Re: [PATCH] btrfs: add ability to query/change feature bits online

[Jeff Mahoney <jeffm@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 3252 bytes --]

On 9/3/13 5:25 AM, Stefan Behrens wrote:
> On Tue, 27 Aug 2013 15:17:54 -0400, Jeff Mahoney wrote:
>> There are some feature bits that require no offline setup and can
>> be enabled online. I've only reviewed extended irefs, but there will
>> probably be more.
>>
>> We introduce three new ioctls:
>> - BTRFS_IOC_GET_SUPPORTED_FEATURES: query the kernel for supported features
>> - BTRFS_IOC_GET_FEATURES: query the kernel for enabled features on a per-fs
>>   basis.
>> - BTRFS_IOC_ADD_FEATURES: enable new features on a per-fs basis.
>>
>> We introduce a new mask _SAFE_ONLINE that allows us to define which
>> features are safe to enable at runtime.
>>
>> The failure modes for BTRFS_IOC_ADD_FEATURES are as follows:
>> - Enabling a completely unsupported feature: warns and returns -ENOTSUPP
>> - Enabling a feature that can only be done offline: warns and returns -EPERM
>>
>> The structure passed in is not modified on return since it is possible
>> to isolate which features would bounce a -EPERM by subtracting the features
>> provided by a GET_SUPPORTED_FEATURES call.
>>
>> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> [...]
>> --- a/include/uapi/linux/btrfs.h	2013-08-27 11:02:35.062626912 -0400
>> +++ b/include/uapi/linux/btrfs.h	2013-08-27 11:03:32.006352802 -0400
>> @@ -184,6 +184,12 @@ struct btrfs_ioctl_fs_info_args {
>>  	__u64 reserved[124];			/* pad to 1k */
>>  };
>>  
>> +struct btrfs_ioctl_feature_flags {
>> +	__u64 compat_flags;
>> +	__u64 compat_ro_flags;
>> +	__u64 incompat_flags;
>> +};
>> +
>>  /* balance control ioctl modes */
>>  #define BTRFS_BALANCE_CTL_PAUSE		1
>>  #define BTRFS_BALANCE_CTL_CANCEL	2
>> @@ -579,4 +585,10 @@ static inline char *btrfs_err_str(enum b
>>  				      struct btrfs_ioctl_get_dev_stats)
>>  #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
>>  				    struct btrfs_ioctl_dev_replace_args)
>> +#define BTRFS_IOC_GET_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 54, \
>> +				   struct btrfs_ioctl_feature_flags)
>> +#define BTRFS_IOC_ADD_FEATURES _IOW(BTRFS_IOCTL_MAGIC, 55, \
>> +				   struct btrfs_ioctl_feature_flags)
> 
> #define BTRFS_IOC_ADD_FEATURES _IOW(BTRFS_IOCTL_MAGIC, 54, \
> 				   struct btrfs_ioctl_feature_flags)
> 
> Using 54 for both would also be possible since the directions are
> different. _IOR(54) for the get ioctl and _IOW(54) for the set/add ioctl.
> 
> 
>> +#define BTRFS_IOC_GET_SUPPORTED_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 56, \
>> +				   struct btrfs_ioctl_feature_flags[2])
> 
> This could share the type value 54 too, since the parameter size is
> different. There are only 8 bits for the type field.

Ah, yep. You're right.

> And in order to support clearing feature bits you could replace the add
> ioctl with a set ioctl and use a mask + value scheme.
> #define BTRFS_IOC_SET_FEATURES _IOW(BTRFS_IOCTL_MAGIC, 54, \
> 				   struct btrfs_ioctl_feature_flags[2])
> btrfs_ioctl_feature_flags[0] is the mask and
> btrfs_ioctl_feature_flags[1] is the value. Only bits that are set to one
> in the mask are taken from the value and modified in the filesystem flags.

Yep. Though for now I think I'd implement the clear as an EOPNOTSUPP.

-Jeff

-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 841 bytes --]