[LTP] [PATCH v6 2/2] ioctl_fiemap01: New test for fiemap ioctl()

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c
> new file mode 100644
> index 000000000..82f67adcf
> --- /dev/null
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * Verify basic fiemap ioctl functionality, including:
> + *
> + * - The ioctl returns EBADR if it receives invalid fm_flags.
> + * - 0 extents are reported for an empty file.
> + * - The ioctl correctly retrieves single and multiple extent mappings after writing to the file.
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/fiemap.h>
> +#include <stdlib.h>
> +#include <sys/statvfs.h>
> +
> +#include "tst_test.h"
> +
> +#define MNTPOINT "mntpoint"
> +#define TESTFILE "testfile"
> +#define NUM_EXTENT 3
> +
> +static void print_extens(struct fiemap *fiemap)
> +{
> +	tst_res(TDEBUG, "File extent count: %u", fiemap->fm_mapped_extents);
> +
> +	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 check_extent(struct fiemap *fiemap, unsigned int fm_mapped_extents,
> +						int index_extents, int fe_flags,
> +						unsigned int min_fe_physical,

There is no point in passing the value of min_fe_physical if we just
pass 1 in all cases, we can just hardcode the value in the function.

> +						unsigned int fe_length)
> +{
> +	TST_EXP_EXPR(fiemap->fm_mapped_extents == fm_mapped_extents,
> +		"Check extent fm_mapped_extents is %d", fm_mapped_extents);

This check should be done in a separate function instead, because we
want to check all three extents in the last test and there is no point
in checking this three times.

Also we do have TST_EXP_EQ_UI() which should be used in the cases we are
testing for equality.

> +	TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_flags & fe_flags,
                                                                ^
This does not really work if we want to assert if the last flag was set
only on the last.

I.e. if for the last test we do:

	check_extent(fiemap, NUM_EXTENT, 0, 0, blk_size);
        check_extent(fiemap, NUM_EXTENT, 1, 0, blk_size);
        check_extent(fiemap, NUM_EXTENT, 2, FIEMAP_EXTENT_LAST, blk_size);

The fe_flags test fails because we get zero. What we have to do instead
is to pass the mask and the value and check for equality as:

	TST_EXP_EQ_UI((fiemap->fm_extents[index_extents].fe_flags & fe_mask) == fe_flags);

And then we call it like:

	check_extent(fiemap, NUM_EXTENT, 0, FIEMAP_EXTENT_LAST, 0, blk_size);
        check_extent(fiemap, NUM_EXTENT, 1, FIEMAP_EXTENT_LAST, 0, blk_size);
        check_extent(fiemap, NUM_EXTENT, 2, FIEMAP_EXTENT_LAST, FIEMAP_EXTENT_LAST, blk_size);

> +		"Check fe_flags is %d", fe_flags);
> +	TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_physical >= min_fe_physical,
> +		"Check fe_physical > %d", min_fe_physical);
> +	TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_length == fe_length,
                        ^
			And lastly but not least it would be shorter to
			get the pointer to the extent and the start as:

struct fiemap_extent *extent = &fieamp->fm_extents[index_extents];

And then do just extent->fe_foo in all cases.

> +		"Check fe_length is %d", fe_length);
> +}
> +
> +static void verify_ioctl(void)
> +{
> +	int fd;
> +	struct fiemap *fiemap;
> +	struct statvfs fs_info;
> +	unsigned long blk_size;
> +
> +	fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, 0644);
> +
> +	SAFE_STATVFS(".", &fs_info);
> +
> +	blk_size = fs_info.f_bsize;
> +
> +	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);
> +
> +	fiemap->fm_flags =  0;
> +	TST_EXP_PASS(ioctl(fd, FS_IOC_FIEMAP, fiemap));
> +	print_extens(fiemap);
> +	TST_EXP_EXPR(fiemap->fm_mapped_extents == 0,
> +		"Empty file should have 0 extends mapped");
                                             ^
					  extents
> +
> +	char *buf = SAFE_MALLOC(blk_size);
> +
> +	SAFE_WRITE(SAFE_WRITE_ANY, fd, buf, blk_size);
> +	fiemap->fm_flags = FIEMAP_FLAG_SYNC;
> +	TST_EXP_PASS(ioctl(fd, FS_IOC_FIEMAP, fiemap));
> +	print_extens(fiemap);
> +	check_extent(fiemap, 1, 0, FIEMAP_EXTENT_LAST, 1, blk_size);
> +
> +	fiemap->fm_extent_count = NUM_EXTENT;
> +	SAFE_LSEEK(fd, 2 * blk_size, SEEK_SET);
> +	SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, blk_size);
> +	SAFE_LSEEK(fd, 4 * blk_size, SEEK_SET);
> +	SAFE_WRITE(SAFE_WRITE_ALL, fd, buf, blk_size);
> +	TST_EXP_PASS(ioctl(fd, FS_IOC_FIEMAP, fiemap));
> +	print_extens(fiemap);
> +	check_extent(fiemap, NUM_EXTENT, NUM_EXTENT - 1, FIEMAP_EXTENT_LAST, 1, blk_size);
> +
> +	free(buf);
> +	free(fiemap);
> +	SAFE_CLOSE(fd);
> +	SAFE_UNLINK(TESTFILE);
> +}
> +
> +static void setup(void)
> +{
> +	SAFE_CHDIR(MNTPOINT);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.mount_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.all_filesystems = 1,
> +	.skip_filesystems = (const char *const[]) {
> +		"exfat", "vfat", "fuse", "ntfs", "tmpfs", NULL
> +	},
> +	.test_all = verify_ioctl,
> +	.needs_root = 1,
> +};
> -- 
> 2.35.3
> 

-- 
Cyril Hrubis
chrubis@suse.cz