On Mon, Feb 19, 2024 at 05:54:33PM +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."
Thanks for the patch. As a general policy though, we're not adding new direct references to the standard error codes (except - the -ENOMEM is fine here). Instead of returning -EINVAL, you'll want to define new errcodes in errcode.h; the left field is the errcode it derives from, by specifying EINVAL there it'll be translated to -EINVAL at the module boundary. Also, our private error codes should generally only be thrown at one source code location, not reused for multiple things. This makes for _much_ better error messages and easier debugging - nothing is worse than chasing down a -EINVAL with inadequate loging. Cheers, Kent > > Signed-off-by: Hongbo Li <[email protected]> > --- > fs/bcachefs/opts.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/bcachefs/opts.c b/fs/bcachefs/opts.c > index b1ed0b9a20d3..a9ee9eaab670 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 = -EINVAL; > goto out; > bad_val: > pr_err("Invalid mount option %s", err.buf); > - ret = -1; > + ret = -EINVAL; > goto out; > out: > kfree(copied_opts_start); > -- > 2.34.1 >
