Re: [PATCH v2] bcachefs: fix the error code when mounting with incorrect options.

[Hongbo Li]

ok, I add mount option error code here.

On 2024/2/19 20:27, Kent Overstreet wrote:

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 <lihongb...@huawei.com>

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