Re: [LTP] [PATCH v1 01/11] syscalls/quotactl01: Also test with vfsv1 format

[Cyril Hrubis <chrubis@suse.cz>]

> +/*\
> + * [Description]
> + * This testcase checks the basic flag of quotactl(2) for non-XFS filesystems
                                                               ^
							       ext4?

It seems that the test is using ext4 only, are there any other
filesystems that should be tested here?

> + * with visible quota files(cover two formats, vfsv0 and vfsv1):
>   *
> - * This testcase checks the basic flag of quotactl(2) for non-XFS filesystems:
>   * 1) quotactl(2) succeeds to turn on quota with Q_QUOTAON flag for user.
> - * 2) quotactl(2) succeeds to set disk quota limits with Q_SETQUOTA flag
> + *
> + * 2 quotactl(2) succeeds to set disk quota limits with Q_SETQUOTA flag
>   *    for user.
> + *
>   * 3) quotactl(2) succeeds to get disk quota limits with Q_GETQUOTA flag
>   *    for user.
> + *
>   * 4) quotactl(2) succeeds to set information about quotafile with Q_SETINFO
>   *    flag for user.
> + *
>   * 5) quotactl(2) succeeds to get information about quotafile with Q_GETINFO
>   *    flag for user.
> + *
>   * 6) quotactl(2) succeeds to get quota format with Q_GETFMT flag for user.
> + *
>   * 7) quotactl(2) succeeds to update quota usages with Q_SYNC flag for user.
> + *
>   * 8) quotactl(2) succeeds to get disk quota limit greater than or equal to
>   *    ID with Q_GETNEXTQUOTA flag for user.
> + *
>   * 9) quotactl(2) succeeds to turn off quota with Q_QUOTAOFF flag for user.
> + *
>   * 10) quotactl(2) succeeds to turn on quota with Q_QUOTAON flag for group.
> + *
>   * 11) quotactl(2) succeeds to set disk quota limits with Q_SETQUOTA flag
>   *     for group.
> + *
>   * 12) quotactl(2) succeeds to get disk quota limits with Q_GETQUOTA flag
>   *     for group.
> + *
>   * 13) quotactl(2) succeeds to set information about quotafile with Q_SETINFO
>   *     flag for group.
> + *
>   * 14) quotactl(2) succeeds to get information about quotafile with Q_GETINFO
>   *     flag for group.
> + *
>   * 15) quotactl(2) succeeds to get quota format with Q_GETFMT flag for group.
> + *
>   * 16) quotactl(2) succeeds to update quota usages with Q_SYNC flag for group.
> + *
>   * 17) quotactl(2) succeeds to get disk quota limit greater than or equal to
>   *     ID with Q_GETNEXTQUOTA flag for group.
> + *
>   * 18) quotactl(2) succeeds to turn off quota with Q_QUOTAOFF flag for group.
>   */

This does not render as a list in asciidoc once the documentation is
rendered. I would have converted it to a bulleted style lists, i.e. the
list items would start with - instead of 1).

Also it's a bit poinless to repeat the quotactl(2) succeeds on each
line. It would make much more sense to put that part to the sentence
that describes the list as:

 * This testcases checks that quotactl(2) succeeds to:
 *
 *  - turn on quota with Q_QUOTAON flag
 *  - ...

> @@ -43,16 +66,12 @@
>  #include "lapi/quotactl.h"
>  #include "tst_test.h"
>  
> -#ifndef QFMT_VFS_V0
> -# define QFMT_VFS_V0	2
> -#endif
>  #define USRPATH MNTPOINT "/aquota.user"
>  #define GRPPATH MNTPOINT "/aquota.group"
> -#define FMTID	QFMT_VFS_V0
>  #define MNTPOINT	"mntpoint"
>  
> -static int32_t fmt_id = FMTID;
> -static int test_id;
> +static int32_t fmt_id;
> +static int test_id, mount_flag;
>  static char usrpath[] = USRPATH;
>  static char grppath[] = GRPPATH;
>  static struct dqblk set_dq = {
> @@ -163,9 +182,22 @@ static struct tcase {
>  
>  static void setup(void)
>  {
> -	const char *const cmd[] = {"quotacheck", "-ugF", "vfsv0", MNTPOINT, NULL};
> -
> -	SAFE_CMD(cmd, NULL, NULL);
> +	const char *const vfsv0_cmd[] = {"quotacheck", "-ugF", "vfsv0", MNTPOINT, NULL};
> +	const char *const vfsv1_cmd[] = {"quotacheck", "-ugF", "vfsv1", MNTPOINT, NULL};
> +
> +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, NULL, NULL);
> +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, "usrquota,grpquota");
> +	mount_flag = 1;

Why are you moving the mount from the tst_test structure here?

Do we have to remount the device to change the quota format?

> +	if (tst_variant) {
> +		tst_res(TINFO, "quotactl() with vfsv1 format");
> +		SAFE_CMD(vfsv1_cmd, NULL, NULL);
> +		fmt_id = QFMT_VFS_V1;
> +	} else {
> +		tst_res(TINFO, "quotactl() with vfsv0 format");
> +		SAFE_CMD(vfsv0_cmd, NULL, NULL);
> +		fmt_id = QFMT_VFS_V0;
> +	}
>  
>  	test_id = geteuid();
>  	if (access(USRPATH, F_OK) == -1)
> @@ -182,6 +214,12 @@ static void setup(void)
>  		getnextquota_nsup = 1;
>  }
>  
> +static void cleanup(void)
> +{
> +	if (mount_flag && tst_umount(MNTPOINT))
> +		tst_res(TWARN | TERRNO, "umount(%s)", MNTPOINT);
> +}
> +
>  static void verify_quota(unsigned int n)
>  {
>  	struct tcase *tc = &tcases[n];
> @@ -222,13 +260,14 @@ static struct tst_test test = {
>  	},
>  	.test = verify_quota,
>  	.tcnt = ARRAY_SIZE(tcases),
> -	.mount_device = 1,
> +	.needs_device = 1,
>  	.dev_fs_type = "ext4",
>  	.mntpoint = MNTPOINT,
> -	.mnt_data = "usrquota,grpquota",
>  	.needs_cmds = (const char *const []) {
>  		"quotacheck",
>  		NULL
>  	},
>  	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_variants = 2,
>  };
> -- 
> 2.23.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp