| Message ID | 20240331021720.9527-1-wegao@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] ioctl_fiemap01: New test for fiemap ioctl() | expand |
Hi Wei, > +#define TMPDIR "mntdir" very nit: We usually use MNTPOINT for .mntpoint #define MNTPOINT "mntpoint" (I even tried to get rid of these defines: https://patchwork.ozlabs.org/project/ltp/list/?series=393028 .) > +#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, unsigned int fe_length) > +{ > + TST_EXP_EXPR(fiemap->fm_mapped_extents == fm_mapped_extents, > + "Check extent fm_mapped_extents is %d", fm_mapped_extents); nit: space here (and below) would be more readable for me. > + TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_flags & fe_flags, > + "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, > + "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; > + > + SAFE_CHDIR(TMPDIR); > + fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, 0644); > + > + TST_EXP_PASS(statvfs(".", &fs_info)); I'd here jut use: if (statvfs(".", &fs_info) != 0) tst_brk(TBROK, "statvfs failed"); Why? IMHO all TEST() and TST_EXP_PASS() should be used for the subject of testing. (We don't have safe_statvfs(), but it'd be needed just on 3 places.) > + 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; nit: 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, > + "Check extent fm_mapped_extents is 0"); > + > + 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); > + unlink(TESTFILE); SAFE_UNLINK(TESTFILE); > +} > + > +static struct tst_test test = { > + .mount_device = 1, > + .mntpoint = TMPDIR, > + .all_filesystems = 1, > + .skip_filesystems = (const char *const[]) { > + "exfat", "vfat", "fuse", "ntfs", "tmpfs", NULL Why do you whitelist fuse? Which filesystem under fuse does not work? > + }, > + .test_all = verify_ioctl, > + .needs_root = 1, > + .needs_tmpdir = 1, needs_tmpdir is not needed (you use .all_filesystems). You would find that when generating docparse. Kind regards, Petr
On Wed, Apr 03, 2024 at 11:28:27AM +0200, Petr Vorel wrote: > Hi Wei, > > > +static struct tst_test test = { > > + .mount_device = 1, > > + .mntpoint = TMPDIR, > > + .all_filesystems = 1, > > + .skip_filesystems = (const char *const[]) { > > + "exfat", "vfat", "fuse", "ntfs", "tmpfs", NULL > > Why do you whitelist fuse? Which filesystem under fuse does not work? I will remove fuse in next patch. But i find fs_type_whitelist not contain "fuse", so this will lead func tst_get_supported_fs_type can not handle "fuse" filesystem, means add/remove "fuse" into skip_filesystems will not take any effect. Correct me if i have any misunderstanding. static const char *const fs_type_whitelist[] = { "ext2", "ext3", "ext4", "xfs", "btrfs", "bcachefs", "vfat", "exfat", "ntfs", "tmpfs", NULL };
> On Wed, Apr 03, 2024 at 11:28:27AM +0200, Petr Vorel wrote: > > Hi Wei, > > > +static struct tst_test test = { > > > + .mount_device = 1, > > > + .mntpoint = TMPDIR, > > > + .all_filesystems = 1, > > > + .skip_filesystems = (const char *const[]) { > > > + "exfat", "vfat", "fuse", "ntfs", "tmpfs", NULL > > Why do you whitelist fuse? Which filesystem under fuse does not work? > I will remove fuse in next patch. > But i find fs_type_whitelist not contain "fuse", so this will lead lib/tst_supported_fs_types.c contains: skip_fuse = tst_fs_in_skiplist("fuse", skiplist); => please use it. Kind regards, Petr > func tst_get_supported_fs_type can not handle "fuse" filesystem, > means add/remove "fuse" into skip_filesystems will not take any effect. > Correct me if i have any misunderstanding. > static const char *const fs_type_whitelist[] = { > "ext2", > "ext3", > "ext4", > "xfs", > "btrfs", > "bcachefs", > "vfat", > "exfat", > "ntfs", > "tmpfs", > NULL > };
diff --git a/runtest/syscalls b/runtest/syscalls index 6e2407879..4e6ce5aef 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -589,6 +589,8 @@ ioctl_ns07 ioctl_ns07 ioctl_sg01 ioctl_sg01 +ioctl_fiemap01 ioctl_fiemap01 + inotify_init1_01 inotify_init1_01 inotify_init1_02 inotify_init1_02 diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore index 5fff7a61d..64adcdfe6 100644 --- a/testcases/kernel/syscalls/ioctl/.gitignore +++ b/testcases/kernel/syscalls/ioctl/.gitignore @@ -22,3 +22,4 @@ /ioctl_ns06 /ioctl_ns07 /ioctl_sg01 +/ioctl_fiemap01 diff --git a/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c new file mode 100644 index 000000000..b81842c6a --- /dev/null +++ b/testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2023 Wei Gao <wegao@suse.com> + */ + +/*\ + * [Description] + * + * Verify basic fiemap ioctl. + */ + +#include <linux/fs.h> +#include <linux/fiemap.h> +#include <stdlib.h> +#include <sys/statvfs.h> + +#include "tst_test.h" + +#define TMPDIR "mntdir" +#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, unsigned int fe_length) +{ + TST_EXP_EXPR(fiemap->fm_mapped_extents == fm_mapped_extents, + "Check extent fm_mapped_extents is %d", fm_mapped_extents); + TST_EXP_EXPR(fiemap->fm_extents[index_extents].fe_flags & fe_flags, + "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, + "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; + + SAFE_CHDIR(TMPDIR); + fd = SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT, 0644); + + TST_EXP_PASS(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, + "Check extent fm_mapped_extents is 0"); + + 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); + unlink(TESTFILE); +} + +static struct tst_test test = { + .mount_device = 1, + .mntpoint = TMPDIR, + .all_filesystems = 1, + .skip_filesystems = (const char *const[]) { + "exfat", "vfat", "fuse", "ntfs", "tmpfs", NULL + }, + .test_all = verify_ioctl, + .needs_root = 1, + .needs_tmpdir = 1, +};
Fixes: #535 Signed-off-by: Wei Gao <wegao@suse.com> --- runtest/syscalls | 2 + testcases/kernel/syscalls/ioctl/.gitignore | 1 + .../kernel/syscalls/ioctl/ioctl_fiemap01.c | 110 ++++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 testcases/kernel/syscalls/ioctl/ioctl_fiemap01.c