[LTP] [PATCH v2] Rewrite statx04 test
Cyril Hrubis
chrubis@suse.cz
Wed Jan 19 16:52:46 CET 2022
Hi!
> statx04 is supposed verify that inode attribute support in statx() matches
> what should actually be implemented. However, we already have functional tests
> for this in statx08 and lack a test of the stx_attribute_mask field which
> reports inode attribute support to userspace.
>
> Rewrite the test to drop the duplicate code and add the missing coverage.
>
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>
> Changes since v1:
> - added known-fail tag for XFS in strict mode
> - changed copyright header to SUSE since this is a near-complete rewrite
>
> This also fixes Btrfs failures on kernels v4.11 and v4.12 where the attributes
> are not supported at all. LTP is supposed to check for bugs, not force people
> to backport features from newer kernels.
>
> If anybody want to backport the Btrfs support, the test now has strict mode
> which disables kernel version exceptions.
>
> I've also disabled FUSE filesystems because they return wrong errno codes
> in the ioctl() check and NTFS3g now has some rudimentary support for inode
> attributes but no support for them in statx().
>
> testcases/kernel/syscalls/statx/statx04.c | 196 +++++++---------------
> 1 file changed, 64 insertions(+), 132 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
> index a3ca436f5..6ac47a8bd 100644
> --- a/testcases/kernel/syscalls/statx/statx04.c
> +++ b/testcases/kernel/syscalls/statx/statx04.c
> @@ -1,14 +1,12 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> - * Copyright (c) Zilogic Systems Pvt. Ltd., 2018
> - * Email: code@zilogic.com
> + * Copyright (c) 2022 SUSE LLC <mdoucha@suse.cz>
> */
>
> /*\
> * [Description]
> *
> - * This code tests if the attributes field of statx received expected value.
> - * File set with following flags by using SAFE_IOCTL:
> + * Test whether the kernel properly advertises support for statx() attributes:
> *
> * - STATX_ATTR_COMPRESSED: The file is compressed by the filesystem.
> * - STATX_ATTR_IMMUTABLE: The file cannot be modified.
> @@ -16,9 +14,6 @@
> * - STATX_ATTR_NODUMP: File is not a candidate for backup when a backup
> * program such as dump(8) is run.
> *
> - * Two directories are tested.
> - * First directory has all flags set. Second directory has no flags set.
> - *
> * xfs filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
> * three other flags.
> *
> @@ -56,165 +51,102 @@
> #include "lapi/stat.h"
>
> #define MOUNT_POINT "mntpoint"
> -#define TESTDIR_FLAGGED MOUNT_POINT"/test_dir1"
> -#define TESTDIR_UNFLAGGED MOUNT_POINT"/test_dir2"
> +#define TESTDIR MOUNT_POINT "/testdir"
> +
> +#define ATTR(x) {.attr = x, .name = #x}
> +
> +static struct {
> + uint64_t attr;
> + const char *name;
> +} attr_list[] = {
> + ATTR(STATX_ATTR_COMPRESSED),
> + ATTR(STATX_ATTR_APPEND),
> + ATTR(STATX_ATTR_IMMUTABLE),
> + ATTR(STATX_ATTR_NODUMP)
> +};
>
> -static int fd, clear_flags;
> +static uint64_t expected_mask;
> +static char *strict;
>
> -static void test_flagged(void)
> +static void setup(void)
> {
> - struct statx buf;
> + int i, fd;
>
> - TEST(statx(AT_FDCWD, TESTDIR_FLAGGED, 0, 0, &buf));
> - if (TST_RET == 0)
> - tst_res(TPASS,
> - "sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> - else
> - tst_brk(TFAIL | TTERRNO,
> - "sys_statx(AT_FDCWD, %s, 0, 0, &buf)", TESTDIR_FLAGGED);
> -
> - if (strcmp(tst_device->fs_type, "xfs")) {
> - if (buf.stx_attributes & STATX_ATTR_COMPRESSED)
> - tst_res(TPASS, "STATX_ATTR_COMPRESSED flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_COMPRESSED flag is not set");
> - }
> + SAFE_MKDIR(TESTDIR, 0777);
>
> - if (buf.stx_attributes & STATX_ATTR_APPEND)
> - tst_res(TPASS, "STATX_ATTR_APPEND flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_APPEND flag is not set");
> + /* Check general inode attribute support */
> + fd = SAFE_OPEN(TESTDIR, O_RDONLY | O_DIRECTORY);
> + TEST(ioctl(fd, FS_IOC_GETFLAGS, &i));
> + SAFE_CLOSE(fd);
>
> - if (buf.stx_attributes & STATX_ATTR_IMMUTABLE)
> - tst_res(TPASS, "STATX_ATTR_IMMUTABLE flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_IMMUTABLE flag is not set");
> + if (TST_RET == -1 && TST_ERR == ENOTTY)
> + tst_brk(TCONF | TTERRNO, "Inode attributes not supported");
>
> - if (buf.stx_attributes & STATX_ATTR_NODUMP)
> - tst_res(TPASS, "STATX_ATTR_NODUMP flag is set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_NODUMP flag is not set");
> -}
> + if (TST_RET)
> + tst_brk(TBROK | TTERRNO, "Unexpected ioctl() error");
>
> -static void test_unflagged(void)
> -{
> - struct statx buf;
> + for (i = 0, expected_mask = 0; i < ARRAY_SIZE(attr_list); i++)
> + expected_mask |= attr_list[i].attr;
>
> - TEST(statx(AT_FDCWD, TESTDIR_UNFLAGGED, 0, 0, &buf));
> - if (TST_RET == 0)
> - tst_res(TPASS,
> - "sys_statx(AT_FDCWD, %s, 0, 0, &buf)",
> - TESTDIR_UNFLAGGED);
> - else
> - tst_brk(TFAIL | TTERRNO,
> - "sys_statx(AT_FDCWD, %s, 0, 0, &buf)",
> - TESTDIR_UNFLAGGED);
> -
> - if ((buf.stx_attributes & STATX_ATTR_COMPRESSED) == 0)
> - tst_res(TPASS, "STATX_ATTR_COMPRESSED flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_COMPRESSED flag is set");
> -
> - if ((buf.stx_attributes & STATX_ATTR_APPEND) == 0)
> - tst_res(TPASS, "STATX_ATTR_APPEND flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_APPEND flag is set");
> -
> - if ((buf.stx_attributes & STATX_ATTR_IMMUTABLE) == 0)
> - tst_res(TPASS, "STATX_ATTR_IMMUTABLE flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_IMMUTABLE flag is set");
> -
> - if ((buf.stx_attributes & STATX_ATTR_NODUMP) == 0)
> - tst_res(TPASS, "STATX_ATTR_NODUMP flag is not set");
> - else
> - tst_res(TFAIL, "STATX_ATTR_NODUMP flag is set");
> -}
> + /* Strict mode: No exceptions, all attributes must be supported */
> + if (strict)
> + return;
>
> -static struct test_cases {
> - void (*tfunc)(void);
> -} tcases[] = {
> - {&test_flagged},
> - {&test_unflagged},
> -};
> + /* STATX_ATTR_COMPRESSED not supported on XFS */
> + if (!strcmp(tst_device->fs_type, "xfs"))
> + expected_mask &= ~STATX_ATTR_COMPRESSED;
>
> -static void run(unsigned int i)
> -{
> - tcases[i].tfunc();
> + /* Attribute support was added to Btrfs statx() in kernel v4.13 */
> + if (!strcmp(tst_device->fs_type, "btrfs") && tst_kvercmp(4, 13, 0) < 0)
> + tst_brk(TCONF, "statx() attributes not supported on Btrfs");
> }
>
> -static void caid_flags_setup(void)
> +static void run(void)
> {
> - int attr, ret;
> -
> - fd = SAFE_OPEN(TESTDIR_FLAGGED, O_RDONLY | O_DIRECTORY);
> -
> - ret = ioctl(fd, FS_IOC_GETFLAGS, &attr);
> - if (ret < 0) {
> - if (errno == ENOTTY)
> - tst_brk(TCONF | TERRNO, "FS_IOC_GETFLAGS not supported");
> -
> - /* ntfs3g fuse fs returns wrong errno for unimplemented ioctls */
> - if (!strcmp(tst_device->fs_type, "ntfs")) {
> - tst_brk(TCONF | TERRNO,
> - "ntfs3g does not support FS_IOC_GETFLAGS");
> - }
> + int i;
> + struct statx buf;
>
> - tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_GETFLAGS, ...)", fd);
> - }
> + TEST(statx(AT_FDCWD, TESTDIR, 0, 0, &buf));
>
> - if (!strcmp(tst_device->fs_type, "xfs"))
> - attr |= FS_APPEND_FL | FS_IMMUTABLE_FL | FS_NODUMP_FL;
> - else
> - attr |= FS_COMPR_FL | FS_APPEND_FL | FS_IMMUTABLE_FL | FS_NODUMP_FL;
> -
> - ret = ioctl(fd, FS_IOC_SETFLAGS, &attr);
> - if (ret < 0) {
> - if (errno == EOPNOTSUPP)
> - tst_brk(TCONF, "Flags not supported");
> - tst_brk(TBROK | TERRNO, "ioctl(%i, FS_IOC_SETFLAGS, %i)", fd, attr);
> + if (TST_RET == -1) {
> + tst_brk(TBROK | TTERRNO, "sys_statx() failed");
> + } else if (TST_RET) {
> + tst_brk(TBROK | TTERRNO,
> + "sys_statx() returned invalid value %ld", TST_RET);
Why not just TST_EXP_PASS_SILENT()?
> - clear_flags = 1;
> -}
> -
> -static void setup(void)
> -{
> - SAFE_MKDIR(TESTDIR_FLAGGED, 0777);
> - SAFE_MKDIR(TESTDIR_UNFLAGGED, 0777);
> + for (i = 0; i < ARRAY_SIZE(attr_list); i++) {
> + if (!(expected_mask & attr_list[i].attr))
> + continue;
>
> - caid_flags_setup();
> -}
> -
> -static void cleanup(void)
> -{
> - int attr;
> -
> - if (clear_flags) {
> - SAFE_IOCTL(fd, FS_IOC_GETFLAGS, &attr);
> - attr &= ~(FS_COMPR_FL | FS_APPEND_FL | FS_IMMUTABLE_FL | FS_NODUMP_FL);
> - SAFE_IOCTL(fd, FS_IOC_SETFLAGS, &attr);
> + if (buf.stx_attributes_mask & attr_list[i].attr)
> + tst_res(TPASS, "%s is supported", attr_list[i].name);
> + else
> + tst_res(TFAIL, "%s not supported", attr_list[i].name);
> }
> -
> - if (fd > 0)
> - SAFE_CLOSE(fd);
> }
>
> static struct tst_test test = {
> - .test = run,
> - .tcnt = ARRAY_SIZE(tcases),
> + .test_all = run,
> .setup = setup,
> - .cleanup = cleanup,
> .needs_root = 1,
> .all_filesystems = 1,
> .mount_device = 1,
> .mntpoint = MOUNT_POINT,
> .min_kver = "4.11",
> + .options = (struct tst_option[]) {
> + {"s", &strict, "Strict mode (disable special exceptions)"}
> + },
This looks like it should have been made a global knob. Maybe
environment variable?
> + .skip_filesystems = (const char *const[]) {
> + "fuse",
> + NULL
> + },
> .tags = (const struct tst_tag[]) {
> {"linux-git", "93bc420ed41d"},
> {"linux-git", "99652ea56a41"},
> {"linux-git", "04a87e347282"},
> {"linux-git", "5f955f26f3d4"},
> + {"known-fail", "STATX_ATTR_COMPRESSED is not implemented on XFS (strict mode)"},
> {}
> },
> };
Otherwise I like the direction where this is going.
I vote for merging this before the release with the TEST() and if()s
replaced byt the TST_EXP_PASS() macro and we can decide where and how to
put the strict knob later on.
--
Cyril Hrubis
chrubis@suse.cz
More information about the ltp
mailing list