Re: [PATCH v3 2/2] generic/486: adjust the max xattr size

[David Disseldorp <ddiss@suse.de>]

On Mon, 13 Jun 2022 12:31:42 +0100, Luís Henriques wrote:

> CephFS doesn't have a maximum xattr size.  Instead, it imposes a maximum
> size for the full set of xattrs names+values, which by default is 64K.  And
> since it reports 4M as the blocksize (the default ceph object size),
> generic/486 will fail in ceph because the XATTR_SIZE_MAX value can't be used
> in attr_replace_test.
> 
> The fix is to add a new argument to the test so that the max size can be
> passed in instead of trying to auto-probe a value for it.
> 
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> ---
>  src/attr_replace_test.c | 30 ++++++++++++++++++++++++++----
>  tests/generic/486       | 11 ++++++++++-
>  2 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/src/attr_replace_test.c b/src/attr_replace_test.c
> index cca8dcf8ff60..1218e7264c8f 100644
> --- a/src/attr_replace_test.c
> +++ b/src/attr_replace_test.c
> @@ -20,19 +20,41 @@ exit(1); } while (0)
>  fprintf(stderr, __VA_ARGS__); exit (1); \
>  } while (0)
>  
> +void usage(char *progname)
> +{
> +	fail("usage: %s [-m max_attr_size] <file>\n", progname);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	int ret;
>  	int fd;
> +	int c;
>  	char *path;
>  	char *name = "user.world";
>  	char *value;
>  	struct stat sbuf;
>  	size_t size = sizeof(value);
> +	size_t maxsize = XATTR_SIZE_MAX;
> +
> +	while ((c = getopt(argc, argv, "m:")) != -1) {
> +		char *endp;
> +
> +		switch (c) {
> +		case 'm':
> +			maxsize = strtoul(optarg, &endp, 0);
> +			if (*endp || (maxsize > XATTR_SIZE_MAX))
> +				fail("Invalid 'max_attr_size' value\n");
> +			break;
> +		default:
> +			usage(argv[0]);
> +		}
> +	}
>  
> -	if (argc != 2)
> -		fail("Usage: %s <file>\n", argv[0]);
> -	path = argv[1];
> +	if (optind == argc - 1)
> +		path = argv[optind];
> +	else
> +		usage(argv[0]);
>  
>  	fd = open(path, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
>  	if (fd < 0) die();
> @@ -46,7 +68,7 @@ int main(int argc, char *argv[])
>  	size = sbuf.st_blksize * 3 / 4;
>  	if (!size)
>  		fail("Invalid st_blksize(%ld)\n", sbuf.st_blksize);
> -	size = MIN(size, XATTR_SIZE_MAX);
> +	size = MIN(size, maxsize);
>  	value = malloc(size);
>  	if (!value)
>  		fail("Failed to allocate memory\n");
> diff --git a/tests/generic/486 b/tests/generic/486
> index 7de198f93a71..7dbfcb9835d9 100755
> --- a/tests/generic/486
> +++ b/tests/generic/486
> @@ -41,7 +41,16 @@ filter_attr_output() {
>  		sed -e 's/has a [0-9]* byte value/has a NNNN byte value/g'
>  }
>  
> -$here/src/attr_replace_test $SCRATCH_MNT/hello
> +max_attr_size=65536
> +
> +# attr_replace_test can't easily auto-probe the attr size for ceph because:
> +# - ceph imposes a maximum value for the total xattr names+values, and
> +# - ceph reports the 'object size' in the block size, which is, by default, much
> +#   larger than XATTR_SIZE_MAX (4M > 64k)
> +# Hence, we need to provide it with a maximum size.
> +[ "$FSTYP" = "ceph" ] && max_attr_size=65000
> +
> +$here/src/attr_replace_test -m $max_attr_size $SCRATCH_MNT/hello
>  $ATTR_PROG -l $SCRATCH_MNT/hello >>$seqres.full 2>&1
>  $ATTR_PROG -l $SCRATCH_MNT/hello | filter_attr_output

Looks okay as an alternative to going through a 73aa648c
("generic/020: move MAX_ATTRS and MAX_ATTRVAL_SIZE logic") revert.

Reviewed-by: David Disseldorp <ddiss@suse.de>