Re: [PATCH 4/4] btrfs: balance: factor out convert profile validation

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3009 bytes --]



On 2020/2/28 上午4:00, David Sterba wrote:
> The validation follows the same steps for all three block group types,
> the existing helper validate_convert_profile can be enhanced and do more
> of the common things.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>  fs/btrfs/volumes.c | 45 +++++++++++++++++++++------------------------
>  1 file changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 3d35466f34b0..b5d7dc561b68 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3762,13 +3762,25 @@ static inline int balance_need_close(struct btrfs_fs_info *fs_info)
>  		 atomic_read(&fs_info->balance_cancel_req) == 0);
>  }
>  
> -/* Non-zero return value signifies invalidity */
> -static inline int validate_convert_profile(struct btrfs_balance_args *bctl_arg,
> -		u64 allowed)
> +/*
> + * Validate target profile against allowed profiles and return true if it's OK.
> + * Otherwise print the error message and return false.
> + */
> +static inline int validate_convert_profile(struct btrfs_fs_info *fs_info,
> +		const struct btrfs_balance_args *bargs,
> +		u64 allowed, const char *type)
>  {
> -	return ((bctl_arg->flags & BTRFS_BALANCE_ARGS_CONVERT) &&
> -		(!alloc_profile_is_valid(bctl_arg->target, 1) ||
> -		 (bctl_arg->target & ~allowed)));
> +	if (!(bargs->flags & BTRFS_BALANCE_ARGS_CONVERT))
> +		return true;
> +
> +	/* Profile is valid and does not have bits outside of the allowed set */
> +	if (alloc_profile_is_valid(bargs->target, 1) &&
> +	    (bargs->target & ~allowed) == 0)
> +		return true;
> +
> +	btrfs_err(fs_info, "balance: invalid convert %s profile %s",
> +			type, btrfs_bg_type_to_raid_name(bargs->target));
> +	return false;
>  }
>  
>  /*
> @@ -3984,24 +3996,9 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>  		if (num_devices >= btrfs_raid_array[i].devs_min)
>  			allowed |= btrfs_raid_array[i].bg_flag;
>  
> -	if (validate_convert_profile(&bctl->data, allowed)) {
> -		btrfs_err(fs_info,
> -			  "balance: invalid convert data profile %s",
> -			  btrfs_bg_type_to_raid_name(bctl->data.target));
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -	if (validate_convert_profile(&bctl->meta, allowed)) {
> -		btrfs_err(fs_info,
> -			  "balance: invalid convert metadata profile %s",
> -			  btrfs_bg_type_to_raid_name(bctl->meta.target));
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -	if (validate_convert_profile(&bctl->sys, allowed)) {
> -		btrfs_err(fs_info,
> -			  "balance: invalid convert system profile %s",
> -			  btrfs_bg_type_to_raid_name(bctl->sys.target));
> +	if (!validate_convert_profile(fs_info, &bctl->data, allowed, "data") ||
> +	    !validate_convert_profile(fs_info, &bctl->meta, allowed, "metadata") ||
> +	    !validate_convert_profile(fs_info, &bctl->sys,  allowed, "system")) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> 


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