cmd_device_add improvements

5 files changed, 166 insertions, 81 deletions

diff --git a/cmd_device.c b/cmd_device.c
index dfb6ef8..32f4492 100644
--- a/cmd_device.c
+++ b/cmd_device.c
@@ -177,20 +177,90 @@ int cmd_device_show(int argc, char *argv[])
 	return 0;
 }
 
+static void device_add_usage(void)
+{
+	puts("bcache device_add - add a device to an existing filesystem\n"
+	     "Usage: bcache device_add [OPTION]... filesystem device\n"
+	     "\n"
+	     "Options:\n"
+	     "      --fs_size=size          Size of filesystem on device\n"
+	     "      --bucket=size           Bucket size\n"
+	     "      --discard               Enable discards\n"
+	     "  -t, --tier=#                Higher tier (e.g. 1) indicates slower devices\n"
+	     "  -f, --force                 Use device even if it appears to already be formatted\n"
+	     "  -h, --help                  Display this help and exit\n"
+	     "\n"
+	     "Report bugs to <linux-bcache@vger.kernel.org>");
+}
+
+static const struct option device_add_opts[] = {
+	{ "fs_size",		required_argument,	NULL, 'S' },
+	{ "bucket",		required_argument,	NULL, 'B' },
+	{ "discard",		no_argument,		NULL, 'D' },
+	{ "tier",		required_argument,	NULL, 't' },
+	{ "force",		no_argument,		NULL, 'f' },
+	{ NULL }
+};
+
 int cmd_device_add(int argc, char *argv[])
 {
-	if (argc < 3)
-		die("Please supply a filesystem and at least one device to add");
+	struct format_opts format_opts = format_opts_default();
+	struct dev_opts dev_opts = { 0 };
+	bool force = false;
+	int opt;
 
-	struct bcache_handle fs = bcache_fs_open(argv[1]);
+	while ((opt = getopt_long(argc, argv, "t:fh",
+				  device_add_opts, NULL)) != -1)
+		switch (opt) {
+		case 'S':
+			if (bch_strtoull_h(optarg, &dev_opts.size))
+				die("invalid filesystem size");
 
-	for (unsigned i = 2; i < argc; i++) {
-		struct bch_ioctl_disk_add ia = {
-			.dev = (__u64) argv[i],
-		};
+			dev_opts.size >>= 9;
+			break;
+		case 'B':
+			dev_opts.bucket_size =
+				hatoi_validate(optarg, "bucket size");
+			break;
+		case 'D':
+			dev_opts.discard = true;
+			break;
+		case 't':
+			if (kstrtouint(optarg, 10, &dev_opts.tier) ||
+			    dev_opts.tier >= BCH_TIER_MAX)
+				die("invalid tier");
+			break;
+		case 'f':
+			force = true;
+			break;
+		case 'h':
+			device_add_usage();
+			exit(EXIT_SUCCESS);
+		}
 
-		xioctl(fs.ioctl_fd, BCH_IOCTL_DISK_ADD, &ia);
-	}
+	if (argc - optind != 2)
+		die("Please supply a filesystem and a device to add");
+
+	struct bcache_handle fs = bcache_fs_open(argv[optind]);
+
+	dev_opts.path = argv[optind + 1];
+	dev_opts.fd = open_for_format(dev_opts.path, force);
+
+	format_opts.block_size	=
+		read_file_u64(fs.sysfs_fd, "block_size_bytes") >> 9;
+	format_opts.btree_node_size =
+		read_file_u64(fs.sysfs_fd, "btree_node_size_bytes") >> 9;
+
+	struct bch_sb *sb = bcache_format(format_opts, &dev_opts, 1);
+	free(sb);
+	fsync(dev_opts.fd);
+	close(dev_opts.fd);
+
+	struct bch_ioctl_disk_add ia = {
+		.dev = (__u64) dev_opts.path,
+	};
+
+	xioctl(fs.ioctl_fd, BCH_IOCTL_DISK_ADD, &ia);
 
 	return 0;
 }
diff --git a/cmd_format.c b/cmd_format.c
index 7334259..3ef862f 100644
--- a/cmd_format.c
+++ b/cmd_format.c
@@ -17,7 +17,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include <blkid.h>
 #include <uuid/uuid.h>
 
 #include "ccan/darray/darray.h"
@@ -28,46 +27,6 @@
 #include "opts.h"
 #include "util.h"
 
-/* Open a block device, do magic blkid stuff: */
-static int open_for_format(const char *dev, bool force)
-{
-	blkid_probe pr;
-	const char *fs_type = NULL, *fs_label = NULL;
-	size_t fs_type_len, fs_label_len;
-
-	int fd = xopen(dev, O_RDWR|O_EXCL);
-
-	if (force)
-		return fd;
-
-	if (!(pr = blkid_new_probe()))
-		die("blkid error 1");
-	if (blkid_probe_set_device(pr, fd, 0, 0))
-		die("blkid error 2");
-	if (blkid_probe_enable_partitions(pr, true))
-		die("blkid error 3");
-	if (blkid_do_fullprobe(pr) < 0)
-		die("blkid error 4");
-
-	blkid_probe_lookup_value(pr, "TYPE", &fs_type, &fs_type_len);
-	blkid_probe_lookup_value(pr, "LABEL", &fs_label, &fs_label_len);
-
-	if (fs_type) {
-		if (fs_label)
-			printf("%s contains a %s filesystem labelled '%s'\n",
-			       dev, fs_type, fs_label);
-		else
-			printf("%s contains a %s filesystem\n",
-			       dev, fs_type);
-		fputs("Proceed anyway?", stdout);
-		if (!ask_yn())
-			exit(EXIT_FAILURE);
-	}
-
-	blkid_free_probe(pr);
-	return fd;
-}
-
 #define OPTS									\
 t("bcache format - create a new bcache filesystem on one or more devices")	\
 t("Usage: bcache format [OPTION]... <devices>")					\
@@ -127,11 +86,11 @@ static void usage(void)
 	     "\n"
 	     "Device specific options:\n"
 	     "      --fs_size=size          Size of filesystem on device\n"
-	     "      --bucket=size           bucket size\n"
+	     "      --bucket=size           Bucket size\n"
 	     "      --discard               Enable discards\n"
-	     "  -t, --tier=#                tier of subsequent devices\n"
+	     "  -t, --tier=#                Higher tier (e.g. 1) indicates slower devices\n"
 	     "\n"
-	     "  -h, --help                  display this help and exit\n"
+	     "  -h, --help                  Display this help and exit\n"
 	     "\n"
 	     "Device specific options must come before corresponding devices, e.g.\n"
 	     "  bcache format --tier 0 /dev/sdb --tier 1 /dev/sdc\n"
@@ -162,27 +121,6 @@ static const struct option format_opts[] = {
 	{ NULL }
 };
 
-static unsigned hatoi_validate(const char *s, const char *msg)
-{
-	u64 v;
-
-	if (bch_strtoull_h(s, &v))
-		die("bad %s %s", msg, s);
-
-	if (v & (v - 1))
-		die("%s must be a power of two", msg);
-
-	v /= 512;
-
-	if (v > USHRT_MAX)
-		die("%s too large\n", msg);
-
-	if (!v)
-		die("%s too small\n", msg);
-
-	return v;
-}
-
 int cmd_format(int argc, char *argv[])
 {
 	darray(struct dev_opts) devices;
diff --git a/libbcache.c b/libbcache.c
index c9c113a..1278fdf 100644
--- a/libbcache.c
+++ b/libbcache.c
@@ -97,8 +97,14 @@ struct bch_sb *bcache_format(struct format_opts opts,
 				die("cannot format %s, too small (%llu sectors, min %llu)",
 				    i->path, i->size, min_size(opts.block_size));
 
+			/* Bucket size must be >= block size: */
+			i->bucket_size = opts.block_size;
+
+			/* Bucket size must be >= btree node size: */
+			i->bucket_size = max(i->bucket_size, opts.btree_node_size);
+
 			/* Want a bucket size of at least 128k, if possible: */
-			i->bucket_size = max(opts.block_size, 256U);
+			i->bucket_size = max(i->bucket_size, 256U);
 
 			if (i->size >= min_size(i->bucket_size)) {
 				unsigned scale = max(1,
@@ -120,6 +126,9 @@ struct bch_sb *bcache_format(struct format_opts opts,
 		if (i->bucket_size < opts.block_size)
 			die("Bucket size cannot be smaller than block size");
 
+		if (i->bucket_size < opts.btree_node_size)
+			die("Bucket size cannot be smaller than btree node size");
+
 		if (i->nbuckets < BCH_MIN_NR_NBUCKETS)
 			die("Not enough buckets: %llu, need %u (bucket size %u)",
 			    i->nbuckets, BCH_MIN_NR_NBUCKETS, i->bucket_size);
diff --git a/tools-util.c b/tools-util.c
index 07fb82d..bb2ac47 100644
--- a/tools-util.c
+++ b/tools-util.c
@@ -13,6 +13,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include <blkid.h>
 #include <uuid/uuid.h>
 
 #include "ccan/crc/crc.h"
@@ -60,11 +61,13 @@ struct units_buf __pr_units(u64 v, enum units units)
 char *read_file_str(int dirfd, const char *path)
 {
 	int fd = xopenat(dirfd, path, O_RDONLY);
-	size_t len = xfstat(fd).st_size;
+	ssize_t len = xfstat(fd).st_size;
 
-	char *buf = malloc(len + 1);
+	char *buf = xmalloc(len + 1);
 
-	xpread(fd, buf, len, 0);
+	len = read(fd, buf, len);
+	if (len < 0)
+		die("read error: %s", strerror(errno));
 
 	buf[len] = '\0';
 	if (len && buf[len - 1] == '\n')
@@ -78,10 +81,11 @@ char *read_file_str(int dirfd, const char *path)
 u64 read_file_u64(int dirfd, const char *path)
 {
 	char *buf = read_file_str(dirfd, path);
-	u64 ret = strtoll(buf, NULL, 10);
-
+	u64 v;
+	if (kstrtou64(buf, 10, &v))
+		die("read_file_u64: error parsing %s (got %s)", path, buf);
 	free(buf);
-	return ret;
+	return v;
 }
 
 /* String list options: */
@@ -122,6 +126,46 @@ unsigned get_blocksize(const char *path, int fd)
 	return ret >> 9;
 }
 
+/* Open a block device, do magic blkid stuff to probe for existing filesystems: */
+int open_for_format(const char *dev, bool force)
+{
+	blkid_probe pr;
+	const char *fs_type = NULL, *fs_label = NULL;
+	size_t fs_type_len, fs_label_len;
+
+	int fd = xopen(dev, O_RDWR|O_EXCL);
+
+	if (force)
+		return fd;
+
+	if (!(pr = blkid_new_probe()))
+		die("blkid error 1");
+	if (blkid_probe_set_device(pr, fd, 0, 0))
+		die("blkid error 2");
+	if (blkid_probe_enable_partitions(pr, true))
+		die("blkid error 3");
+	if (blkid_do_fullprobe(pr) < 0)
+		die("blkid error 4");
+
+	blkid_probe_lookup_value(pr, "TYPE", &fs_type, &fs_type_len);
+	blkid_probe_lookup_value(pr, "LABEL", &fs_label, &fs_label_len);
+
+	if (fs_type) {
+		if (fs_label)
+			printf("%s contains a %s filesystem labelled '%s'\n",
+			       dev, fs_type, fs_label);
+		else
+			printf("%s contains a %s filesystem\n",
+			       dev, fs_type);
+		fputs("Proceed anyway?", stdout);
+		if (!ask_yn())
+			exit(EXIT_FAILURE);
+	}
+
+	blkid_free_probe(pr);
+	return fd;
+}
+
 /* Global control device: */
 int bcachectl_open(void)
 {
@@ -270,3 +314,24 @@ const char *strcmp_prefix(const char *a, const char *a_prefix)
 	}
 	return *a_prefix ? NULL : a;
 }
+
+unsigned hatoi_validate(const char *s, const char *msg)
+{
+	u64 v;
+
+	if (bch_strtoull_h(s, &v))
+		die("bad %s %s", msg, s);
+
+	if (v & (v - 1))
+		die("%s must be a power of two", msg);
+
+	v /= 512;
+
+	if (v > USHRT_MAX)
+		die("%s too large\n", msg);
+
+	if (!v)
+		die("%s too small\n", msg);
+
+	return v;
+}
diff --git a/tools-util.h b/tools-util.h
index 1aac56a..8451b11 100644
--- a/tools-util.h
+++ b/tools-util.h
@@ -121,6 +121,7 @@ ssize_t read_string_list_or_die(const char *, const char * const[],
 
 u64 get_size(const char *, int);
 unsigned get_blocksize(const char *, int);
+int open_for_format(const char *, bool);
 
 int bcachectl_open(void);
 
@@ -203,4 +204,6 @@ struct fiemap_extent fiemap_iter_next(struct fiemap_iter *);
 
 const char *strcmp_prefix(const char *, const char *);
 
+unsigned hatoi_validate(const char *, const char *);
+
 #endif /* _TOOLS_UTIL_H */