On Mon, Feb 19, 2024 at 08:24:32PM +0800, Hongbo Li wrote: > When mount with incorrect options such as: > "mount -t bcachefs -o errors=back /dev/loop1 /mnt/bcachefs/". > It rebacks the error "mount: /mnt/bcachefs: permission denied." > cause bch2_parse_mount_opts returns -1 and bch2_mount throws > it up. This is unreasonable. > > The real error message should be like this: > "mount: /mnt/bcachefs: wrong fs type, bad option, bad > superblock on /dev/loop1, missing codepage or helper program, > or other error." > > Adding three private error codes for mounting error. Here are: > - BCH_ERR_mount_option as the parent class for option error. > - BCH_ERR_option_name represents the invalid option name. > - BCH_ERR_option_value represents the invalid option value. > > Signed-off-by: Hongbo Li <[email protected]>
thanks! I'll give this more detailed review tomorrow but at a glance, looks good. > --- > Thanks for reviewing my patch. Here is my new changelog: > > v2: > - return private error codes for internal functions. > v1: > https://lore.kernel.org/linux-bcachefs/b3edruyblko7tbpbbpftov75tmnmwir4mx7m2mwpdzzaylt3av@ttwlfwzzoc6s/T/#t > --- > fs/bcachefs/errcode.h | 3 +++ > fs/bcachefs/fs.c | 4 +++- > fs/bcachefs/opts.c | 6 +++--- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h > index fe3fc14d3c9a..3653ff2c0c12 100644 > --- a/fs/bcachefs/errcode.h > +++ b/fs/bcachefs/errcode.h > @@ -5,6 +5,9 @@ > #define BCH_ERRCODES() > \ > x(ERANGE, ERANGE_option_too_small) > \ > x(ERANGE, ERANGE_option_too_big) > \ > + x(EINVAL, mount_option) > \ > + x(BCH_ERR_mount_option, option_name) > \ > + x(BCH_ERR_mount_option, option_value) > \ > x(ENOMEM, ENOMEM_stripe_buf) > \ > x(ENOMEM, ENOMEM_replicas_table) > \ > x(ENOMEM, ENOMEM_cpu_replicas) > \ > diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c > index 093f5404a655..3f073845bbd7 100644 > --- a/fs/bcachefs/fs.c > +++ b/fs/bcachefs/fs.c > @@ -1870,8 +1870,10 @@ static struct dentry *bch2_mount(struct > file_system_type *fs_type, > opt_set(opts, read_only, (flags & SB_RDONLY) != 0); > > ret = bch2_parse_mount_opts(NULL, &opts, data); > - if (ret) > + if (ret) { > + ret = bch2_err_class(ret); > return ERR_PTR(ret); > + } > > if (!dev_name || strlen(dev_name) == 0) > return ERR_PTR(-EINVAL); > diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c > index b1ed0b9a20d3..1db11c15b2b9 100644 > --- a/fs/bcachefs/opts.c > +++ b/fs/bcachefs/opts.c > @@ -456,7 +456,7 @@ int bch2_parse_mount_opts(struct bch_fs *c, struct > bch_opts *opts, > > copied_opts = kstrdup(options, GFP_KERNEL); > if (!copied_opts) > - return -1; > + return -ENOMEM; > copied_opts_start = copied_opts; > > while ((opt = strsep(&copied_opts, ",")) != NULL) { > @@ -501,11 +501,11 @@ int bch2_parse_mount_opts(struct bch_fs *c, struct > bch_opts *opts, > > bad_opt: > pr_err("Bad mount option %s", name); > - ret = -1; > + ret = -BCH_ERR_option_name; > goto out; > bad_val: > pr_err("Invalid mount option %s", err.buf); > - ret = -1; > + ret = -BCH_ERR_option_value; > goto out; > out: > kfree(copied_opts_start); > -- > 2.34.1 >
