Re: [PATCH 1/2] gfs2: Convert gfs2 to fs_context

[Andrew Price <anprice@redhat.com>]

On 18/03/2019 13:18, David Howells wrote:
> Andrew Price <anprice@redhat.com> wrote:
> 
>> sget() is still used instead of sget_fc() as there doesn't seem to be an
>> obvious replacement for the bdev pointer propagation to the test/set
>> functions (yet?)
> 
> Umm...  What about the fs_context struct?  Why can't that be used to propagate
> the bdev pointer?  That's kind of what it's for...

It would be useful to have the block device pointer in the fs_context 
since so many of the filesystems use them and it makes for an obvious 
API migration.

> 	struct super_block *sget_fc(
> 		struct fs_context *fc,
> 		int (*test)(struct super_block *, struct fs_context *),
> 		int (*set)(struct super_block *, struct fs_context *))
> 
> It looks like you should be able to stash the bdev pointer in the gfs2_args
> struct.

Sure, but since the new API is young I figured I'd hold off until we had 
this conversation because adding it to the fs_context might be agreeable :)

>> +	fsparam_s32    ("commit",             Opt_commit),
>> +	fsparam_s32    ("statfs_quantum",     Opt_statfs_quantum),
>> +	fsparam_s32    ("statfs_percent",     Opt_statfs_percent),
> 
> Why s32?  Why not u32?

They were signed before, I guess because if the user passes a negative 
number it allows us to range check properly and provide a sensible error 
message. Is there an issue with them being s32 that I hadn't considered?

Thanks,
Andy