<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">
<pre>on 2019/11/20 23:12, Petr Vorel wrote:</pre>
</div>
<blockquote type="cite" cite="mid:20191120151244.GA28197@dell5510">
<pre class="moz-quote-pre" wrap="">Hi Jan, Cyril, Xu,
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">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.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Signed-off-by: Yang Xu <a class="moz-txt-link-rfc2396E" href="mailto:xuyang2018.jy@cn.fujitsu.com"><xuyang2018.jy@cn.fujitsu.com></a>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Reviewed-by: Petr Vorel <a class="moz-txt-link-rfc2396E" href="mailto:pvorel@suse.cz"><pvorel@suse.cz></a>
LGTM, with 2 questions.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> #ifndef LAPI_QUOTACTL_H__
# define LAPI_QUOTACTL_H__
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+#include <sys/quota.h>
+
+#ifdef HAVE_STRUCT_IF_NEXTDQBLK
+# include <linux/quota.h>
+#else
+# ifdef HAVE_LINUX_TYPES_H
+# include <linux/types.h>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">@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
<a class="moz-txt-link-freetext" href="https://patchwork.ozlabs.org/patch/1166786/">https://patchwork.ozlabs.org/patch/1166786/</a>).
</pre>
</blockquote>
<pre>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.
</pre>
<blockquote type="cite" cite="mid:20191120151244.GA28197@dell5510">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+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;
+};
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
...
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+++ 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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">BTW correct formatting is
/*
*
*/
Not
/*
*
*/
I'll change it during merge (nit, the code is what matters, not formatting, of course).
...
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+static int getnextquota_nsup;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">...
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> static void setup(void)
{
const char *const cmd[] = {"quotacheck", "-ugF", "vfsv0", MNTPOINT, NULL};
int ret;
+ getnextquota_nsup = 0;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">This is not needed (getnextquota_nsup is static and it's called just once, I'll
remove it before merge).</pre>
</blockquote>
<pre>Yes.</pre>
<blockquote type="cite" cite="mid:20191120151244.GA28197@dell5510">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> ret = tst_run_cmd(cmd, NULL, NULL, 1);
switch (ret) {
@@ -146,6 +183,11 @@ static void setup(void)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> 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)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Does EINVAL really mans not supported? Shouldn't be just for ENOSYS.</pre>
</blockquote>
<pre>EINVAL can mean non-supported by using correct argument.</pre>
<blockquote type="cite" cite="mid:20191120151244.GA28197@dell5510">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ getnextquota_nsup = 1;
}
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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;
}
</pre>
</blockquote>
<pre>Hi Petr</pre>
<pre>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)
{</pre>
<pre> 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:
......</pre>
<pre> default:
return -EINVAL;
}
}</pre>
<pre>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).</pre>
<blockquote type="cite" cite="mid:20191120151244.GA28197@dell5510">
<pre class="moz-quote-pre" wrap="">
Kind regards,
Petr
</pre>
</blockquote>
</body>
</html>