Re: [LTP] [PATCH v2] ioctl_fiemap01: New test for fiemap ioctl()

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

> Fixes: #535

> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c
> new file mode 100644
> index 000000000..a626bb03c
> --- /dev/null
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify basic fiemap ioctl
> + *
nit: missing space and dot at the end.
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/fiemap.h>
> +#include <stdlib.h>
> +
> +#include "tst_test.h"
> +
> +#define TESTFILE "testfile"
> +#define NUM_EXTENT 2
> +#define FILE_OFFSET ((rand() % 8 + 2) * getpagesize())
> +
> +static char *buf;
> +
> +static void print_extens(struct fiemap *fiemap)
> +{
> +
nit: please no space above.
> +	tst_res(TDEBUG, "File extent count: %u", fiemap->fm_mapped_extents);
nit: please add space here (readability)
> +	for (unsigned int i = 0; i < fiemap->fm_mapped_extents; ++i) {
> +		tst_res(TDEBUG, "Extent %u: Logical offset: %llu, Physical offset: %llu, flags: %u, Length: %llu",
> +				i + 1,
> +				fiemap->fm_extents[i].fe_logical,
> +				fiemap->fm_extents[i].fe_physical,
> +				fiemap->fm_extents[i].fe_flags,
> +				fiemap->fm_extents[i].fe_length);
> +	}
> +}
> +
> +static void verify_ioctl(void)
> +{
> +	int fd;
> +	struct fiemap *fiemap;
> +
> +	fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, 0644);
> +
> +	fiemap = SAFE_MALLOC(sizeof(struct fiemap) + sizeof(struct fiemap_extent) * NUM_EXTENT);
> +	fiemap->fm_start = 0;
> +	fiemap->fm_length = ~0ULL;
> +	fiemap->fm_extent_count = 1;
> +
> +	fiemap->fm_flags =  -1;
> +	TST_EXP_FAIL(ioctl(fd, FS_IOC_FIEMAP, fiemap), EBADR);

I get on Tumbleweed with 6.8.0-rc4-1.g9b23bf2-default EOPNOTSUPP:
ioctl_fiemap01.c:52: TFAIL: ioctl(fd, FS_IOC_FIEMAP, fiemap) expected EBADR: EOPNOTSUPP (95)
What is the missing dependency for FS_IOC_FIEMAP? Or is there wrong fiemap use
which causes that?

> +
> +	fiemap->fm_flags =  0;
> +	SAFE_IOCTL(fd, FS_IOC_FIEMAP, fiemap);
And on the same kernel another EOPNOTSUPP:

ioctl_fiemap01.c:55: TBROK: ioctl(3,((((2U|1U) << (((0+8)+8)+14)) | ((('f')) << (0+8)) | (((11)) << 0) | ((((sizeof(struct fiemap)))) << ((0+8)+8)))),...) failed: EOPNOTSUPP (95)
> +	print_extens(fiemap);
> +	if (fiemap->fm_mapped_extents == 0)
> +		tst_res(TPASS, "Check fiemap iotct zero fm_mapped_extents pass");
s/iotct/ioctl/

> +	else
> +		tst_res(TFAIL, "Check fiemap iotct zero fm_mapped_extents failed");
s/iotct/ioctl/
> +
> +	SAFE_WRITE(SAFE_WRITE_ANY, fd, buf, getpagesize());
> +	SAFE_IOCTL(fd, FS_IOC_FIEMAP, fiemap);
> +	print_extens(fiemap);
Maybe print only on failure?
> +	if ((fiemap->fm_mapped_extents == 1) && (fiemap->fm_extents[0].fe_physical == 0))
NOTE: brackets are not needed (== has higher precedence than &&), thus:
	if (fiemap->fm_mapped_extents == 1 && fiemap->fm_extents[0].fe_physical == 0)

> +		tst_res(TPASS, "Check fiemap iotct one fm_mapped_extents pass");
s/iotct/ioctl/
> +	else
> +		tst_res(TFAIL, "Check fiemap iotct one fm_mapped_extents failed");
s/iotct/ioctl/
> +
> +	fiemap->fm_flags = FIEMAP_FLAG_SYNC;
> +	SAFE_IOCTL(fd, FS_IOC_FIEMAP, fiemap);
> +	print_extens(fiemap);
> +	if ((fiemap->fm_mapped_extents == 1) &&
nit: again == does not need to be in ()
> +		(fiemap->fm_extents[0].fe_flags == FIEMAP_EXTENT_LAST) &&
> +		(fiemap->fm_extents[0].fe_physical > 0) &&
> +		(fiemap->fm_extents[0].fe_length == (__u64)getpagesize()))
> +		tst_res(TPASS, "Check fiemap iotct FIEMAP_FLAG_SYNC fm_flags pass");
s/iotct/ioctl/
> +	else
> +		tst_res(TFAIL, "Check fiemap iotct FIEMAP_FLAG_SYNC fm_flags failed");
s/iotct/ioctl/
There are 4x checks, I guess instead it would be worth to write what exactly failed.

> +
> +	fiemap->fm_extent_count = NUM_EXTENT;
> +	srand(time(NULL));
> +	SAFE_LSEEK(fd, FILE_OFFSET, SEEK_SET);
> +	SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, getpagesize());
> +	SAFE_IOCTL(fd, FS_IOC_FIEMAP, fiemap);
> +	print_extens(fiemap);
> +	if ((fiemap->fm_mapped_extents == NUM_EXTENT) &&
nit: If this check was repeated more than twice, I would put it into separate
function.
> +		(fiemap->fm_extents[NUM_EXTENT - 1].fe_flags == FIEMAP_EXTENT_LAST) &&
> +		(fiemap->fm_extents[NUM_EXTENT - 1].fe_physical > 0) &&
> +		(fiemap->fm_extents[NUM_EXTENT - 1].fe_length == (__u64)getpagesize()))
> +		tst_res(TPASS, "Check fiemap iotct multiple fm_mapped_extents pass");
> +	else
> +		tst_res(TFAIL, "Check fiemap iotct multiple fm_mapped_extents failed");
> +
> +	free(fiemap);
> +	SAFE_CLOSE(fd);
> +	unlink(TESTFILE);
> +}
...

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp