xclose()

Add a helper to check for close errrors - especially bad file
descriptors, that can be a fun source of heisenbugs.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

12 files changed, 38 insertions, 35 deletions

diff --git a/c_src/cmd_attr.c b/c_src/cmd_attr.c
index 1da41265..78df1be8 100644
--- a/c_src/cmd_attr.c
+++ b/c_src/cmd_attr.c
@@ -48,7 +48,7 @@ static void propagate_recurse(int dirfd)
 			continue;
 		}
 		propagate_recurse(fd);
-		close(fd);
+		xclose(fd);
 	}
 
 	if (errno)
@@ -80,7 +80,7 @@ static void do_setattr(char *path, struct bch_opt_strs opts)
 		die("error opening %s: %m", path);
 
 	propagate_recurse(dirfd);
-	close(dirfd);
+	xclose(dirfd);
 }
 
 static void setattr_usage(void)
diff --git a/c_src/cmd_data.c b/c_src/cmd_data.c
index 302c5a8c..5a1a1485 100644
--- a/c_src/cmd_data.c
+++ b/c_src/cmd_data.c
@@ -167,7 +167,7 @@ static int cmd_data_scrub(int argc, char *argv[])
 
 			if (dev->progress_fd >= 0 &&
 			    read(dev->progress_fd, &e, sizeof(e)) != sizeof(e)) {
-				close(dev->progress_fd);
+				xclose(dev->progress_fd);
 				dev->progress_fd = -1;
 			}
 
@@ -186,7 +186,7 @@ static int cmd_data_scrub(int argc, char *argv[])
 			}
 
 			if (dev->progress_fd >= 0 && e.ret) {
-				close(dev->progress_fd);
+				xclose(dev->progress_fd);
 				dev->progress_fd = -1;
 				dev->ret = e.ret;
 			}
diff --git a/c_src/cmd_dump.c b/c_src/cmd_dump.c
index c9e417f2..9b182325 100644
--- a/c_src/cmd_dump.c
+++ b/c_src/cmd_dump.c
@@ -172,7 +172,7 @@ int cmd_dump(int argc, char *argv[])
 		free(path);
 
 		dump_one_device(c, ca, fd, entire_journal);
-		close(fd);
+		xclose(fd);
 	}
 
 	up_read(&c->state_lock);
diff --git a/c_src/cmd_format.c b/c_src/cmd_format.c
index 6385cff4..6534469f 100644
--- a/c_src/cmd_format.c
+++ b/c_src/cmd_format.c
@@ -116,10 +116,6 @@ static void build_fs(struct bch_fs *c, const char *src_path)
 {
 	struct copy_fs_state s = {};
 	int src_fd = xopen(src_path, O_RDONLY|O_NOATIME);
-	struct stat stat = xfstat(src_fd);
-
-	if (!S_ISDIR(stat.st_mode))
-		die("%s is not a directory", src_path);
 
 	copy_fs(c, src_fd, src_path, &s, 0);
 }
diff --git a/c_src/cmd_fsck.c b/c_src/cmd_fsck.c
index 06f131bc..aa81fa11 100644
--- a/c_src/cmd_fsck.c
+++ b/c_src/cmd_fsck.c
@@ -86,7 +86,8 @@ static int splice_fd_to_stdinout(int fd)
 			stdin_closed = true;
 	}
 
-	return close(fd);
+	xclose(fd);
+	return 0;
 }
 
 static int fsck_online(const char *dev_path, const char *opt_str)
diff --git a/c_src/cmd_migrate.c b/c_src/cmd_migrate.c
index 639aba64..7b4004f9 100644
--- a/c_src/cmd_migrate.c
+++ b/c_src/cmd_migrate.c
@@ -147,7 +147,7 @@ static ranges reserve_new_fs_space(const char *file_path, unsigned block_size,
 		range_add(&extents, e.fe_physical, e.fe_length);
 	}
 	fiemap_iter_exit(&iter);
-	close(fd);
+	xclose(fd);
 
 	ranges_sort_merge(&extents);
 	return extents;
@@ -425,7 +425,7 @@ int cmd_migrate_superblock(int argc, char *argv[])
 	xpwrite(fd, zeroes, BCH_SB_SECTOR << 9, 0, "zeroing start of disk");
 
 	bch2_super_write(fd, sb);
-	close(fd);
+	xclose(fd);
 
 	/* mark new superblocks */
 
diff --git a/c_src/libbcachefs.c b/c_src/libbcachefs.c
index 748fa182..c8503839 100644
--- a/c_src/libbcachefs.c
+++ b/c_src/libbcachefs.c
@@ -329,7 +329,7 @@ struct bch_sb *bch2_format(struct bch_opt_strs	fs_opt_strs,
 		}
 
 		bch2_super_write(i->bdev->bd_fd, sb.sb);
-		close(i->bdev->bd_fd);
+		xclose(i->bdev->bd_fd);
 	}
 
 	return sb.sb;
@@ -402,8 +402,8 @@ int bcachectl_open(void)
 
 void bcache_fs_close(struct bchfs_handle fs)
 {
-	close(fs.ioctl_fd);
-	close(fs.sysfs_fd);
+	xclose(fs.ioctl_fd);
+	xclose(fs.sysfs_fd);
 }
 
 static int bcache_fs_open_by_uuid(const char *uuid_str, struct bchfs_handle *fs)
@@ -460,7 +460,7 @@ int bcache_fs_open_fallible(const char *path, struct bchfs_handle *fs)
 	char buf[1024], *uuid_str;
 
 	struct stat stat = xstat(path);
-	close(path_fd);
+	xclose(path_fd);
 
 	if (S_ISBLK(stat.st_mode)) {
 		char *sysfs = mprintf("/sys/dev/block/%u:%u/bcachefs",
@@ -607,7 +607,7 @@ int bchu_data(struct bchfs_handle fs, struct bch_ioctl_data cmd)
 	}
 	printf("\nDone\n");
 
-	close(progress_fd);
+	xclose(progress_fd);
 	return 0;
 }
 
@@ -830,7 +830,7 @@ void bch2_opts_usage(unsigned opt_types)
 
 dev_names bchu_fs_get_devices(struct bchfs_handle fs)
 {
-	DIR *dir = fdopendir(fs.sysfs_fd);
+	DIR *dir = fdopendir(dup(fs.sysfs_fd));
 	struct dirent *d;
 	dev_names devs;
 
diff --git a/c_src/posix_to_bcachefs.c b/c_src/posix_to_bcachefs.c
index 15455a66..4ff776f3 100644
--- a/c_src/posix_to_bcachefs.c
+++ b/c_src/posix_to_bcachefs.c
@@ -160,7 +160,7 @@ static void write_data(struct bch_fs *c,
 	op.nr_replicas	= 1;
 	op.subvol	= 1;
 	op.pos		= SPOS(dst_inode->bi_inum, dst_offset >> 9, U32_MAX);
-	op.flags |= BCH_WRITE_sync;
+	op.flags |= BCH_WRITE_sync|BCH_WRITE_only_specified_devs;
 
 	int ret = bch2_disk_reservation_get(c, &op.res, len >> 9,
 					    c->opts.data_replicas, 0);
@@ -345,7 +345,7 @@ static void copy_dir(struct copy_fs_state *s,
 		int fd;
 
 		if (fchdir(src_fd))
-			die("chdir error: %m");
+			die("fchdir error: %m");
 
 		struct stat stat =
 			xfstatat(src_fd, d->d_name, AT_SYMLINK_NOFOLLOW);
@@ -387,7 +387,7 @@ static void copy_dir(struct copy_fs_state *s,
 		case DT_DIR:
 			fd = xopen(d->d_name, O_RDONLY|O_NOATIME);
 			copy_dir(s, c, &inode, fd, child_path, reserve_start);
-			close(fd);
+			xclose(fd);
 			break;
 		case DT_REG:
 			inode.bi_size = stat.st_size;
@@ -395,7 +395,7 @@ static void copy_dir(struct copy_fs_state *s,
 			fd = xopen(d->d_name, O_RDONLY|O_NOATIME);
 			copy_file(c, &inode, fd, stat.st_size,
 				  child_path, s, reserve_start);
-			close(fd);
+			xclose(fd);
 			break;
 		case DT_LNK:
 			inode.bi_size = stat.st_size;
@@ -420,7 +420,6 @@ next:
 	}
 
 	darray_exit(&dirents);
-	closedir(dir);
 }
 
 static void reserve_old_fs_space(struct bch_fs *c,
@@ -454,7 +453,11 @@ static void reserve_old_fs_space(struct bch_fs *c,
 void copy_fs(struct bch_fs *c, int src_fd, const char *src_path,
 	     struct copy_fs_state *s, u64 reserve_start)
 {
-	syncfs(src_fd);
+	if (!S_ISDIR(xfstat(src_fd).st_mode))
+		die("%s is not a directory", src_path);
+
+	if (s->type == BCH_MIGRATE_migrate)
+		syncfs(src_fd);
 
 	struct bch_inode_unpacked root_inode;
 	int ret = bch2_inode_find_by_inum(c, (subvol_inum) { 1, BCACHEFS_ROOT_INO },
@@ -463,23 +466,20 @@ void copy_fs(struct bch_fs *c, int src_fd, const char *src_path,
 		die("error looking up root directory: %s", bch2_err_str(ret));
 
 	if (fchdir(src_fd))
-		die("chdir error: %m");
+		die("fchdir error: %m");
 
 	struct stat stat = xfstat(src_fd);
 	copy_times(c, &root_inode, &stat);
 	copy_xattrs(c, &root_inode, ".");
 
-
 	/* now, copy: */
 	copy_dir(s, c, &root_inode, src_fd, src_path, reserve_start);
 
-	if (BCH_MIGRATE_migrate == s->type)
+	if (s->type == BCH_MIGRATE_migrate)
 		reserve_old_fs_space(c, &root_inode, &s->extents, reserve_start);
 
 	update_inode(c, &root_inode);
 
-	if (BCH_MIGRATE_migrate == s->type)
-		darray_exit(&s->extents);
-
+	darray_exit(&s->extents);
 	genradix_free(&s->hardlinks);
 }
diff --git a/c_src/posix_to_bcachefs.h b/c_src/posix_to_bcachefs.h
index ed87366a..3ff18a44 100644
--- a/c_src/posix_to_bcachefs.h
+++ b/c_src/posix_to_bcachefs.h
@@ -50,5 +50,5 @@ struct copy_fs_state {
  * initialized (`hardlinks` is initialized with zeroes).
  */
 void copy_fs(struct bch_fs *c, int src_fd, const char *src_path,
-		    struct copy_fs_state *s, u64);
+	     struct copy_fs_state *s, u64);
 #endif /* _LIBBCACHE_H */
diff --git a/c_src/tools-util.c b/c_src/tools-util.c
index 8403bc8b..496c875c 100644
--- a/c_src/tools-util.c
+++ b/c_src/tools-util.c
@@ -107,7 +107,7 @@ void write_file_str(int dirfd, const char *path, const char *str)
 	wrote = write(fd, str, len);
 	if (wrote != len)
 		die("read error: %m");
-	close(fd);
+	xclose(fd);
 }
 
 char *read_file_str(int dirfd, const char *path)
@@ -129,7 +129,7 @@ char *read_file_str(int dirfd, const char *path)
 		buf = NULL;
 	}
 
-	close(fd);
+	xclose(fd);
 
 	return buf;
 }
diff --git a/c_src/tools-util.h b/c_src/tools-util.h
index ca84dd01..d3d6b14b 100644
--- a/c_src/tools-util.h
+++ b/c_src/tools-util.h
@@ -82,6 +82,12 @@ static inline void *xrealloc(void *p, size_t size)
 	_ret;								\
 })
 
+#define xclose(_fd)							\
+do {									\
+	if (close(_fd))							\
+		die("error closing fd: %m at %s:%u", __FILE__, __LINE__);\
+} while (0)
+
 void write_file_str(int, const char *, const char *);
 char *read_file_str(int, const char *);
 u64 read_file_u64(int, const char *);
diff --git a/linux/blkdev.c b/linux/blkdev.c
index eb257d8b..d073ed6a 100644
--- a/linux/blkdev.c
+++ b/linux/blkdev.c
@@ -356,8 +356,8 @@ static void aio_cleanup(void)
 
 	put_task_struct(p);
 
-	close(fds[0]);
-	close(fds[1]);
+	xclose(fds[0]);
+	xclose(fds[1]);
 }
 
 static void aio_op(struct bio *bio, struct iovec *iov, unsigned i, int opcode)