| Message ID | 1641973691-22981-2-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v6,1/3] syscalls/quotactl09: Test error when quota info hidden in filesystem | expand |
Hi! > diff --git a/testcases/kernel/syscalls/quotactl/quotactl07.c b/testcases/kernel/syscalls/quotactl/quotactl07.c > index 2992a6112..2427ef682 100644 > --- a/testcases/kernel/syscalls/quotactl/quotactl07.c > +++ b/testcases/kernel/syscalls/quotactl/quotactl07.c > @@ -7,7 +7,9 @@ > /*\ > * [Description] > * > - * This is a regresstion test for kernel commit 3dd4d40b4208 > + * This is not only a functional test but also a error test for Q_XQUOTARM. > + * > + * It is a regresstion test for kernel commit 3dd4d40b4208 > * ("xfs: Sanity check flags of Q_XQUOTARM call"). > */ > > @@ -15,51 +17,71 @@ > #include <unistd.h> > #include <stdio.h> > #include <sys/quota.h> > +#include <sys/statvfs.h> > #include "tst_test.h" > -#include "lapi/quotactl.h" > +#include "quotactl_syscall_var.h" > > #ifdef HAVE_XFS_XQM_H > # include <xfs/xqm.h> > > -#define MNTPOINT "mntpoint" > - > -static uint32_t qflag_acct = XFS_QUOTA_UDQ_ACCT; > -static unsigned int valid_type = XFS_USER_QUOTA; > /* Include a valid quota type to avoid other EINVAL error */ > static unsigned int invalid_type = XFS_GROUP_QUOTA << 1 | XFS_USER_QUOTA; > +static unsigned int valid_type = XFS_USER_QUOTA; > +static int mount_flag; > > static void verify_quota(void) > { > - TEST(quotactl(QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, (void *)&invalid_type)); > - if (TST_ERR == EINVAL) > - tst_res(TPASS, "Q_XQUOTARM has quota type check"); > + struct statfs before, after; > + > + SAFE_STATFS(MNTPOINT, &before); > + TST_EXP_PASS(do_quotactl(fd, QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, > + (void *)&valid_type), "do_quotactl(Q_XQUOTARM,valid_type)"); > + SAFE_STATFS(MNTPOINT, &after); > + if (before.f_bavail <= after.f_bavail) > + tst_res(TPASS, "Q_XQUOTARM to free space, delta(%lu)", after.f_bavail - before.f_bavail); > else > - tst_res(TFAIL, "Q_XQUOTARM doesn't have quota type check"); > + tst_res(TFAIL, "Q_XQUOTARM to free space, delta(-%lu)", before.f_bavail - after.f_bavail); > + > + TST_EXP_FAIL(do_quotactl(fd, QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, > + (void *)&invalid_type), EINVAL, "do_quotactl(Q_XQUOTARM, invalid_type)"); > } > > static void setup(void) > { > - TEST(quotactl(QCMD(Q_XQUOTAOFF, USRQUOTA), tst_device->dev, 0, (void *)&qflag_acct)); > - if (TST_RET == -1) > - tst_brk(TBROK | TTERRNO, "quotactl with Q_XQUOTAOFF failed"); > + quotactl_info(); > + > + /* ensure superblock has quota data, but not running */ > + SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "usrquota"); > + mount_flag = 1; > + SAFE_UMOUNT(MNTPOINT); > + mount_flag = 0; > + SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "noquota"); > + mount_flag = 1; Maybe just SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "remount, noquota") ? If we can remount the device without umounting it the code would be a bit shorter. Anyways looks good to me: Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Hi Cyril > Hi! >> diff --git a/testcases/kernel/syscalls/quotactl/quotactl07.c b/testcases/kernel/syscalls/quotactl/quotactl07.c >> index 2992a6112..2427ef682 100644 >> --- a/testcases/kernel/syscalls/quotactl/quotactl07.c >> +++ b/testcases/kernel/syscalls/quotactl/quotactl07.c >> @@ -7,7 +7,9 @@ >> /*\ >> * [Description] >> * >> - * This is a regresstion test for kernel commit 3dd4d40b4208 >> + * This is not only a functional test but also a error test for Q_XQUOTARM. >> + * >> + * It is a regresstion test for kernel commit 3dd4d40b4208 >> * ("xfs: Sanity check flags of Q_XQUOTARM call"). >> */ >> >> @@ -15,51 +17,71 @@ >> #include<unistd.h> >> #include<stdio.h> >> #include<sys/quota.h> >> +#include<sys/statvfs.h> >> #include "tst_test.h" >> -#include "lapi/quotactl.h" >> +#include "quotactl_syscall_var.h" >> >> #ifdef HAVE_XFS_XQM_H >> # include<xfs/xqm.h> >> >> -#define MNTPOINT "mntpoint" >> - >> -static uint32_t qflag_acct = XFS_QUOTA_UDQ_ACCT; >> -static unsigned int valid_type = XFS_USER_QUOTA; >> /* Include a valid quota type to avoid other EINVAL error */ >> static unsigned int invalid_type = XFS_GROUP_QUOTA<< 1 | XFS_USER_QUOTA; >> +static unsigned int valid_type = XFS_USER_QUOTA; >> +static int mount_flag; >> >> static void verify_quota(void) >> { >> - TEST(quotactl(QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, (void *)&invalid_type)); >> - if (TST_ERR == EINVAL) >> - tst_res(TPASS, "Q_XQUOTARM has quota type check"); >> + struct statfs before, after; >> + >> + SAFE_STATFS(MNTPOINT,&before); >> + TST_EXP_PASS(do_quotactl(fd, QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, >> + (void *)&valid_type), "do_quotactl(Q_XQUOTARM,valid_type)"); >> + SAFE_STATFS(MNTPOINT,&after); >> + if (before.f_bavail<= after.f_bavail) >> + tst_res(TPASS, "Q_XQUOTARM to free space, delta(%lu)", after.f_bavail - before.f_bavail); >> else >> - tst_res(TFAIL, "Q_XQUOTARM doesn't have quota type check"); >> + tst_res(TFAIL, "Q_XQUOTARM to free space, delta(-%lu)", before.f_bavail - after.f_bavail); >> + >> + TST_EXP_FAIL(do_quotactl(fd, QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, >> + (void *)&invalid_type), EINVAL, "do_quotactl(Q_XQUOTARM, invalid_type)"); >> } >> >> static void setup(void) >> { >> - TEST(quotactl(QCMD(Q_XQUOTAOFF, USRQUOTA), tst_device->dev, 0, (void *)&qflag_acct)); >> - if (TST_RET == -1) >> - tst_brk(TBROK | TTERRNO, "quotactl with Q_XQUOTAOFF failed"); >> + quotactl_info(); >> + >> + /* ensure superblock has quota data, but not running */ >> + SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "usrquota"); >> + mount_flag = 1; >> + SAFE_UMOUNT(MNTPOINT); >> + mount_flag = 0; >> + SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "noquota"); >> + mount_flag = 1; > > Maybe just SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "remount, noquota") ? > > If we can remount the device without umounting it the code would be a > bit shorter. > It seems use MS_REMOUNT flag doesn't work well, the quota feature is still on and will report EINVAL error when using Q_XQUOTARM syscall. Also, send a email to xfs maintainer to prove whether it is a expected behaviour. ps: I will merge this first, then I will add a comment to clarify this when I know the reason for xfs maintainer. Best Regards Yang Xu > Anyways looks good to me: > > Reviewed-by: Cyril Hrubis<chrubis@suse.cz> >
diff --git a/testcases/kernel/syscalls/quotactl/quotactl07.c b/testcases/kernel/syscalls/quotactl/quotactl07.c index 2992a6112..2427ef682 100644 --- a/testcases/kernel/syscalls/quotactl/quotactl07.c +++ b/testcases/kernel/syscalls/quotactl/quotactl07.c @@ -7,7 +7,9 @@ /*\ * [Description] * - * This is a regresstion test for kernel commit 3dd4d40b4208 + * This is not only a functional test but also a error test for Q_XQUOTARM. + * + * It is a regresstion test for kernel commit 3dd4d40b4208 * ("xfs: Sanity check flags of Q_XQUOTARM call"). */ @@ -15,51 +17,71 @@ #include <unistd.h> #include <stdio.h> #include <sys/quota.h> +#include <sys/statvfs.h> #include "tst_test.h" -#include "lapi/quotactl.h" +#include "quotactl_syscall_var.h" #ifdef HAVE_XFS_XQM_H # include <xfs/xqm.h> -#define MNTPOINT "mntpoint" - -static uint32_t qflag_acct = XFS_QUOTA_UDQ_ACCT; -static unsigned int valid_type = XFS_USER_QUOTA; /* Include a valid quota type to avoid other EINVAL error */ static unsigned int invalid_type = XFS_GROUP_QUOTA << 1 | XFS_USER_QUOTA; +static unsigned int valid_type = XFS_USER_QUOTA; +static int mount_flag; static void verify_quota(void) { - TEST(quotactl(QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, (void *)&invalid_type)); - if (TST_ERR == EINVAL) - tst_res(TPASS, "Q_XQUOTARM has quota type check"); + struct statfs before, after; + + SAFE_STATFS(MNTPOINT, &before); + TST_EXP_PASS(do_quotactl(fd, QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, + (void *)&valid_type), "do_quotactl(Q_XQUOTARM,valid_type)"); + SAFE_STATFS(MNTPOINT, &after); + if (before.f_bavail <= after.f_bavail) + tst_res(TPASS, "Q_XQUOTARM to free space, delta(%lu)", after.f_bavail - before.f_bavail); else - tst_res(TFAIL, "Q_XQUOTARM doesn't have quota type check"); + tst_res(TFAIL, "Q_XQUOTARM to free space, delta(-%lu)", before.f_bavail - after.f_bavail); + + TST_EXP_FAIL(do_quotactl(fd, QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, + (void *)&invalid_type), EINVAL, "do_quotactl(Q_XQUOTARM, invalid_type)"); } static void setup(void) { - TEST(quotactl(QCMD(Q_XQUOTAOFF, USRQUOTA), tst_device->dev, 0, (void *)&qflag_acct)); - if (TST_RET == -1) - tst_brk(TBROK | TTERRNO, "quotactl with Q_XQUOTAOFF failed"); + quotactl_info(); + + /* ensure superblock has quota data, but not running */ + SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "usrquota"); + mount_flag = 1; + SAFE_UMOUNT(MNTPOINT); + mount_flag = 0; + SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "noquota"); + mount_flag = 1; - TEST(quotactl(QCMD(Q_XQUOTARM, USRQUOTA), tst_device->dev, 0, (void *)&valid_type)); - if (TST_ERR == EINVAL) - tst_brk(TCONF, "current system doesn't support Q_XQUOTARM, skip it"); + fd = SAFE_OPEN(MNTPOINT, O_RDONLY); +} + +static void cleanup(void) +{ + if (fd > -1) + SAFE_CLOSE(fd); + if (mount_flag && tst_umount(MNTPOINT)) + tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT); } static struct tst_test test = { .setup = setup, + .cleanup = cleanup, .needs_root = 1, .needs_kconfigs = (const char *[]) { "CONFIG_XFS_QUOTA", NULL }, .test_all = verify_quota, - .mount_device = 1, + .format_device = 1, .dev_fs_type = "xfs", - .mnt_data = "usrquota", .mntpoint = MNTPOINT, + .test_variants = QUOTACTL_SYSCALL_VARIANTS, .tags = (const struct tst_tag[]) { {"linux-git", "3dd4d40b4208"}, {}
man-pages has error that Q_XQUOTARM was not introduced by kernel 3.16[1][2]. It only doesn't have owner handler so the flag spaces overlap a bit[3]. Add a functional test for Q_XQUOTARM and check whether available block is greater than before instead of a regression test for buggy 3.16 kernel because it is about 8 year old. Functional test is more meaningful. Since kernel commit 40b52225e ("xfs: remove support for disabling quota accounting on a mounted file system")[4] was introduced, if we want to disable quota account feature before Q_XQUOTARM, we must need to remount with noquota like xfs maintainer does in xfstests xfs/220 [5]. So, change this by mount with usrquota and remount with noquota to create a test environment that superblock has quota data but quota feature not running. We also add quotactl_fd test variant for this test. [1]https://github.com/alejandro-colomar/man-pages/commit/38bccbcf4f51c5370a1060e6a80b90d68b0dcdc8 [2]https://github.com/alejandro-colomar/man-pages/commit/26f3978f04a1aeeb5397a5facebaef40a341afb6 [3]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9da93f9b7c [4]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=40b52225e58 [5]https://patchwork.kernel.org/project/fstests/patch/20220105195352.GM656707@magnolia/ Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- .../kernel/syscalls/quotactl/quotactl07.c | 58 +++++++++++++------ 1 file changed, 40 insertions(+), 18 deletions(-)