Re: [PATCH 1/2] generic/020: adjust max_attrval_size for ceph

Xiubo Li
2022-06-08T09:19:48Z

On 6/7/22 11:15 PM, Luís Henriques wrote:
> CephFS doesn't had a maximum xattr size. Instead, it imposes a maximum
> size for the full set of an inode's xattrs names+values, which by default
> is 64K but it can be changed by a cluster admin.
>
> Adjust max_attrval_size so that the test can be executed in this
> filesystem.
>
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> ---
> tests/generic/020 | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tests/generic/020 b/tests/generic/020
> index d8648e96286e..cadfce5f45e3 100755
> --- a/tests/generic/020
> +++ b/tests/generic/020
> @@ -128,15 +128,16 @@ _attr_get_max()
> pvfs2)
> max_attrval_size=8192
> ;;
> - xfs|udf|9p|ceph)
> + xfs|udf|9p)
> max_attrval_size=65536
> ;;
> bcachefs)
> max_attrval_size=1024
> ;;
> - nfs)
> - # NFS doesn't provide a way to find out the max_attrval_size for
> - # the underlying filesystem, so just use the lowest value above.
> + nfs|ceph)
> + # NFS and CephFS don't provide a way to find out the
> + # max_attrval_size for the underlying filesystem, so just use
> + # the lowest value above.
> max_attrval_size=1024
> ;;
> *)

Why not fixing this by making sure that the total length of 'name' +
'value' == 64K instead for ceph case ?

IMO we shouldn't worry about the case that the max could be changeable,
we just need to test the framework works well with the default is
enough. And then print a warning if the test fails to let users to know
that the max size must be as default, which is 64K, or if users didn't
change it then it should be a real bug in ceph.