| Message ID | 1574241216-15168-2-git-send-email-xuyang2018.jy@cn.fujitsu.com |
|---|---|
| State | Accepted |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | optimize quotactl test code | expand |
Hi Jan, Cyril, Xu, > 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, with 2 questions. > #ifndef LAPI_QUOTACTL_H__ > # define LAPI_QUOTACTL_H__ > +#include <sys/quota.h> > + > +#ifdef HAVE_STRUCT_IF_NEXTDQBLK > +# include <linux/quota.h> > +#else > +# ifdef HAVE_LINUX_TYPES_H > +# include <linux/types.h> @Jan, @Cyril: Do we want to generally avoid loading <linux/types.h> if not really needed? __u64 can be uint64_t etc (as it's also visible in struct dqblk in <sys/quota.h> in various libc headers). We used this approach for /usr/include/linux/bpf.h and for fanotify fixes for musl (testcases/kernel/syscalls/fanotify/fanotify.h). So unless you're against this approach here I'll change it before merge (and add this info to next version of library API writing guidelines patch https://patchwork.ozlabs.org/patch/1166786/). > +struct if_nextdqblk { > + __u64 dqb_bhardlimit; > + __u64 dqb_bsoftlimit; > + __u64 dqb_curspace; > + __u64 dqb_ihardlimit; > + __u64 dqb_isoftlimit; > + __u64 dqb_curinodes; > + __u64 dqb_btime; > + __u64 dqb_itime; > + __u32 dqb_valid; > + __u32 dqb_id; > +}; ... > +++ b/testcases/kernel/syscalls/quotactl/quotactl01.c > @@ -1,7 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) Crackerjack Project., 2007 > -* Copyright (c) 2016 Fujitsu Ltd. > +* Copyright (c) 2016-2019 FUJITSU LIMITED. All rights reserved BTW correct formatting is /* * */ Not /* * */ I'll change it during merge (nit, the code is what matters, not formatting, of course). ... > +static int getnextquota_nsup; ... > static void setup(void) > { > const char *const cmd[] = {"quotacheck", "-ugF", "vfsv0", MNTPOINT, NULL}; > int ret; > + getnextquota_nsup = 0; This is not needed (getnextquota_nsup is static and it's called just once, I'll remove it before merge). > ret = tst_run_cmd(cmd, NULL, NULL, 1); > switch (ret) { > @@ -146,6 +183,11 @@ static void setup(void) > if (access(GRPPATH, F_OK) == -1) > tst_brk(TFAIL | TERRNO, "group quotafile didn't exist"); > + > + TEST(quotactl(QCMD(Q_GETNEXTQUOTA, USRQUOTA), tst_device->dev, > + test_id, (void *) &res_ndq)); > + if (TST_ERR == EINVAL || TST_ERR == ENOSYS) Does EINVAL really mans not supported? Shouldn't be just for ENOSYS > + getnextquota_nsup = 1; > } Looking at kernel sources - this does not look as not supported, but rather a failure (we might want to add some test for EINVAL): if (!qid_has_mapping(sb->s_user_ns, qid)) return -EINVAL; kernel fs/quota/quota.c /* * Return quota for next active quota >= this id, if any exists, * otherwise return -ENOENT via ->get_nextdqblk */ static int quota_getnextquota(struct super_block *sb, int type, qid_t id, void __user *addr) { struct kqid qid; struct qc_dqblk fdq; struct if_nextdqblk idq; int ret; if (!sb->s_qcop->get_nextdqblk) return -ENOSYS; qid = make_kqid(current_user_ns(), type, id); if (!qid_has_mapping(sb->s_user_ns, qid)) return -EINVAL; ret = sb->s_qcop->get_nextdqblk(sb, &qid, &fdq); if (ret) return ret; /* struct if_nextdqblk is a superset of struct if_dqblk */ copy_to_if_dqblk((struct if_dqblk *)&idq, &fdq); idq.dqb_id = from_kqid(current_user_ns(), qid); if (copy_to_user(addr, &idq, sizeof(idq))) return -EFAULT; return 0; } Kind regards, Petr
Hi Jan, Cyril, Xu, > > +#ifdef HAVE_STRUCT_IF_NEXTDQBLK > > +# include <linux/quota.h> > > +#else > > +# ifdef HAVE_LINUX_TYPES_H > > +# include <linux/types.h> > @Jan, @Cyril: Do we want to generally avoid loading <linux/types.h> if not really needed? > __u64 can be uint64_t etc (as it's also visible in struct dqblk in <sys/quota.h> > in various libc headers). > We used this approach for /usr/include/linux/bpf.h and for fanotify fixes for > musl (testcases/kernel/syscalls/fanotify/fanotify.h). > So unless you're against this approach here I'll change it before merge > (and add this info to next version of library API writing guidelines patch > https://patchwork.ozlabs.org/patch/1166786/). + general question: do we want always test against kernel headers or libc headers? Libc is often outdated, so mostly it'd be our fallback to be tested. Ideally both kernel and libc header should be tested, but that's not easily achievable. Kind regards, Petr
on 2019/11/20 23:12, Petr Vorel wrote: > Hi Jan, Cyril, Xu, > >> 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, with 2 questions. > >> #ifndef LAPI_QUOTACTL_H__ >> # define LAPI_QUOTACTL_H__ >> +#include <sys/quota.h> >> + >> +#ifdef HAVE_STRUCT_IF_NEXTDQBLK >> +# include <linux/quota.h> >> +#else >> +# ifdef HAVE_LINUX_TYPES_H >> +# include <linux/types.h> > @Jan, @Cyril: Do we want to generally avoid loading <linux/types.h> if not really needed? > __u64 can be uint64_t etc (as it's also visible in struct dqblk in <sys/quota.h> > in various libc headers). > We used this approach for /usr/include/linux/bpf.h and for fanotify fixes for > musl (testcases/kernel/syscalls/fanotify/fanotify.h). > > So unless you're against this approach here I'll change it before merge > (and add this info to next version of library API writing guidelines patch > https://patchwork.ozlabs.org/patch/1166786/). I have no objection about using uint64_t becuase Q_GETNEXTQUOTA man-pages also uses it. I used struct if_nextdqblk as same as <linux/quota.h> defined. But I don't know why we can't use <linux/type.h> in lapi/quotactl.h and I also use it in lapi/seccomp.h. IMHO, they affected nothing. Or, they have some redefined errors or not having this headers files in special linux distribution. >> +struct if_nextdqblk { >> + __u64 dqb_bhardlimit; >> + __u64 dqb_bsoftlimit; >> + __u64 dqb_curspace; >> + __u64 dqb_ihardlimit; >> + __u64 dqb_isoftlimit; >> + __u64 dqb_curinodes; >> + __u64 dqb_btime; >> + __u64 dqb_itime; >> + __u32 dqb_valid; >> + __u32 dqb_id; >> +}; > ... >> +++ b/testcases/kernel/syscalls/quotactl/quotactl01.c >> @@ -1,7 +1,7 @@ >> // SPDX-License-Identifier: GPL-2.0-or-later >> /* >> * Copyright (c) Crackerjack Project., 2007 >> -* Copyright (c) 2016 Fujitsu Ltd. >> +* Copyright (c) 2016-2019 FUJITSU LIMITED. All rights reserved > BTW correct formatting is > /* > * > */ > Not > /* > * > */ > I'll change it during merge (nit, the code is what matters, not formatting, of course). > > ... >> +static int getnextquota_nsup; > ... >> static void setup(void) >> { >> const char *const cmd[] = {"quotacheck", "-ugF", "vfsv0", MNTPOINT, NULL}; >> int ret; >> + getnextquota_nsup = 0; > This is not needed (getnextquota_nsup is static and it's called just once, I'll > remove it before merge). Yes. > >> ret = tst_run_cmd(cmd, NULL, NULL, 1); >> switch (ret) { >> @@ -146,6 +183,11 @@ static void setup(void) >> if (access(GRPPATH, F_OK) == -1) >> tst_brk(TFAIL | TERRNO, "group quotafile didn't exist"); >> + >> + TEST(quotactl(QCMD(Q_GETNEXTQUOTA, USRQUOTA), tst_device->dev, >> + test_id, (void *) &res_ndq)); >> + if (TST_ERR == EINVAL || TST_ERR == ENOSYS) > Does EINVAL really mans not supported? Shouldn't be just for ENOSYS. EINVAL can mean non-supported by using correct argument. >> + getnextquota_nsup = 1; >> } > Looking at kernel sources - this does not look as not supported, but rather a > failure (we might want to add some test for EINVAL): > if (!qid_has_mapping(sb->s_user_ns, qid)) > return -EINVAL; > > kernel fs/quota/quota.c > /* > * Return quota for next active quota >= this id, if any exists, > * otherwise return -ENOENT via ->get_nextdqblk > */ > static int quota_getnextquota(struct super_block *sb, int type, qid_t id, > void __user *addr) > { > struct kqid qid; > struct qc_dqblk fdq; > struct if_nextdqblk idq; > int ret; > > if (!sb->s_qcop->get_nextdqblk) > return -ENOSYS; > qid = make_kqid(current_user_ns(), type, id); > if (!qid_has_mapping(sb->s_user_ns, qid)) > return -EINVAL; > ret = sb->s_qcop->get_nextdqblk(sb, &qid, &fdq); > if (ret) > return ret; > /* struct if_nextdqblk is a superset of struct if_dqblk */ > copy_to_if_dqblk((struct if_dqblk *)&idq, &fdq); > idq.dqb_id = from_kqid(current_user_ns(), qid); > if (copy_to_user(addr, &idq, sizeof(idq))) > return -EFAULT; > return 0; > } Hi Petr look do_quotactl function in fs/quota/quota.c. static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id, void __user *addr, const struct path *path) { switch (cmd) { case Q_QUOTAON: return quota_quotaon(sb, type, id, path); case Q_QUOTAOFF: return quota_quotaoff(sb, type); case Q_GETFMT: return quota_getfmt(sb, type, addr); case Q_GETINFO: ...... default: return -EINVAL; } } So if it doesn't have Q_GETNEXTQUOTA cmd, it should report EINVAL(we use correct argument and correct environment, so there is no failure). > Kind regards, > Petr > >
on 2019/11/20 23:16, Petr Vorel wrote: > Hi Jan, Cyril, Xu, > >>> +#ifdef HAVE_STRUCT_IF_NEXTDQBLK >>> +# include <linux/quota.h> >>> +#else >>> +# ifdef HAVE_LINUX_TYPES_H >>> +# include <linux/types.h> >> @Jan, @Cyril: Do we want to generally avoid loading <linux/types.h> if not really needed? >> __u64 can be uint64_t etc (as it's also visible in struct dqblk in <sys/quota.h> >> in various libc headers). >> We used this approach for /usr/include/linux/bpf.h and for fanotify fixes for >> musl (testcases/kernel/syscalls/fanotify/fanotify.h). >> So unless you're against this approach here I'll change it before merge >> (and add this info to next version of library API writing guidelines patch >> https://patchwork.ozlabs.org/patch/1166786/). > + general question: do we want always test against kernel headers or libc > headers? Libc is often outdated, so mostly it'd be our fallback to be tested. > Ideally both kernel and libc header should be tested, but that's not easily > achievable. IMHO, We often test libc and it usually includes kernel headers ie. <sys/quota.h> <sys/prctl.h>. I perfet to check one except that glibc and kernel they have themselves implementation . If the struct or variable is not defined, we can define it in ltp lapi headers. Then we can avoid build error and increase coverage(because kernel may implement it). > > Kind regards, > Petr >
Hi Xu, > > + general question: do we want always test against kernel headers or libc > > headers? Libc is often outdated, so mostly it'd be our fallback to be tested. > > Ideally both kernel and libc header should be tested, but that's not easily > > achievable. > IMHO, We often test libc and it usually includes kernel headers ie. > <sys/quota.h> <sys/prctl.h>. I perfet to check one except that glibc and > kernel they have themselves implementation . If the struct or variable is > not defined, we can define it in ltp lapi headers. Then we can avoid build > error and increase coverage(because kernel may implement it). Yep. I'm ok with using libc headers (increased coverage), but we need good checks anyway for other libc (at least for musl; bionic also like glibc uses internally kernel headers, uclibc-ng usually embeds kernel header parts and strives to be glibc compatible anyway). Kind regards, Petr
Hi Xu, > > @Jan, @Cyril: Do we want to generally avoid loading <linux/types.h> if not really needed? > > __u64 can be uint64_t etc (as it's also visible in struct dqblk in <sys/quota.h> > > in various libc headers). > > We used this approach for /usr/include/linux/bpf.h and for fanotify fixes for > > musl (testcases/kernel/syscalls/fanotify/fanotify.h). > > So unless you're against this approach here I'll change it before merge > > (and add this info to next version of library API writing guidelines patch > > https://patchwork.ozlabs.org/patch/1166786/). > I have no objection about using uint64_t becuase Q_GETNEXTQUOTA man-pages also uses it. > I used struct if_nextdqblk as same as <linux/quota.h> defined. But I don't know why we can't use > <linux/type.h> in lapi/quotactl.h and I also use it in lapi/seccomp.h. IMHO, they affected nothing. > Or, they have some redefined errors or not having this headers files in special linux distribution. Based on Jan's comment [1], maybe he meant it specifically for __kernel_fsid_t in fanotify.h, not as a general approach. __kernel_fsid_t is more complicated than uint64_t. That's why I'm asking whether there is a general approach we want to take. > > > + TEST(quotactl(QCMD(Q_GETNEXTQUOTA, USRQUOTA), tst_device->dev, > > > + test_id, (void *) &res_ndq)); > > > + if (TST_ERR == EINVAL || TST_ERR == ENOSYS) > > Does EINVAL really mans not supported? Shouldn't be just for ENOSYS. > EINVAL can mean non-supported by using correct argument. > look do_quotactl function in fs/quota/quota.c. > static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id, > void __user *addr, const struct path *path) > { > switch (cmd) { > case Q_QUOTAON: > return quota_quotaon(sb, type, id, path); > case Q_QUOTAOFF: > return quota_quotaoff(sb, type); > case Q_GETFMT: > return quota_getfmt(sb, type, addr); > case Q_GETINFO: > ...... > default: > return -EINVAL; > } > } > So if it doesn't have Q_GETNEXTQUOTA cmd, it should report EINVAL(we use correct argument and correct environment, so there is no failure). OK, you're right, thanks for info :). Kind regards, Petr [1] https://patchwork.ozlabs.org/patch/1178182/#2281586
on 2019/11/21 13:10, Petr Vorel wrote: > Hi Xu, > >>> + general question: do we want always test against kernel headers or libc >>> headers? Libc is often outdated, so mostly it'd be our fallback to be tested. >>> Ideally both kernel and libc header should be tested, but that's not easily >>> achievable. >> IMHO, We often test libc and it usually includes kernel headers ie. >> <sys/quota.h> <sys/prctl.h>. I perfet to check one except that glibc and >> kernel they have themselves implementation . If the struct or variable is >> not defined, we can define it in ltp lapi headers. Then we can avoid build >> error and increase coverage(because kernel may implement it). > Yep. I'm ok with using libc headers (increased coverage), but we need good > checks anyway for other libc (at least for musl; bionic also like glibc uses > internally kernel headers, uclibc-ng usually embeds kernel header parts and > strives to be glibc compatible anyway). Hi Petr Yes. I check <sys/quota.h> and <sys/prctl.h> on musl libc[1] and they don't include linux header files. So I think checking both kernel and libc headers on other libc(musl,bionic) is meaningful. ps: If our travis-ci has a target with musl, I think it will be better. I don't know whether possible. [1]http://git.musl-libc.org/cgit/musl/tree/include/sys/quota.h Thanks Yang Xu > > Kind regards, > Petr > >
on 2019/11/21 13:45, Petr Vorel wrote: > Based on Jan's comment [1], maybe he meant it specifically for __kernel_fsid_t in > fanotify.h, not as a general approach. __kernel_fsid_t is more complicated than > uint64_t. That's why I'm asking whether there is a general approach we want to > take. Hi Petr I see. Now, I think we should avoid to use <linux/types.h> because on musl libc doesn't have it. Also ,If we use uint64_t, they still failed on 2.6.32-754.el6.x86_64 with undefined . Or, we should use TST_ABI to define uint64_t them on myself. Thanks Yang Xu
Hi Xu, > Yes. I check <sys/quota.h> and <sys/prctl.h> on musl libc[1] and they don't include linux header files. > So I think checking both kernel and libc headers on other libc(musl,bionic) is meaningful. But in single C file we decide only on one of these two. It's a similar problem as testing raw syscall or libc wrapper (which we already solved with .test_variants). NOTE: Bionic has generate it's headers from linux headers. > ps: If our travis-ci has a target with musl, I think it will be better. I don't know whether possible. It's here. But it requires some more fixes (in a meantime files are deleted), so I'll post it once it's everything fixed https://github.com/pevik/ltp/commits/travis/musl https://travis-ci.org/pevik/ltp/builds/614575796 There is also CI on LTP on all glibc/uclibc-ng/musl in Buildroot http://autobuild.buildroot.net/index.php?reason=ltp-testsuite-20190930 + There are other CI based on yocto/openembedded on glibc/uclibc-ng/musl > [1]http://git.musl-libc.org/cgit/musl/tree/include/sys/quota.h Kind regards, Petr
Hi Xu, > I see. Now, I think we should avoid to use <linux/types.h> because on musl libc doesn't have it. IMHO <linux/types.h> are always installed from kernel, not from libc (packaged result of make headers_install run from kernel sources). > Also ,If we use uint64_t, they still failed on 2.6.32-754.el6.x86_64 with undefined . Or, we should use TST_ABI to define uint64_t them Hm, that what I said: using kernel headers is imho easier that using libc headers (fewer problems with compatibility). Anyway, I don't want to block this patchset. We can always merge it as it is and sort/fix this problem later. Kind regards, Petr
on 2019/11/21 16:32, Petr Vorel wrote: > Hi Xu, > >> I see. Now, I think we should avoid to use <linux/types.h> because on musl libc doesn't have it. > IMHO <linux/types.h> are always installed from kernel, not from libc > (packaged result of make headers_install run from kernel sources). Oh, you are right. For this case, using <linux/types.h> is right that we don't have situation such as fanotify . > >> Also ,If we use uint64_t, they still failed on 2.6.32-754.el6.x86_64 with undefined . Or, we should use TST_ABI to define uint64_t them > Hm, that what I said: using kernel headers is imho easier that using libc > headers (fewer problems with compatibility). > Anyway, I don't want to block this patchset. > We can always merge it as it is and sort/fix this problem later. Yes. kernel header is more eaiser that libc. Thanks. > > Kind regards, > Petr > >
----- Original Message ----- > @Jan, @Cyril: Do we want to generally avoid loading <linux/types.h> if not > really needed? Yes, we generally try to avoid including kernel headers. Our style-guide says "Don't use +linux/+ headers if at all possible". uapi on older distros was more prone to cause LTP build errors.
on 2019/11/21 16:21, Petr Vorel wrote: > Hi Xu, > >> Yes. I check <sys/quota.h> and <sys/prctl.h> on musl libc[1] and they don't include linux header files. >> So I think checking both kernel and libc headers on other libc(musl,bionic) is meaningful. > But in single C file we decide only on one of these two. > It's a similar problem as testing raw syscall or libc wrapper (which we already > solved with .test_variants). > NOTE: Bionic has generate it's headers from linux headers. I see. Thanks again for reminding Bionic like glibc uses linux headers. > >> ps: If our travis-ci has a target with musl, I think it will be better. I don't know whether possible. > It's here. But it requires some more fixes (in a meantime files are deleted), > so I'll post it once it's everything fixed > https://github.com/pevik/ltp/commits/travis/musl > https://travis-ci.org/pevik/ltp/builds/614575796 > > There is also CI on LTP on all glibc/uclibc-ng/musl in Buildroot > http://autobuild.buildroot.net/index.php?reason=ltp-testsuite-20190930 > > + There are other CI based on yocto/openembedded on glibc/uclibc-ng/musl It is so cool. And I think I can move your musl patches to my ltp fork, so that I can also test them on musl when I contribute my patches to ltp. ps: I also think we should not block this patchset. > >> [1]http://git.musl-libc.org/cgit/musl/tree/include/sys/quota.h > Kind regards, > Petr > >
Hi Jan, Li, > ----- Original Message ----- > > @Jan, @Cyril: Do we want to generally avoid loading <linux/types.h> if not > > really needed? > Yes, we generally try to avoid including kernel headers. Our style-guide says > "Don't use +linux/+ headers if at all possible". uapi on older distros > was more prone to cause LTP build errors. Thank you for pointing out docs, I completely forgot on this page. BTW it needs some update (examples use old API, linux_syscall_numbers.h). Also not sure if everything else still applies (4. Call APIs that don't require freeing up resources on failure first, 2. Sort headers). > > Also ,If we use uint64_t, they still failed on 2.6.32-754.el6.x86_64 with undefined . Or, we should use TST_ABI to define uint64_t them Jan, are you aware of this problem? Xu, I'm not sure if you're talking about uint64_t problematic in <linux/types.h> (as you mention kernel) or problem in glibc <sys/types.h> / <stdint.h> / <inttypes.h>? We have lots of code which is using some of these 3 libc headers, does it fail on 2.6.32? Does anybody compile for 2.6.32? I know we want to keep support for 2.6.x (we had some discussion in the past). Than it'd be good to have travis build for kernel 2.6.x. Back to patchset. I suggest to merge it as it is and I'll prepare patches, which use <sys/types.h> in our tests. Kind regards, Petr
----- Original Message ----- > > > Also ,If we use uint64_t, they still failed on 2.6.32-754.el6.x86_64 with > > > undefined . Or, we should use TST_ABI to define uint64_t them > Jan, are you aware of this problem? I'm not, it should be defined in stdint.h. # cat /etc/redhat-release Red Hat Enterprise Linux Server release 6.10 (Santiago) # grep uint64_t -r /usr/include/ | grep stdint /usr/include/stdint.h:typedef unsigned long int uint64_t; > Xu, I'm not sure if you're talking about uint64_t problematic in > <linux/types.h> > (as you mention kernel) or problem in glibc <sys/types.h> / <stdint.h> / > <inttypes.h>? > We have lots of code which is using some of these 3 libc headers, does it > fail > on 2.6.32? > > Does anybody compile for 2.6.32? [CC Li] I think RH still does compile latest for regression tests. RHEL6 will be in sustaining for couple more years.
Hi Jan, > ----- Original Message ----- > > > > Also ,If we use uint64_t, they still failed on 2.6.32-754.el6.x86_64 with > > > > undefined . Or, we should use TST_ABI to define uint64_t them > > Jan, are you aware of this problem? > I'm not, it should be defined in stdint.h. > # cat /etc/redhat-release > Red Hat Enterprise Linux Server release 6.10 (Santiago) > # grep uint64_t -r /usr/include/ | grep stdint > /usr/include/stdint.h:typedef unsigned long int uint64_t; > > Xu, I'm not sure if you're talking about uint64_t problematic in > > <linux/types.h> > > (as you mention kernel) or problem in glibc <sys/types.h> / <stdint.h> / > > <inttypes.h>? > > We have lots of code which is using some of these 3 libc headers, does it > > fail > > on 2.6.32? > > Does anybody compile for 2.6.32? > [CC Li] > I think RH still does compile latest for regression tests. RHEL6 will be in > sustaining for couple more years. Thanks for info. Sorry I forgot, we have CentOS 6.10 in travis, that should be enough for testing build in old toolchain.
diff --git a/configure.ac b/configure.ac index c8c0db2dc..50d14967d 100644 --- a/configure.ac +++ b/configure.ac @@ -251,6 +251,7 @@ AC_DEFINE_UNQUOTED(NUMA_ERROR_MSG, ["$numa_error_msg"], [Error message when no N LTP_CHECK_SYSCALL_PERF_EVENT_OPEN +LTP_CHECK_SYSCALL_QUOTACTL LTP_CHECK_SYSCALL_SIGNALFD LTP_CHECK_SYSCALL_UTIMENSAT LTP_CHECK_TASKSTATS diff --git a/include/lapi/quotactl.h b/include/lapi/quotactl.h index 729472f69..5c49cedce 100644 --- a/include/lapi/quotactl.h +++ b/include/lapi/quotactl.h @@ -1,26 +1,41 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* - * Copyright (c) 2017 Fujitsu Ltd. + * Copyright (c) 2017-2019 Fujitsu Ltd. * Author: Xiao Yang <yangx.jy@cn.fujitsu.com> - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. + * Author: Yang Xu <xuyang2018.jy@cn.jujitsu.com> */ #ifndef LAPI_QUOTACTL_H__ # define LAPI_QUOTACTL_H__ +#include <sys/quota.h> + +#ifdef HAVE_STRUCT_IF_NEXTDQBLK +# include <linux/quota.h> +#else +# ifdef HAVE_LINUX_TYPES_H +# include <linux/types.h> +struct if_nextdqblk { + __u64 dqb_bhardlimit; + __u64 dqb_bsoftlimit; + __u64 dqb_curspace; + __u64 dqb_ihardlimit; + __u64 dqb_isoftlimit; + __u64 dqb_curinodes; + __u64 dqb_btime; + __u64 dqb_itime; + __u32 dqb_valid; + __u32 dqb_id; +}; +#endif +#endif + # ifndef Q_XGETNEXTQUOTA # define Q_XGETNEXTQUOTA XQM_CMD(9) # endif +# ifndef Q_GETNEXTQUOTA +# define Q_GETNEXTQUOTA 0x800009 /* get disk limits and usage >= ID */ +# endif + #endif /* LAPI_QUOTACTL_H__ */ diff --git a/m4/ltp-quota.m4 b/m4/ltp-quota.m4 new file mode 100644 index 000000000..e8d08c6b7 --- /dev/null +++ b/m4/ltp-quota.m4 @@ -0,0 +1,7 @@ +dnl SPDX-License-Identifier: GPL-2.0-or-later +dnl Copyright (c) 2019 Fujitsu Ltd. +dnl Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com> + +AC_DEFUN([LTP_CHECK_SYSCALL_QUOTACTL],[ +AC_CHECK_TYPES([struct if_nextdqblk],,,[#include <linux/quota.h>]) +]) diff --git a/testcases/kernel/syscalls/quotactl/quotactl01.c b/testcases/kernel/syscalls/quotactl/quotactl01.c index 2f563515d..41662a818 100644 --- a/testcases/kernel/syscalls/quotactl/quotactl01.c +++ b/testcases/kernel/syscalls/quotactl/quotactl01.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) Crackerjack Project., 2007 -* Copyright (c) 2016 Fujitsu Ltd. +* Copyright (c) 2016-2019 FUJITSU LIMITED. All rights reserved * Author: Xiao Yang <yangx.jy@cn.fujitsu.com> * * This testcase checks the basic flag of quotactl(2) for non-XFS filesystems: @@ -16,19 +16,23 @@ * 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 turn off quota with Q_QUOTAOFF flag for user. -* 9) quotactl(2) succeeds to turn on quota with Q_QUOTAON flag for group. -* 10) quotactl(2) succeeds to set disk quota limits with Q_SETQUOTA flag +* 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. -* 11) quotactl(2) succeeds to get disk quota limits with Q_GETQUOTA flag +* 12) quotactl(2) succeeds to get disk quota limits with Q_GETQUOTA flag * for group. -* 12) quotactl(2) succeeds to set information about quotafile with Q_SETINFO +* 13) quotactl(2) succeeds to set information about quotafile with Q_SETINFO * flag for group. -* 13) quotactl(2) succeeds to get information about quotafile with Q_GETINFO +* 14) quotactl(2) succeeds to get information about quotafile with Q_GETINFO * flag for group. -* 14) quotactl(2) succeeds to get quota format with Q_GETFMT flag for group. -* 15) quotactl(2) succeeds to update quota usages with Q_SYNC flag for group. -* 16) quotactl(2) succeeds to turn off quota with Q_QUOTAOFF 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. */ #include "config.h" @@ -36,11 +40,13 @@ #include <string.h> #include <unistd.h> #include <stdio.h> -#include <sys/quota.h> +#include "lapi/quotactl.h" #include "tst_test.h" -#define QFMT_VFS_V0 2 +#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 @@ -60,6 +66,9 @@ static struct dqinfo set_qf = { }; static struct dqinfo res_qf; static int32_t fmt_buf; +static int getnextquota_nsup; + +static struct if_nextdqblk res_ndq; static struct tcase { int cmd; @@ -69,65 +78,93 @@ static struct tcase { void *res_data; int sz; char *des; + char *tname; } tcases[] = { {QCMD(Q_QUOTAON, USRQUOTA), &fmt_id, USRPATH, - NULL, NULL, 0, "turn on quota for user"}, + NULL, NULL, 0, "turn on quota for user", + "QCMD(Q_QUOTAON, USRQUOTA)"}, {QCMD(Q_SETQUOTA, USRQUOTA), &test_id, &set_dq, - NULL, NULL, 0, "set disk quota limit for user"}, + NULL, NULL, 0, "set disk quota limit for user", + "QCMD(Q_SETQUOTA, USRQUOTA)"}, {QCMD(Q_GETQUOTA, USRQUOTA), &test_id, &res_dq, &set_dq.dqb_bsoftlimit, &res_dq.dqb_bsoftlimit, - sizeof(res_dq.dqb_bsoftlimit), "get disk quota limit for user"}, + sizeof(res_dq.dqb_bsoftlimit), "get disk quota limit for user", + "QCMD(Q_GETQUOTA, USRQUOTA)"}, {QCMD(Q_SETINFO, USRQUOTA), &test_id, &set_qf, - NULL, NULL, 0, "set information about quotafile for user"}, + NULL, NULL, 0, "set information about quotafile for user", + "QCMD(Q_SETINFO, USRQUOTA)"}, {QCMD(Q_GETINFO, USRQUOTA), &test_id, &res_qf, &set_qf.dqi_bgrace, &res_qf.dqi_bgrace, sizeof(res_qf.dqi_bgrace), - "get information about quotafile for user"}, + "get information about quotafile for user", + "QCMD(Q_GETINFO, USRQUOTA)"}, {QCMD(Q_GETFMT, USRQUOTA), &test_id, &fmt_buf, &fmt_id, &fmt_buf, sizeof(fmt_buf), - "get quota format for user"}, + "get quota format for user", + "QCMD(Q_GETFMT, USRQUOTA)"}, {QCMD(Q_SYNC, USRQUOTA), &test_id, &res_dq, - NULL, NULL, 0, "update quota usages for user"}, + NULL, NULL, 0, "update quota usages for user", + "QCMD(Q_SYNC, USRQUOTA)"}, + + {QCMD(Q_GETNEXTQUOTA, USRQUOTA), &test_id, &res_ndq, + &test_id, &res_ndq.dqb_id, sizeof(res_ndq.dqb_id), + "get next disk quota limit for user", + "QCMD(Q_GETNEXTQUOTA, USRQUOTA)"}, {QCMD(Q_QUOTAOFF, USRQUOTA), &test_id, USRPATH, - NULL, NULL, 0, "turn off quota for user"}, + NULL, NULL, 0, "turn off quota for user", + "QCMD(Q_QUOTAOFF, USRQUOTA)"}, {QCMD(Q_QUOTAON, GRPQUOTA), &fmt_id, GRPPATH, - NULL, NULL, 0, "turn on quota for group"}, + NULL, NULL, 0, "turn on quota for group", + "QCMD(Q_QUOTAON, GRPQUOTA)"}, {QCMD(Q_SETQUOTA, GRPQUOTA), &test_id, &set_dq, - NULL, NULL, 0, "set disk quota limit for group"}, + NULL, NULL, 0, "set disk quota limit for group", + "QCMD(Q_SETQUOTA, GRPQUOTA)"}, {QCMD(Q_GETQUOTA, GRPQUOTA), &test_id, &res_dq, &set_dq.dqb_bsoftlimit, &res_dq.dqb_bsoftlimit, sizeof(res_dq.dqb_bsoftlimit), - "set disk quota limit for group"}, + "set disk quota limit for group", + "QCMD(Q_GETQUOTA, GRPQUOTA)"}, {QCMD(Q_SETINFO, GRPQUOTA), &test_id, &set_qf, - NULL, NULL, 0, "set information about quotafile for group"}, + NULL, NULL, 0, "set information about quotafile for group", + "QCMD(Q_SETINFO, GRPQUOTA)"}, {QCMD(Q_GETINFO, GRPQUOTA), &test_id, &res_qf, &set_qf.dqi_bgrace, &res_qf.dqi_bgrace, sizeof(res_qf.dqi_bgrace), - "get information about quotafile for group"}, + "get information about quotafile for group", + "QCMD(Q_GETINFO, GRPQUOTA)"}, {QCMD(Q_GETFMT, GRPQUOTA), &test_id, &fmt_buf, - &fmt_id, &fmt_buf, sizeof(fmt_buf), "get quota format for group"}, + &fmt_id, &fmt_buf, sizeof(fmt_buf), "get quota format for group", + "QCMD(Q_GETFMT, GRPQUOTA)"}, {QCMD(Q_SYNC, GRPQUOTA), &test_id, &res_dq, - NULL, NULL, 0, "update quota usages for group"}, + NULL, NULL, 0, "update quota usages for group", + "QCMD(Q_SYNC, GRPQUOTA)"}, + + {QCMD(Q_GETNEXTQUOTA, GRPQUOTA), &test_id, &res_ndq, + &test_id, &res_ndq.dqb_id, sizeof(res_ndq.dqb_id), + "get next disk quota limit for group", + "QCMD(Q_GETNEXTQUOTA, GRPQUOTA)"}, {QCMD(Q_QUOTAOFF, GRPQUOTA), &test_id, GRPPATH, - NULL, NULL, 0, "turn off quota for group"} + NULL, NULL, 0, "turn off quota for group", + "QCMD(Q_QUOTAOFF, GRPQUOTA)"}, }; static void setup(void) { const char *const cmd[] = {"quotacheck", "-ugF", "vfsv0", MNTPOINT, NULL}; int ret; + getnextquota_nsup = 0; ret = tst_run_cmd(cmd, NULL, NULL, 1); switch (ret) { @@ -146,6 +183,11 @@ static void setup(void) if (access(GRPPATH, F_OK) == -1) tst_brk(TFAIL | TERRNO, "group quotafile didn't exist"); + + TEST(quotactl(QCMD(Q_GETNEXTQUOTA, USRQUOTA), tst_device->dev, + test_id, (void *) &res_ndq)); + if (TST_ERR == EINVAL || TST_ERR == ENOSYS) + getnextquota_nsup = 1; } static void verify_quota(unsigned int n) @@ -155,7 +197,15 @@ static void verify_quota(unsigned int n) res_dq.dqb_bsoftlimit = 0; res_qf.dqi_igrace = 0; fmt_buf = 0; + res_ndq.dqb_id = -1; + tst_res(TINFO, "Test #%d: %s", n, tc->tname); + if ((tc->cmd == QCMD(Q_GETNEXTQUOTA, USRQUOTA) || + tc->cmd == QCMD(Q_GETNEXTQUOTA, GRPQUOTA)) && + getnextquota_nsup) { + tst_res(TCONF, "current system doesn't support this cmd, skip it"); + return; + } TEST(quotactl(tc->cmd, tst_device->dev, *tc->id, tc->addr)); if (TST_RET == -1) { tst_res(TFAIL | TTERRNO, "quotactl failed to %s", tc->des); @@ -189,4 +239,3 @@ static struct tst_test test = { .mnt_data = "usrquota,grpquota", .setup = setup, }; -
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 | 43 ++++--- m4/ltp-quota.m4 | 7 ++ .../kernel/syscalls/quotactl/quotactl01.c | 107 +++++++++++++----- 4 files changed, 115 insertions(+), 43 deletions(-) create mode 100644 m4/ltp-quota.m4