[LTP] [PATCH 1/3] quotactl06: Replace tst_brk(TFAIL) with TBROK

[LTP] [PATCH 1/3] quotactl06: Replace tst_brk(TFAIL) with TBROK

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/quotactl/quotactl06.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
index 4fb198268..ca81a742f 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl06.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
@@ -178,7 +178,7 @@ static void setup(void)
 	set_dqmax.dqb_bsoftlimit = tst_variant ? 0x20000000000000 : 0x100000000;
 
 	if (access(USRPATH, F_OK) == -1)
-		tst_brk(TFAIL | TERRNO, "user quotafile didn't exist");
+		tst_brk(TBROK | TERRNO, "user quotafile didn't exist");
 
 	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
 
-- 
2.35.1


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

[LTP] [PATCH 2/3] quotactl06: Code style fixes

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 .../kernel/syscalls/quotactl/quotactl06.c     | 49 +++++++++++--------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
index ca81a742f..8e9a17393 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl06.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
@@ -12,22 +12,23 @@
  *
  * - EACCES when cmd is Q_QUOTAON and addr existed but not a regular file
  * - ENOENT when the file specified by special or addr does not exist
- * - EBUSY when cmd is Q_QUOTAON and another Q_QUOTAON had already been performed
+ * - EBUSY when cmd is Q_QUOTAON and another Q_QUOTAON had already been
+ *   performed
  * - EFAULT when addr or special is invalid
  * - EINVAL when cmd or type is invalid
  * - ENOTBLK when special is not a block device
- * - ESRCH when no disk quota is found for the indicated user and quotas have not been
- *   turned on for this fs
+ * - ESRCH when no disk quota is found for the indicated user and quotas have
+ *   not been turned on for this fs
  * - ESRCH when cmd is Q_QUOTAON, but the quota format was not found
- * - ESRCH when cmd is Q_GETNEXTQUOTA, but there is no ID greater than or equal to id that
- *   has an active quota
- * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range allowed
- *   by the quota format
- * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for the specified
- *   operation
+ * - ESRCH when cmd is Q_GETNEXTQUOTA, but there is no ID greater than or
+ *   equal to id that has an active quota
+ * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the
+ *   range allowed by the quota format
+ * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for
+ *   the specified operation
  *
- * For ERANGE error, the vfsv0 and vfsv1 format's maximum quota limit setting have been
- * fixed since the following kernel patch:
+ * For ERANGE error, the vfsv0 and vfsv1 format's maximum quota limit setting
+ * have been fixed since the following kernel patch:
  *
  *  commit 7e08da50cf706151f324349f9235ebd311226997
  *  Author: Jan Kara <jack@suse.cz>
@@ -135,10 +136,13 @@ static void verify_quotactl(unsigned int n)
 	}
 
 	if (tc->on_flag) {
-		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAON, USRQUOTA), tst_device->dev,
-					fmt_id, usrpath), "quotactl with Q_QUOTAON");
+		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAON, USRQUOTA),
+			tst_device->dev, fmt_id, usrpath),
+			"quotactl with Q_QUOTAON");
+
 		if (!TST_PASS)
 			return;
+
 		quota_on = 1;
 	}
 
@@ -147,16 +151,19 @@ static void verify_quotactl(unsigned int n)
 		drop_flag = 1;
 	}
 
-	if (tc->exp_err == ENOTBLK)
+	if (tc->exp_err == ENOTBLK) {
 		TST_EXP_FAIL(quotactl(tc->cmd, "/dev/null", *tc->id, tc->addr),
 			ENOTBLK, "quotactl()");
-	else
-		TST_EXP_FAIL(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr),
-			tc->exp_err, "quotactl()");
+	} else {
+		TST_EXP_FAIL(quotactl(tc->cmd, tst_device->dev, *tc->id,
+			tc->addr), tc->exp_err, "quotactl()");
+	}
 
 	if (quota_on) {
-		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAOFF, USRQUOTA), tst_device->dev,
-					fmt_id, usrpath), "quotactl with Q_QUOTAOFF");
+		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAOFF, USRQUOTA),
+			tst_device->dev, fmt_id, usrpath),
+			"quotactl with Q_QUOTAOFF");
+
 		if (!TST_PASS)
 			return;
 	}
@@ -169,7 +176,9 @@ static void setup(void)
 {
 	unsigned int i;
 	const struct quotactl_fmt_variant *var = &fmt_variants[tst_variant];
-	const char *const cmd[] = {"quotacheck", "-ugF", var->fmt_name, MNTPOINT, NULL};
+	const char *const cmd[] = {
+		"quotacheck", "-ugF", var->fmt_name, MNTPOINT, NULL
+	};
 
 	tst_res(TINFO, "quotactl() with %s format", var->fmt_name);
 	SAFE_CMD(cmd, NULL, NULL);
-- 
2.35.1


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

Re: [LTP] [PATCH 2/3] quotactl06: Code style fixes

[xuyang2018.jy]

Hi Martin

Looks good to me,
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Best Regards
Yang Xu
> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
> ---
>   .../kernel/syscalls/quotactl/quotactl06.c     | 49 +++++++++++--------
>   1 file changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
> index ca81a742f..8e9a17393 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl06.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
> @@ -12,22 +12,23 @@
>    *
>    * - EACCES when cmd is Q_QUOTAON and addr existed but not a regular file
>    * - ENOENT when the file specified by special or addr does not exist
> - * - EBUSY when cmd is Q_QUOTAON and another Q_QUOTAON had already been performed
> + * - EBUSY when cmd is Q_QUOTAON and another Q_QUOTAON had already been
> + *   performed
>    * - EFAULT when addr or special is invalid
>    * - EINVAL when cmd or type is invalid
>    * - ENOTBLK when special is not a block device
> - * - ESRCH when no disk quota is found for the indicated user and quotas have not been
> - *   turned on for this fs
> + * - ESRCH when no disk quota is found for the indicated user and quotas have
> + *   not been turned on for this fs
>    * - ESRCH when cmd is Q_QUOTAON, but the quota format was not found
> - * - ESRCH when cmd is Q_GETNEXTQUOTA, but there is no ID greater than or equal to id that
> - *   has an active quota
> - * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the range allowed
> - *   by the quota format
> - * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for the specified
> - *   operation
> + * - ESRCH when cmd is Q_GETNEXTQUOTA, but there is no ID greater than or
> + *   equal to id that has an active quota
> + * - ERANGE when cmd is Q_SETQUOTA, but the specified limits are out of the
> + *   range allowed by the quota format
> + * - EPERM when the caller lacked the required privilege (CAP_SYS_ADMIN) for
> + *   the specified operation
>    *
> - * For ERANGE error, the vfsv0 and vfsv1 format's maximum quota limit setting have been
> - * fixed since the following kernel patch:
> + * For ERANGE error, the vfsv0 and vfsv1 format's maximum quota limit setting
> + * have been fixed since the following kernel patch:
>    *
>    *  commit 7e08da50cf706151f324349f9235ebd311226997
>    *  Author: Jan Kara<jack@suse.cz>
> @@ -135,10 +136,13 @@ static void verify_quotactl(unsigned int n)
>   	}
> 
>   	if (tc->on_flag) {
> -		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAON, USRQUOTA), tst_device->dev,
> -					fmt_id, usrpath), "quotactl with Q_QUOTAON");
> +		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAON, USRQUOTA),
> +			tst_device->dev, fmt_id, usrpath),
> +			"quotactl with Q_QUOTAON");
> +
>   		if (!TST_PASS)
>   			return;
> +
>   		quota_on = 1;
>   	}
> 
> @@ -147,16 +151,19 @@ static void verify_quotactl(unsigned int n)
>   		drop_flag = 1;
>   	}
> 
> -	if (tc->exp_err == ENOTBLK)
> +	if (tc->exp_err == ENOTBLK) {
>   		TST_EXP_FAIL(quotactl(tc->cmd, "/dev/null", *tc->id, tc->addr),
>   			ENOTBLK, "quotactl()");
> -	else
> -		TST_EXP_FAIL(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr),
> -			tc->exp_err, "quotactl()");
> +	} else {
> +		TST_EXP_FAIL(quotactl(tc->cmd, tst_device->dev, *tc->id,
> +			tc->addr), tc->exp_err, "quotactl()");
> +	}
> 
>   	if (quota_on) {
> -		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAOFF, USRQUOTA), tst_device->dev,
> -					fmt_id, usrpath), "quotactl with Q_QUOTAOFF");
> +		TST_EXP_PASS_SILENT(quotactl(QCMD(Q_QUOTAOFF, USRQUOTA),
> +			tst_device->dev, fmt_id, usrpath),
> +			"quotactl with Q_QUOTAOFF");
> +
>   		if (!TST_PASS)
>   			return;
>   	}
> @@ -169,7 +176,9 @@ static void setup(void)
>   {
>   	unsigned int i;
>   	const struct quotactl_fmt_variant *var =&fmt_variants[tst_variant];
> -	const char *const cmd[] = {"quotacheck", "-ugF", var->fmt_name, MNTPOINT, NULL};
> +	const char *const cmd[] = {
> +		"quotacheck", "-ugF", var->fmt_name, MNTPOINT, NULL
> +	};
> 
>   	tst_res(TINFO, "quotactl() with %s format", var->fmt_name);
>   	SAFE_CMD(cmd, NULL, NULL);

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

[LTP] [PATCH 3/3] quotactl06: Fix TESTDIR1 cleanup if setup() exits early

[Martin Doucha]

When setup() exits with TCONF before TESTDIR1 gets created, cleanup() will
trigger TWARN when it tries to delete it. Check whether the directory exists
before calling SAFE_RMDIR().

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/quotactl/quotactl06.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
index 8e9a17393..d470486d5 100644
--- a/testcases/kernel/syscalls/quotactl/quotactl06.c
+++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
@@ -207,7 +207,9 @@ static void setup(void)
 static void cleanup(void)
 {
 	SAFE_UNLINK(USRPATH);
-	SAFE_RMDIR(TESTDIR1);
+
+	if (!access(TESTDIR1, F_OK))
+		SAFE_RMDIR(TESTDIR1);
 }
 
 static struct tst_test test = {
-- 
2.35.1


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

Re: [LTP] [PATCH 3/3] quotactl06: Fix TESTDIR1 cleanup if setup() exits early

[xuyang2018.jy]

Hi Martin
> When setup() exits with TCONF before TESTDIR1 gets created, cleanup() will
> trigger TWARN when it tries to delete it. Check whether the directory exists
> before calling SAFE_RMDIR().
> 
> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
> ---
>   testcases/kernel/syscalls/quotactl/quotactl06.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
> index 8e9a17393..d470486d5 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl06.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
> @@ -207,7 +207,9 @@ static void setup(void)
>   static void cleanup(void)
>   {
>   	SAFE_UNLINK(USRPATH);
It seems we also should check USRPATH whether existed.

Best Regards
Yang Xu
> -	SAFE_RMDIR(TESTDIR1);
> +
> +	if (!access(TESTDIR1, F_OK))
> +		SAFE_RMDIR(TESTDIR1);
>   }
> 
>   static struct tst_test test = {

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

Re: [LTP] [PATCH 3/3] quotactl06: Fix TESTDIR1 cleanup if setup() exits early

[Martin Doucha]

On 04. 03. 22 2:38, xuyang2018.jy@fujitsu.com wrote:
> Hi Martin
>> When setup() exits with TCONF before TESTDIR1 gets created, cleanup() will
>> trigger TWARN when it tries to delete it. Check whether the directory exists
>> before calling SAFE_RMDIR().
>>
>> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
>> ---
>>   testcases/kernel/syscalls/quotactl/quotactl06.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
>> index 8e9a17393..d470486d5 100644
>> --- a/testcases/kernel/syscalls/quotactl/quotactl06.c
>> +++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
>> @@ -207,7 +207,9 @@ static void setup(void)
>>   static void cleanup(void)
>>   {
>>   	SAFE_UNLINK(USRPATH);
> It seems we also should check USRPATH whether existed.

If I understand correctly, the file will be created by mount(), even
before setup() gets executed. So if the file doesn't exist during
cleanup(), the error will be valid.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

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

Re: [LTP] [PATCH 3/3] quotactl06: Fix TESTDIR1 cleanup if setup() exits early

[xuyang2018.jy]

Hi Martin
> On 04. 03. 22 2:38, xuyang2018.jy@fujitsu.com wrote:
>> Hi Martin
>>> When setup() exits with TCONF before TESTDIR1 gets created, cleanup() will
>>> trigger TWARN when it tries to delete it. Check whether the directory exists
>>> before calling SAFE_RMDIR().
>>>
>>> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
>>> ---
>>>    testcases/kernel/syscalls/quotactl/quotactl06.c | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
>>> index 8e9a17393..d470486d5 100644
>>> --- a/testcases/kernel/syscalls/quotactl/quotactl06.c
>>> +++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
>>> @@ -207,7 +207,9 @@ static void setup(void)
>>>    static void cleanup(void)
>>>    {
>>>    	SAFE_UNLINK(USRPATH);
>> It seems we also should check USRPATH whether existed.
>
> If I understand correctly, the file will be created by mount(), even
> before setup() gets executed. So if the file doesn't exist during
> cleanup(), the error will be valid.
The USRPATH file is generated by quotacheck command.

Best Regards
Yang Xu
>

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

Re: [LTP] [PATCH 1/3] quotactl06: Replace tst_brk(TFAIL) with TBROK

[xuyang2018.jy]

Hi Martin

Looks good to me,
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Also, quotactl01 also use this similar code, I guess we can also replace
it. It seems we miss SAFE_ACCESS macro, we can add this macro and use it
in all similar code.

Best Regards
Yang Xu
> Signed-off-by: Martin Doucha<mdoucha@suse.cz>
> ---
>   testcases/kernel/syscalls/quotactl/quotactl06.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/quotactl/quotactl06.c b/testcases/kernel/syscalls/quotactl/quotactl06.c
> index 4fb198268..ca81a742f 100644
> --- a/testcases/kernel/syscalls/quotactl/quotactl06.c
> +++ b/testcases/kernel/syscalls/quotactl/quotactl06.c
> @@ -178,7 +178,7 @@ static void setup(void)
>   	set_dqmax.dqb_bsoftlimit = tst_variant ? 0x20000000000000 : 0x100000000;
> 
>   	if (access(USRPATH, F_OK) == -1)
> -		tst_brk(TFAIL | TERRNO, "user quotafile didn't exist");
> +		tst_brk(TBROK | TERRNO, "user quotafile didn't exist");
> 
>   	tst_require_quota_support(tst_device->dev, fmt_id, usrpath);
> 

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