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

[Wei Gao]

On Wed, Feb 28, 2024 at 06:07:29PM +0100, Petr Vorel wrote:
> 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?
Thanks for your test and i also reproduce this issue.
This is caused by Tumbleweed mount tmpfs on /tmp and tmpfs seems not support fiemap action.
tmpfs on /tmp type tmpfs (rw,nosuid,nodev,size=494452k,nr_inodes=1048576,inode64)

I will sent new patch for cover all supported filesystem.
> 
> > +
> > +	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