[v3,1/5] syscalls/quotactl01.c: Add Q_GETNEXQUOTA test

Q_GETNEXTQUOTA was introduced since linux 4.6, this operation is the
same as Q_GETQUOTA, but it returns quota information for the next ID
greater than or equal to id that has a quota set.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 configure.ac                                  |  1 +
 include/lapi/quotactl.h                       | 26 +++++----
 m4/ltp-quota.m4                               |  7 +++
 .../kernel/syscalls/quotactl/quotactl01.c     | 53 ++++++++++++++-----
 4 files changed, 61 insertions(+), 26 deletions(-)
 create mode 100644 m4/ltp-quota.m4

> Q_GETNEXTQUOTA was introduced since linux 4.6, this operation is the
> same as Q_GETQUOTA, but it returns quota information for the next ID
> greater than or equal to id that has a quota set.

> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
LGTM, minor note below.

> diff --git a/include/lapi/quotactl.h b/include/lapi/quotactl.h
...
>  #ifndef LAPI_QUOTACTL_H__
>  # define LAPI_QUOTACTL_H__

> +#ifdef HAVE_STRUCT_IF_NEXTDQBLK
> +# include <linux/quota.h>
You test <linux/quota.h> in LTP_CHECK_SYSCALL_QUOTACTL (indirectly via AC_CHECK_TYPES, but that should be ok)
> +#endif
> +#include <sys/quota.h>
> +

...
> @@ -155,8 +177,15 @@ static void verify_quota(unsigned int n)
>  	res_dq.dqb_bsoftlimit = 0;
>  	res_qf.dqi_igrace = 0;
>  	fmt_buf = 0;
> +#if defined(HAVE_STRUCT_IF_NEXTDQBLK)
> +	res_ndq.dqb_id = -1;
> +#endif

>  	TEST(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr));
> +	if (TST_ERR == EINVAL) {
> +		tst_res(TCONF, "Current system doesn't support this cmd");
nit: cmd? Maybe something like "quotactl() syscall does not support this command"
or "quotactl() syscall does not support to %s", tc->des
to follow other messages. I'd actually prefer to have some macro, which prints
QCMD flags, but that's a detail which we can ignore.

> +		return;
> +	}
>  	if (TST_RET == -1) {
>  		tst_res(TFAIL | TTERRNO, "quotactl failed to %s", tc->des);
>  		return;

Kind regards,
Petr

Subject: [PATCH v3 1/5] syscalls/quotactl01.c: Add Q_GETNEXQUOTA test
=> s/Q_GETNEXQUOTA/Q_GETNEXTQUOTA/ :)
Generally there are more typos in docs and formatting issues. As these are just
a small details, I usually fix them during merge, but copy paste on constants is
you friend.

Kind regards,
Petr

Hi Petr
 I will check it and correct them.
>Subject: Re: [PATCH v3 1/5] syscalls/quotactl01.c: Add Q_GETNEXQUOTA test
>
>Subject: [PATCH v3 1/5] syscalls/quotactl01.c: Add Q_GETNEXQUOTA test => s/Q_GETNEXQUOTA/Q_GETNEXTQUOTA/ :) Generally there are more typos in docs and >formatting 》issues. As these are just a small details, I usually fix them during merge, but copy paste on constants is you friend.
>
>Kind regards,
>Petr

Hi Xu,

> Hi Petr
>  I will check it and correct them.
No need to send a new patchset just for this, I'll fix that during merge.

> >Subject: Re: [PATCH v3 1/5] syscalls/quotactl01.c: Add Q_GETNEXQUOTA test

> >Subject: [PATCH v3 1/5] syscalls/quotactl01.c: Add Q_GETNEXQUOTA test => s/Q_GETNEXQUOTA/Q_GETNEXTQUOTA/ :) Generally there are more typos in docs and >formatting 》issues. As these are just a small details, I usually fix them during merge, but copy paste on constants is you friend.

Kind regards,
Petr

>> Q_GETNEXTQUOTA was introduced since linux 4.6, this operation is the 
>>same as Q_GETQUOTA, but it returns quota information for the next ID 
>> greater than or equal to id that has a quota set.
>
>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>Reviewed-by: Petr Vorel <pvorel@suse.cz> LGTM, minor note below.
Thanks for your review.

>> +#ifdef HAVE_STRUCT_IF_NEXTDQBLK
>> +# include <linux/quota.h>
>You test <linux/quota.h> in LTP_CHECK_SYSCALL_QUOTACTL (indirectly via AC_CHECK_TYPES, but that should be ok)
...
>>  	TEST(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr));
>>+	if (TST_ERR == EINVAL) {
>> +		tst_res(TCONF, "Current system doesn't support this cmd");
>nit: cmd? Maybe something like "quotactl() syscall does not support this command"
>or "quotactl() syscall does not support to %s", tc->des to follow other messages. I'd actually prefer to have some macro, which prints QCMD flags, but that's a detail which we can >ignore.
I think we can add some info before each run (such as prctl02.c and copy_file_range02.c, make error cases more clear ), as below:
	tst_res(TINFO, "Test #%d: %s", n, tc->tname);  // this tname is a QCMD_string
>Kind regards,
>Petr

Hi Xu,

> >>  	TEST(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr));
> >>+	if (TST_ERR == EINVAL) {
> >> +		tst_res(TCONF, "Current system doesn't support this cmd");
> >nit: cmd? Maybe something like "quotactl() syscall does not support this command"
> >or "quotactl() syscall does not support to %s", tc->des to follow other messages. I'd actually prefer to have some macro, which prints QCMD flags, but that's a detail which we can >ignore.
> I think we can add some info before each run (such as prctl02.c and copy_file_range02.c, make error cases more clear ), as below:
> 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);  // this tname is a QCMD_string

+1.  It could use similar approach as it's in testcases/kernel/syscalls/mbind/mbind01.c

#define POLICY_DESC(x) .policy = x, .desc = #x
#define POLICY_DESC_TEXT(x, y) .policy = x, .desc = #x" ("y")"

static struct test_case tcase[] = {
	{
		POLICY_DESC(MPOL_DEFAULT),
		...
	},
	{
		POLICY_DESC_TEXT(MPOL_DEFAULT, "target exists"),

But I'd prefer to have it as a separate patch after v4 of this patchset is merged.
I'd prefer to finish v4 (I've already fixed small formatting things, I can fix
the rest of needed things myself).

Kind regards,
Petr

on 2019/11/21 1:09, Petr Vorel wrote:

> Hi Xu,
>
>>>>   	TEST(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr));
>>>> +	if (TST_ERR == EINVAL) {
>>>> +		tst_res(TCONF, "Current system doesn't support this cmd");
>>> nit: cmd? Maybe something like "quotactl() syscall does not support this command"
>>> or "quotactl() syscall does not support to %s", tc->des to follow other messages. I'd actually prefer to have some macro, which prints QCMD flags, but that's a detail which we can >ignore.
>> I think we can add some info before each run (such as prctl02.c and copy_file_range02.c, make error cases more clear ), as below:
>> 	tst_res(TINFO, "Test #%d: %s", n, tc->tname);  // this tname is a QCMD_string
> +1.  It could use similar approach as it's in testcases/kernel/syscalls/mbind/mbind01.c
>
> #define POLICY_DESC(x) .policy = x, .desc = #x
> #define POLICY_DESC_TEXT(x, y) .policy = x, .desc = #x" ("y")"
>
> static struct test_case tcase[] = {
> 	{
> 		POLICY_DESC(MPOL_DEFAULT),
> 		...
> 	},
> 	{
> 		POLICY_DESC_TEXT(MPOL_DEFAULT, "target exists"),
>
> But I'd prefer to have it as a separate patch after v4 of this patchset is merged.
> I'd prefer to finish v4 (I've already fixed small formatting things, I can fix
> the rest of needed things myself).

Hi Petr

  Using a macro is a better way.

Thanks
Yang Xu

>
> Kind regards,
> Petr
>
>