Re: [PATCH 1/4] xfs: clean up the XFS_IOC_{GS}ET_RESBLKS handler

["Darrick J. Wong" <djwong@kernel.org>]

On Sun, Nov 26, 2023 at 02:01:21PM +0100, Christoph Hellwig wrote:
> The XFS_IOC_GET_RESBLKS and XFS_IOC_SET_RESBLKS already share a fair
> amount of code, and will share even more soon.  Move the logic for both
> of them out of the main xfs_file_ioctl function into a
> xfs_ioctl_getset_resblocks helper to share the code and prepare for
> additional changes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_ioctl.c | 87 +++++++++++++++++++++++-----------------------
>  1 file changed, 43 insertions(+), 44 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index a82470e027f727..8faaf2ef67a7b8 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1862,6 +1862,46 @@ xfs_fs_eofblocks_from_user(
>  	return 0;
>  }
>  
> +static int
> +xfs_ioctl_getset_resblocks(
> +	struct file		*filp,
> +	unsigned int		cmd,
> +	void __user		*arg)
> +{
> +	struct xfs_mount	*mp = XFS_I(file_inode(filp))->i_mount;
> +	struct xfs_fsop_resblks	fsop = { };
> +	int			error;
> +	uint64_t		in;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (cmd == XFS_IOC_SET_RESBLKS) {
> +		if (xfs_is_readonly(mp))
> +			return -EROFS;
> +
> +		if (copy_from_user(&fsop, arg, sizeof(fsop)))
> +			return -EFAULT;
> +
> +		error = mnt_want_write_file(filp);
> +		if (error)
> +			return error;
> +		in = fsop.resblks;
> +		error = xfs_reserve_blocks(mp, &in, &fsop);

<shudder> what an interface...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> +		mnt_drop_write_file(filp);
> +		if (error)
> +			return error;
> +	} else {
> +		error = xfs_reserve_blocks(mp, NULL, &fsop);
> +		if (error)
> +			return error;
> +	}
> +
> +	if (copy_to_user(arg, &fsop, sizeof(fsop)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
>  /*
>   * These long-unused ioctls were removed from the official ioctl API in 5.17,
>   * but retain these definitions so that we can log warnings about them.
> @@ -2008,50 +2048,9 @@ xfs_file_ioctl(
>  		return 0;
>  	}
>  
> -	case XFS_IOC_SET_RESBLKS: {
> -		xfs_fsop_resblks_t inout;
> -		uint64_t	   in;
> -
> -		if (!capable(CAP_SYS_ADMIN))
> -			return -EPERM;
> -
> -		if (xfs_is_readonly(mp))
> -			return -EROFS;
> -
> -		if (copy_from_user(&inout, arg, sizeof(inout)))
> -			return -EFAULT;
> -
> -		error = mnt_want_write_file(filp);
> -		if (error)
> -			return error;
> -
> -		/* input parameter is passed in resblks field of structure */
> -		in = inout.resblks;
> -		error = xfs_reserve_blocks(mp, &in, &inout);
> -		mnt_drop_write_file(filp);
> -		if (error)
> -			return error;
> -
> -		if (copy_to_user(arg, &inout, sizeof(inout)))
> -			return -EFAULT;
> -		return 0;
> -	}
> -
> -	case XFS_IOC_GET_RESBLKS: {
> -		xfs_fsop_resblks_t out;
> -
> -		if (!capable(CAP_SYS_ADMIN))
> -			return -EPERM;
> -
> -		error = xfs_reserve_blocks(mp, NULL, &out);
> -		if (error)
> -			return error;
> -
> -		if (copy_to_user(arg, &out, sizeof(out)))
> -			return -EFAULT;
> -
> -		return 0;
> -	}
> +	case XFS_IOC_SET_RESBLKS:
> +	case XFS_IOC_GET_RESBLKS:
> +		return xfs_ioctl_getset_resblocks(filp, cmd, arg);
>  
>  	case XFS_IOC_FSGROWFSDATA: {
>  		struct xfs_growfs_data in;
> -- 
> 2.39.2
> 
>