Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
Andrew Price wrote: > Would this fly? Can you update the comment to document what it's for? David
Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
On 19/03/2019 17:53, Andrew Price wrote: On 19/03/2019 17:05, David Howells wrote: Can you use vfs_get_block_super()? It might be possible if we can rearrange things so that this can be done outside of the function: if (args->ar_meta) fc->root = dget(sdp->sd_master_dir); else fc->root = dget(sdp->sd_root_dir); but we can't do that in our fill_super() because it needs to be selected whether we have an existing mount or not. Would this fly? diff --git a/fs/super.c b/fs/super.c index 6d8dbf309241..0cdeaf28126f 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1232,11 +1232,12 @@ static int test_bdev_super_fc(struct super_block *s, struct fs_context *fc) * @fill_super: Helper to initialise a new superblock */ int vfs_get_block_super(struct fs_context *fc, - int (*fill_super)(struct super_block *, - struct fs_context *)) + int (*fill_super)(struct super_block *, struct fs_context *), + struct dentry* (*select_root)(struct fs_context *, struct super_block *)) { struct block_device *bdev; struct super_block *s; + struct dentry *root; int error = 0; fc->bdev_mode = FMODE_READ | FMODE_EXCL; @@ -1302,7 +1303,11 @@ int vfs_get_block_super(struct fs_context *fc, } BUG_ON(fc->root); - fc->root = dget(s->s_root); + if (select_root) + root = select_root(fc, s); + else + root = s->s_root; + fc->root = dget(root); return 0; error_sb: diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h index 8233c873af73..c2e9dc5826ce 100644 --- a/include/linux/fs_context.h +++ b/include/linux/fs_context.h @@ -145,8 +145,8 @@ extern int vfs_get_super(struct fs_context *fc, struct fs_context *fc)); extern int vfs_get_block_super(struct fs_context *fc, - int (*fill_super)(struct super_block *sb, -struct fs_context *fc)); + int (*fill_super)(struct super_block *, struct fs_context *), + struct dentry* (*select_root)(struct fs_context *, struct super_block *)); extern const struct file_operations fscontext_fops;
Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
On 19/03/2019 17:05, David Howells wrote: Andrew Price wrote: + pr_warn("-o debug and -o errors=panic are mutually exclusive\n"); + return -EINVAL; return invalf(fc, "gfs2: -o debug and -o errors=panic are mutually exclusive"); Thanks. (Note: no "\n") + if (result.int_32 > 0) + args->ar_quota = opt_quota_values[result.int_32]; + else if (result.negated) + args->ar_quota = GFS2_QUOTA_OFF; + else + args->ar_quota = GFS2_QUOTA_ON; I recommend checking result.negated first. OK + /* Not allowed to change locking details */ + if (strcmp(newargs->ar_lockproto, oldargs->ar_lockproto) || + strcmp(newargs->ar_locktable, oldargs->ar_locktable) || + strcmp(newargs->ar_hostdata, oldargs->ar_hostdata)) + return -EINVAL; Use errorf(). (Not invalf - the parameter isn't exactly invalid, it's just that you're not allowed to do this operation). Yes, that makes more sense. + error = gfs2_make_fs_ro(sdp); + else + error = gfs2_make_fs_rw(sdp); + if (error) + return error; Might want to call errorf() here too. - s = sget(&gfs2_fs_type, test_gfs2_super, set_meta_super, flags, + s = sget(&gfs2_fs_type, test_meta_super, set_meta_super, flags, Try and use sget_fc() please. That happens in patch 2/2 for gfs2meta. I might just roll both patches together in v3 so there's no intermediate churn. If you look at the fuse patchset I cc'd you on, there's a commit there that adds a ->bdev and ->bdev_mode to fs_context that may be of use to you. Yes, that looks useful - thanks. Can you use vfs_get_block_super()? It might be possible if we can rearrange things so that this can be done outside of the function: if (args->ar_meta) fc->root = dget(sdp->sd_master_dir); else fc->root = dget(sdp->sd_root_dir); but we can't do that in our fill_super() because it needs to be selected whether we have an existing mount or not. Would it be of use to export test_bdev_super_fc() and set_bdev_super_fc()? There's only a little maintenance value in it, I think, but we could certainly use them in gfs2 if we can't solve the fc->root selection issue. Andy
Re: [Cluster-devel] [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
Andrew Price wrote: > + pr_warn("-o debug and -o errors=panic are mutually > exclusive\n"); > + return -EINVAL; return invalf(fc, "gfs2: -o debug and -o errors=panic are mutually exclusive"); (Note: no "\n") > + if (result.int_32 > 0) > + args->ar_quota = opt_quota_values[result.int_32]; > + else if (result.negated) > + args->ar_quota = GFS2_QUOTA_OFF; > + else > + args->ar_quota = GFS2_QUOTA_ON; I recommend checking result.negated first. > + /* Not allowed to change locking details */ > + if (strcmp(newargs->ar_lockproto, oldargs->ar_lockproto) || > + strcmp(newargs->ar_locktable, oldargs->ar_locktable) || > + strcmp(newargs->ar_hostdata, oldargs->ar_hostdata)) > + return -EINVAL; Use errorf(). (Not invalf - the parameter isn't exactly invalid, it's just that you're not allowed to do this operation). > + error = gfs2_make_fs_ro(sdp); > + else > + error = gfs2_make_fs_rw(sdp); > + if (error) > + return error; Might want to call errorf() here too. > - s = sget(&gfs2_fs_type, test_gfs2_super, set_meta_super, flags, > + s = sget(&gfs2_fs_type, test_meta_super, set_meta_super, flags, Try and use sget_fc() please. If you look at the fuse patchset I cc'd you on, there's a commit there that adds a ->bdev and ->bdev_mode to fs_context that may be of use to you. Can you use vfs_get_block_super()? Would it be of use to export test_bdev_super_fc() and set_bdev_super_fc()? David