Unit handling cleanups

The option code has been switching to keeping things in display units -
bytes - and this transitions more libbcachefs.c code to bytes as well,
to match, and also fixes device add.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>

cmd_device.c

@@ -78,12 +78,10 @@ int cmd_device_add(int argc, char *argv[])
 		case 'S':
 			if (bch2_strtoull_h(optarg, &dev_opts.size))
 				die("invalid filesystem size");
-
-			dev_opts.size >>= 9;
 			break;
 		case 'B':
-			dev_opts.bucket_size =
+			if (bch2_strtoull_h(optarg, &dev_opts.bucket_size))
-				hatoi_validate(optarg, "bucket size");
+				die("bad bucket_size %s", optarg);
 			break;
 		case 'D':
 			dev_opts.discard = true;
@@ -121,9 +119,9 @@ int cmd_device_add(int argc, char *argv[])
 	struct bch_opts fs_opts = bch2_parse_opts(fs_opt_strs);
 
 	opt_set(fs_opts, block_size,
-		read_file_u64(fs.sysfs_fd, "options/block_size") >> 9);
+		read_file_u64(fs.sysfs_fd, "options/block_size"));
 	opt_set(fs_opts, btree_node_size,
-		read_file_u64(fs.sysfs_fd, "options/btree_node_size") >> 9);
+		read_file_u64(fs.sysfs_fd, "options/btree_node_size"));
 
 	struct bch_sb *sb = bch2_format(fs_opt_strs,
 					fs_opts,

cmd_format.c

@@ -163,8 +163,6 @@ int cmd_format(int argc, char *argv[])
 		case O_fs_size:
 			if (bch2_strtoull_h(optarg, &dev_opts.size))
 				die("invalid filesystem size");
-
-			dev_opts.size >>= 9;
 			break;
 		case O_superblock_size:
 			if (bch2_strtouint_h(optarg, &opts.superblock_size))
@@ -173,8 +171,8 @@ int cmd_format(int argc, char *argv[])
 			opts.superblock_size >>= 9;
 			break;
 		case O_bucket_size:
-			dev_opts.bucket_size =
+			if (bch2_strtoull_h(optarg, &dev_opts.bucket_size))
-				hatoi_validate(optarg, "bucket size");
+				die("bad bucket_size %s", optarg);
 			break;
 		case O_label:
 		case 'l':
@@ -258,7 +256,7 @@ int cmd_format(int argc, char *argv[])
 						darray_size(device_paths),
 						bch2_opts_empty());
 		if (IS_ERR(c))
-			die("error opening %s: %s", device_paths.item,
+			die("error opening %s: %s", device_paths.item[0],
 			    strerror(-PTR_ERR(c)));
 
 		bch2_fs_stop(c);

libbcachefs.c

@@ -29,12 +29,6 @@
 
 #define NSEC_PER_SEC	1000000000L
 
-/* minimum size filesystem we can create, given a bucket size: */
-static u64 min_size(unsigned bucket_size)
-{
-	return BCH_MIN_NR_NBUCKETS * bucket_size;
-}
-
 static void init_layout(struct bch_sb_layout *l,
 			unsigned block_size,
 			unsigned sb_size,
@@ -64,14 +58,20 @@ static void init_layout(struct bch_sb_layout *l,
 		    sb_start, sb_pos, sb_end, sb_size);
 }
 
+/* minimum size filesystem we can create, given a bucket size: */
+static u64 min_size(unsigned bucket_size)
+{
+	return BCH_MIN_NR_NBUCKETS * bucket_size;
+}
+
 void bch2_pick_bucket_size(struct bch_opts opts, struct dev_opts *dev)
 {
 	if (!dev->size)
-		dev->size = get_size(dev->path, dev->fd) >> 9;
+		dev->size = get_size(dev->path, dev->fd);
 
 	if (!dev->bucket_size) {
 		if (dev->size < min_size(opts.block_size))
-			die("cannot format %s, too small (%llu sectors, min %llu)",
+			die("cannot format %s, too small (%llu bytes, min %llu)",
 			    dev->path, dev->size, min_size(opts.block_size));
 
 		/* Bucket size must be >= block size: */
@@ -83,16 +83,16 @@ void bch2_pick_bucket_size(struct bch_opts opts, struct dev_opts *dev)
 						 opts.btree_node_size);
 
 		/* Want a bucket size of at least 128k, if possible: */
-		dev->bucket_size = max(dev->bucket_size, 256U);
+		dev->bucket_size = max(dev->bucket_size, 128ULL << 10);
 
 		if (dev->size >= min_size(dev->bucket_size)) {
 			unsigned scale = max(1,
-					     ilog2(dev->size / min_size(dev->bucket_size)) / 4);
+				ilog2(dev->size / min_size(dev->bucket_size)) / 4);
 
 			scale = rounddown_pow_of_two(scale);
 
 			/* max bucket size 1 mb */
-			dev->bucket_size = min(dev->bucket_size * scale, 1U << 11);
+			dev->bucket_size = min(dev->bucket_size * scale, 1ULL << 20);
 		} else {
 			do {
 				dev->bucket_size /= 2;
@@ -100,21 +100,24 @@ void bch2_pick_bucket_size(struct bch_opts opts, struct dev_opts *dev)
 		}
 	}
 
-	dev->nbuckets	= dev->size / dev->bucket_size;
+	dev->nbuckets = dev->size / dev->bucket_size;
 
-	if (dev->bucket_size << 9 < opts.block_size)
+	if (dev->bucket_size < opts.block_size)
-		die("Bucket size (%u) cannot be smaller than block size (%u)",
+		die("Bucket size (%llu) cannot be smaller than block size (%u)",
-		    dev->bucket_size << 9, opts.block_size);
+		    dev->bucket_size, opts.block_size);
 
 	if (opt_defined(opts, btree_node_size) &&
-	    dev->bucket_size << 9 < opts.btree_node_size)
+	    dev->bucket_size < opts.btree_node_size)
-		die("Bucket size (%u) cannot be smaller than btree node size (%u)",
+		die("Bucket size (%llu) cannot be smaller than btree node size (%u)",
-		    dev->bucket_size << 9, opts.btree_node_size);
+		    dev->bucket_size, opts.btree_node_size);
 
 	if (dev->nbuckets < BCH_MIN_NR_NBUCKETS)
-		die("Not enough buckets: %llu, need %u (bucket size %u)",
+		die("Not enough buckets: %llu, need %u (bucket size %llu)",
 		    dev->nbuckets, BCH_MIN_NR_NBUCKETS, dev->bucket_size);
 
+	if (dev->bucket_size > (u32) U16_MAX << 9)
+		die("Bucket size (%llu) too big (max %u)",
+		    dev->bucket_size, (u32) U16_MAX << 9);
 }
 
 static unsigned parse_target(struct bch_sb_handle *sb,
@@ -174,7 +177,7 @@ struct bch_sb *bch2_format(struct bch_opt_strs	fs_opt_strs,
 		for (i = devs; i < devs + nr_devs; i++)
 			fs_opts.btree_node_size =
 				min_t(unsigned, fs_opts.btree_node_size,
-				      i->bucket_size << 9);
+				      i->bucket_size);
 	}
 
 	if (uuid_is_null(opts.uuid.b))
@@ -229,7 +232,7 @@ struct bch_sb *bch2_format(struct bch_opt_strs	fs_opt_strs,
 		uuid_generate(m->uuid.b);
 		m->nbuckets	= cpu_to_le64(i->nbuckets);
 		m->first_bucket	= 0;
-		m->bucket_size	= cpu_to_le16(i->bucket_size);
+		m->bucket_size	= cpu_to_le16(i->bucket_size >> 9);
 
 		SET_BCH_MEMBER_DISCARD(m,	i->discard);
 		SET_BCH_MEMBER_DATA_ALLOWED(m,	i->data_allowed);
@@ -246,7 +249,7 @@ struct bch_sb *bch2_format(struct bch_opt_strs	fs_opt_strs,
 
 		idx = bch2_disk_path_find_or_create(&sb, i->label);
 		if (idx < 0)
-			die("error creating disk path: %s", idx);
+			die("error creating disk path: %s", strerror(-idx));
 
 		SET_BCH_MEMBER_GROUP(m,	idx + 1);
 	}
@@ -270,11 +273,13 @@ struct bch_sb *bch2_format(struct bch_opt_strs	fs_opt_strs,
 	}
 
 	for (i = devs; i < devs + nr_devs; i++) {
+		u64 size_sectors = i->size >> 9;
+
 		sb.sb->dev_idx = i - devs;
 
 		if (!i->sb_offset) {
 			i->sb_offset	= BCH_SB_SECTOR;
-			i->sb_end	= i->size;
+			i->sb_end	= size_sectors;
 		}
 
 		init_layout(&sb.sb->layout, fs_opts.block_size,
@@ -290,9 +295,9 @@ struct bch_sb *bch2_format(struct bch_opt_strs	fs_opt_strs,
 		 */
 		if (i->sb_offset == BCH_SB_SECTOR) {
 			struct bch_sb_layout *l = &sb.sb->layout;
-			u64 backup_sb = i->size - (1 << l->sb_max_size_bits);
+			u64 backup_sb = size_sectors - (1 << l->sb_max_size_bits);
 
-			backup_sb = rounddown(backup_sb, i->bucket_size);
+			backup_sb = rounddown(backup_sb, i->bucket_size >> 9);
 			l->sb_offset[l->nr_superblocks++] = cpu_to_le64(backup_sb);
 		}
 
@@ -300,7 +305,8 @@ struct bch_sb *bch2_format(struct bch_opt_strs	fs_opt_strs,
 			/* Zero start of disk */
 			static const char zeroes[BCH_SB_SECTOR << 9];
 
-			xpwrite(i->fd, zeroes, BCH_SB_SECTOR << 9, 0);
+			xpwrite(i->fd, zeroes, BCH_SB_SECTOR << 9, 0,
+				"zeroing start of disk");
 		}
 
 		bch2_super_write(i->fd, sb.sb);
@@ -321,12 +327,14 @@ void bch2_super_write(int fd, struct bch_sb *sb)
 		if (sb->offset == BCH_SB_SECTOR) {
 			/* Write backup layout */
 			xpwrite(fd, &sb->layout, sizeof(sb->layout),
-				BCH_SB_LAYOUT_SECTOR << 9);
+				BCH_SB_LAYOUT_SECTOR << 9,
+				"backup layout");
 		}
 
 		sb->csum = csum_vstruct(NULL, BCH_SB_CSUM_TYPE(sb), nonce, sb);
 		xpwrite(fd, sb, vstruct_bytes(sb),
-			le64_to_cpu(sb->offset) << 9);
+			le64_to_cpu(sb->offset) << 9,
+			"superblock");
 	}
 
 	fsync(fd);

libbcachefs.h

@@ -50,8 +50,8 @@ static inline struct format_opts format_opts_default()
 struct dev_opts {
 	int		fd;
 	char		*path;
-	u64		size; /* 512 byte sectors */
+	u64		size;		/* bytes*/
-	unsigned	bucket_size;
+	u64		bucket_size;	/* bytes */
 	const char	*label;
 	unsigned	data_allowed;
 	unsigned	durability;

libbcachefs/fsck.c

@@ -1132,9 +1132,9 @@ static int check_i_sectors(struct btree_trans *trans, struct inode_walker *w)
 		count2 = lockrestart_do(trans,
 			bch2_count_inode_sectors(trans, w->cur_inum, i->snapshot));
 
-		if (i->count != count2) {
+		if (fsck_err_on(i->count != count2, c,
-			bch_err(c, "fsck counted i_sectors wrong: got %llu should be %llu",
+				"fsck counted i_sectors wrong: got %llu should be %llu",
-				i->count, count2);
+				i->count, count2)) {
 			i->count = count2;
 			if (i->inode.bi_sectors == i->count)
 				continue;
@@ -1316,9 +1316,9 @@ static int check_subdir_count(struct btree_trans *trans, struct inode_walker *w)
 		count2 = lockrestart_do(trans,
 				bch2_count_subdirs(trans, w->cur_inum, i->snapshot));
 
-		if (i->count != count2) {
+		if (fsck_err_on(i->count != count2, c,
-			bch_err(c, "fsck counted subdirectories wrong: got %llu should be %llu",
+				"directory %llu:%u: fsck counted subdirectories wrong, got %llu should be %llu",
-				i->count, count2);
+				w->cur_inum, i->snapshot, i->count, count2)) {
 			i->count = count2;
 			if (i->inode.bi_nlink == i->count)
 				continue;

qcow2.c

@@ -46,7 +46,8 @@ static void flush_l2(struct qcow2_image *img)
 	if (img->l1_index != -1) {
 		img->l1_table[img->l1_index] =
 			cpu_to_be64(img->offset|QCOW_OFLAG_COPIED);
-		xpwrite(img->fd, img->l2_table, img->block_size, img->offset);
+		xpwrite(img->fd, img->l2_table, img->block_size, img->offset,
+			"qcow2 l2 table");
 		img->offset += img->block_size;
 
 		memset(img->l2_table, 0, img->block_size);
@@ -101,7 +102,8 @@ void qcow2_write_image(int infd, int outfd, ranges *data,
 			img.offset += img.block_size;
 
 			xpread(infd, buf, block_size, src_offset);
-			xpwrite(outfd, buf, block_size, dst_offset);
+			xpwrite(outfd, buf, block_size, dst_offset,
+				"qcow2 data");
 
 			add_l2(&img, src_offset / block_size, dst_offset);
 		}
@@ -111,7 +113,8 @@ void qcow2_write_image(int infd, int outfd, ranges *data,
 	/* Write L1 table: */
 	dst_offset		= img.offset;
 	img.offset		+= round_up(l1_size * sizeof(u64), block_size);
-	xpwrite(img.fd, img.l1_table, l1_size * sizeof(u64), dst_offset);
+	xpwrite(img.fd, img.l1_table, l1_size * sizeof(u64), dst_offset,
+		"qcow2 l1 table");
 
 	/* Write header: */
 	hdr.magic		= cpu_to_be32(QCOW_MAGIC);
@@ -123,7 +126,8 @@ void qcow2_write_image(int infd, int outfd, ranges *data,
 
 	memset(buf, 0, block_size);
 	memcpy(buf, &hdr, sizeof(hdr));
-	xpwrite(img.fd, buf, block_size, 0);
+	xpwrite(img.fd, buf, block_size, 0,
+		"qcow2 header");
 
 	free(img.l2_table);
 	free(img.l1_table);

tools-util.c

@@ -94,12 +94,12 @@ void xpread(int fd, void *buf, size_t count, off_t offset)
 	}
 }
 
-void xpwrite(int fd, const void *buf, size_t count, off_t offset)
+void xpwrite(int fd, const void *buf, size_t count, off_t offset, const char *msg)
 {
 	ssize_t r = pwrite(fd, buf, count, offset);
 
 	if (r != count)
-		die("write error (ret %zi err %m)", r);
+		die("error writing %s (ret %zi err %m)", msg, r);
 }
 
 struct stat xfstatat(int dirfd, const char *path, int flags)
@@ -242,7 +242,7 @@ u64 get_size(const char *path, int fd)
 	return ret;
 }
 
-/* Returns blocksize in units of 512 byte sectors: */
+/* Returns blocksize, in bytes: */
 unsigned get_blocksize(const char *path, int fd)
 {
 	struct stat statbuf = xfstat(fd);
@@ -403,24 +403,6 @@ char *strcmp_prefix(char *a, const char *a_prefix)
 	return *a_prefix ? NULL : a;
 }
 
-unsigned hatoi_validate(const char *s, const char *msg)
-{
-	u64 v;
-
-	if (bch2_strtoull_h(s, &v))
-		die("bad %s %s", msg, s);
-
-	v /= 512;
-
-	if (v > USHRT_MAX)
-		die("%s too large\n", msg);
-
-	if (!v)
-		die("%s too small\n", msg);
-
-	return v;
-}
-
 /* crc32c */
 
 static u32 crc32c_default(u32 crc, const void *buf, size_t size)

tools-util.h

@@ -22,14 +22,15 @@
 
 #define noreturn __attribute__((noreturn))
 
-void die(const char *, ...) noreturn;
+void die(const char *, ...)
+	__attribute__ ((format (printf, 1, 2))) noreturn;
 char *mprintf(const char *, ...)
 	__attribute__ ((format (printf, 1, 2)));
 void *xcalloc(size_t, size_t);
 void *xmalloc(size_t);
 void *xrealloc(void *, size_t);
 void xpread(int, void *, size_t, off_t);
-void xpwrite(int, const void *, size_t, off_t);
+void xpwrite(int, const void *, size_t, off_t, const char *);
 struct stat xfstatat(int, const char *, int);
 struct stat xfstat(int);
 struct stat xstat(const char *);
@@ -150,8 +151,6 @@ struct fiemap_extent fiemap_iter_next(struct fiemap_iter *);
 
 char *strcmp_prefix(char *, const char *);
 
-unsigned hatoi_validate(const char *, const char *);
-
 u32 crc32c(u32, const void *, size_t);
 
 char *dev_to_name(dev_t);