Re: [PATCH v4 1/3] common/xfs: add _require_xfs_scratch_shrink helper

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

On Fri, Apr 02, 2021 at 05:49:35PM +0800, Gao Xiang wrote:
> In order to detect whether the current kernel supports XFS shrinking.
> 
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
>  common/xfs | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 69f76d6e..c6c2e3f5 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -766,6 +766,20 @@ _require_xfs_mkfs_without_validation()
>  	fi
>  }
>  
> +_require_xfs_scratch_shrink()
> +{
> +	_require_scratch
> +	_require_command "$XFS_GROWFS_PROG" xfs_growfs
> +
> +	_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
> +	. $tmp.mkfs
> +	_scratch_mount
> +	# here just to check if kernel supports, no need do more extra work
> +	$XFS_GROWFS_PROG -D$((dblocks-1)) "$SCRATCH_MNT" > /dev/null 2>&1 || \
> +		_notrun "kernel does not support shrinking"

I think isn't sufficiently precise -- if xfs_growfs (userspace) doesn't
support shrinking it'll error out with "data size XXX too small", and if
the kernel doesn't support shrink, it'll return EINVAL.

As written, this code attempts a single-block shrink and disables the
entire test if that fails for any reason, even if that reason is that
the last block in the filesystem isn't free, or we ran out of memory, or
something like that.

I think this needs to check the output of xfs_growfs to make the
decision to _notrun.

--D

> +	_scratch_unmount
> +}
> +
>  # XFS ability to change UUIDs on V5/CRC filesystems
>  #
>  _require_meta_uuid()
> -- 
> 2.27.0
>