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(_fs_type, test_gfs2_super, set_meta_super, flags,
+ s = sget(_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(_fs_type, test_gfs2_super, set_meta_super, flags,
> + s = sget(_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
