bcachefs: return error codes from mount more correctly

Error reporting is still a mess and really needs to be revamped

6 files changed, 156 insertions, 160 deletions

diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
index 1618ffe70e23..1498832ba28f 100644
--- a/fs/bcachefs/chardev.c
+++ b/fs/bcachefs/chardev.c
@@ -64,7 +64,7 @@ found:
 static long bch2_ioctl_assemble(struct bch_ioctl_assemble __user *user_arg)
 {
 	struct bch_ioctl_assemble arg;
-	const char *err;
+	struct bch_fs *c;
 	u64 *user_devs = NULL;
 	char **devs = NULL;
 	unsigned i;
@@ -96,14 +96,10 @@ static long bch2_ioctl_assemble(struct bch_ioctl_assemble __user *user_arg)
 		}
 	}
 
-	err = bch2_fs_open(devs, arg.nr_devs, bch2_opts_empty(), NULL);
-	if (err) {
-		pr_err("Could not open filesystem: %s", err);
-		ret = -EINVAL;
-		goto err;
-	}
-
-	ret = 0;
+	c = bch2_fs_open(devs, arg.nr_devs, bch2_opts_empty());
+	ret = PTR_ERR_OR_ZERO(c);
+	if (!ret)
+		closure_put(&c->cl);
 err:
 	if (devs)
 		for (i = 0; i < arg.nr_devs; i++)
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 72aa52e2f5d4..2ab2efc050ce 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -1036,81 +1036,100 @@ static int bch2_sync_fs(struct super_block *sb, int wait)
 	return bch2_journal_flush(&c->journal);
 }
 
-static struct bch_fs *bch2_open_as_blockdevs(const char *_dev_name,
-					     struct bch_opts opts)
+static struct bch_fs *bch2_path_to_fs(const char *dev)
 {
-	size_t nr_devs = 0, i = 0;
-	char *dev_name, *s, **devs;
-	struct bch_fs *c = NULL;
-	const char *err = "cannot allocate memory";
+	struct bch_fs *c;
+	struct block_device *bdev = lookup_bdev(dev);
 
-	dev_name = kstrdup(_dev_name, GFP_KERNEL);
-	if (!dev_name)
-		return NULL;
+	if (IS_ERR(bdev))
+		return ERR_CAST(bdev);
 
-	for (s = dev_name; s; s = strchr(s + 1, ':'))
-		nr_devs++;
+	c = bch2_bdev_to_fs(bdev);
+	bdput(bdev);
+	return c ?: ERR_PTR(-ENOENT);
+}
 
-	devs = kcalloc(nr_devs, sizeof(const char *), GFP_KERNEL);
-	if (!devs)
-		goto err;
+static struct bch_fs *__bch2_open_as_blockdevs(const char *dev_name, char * const *devs,
+					       unsigned nr_devs, struct bch_opts opts)
+{
+	struct bch_fs *c, *c1, *c2;
+	size_t i;
 
-	for (i = 0, s = dev_name;
-	     s;
-	     (s = strchr(s, ':')) && (*s++ = '\0'))
-		devs[i++] = s;
+	if (!nr_devs)
+		return ERR_PTR(-EINVAL);
 
-	err = bch2_fs_open(devs, nr_devs, opts, &c);
-	if (err) {
+	c = bch2_fs_open(devs, nr_devs, opts);
+
+	if (IS_ERR(c) && PTR_ERR(c) == -EBUSY) {
 		/*
 		 * Already open?
 		 * Look up each block device, make sure they all belong to a
 		 * filesystem and they all belong to the _same_ filesystem
 		 */
 
-		for (i = 0; i < nr_devs; i++) {
-			struct block_device *bdev = lookup_bdev(devs[i]);
-			struct bch_fs *c2;
-
-			if (IS_ERR(bdev))
-				goto err;
-
-			c2 = bch2_bdev_to_fs(bdev);
-			bdput(bdev);
+		c1 = bch2_path_to_fs(devs[0]);
+		if (!c1)
+			return c;
 
-			if (!c)
-				c = c2;
-			else if (c2)
+		for (i = 1; i < nr_devs; i++) {
+			c2 = bch2_path_to_fs(devs[i]);
+			if (!IS_ERR(c2))
 				closure_put(&c2->cl);
 
-			if (!c)
-				goto err;
-			if (c != c2) {
-				closure_put(&c->cl);
-				goto err;
+			if (c1 != c2) {
+				closure_put(&c1->cl);
+				return c;
 			}
 		}
 
-		mutex_lock(&c->state_lock);
+		c = c1;
+	}
 
-		if (!bch2_fs_running(c)) {
-			mutex_unlock(&c->state_lock);
-			closure_put(&c->cl);
-			err = "incomplete filesystem";
-			c = NULL;
-			goto err;
-		}
+	if (IS_ERR(c))
+		return c;
+
+	mutex_lock(&c->state_lock);
 
+	if (!bch2_fs_running(c)) {
 		mutex_unlock(&c->state_lock);
+		closure_put(&c->cl);
+		pr_err("err mounting %s: incomplete filesystem", dev_name);
+		return ERR_PTR(-EINVAL);
 	}
 
+	mutex_unlock(&c->state_lock);
+
 	set_bit(BCH_FS_BDEV_MOUNTED, &c->flags);
+	return c;
+}
+
+static struct bch_fs *bch2_open_as_blockdevs(const char *_dev_name,
+					     struct bch_opts opts)
+{
+	char *dev_name = NULL, **devs = NULL, *s;
+	struct bch_fs *c = ERR_PTR(-ENOMEM);
+	size_t i, nr_devs = 0;
+
+	dev_name = kstrdup(_dev_name, GFP_KERNEL);
+	if (!dev_name)
+		goto err;
+
+	for (s = dev_name; s; s = strchr(s + 1, ':'))
+		nr_devs++;
+
+	devs = kcalloc(nr_devs, sizeof(const char *), GFP_KERNEL);
+	if (!devs)
+		goto err;
+
+	for (i = 0, s = dev_name;
+	     s;
+	     (s = strchr(s, ':')) && (*s++ = '\0'))
+		devs[i++] = s;
+
+	c = __bch2_open_as_blockdevs(_dev_name, devs, nr_devs, opts);
 err:
 	kfree(devs);
 	kfree(dev_name);
-
-	if (!c)
-		pr_err("bch_fs_open err %s", err);
 	return c;
 }
 
@@ -1233,8 +1252,8 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
 		return ERR_PTR(ret);
 
 	c = bch2_open_as_blockdevs(dev_name, opts);
-	if (!c)
-		return ERR_PTR(-ENOENT);
+	if (IS_ERR(c))
+		return ERR_CAST(c);
 
 	sb = sget(fs_type, bch2_test_super, bch2_set_super, flags|MS_NOSEC, c);
 	if (IS_ERR(sb)) {
diff --git a/fs/bcachefs/super-io.c b/fs/bcachefs/super-io.c
index c3eef9c11509..baa0f0175351 100644
--- a/fs/bcachefs/super-io.c
+++ b/fs/bcachefs/super-io.c
@@ -380,27 +380,6 @@ const char *bch2_sb_validate(struct bch_sb_handle *disk_sb)
 
 /* device open: */
 
-static const char *bch2_blkdev_open(const char *path, fmode_t mode,
-				   void *holder, struct block_device **ret)
-{
-	struct block_device *bdev;
-
-	*ret = NULL;
-	bdev = blkdev_get_by_path(path, mode, holder);
-	if (bdev == ERR_PTR(-EBUSY))
-		return "device busy";
-
-	if (IS_ERR(bdev))
-		return "failed to open device";
-
-	if (mode & FMODE_WRITE)
-		bdev_get_queue(bdev)->backing_dev_info->capabilities
-			|= BDI_CAP_STABLE_WRITES;
-
-	*ret = bdev;
-	return NULL;
-}
-
 static void bch2_sb_update(struct bch_fs *c)
 {
 	struct bch_sb *src = c->disk_sb;
@@ -552,44 +531,45 @@ reread:
 	return NULL;
 }
 
-const char *bch2_read_super(const char *path,
-			    struct bch_opts opts,
-			    struct bch_sb_handle *ret)
+int bch2_read_super(const char *path, struct bch_opts opts,
+		    struct bch_sb_handle *sb)
 {
 	u64 offset = opt_get(opts, sb);
 	struct bch_sb_layout layout;
 	const char *err;
-	unsigned i;
+	__le64 *i;
+	int ret;
 
-	memset(ret, 0, sizeof(*ret));
-	ret->mode = FMODE_READ;
+	memset(sb, 0, sizeof(*sb));
+	sb->mode = FMODE_READ;
 
 	if (!opt_get(opts, noexcl))
-		ret->mode |= FMODE_EXCL;
+		sb->mode |= FMODE_EXCL;
 
 	if (!opt_get(opts, nochanges))
-		ret->mode |= FMODE_WRITE;
+		sb->mode |= FMODE_WRITE;
 
-	err = bch2_blkdev_open(path, ret->mode, ret, &ret->bdev);
-	if (err)
-		return err;
+	sb->bdev = blkdev_get_by_path(path, sb->mode, sb);
+	if (IS_ERR(sb->bdev))
+		return PTR_ERR(sb->bdev);
 
 	err = "cannot allocate memory";
-	if (__bch2_super_realloc(ret, 0))
+	ret = __bch2_super_realloc(sb, 0);
+	if (ret)
 		goto err;
 
+	ret = -EFAULT;
 	err = "dynamic fault";
 	if (bch2_fs_init_fault("read_super"))
 		goto err;
 
-	err = read_one_super(ret, offset);
+	ret = -EINVAL;
+	err = read_one_super(sb, offset);
 	if (!err)
 		goto got_super;
 
-	if (offset != BCH_SB_SECTOR) {
-		pr_err("error reading superblock: %s", err);
+	if (opt_defined(opts, sb))
 		goto err;
-	}
 
 	pr_err("error reading default superblock: %s", err);
 
@@ -597,53 +577,57 @@ const char *bch2_read_super(const char *path,
 	 * Error reading primary superblock - read location of backup
 	 * superblocks:
 	 */
-	bio_reset(ret->bio);
-	bio_set_dev(ret->bio, ret->bdev);
-	ret->bio->bi_iter.bi_sector = BCH_SB_LAYOUT_SECTOR;
-	ret->bio->bi_iter.bi_size = sizeof(struct bch_sb_layout);
-	bio_set_op_attrs(ret->bio, REQ_OP_READ, REQ_SYNC|REQ_META);
+	bio_reset(sb->bio);
+	bio_set_dev(sb->bio, sb->bdev);
+	sb->bio->bi_iter.bi_sector = BCH_SB_LAYOUT_SECTOR;
+	sb->bio->bi_iter.bi_size = sizeof(struct bch_sb_layout);
+	bio_set_op_attrs(sb->bio, REQ_OP_READ, REQ_SYNC|REQ_META);
 	/*
 	 * use sb buffer to read layout, since sb buffer is page aligned but
 	 * layout won't be:
 	 */
-	bch2_bio_map(ret->bio, ret->sb);
+	bch2_bio_map(sb->bio, sb->sb);
 
 	err = "IO error";
-	if (submit_bio_wait(ret->bio))
+	if (submit_bio_wait(sb->bio))
 		goto err;
 
-	memcpy(&layout, ret->sb, sizeof(layout));
+	memcpy(&layout, sb->sb, sizeof(layout));
 	err = validate_sb_layout(&layout);
 	if (err)
 		goto err;
 
-	for (i = 0; i < layout.nr_superblocks; i++) {
-		u64 offset = le64_to_cpu(layout.sb_offset[i]);
+	for (i = layout.sb_offset;
+	     i < layout.sb_offset + layout.nr_superblocks; i++) {
+		offset = le64_to_cpu(*i);
 
-		if (offset == BCH_SB_SECTOR)
+		if (offset == opt_get(opts, sb))
 			continue;
 
-		err = read_one_super(ret, offset);
+		err = read_one_super(sb, offset);
 		if (!err)
 			goto got_super;
 	}
+
+	ret = -EINVAL;
 	goto err;
-got_super:
-	pr_debug("read sb version %llu, flags %llu, seq %llu, journal size %u",
-		 le64_to_cpu(ret->sb->version),
-		 le64_to_cpu(ret->sb->flags[0]),
-		 le64_to_cpu(ret->sb->seq),
-		 le32_to_cpu(ret->sb->u64s));
 
+got_super:
 	err = "Superblock block size smaller than device block size";
-	if (le16_to_cpu(ret->sb->block_size) << 9 <
-	    bdev_logical_block_size(ret->bdev))
+	ret = -EINVAL;
+	if (le16_to_cpu(sb->sb->block_size) << 9 <
+	    bdev_logical_block_size(sb->bdev))
 		goto err;
 
-	return NULL;
+	if (sb->mode & FMODE_WRITE)
+		bdev_get_queue(sb->bdev)->backing_dev_info->capabilities
+			|= BDI_CAP_STABLE_WRITES;
+
+	return 0;
 err:
-	bch2_free_super(ret);
-	return err;
+	bch2_free_super(sb);
+	pr_err("error reading superblock: %s", err);
+	return ret;
 }
 
 /* write superblock: */
diff --git a/fs/bcachefs/super-io.h b/fs/bcachefs/super-io.h
index e0dd26e31078..32ac0e875fa0 100644
--- a/fs/bcachefs/super-io.h
+++ b/fs/bcachefs/super-io.h
@@ -94,8 +94,7 @@ int bch2_super_realloc(struct bch_sb_handle *, unsigned);
 
 const char *bch2_sb_validate(struct bch_sb_handle *);
 
-const char *bch2_read_super(const char *, struct bch_opts,
-			    struct bch_sb_handle *);
+int bch2_read_super(const char *, struct bch_opts, struct bch_sb_handle *);
 void bch2_write_super(struct bch_fs *);
 
 /* BCH_SB_FIELD_journal: */
diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index c42629563686..6f541ddd6aa0 100644
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -1438,11 +1438,11 @@ int bch2_dev_add(struct bch_fs *c, const char *path)
 	struct bch_sb_field_members *mi, *dev_mi;
 	struct bch_member saved_mi;
 	unsigned dev_idx, nr_devices, u64s;
-	int ret = -EINVAL;
+	int ret;
 
-	err = bch2_read_super(path, bch2_opts_empty(), &sb);
-	if (err)
-		return -EINVAL;
+	ret = bch2_read_super(path, bch2_opts_empty(), &sb);
+	if (ret)
+		return ret;
 
 	err = bch2_sb_validate(&sb);
 	if (err)
@@ -1546,12 +1546,15 @@ int bch2_dev_online(struct bch_fs *c, const char *path)
 	struct bch_dev *ca;
 	unsigned dev_idx;
 	const char *err;
+	int ret;
 
 	mutex_lock(&c->state_lock);
 
-	err = bch2_read_super(path, bch2_opts_empty(), &sb);
-	if (err)
-		goto err;
+	ret = bch2_read_super(path, bch2_opts_empty(), &sb);
+	if (ret) {
+		mutex_unlock(&c->state_lock);
+		return ret;
+	}
 
 	dev_idx = sb.sb->dev_idx;
 
@@ -1685,33 +1688,33 @@ err:
 
 /* Filesystem open: */
 
-const char *bch2_fs_open(char * const *devices, unsigned nr_devices,
-			 struct bch_opts opts, struct bch_fs **ret)
+struct bch_fs *bch2_fs_open(char * const *devices, unsigned nr_devices,
+			    struct bch_opts opts)
 {
-	const char *err;
+	struct bch_sb_handle *sb = NULL;
 	struct bch_fs *c = NULL;
-	struct bch_sb_handle *sb;
 	unsigned i, best_sb = 0;
+	const char *err;
+	int ret = -ENOMEM;
 
 	if (!nr_devices)
-		return "need at least one device";
+		return ERR_PTR(-EINVAL);
 
 	if (!try_module_get(THIS_MODULE))
-		return "module unloading";
+		return ERR_PTR(-ENODEV);
 
-	err = "cannot allocate memory";
 	sb = kcalloc(nr_devices, sizeof(*sb), GFP_KERNEL);
 	if (!sb)
 		goto err;
 
 	for (i = 0; i < nr_devices; i++) {
-		err = bch2_read_super(devices[i], opts, &sb[i]);
-		if (err)
+		ret = bch2_read_super(devices[i], opts, &sb[i]);
+		if (ret)
 			goto err;
 
 		err = bch2_sb_validate(&sb[i]);
 		if (err)
-			goto err;
+			goto err_print;
 	}
 
 	for (i = 1; i < nr_devices; i++)
@@ -1722,10 +1725,10 @@ const char *bch2_fs_open(char * const *devices, unsigned nr_devices,
 	for (i = 0; i < nr_devices; i++) {
 		err = bch2_dev_in_fs(sb[best_sb].sb, sb[i].sb);
 		if (err)
-			goto err;
+			goto err_print;
 	}
 
-	err = "cannot allocate memory";
+	ret = -ENOMEM;
 	c = bch2_fs_alloc(sb[best_sb].sb, opts);
 	if (!c)
 		goto err;
@@ -1735,43 +1738,40 @@ const char *bch2_fs_open(char * const *devices, unsigned nr_devices,
 	for (i = 0; i < nr_devices; i++)
 		if (__bch2_dev_online(c, &sb[i])) {
 			mutex_unlock(&c->state_lock);
-			goto err;
+			goto err_print;
 		}
 	mutex_unlock(&c->state_lock);
 
 	err = "insufficient devices";
 	if (!bch2_fs_may_start(c))
-		goto err;
+		goto err_print;
 
 	if (!c->opts.nostart) {
 		err = __bch2_fs_start(c);
 		if (err)
-			goto err;
+			goto err_print;
 	}
 
 	err = bch2_fs_online(c);
 	if (err)
-		goto err;
+		goto err_print;
 
-	if (ret)
-		*ret = c;
-	else
-		closure_put(&c->cl);
-
-	err = NULL;
-out:
 	kfree(sb);
 	module_put(THIS_MODULE);
-	if (err)
-		c = NULL;
-	return err;
+	return c;
+err_print:
+	pr_err("bch_fs_open err opening %s: %s",
+	       devices[0], err);
+	ret = -EINVAL;
 err:
 	if (c)
 		bch2_fs_stop(c);
 
 	for (i = 0; i < nr_devices; i++)
 		bch2_free_super(&sb[i]);
-	goto out;
+	kfree(sb);
+	module_put(THIS_MODULE);
+	return ERR_PTR(ret);
 }
 
 static const char *__bch2_fs_open_incremental(struct bch_sb_handle *sb,
@@ -1842,9 +1842,8 @@ const char *bch2_fs_open_incremental(const char *path)
 	struct bch_opts opts = bch2_opts_empty();
 	const char *err;
 
-	err = bch2_read_super(path, opts, &sb);
-	if (err)
-		return err;
+	if (bch2_read_super(path, opts, &sb))
+		return "error reading superblock";
 
 	err = __bch2_fs_open_incremental(&sb, opts);
 	bch2_free_super(&sb);
diff --git a/fs/bcachefs/super.h b/fs/bcachefs/super.h
index 6f628830305c..a35ee3db0ea2 100644
--- a/fs/bcachefs/super.h
+++ b/fs/bcachefs/super.h
@@ -198,8 +198,7 @@ const char *bch2_fs_read_write(struct bch_fs *);
 void bch2_fs_stop(struct bch_fs *);
 
 const char *bch2_fs_start(struct bch_fs *);
-const char *bch2_fs_open(char * const *, unsigned, struct bch_opts,
-			struct bch_fs **);
+struct bch_fs *bch2_fs_open(char * const *, unsigned, struct bch_opts);
 const char *bch2_fs_open_incremental(const char *path);
 
 #endif /* _BCACHEFS_SUPER_H */