| Message ID | ca1d2bb6-6f37-255c-1015-a20c6060d81c@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] xfs: allow adjusting individual quota grace times | expand |
The userspace change is basically just modifying timer_f so we can
specify an id; if it's not id 0, we set the grace time to now() + value,
iff the user is over the soft limit (this mirrors what setquota does):
+ /*
+ * If id is specified we are extending grace time by value
+ * Otherwise we are setting the default grace time
+ */
+ if (id) {
+ time_t now;
+
+ if (xfsquotactl(XFS_GETQUOTA, dev, type, id, (void *)&d) < 0) {
+ exitcode = 1;
+ fprintf(stderr, _("%s: cannot get quota: %s\n"),
+ progname, strerror(errno));
+ return;
+ }
+
+ time(&now);
+
+ if (d.d_blk_hardlimit && d.d_bcount > d.d_blk_hardlimit)
+ d.d_btimer = now + value;
+ if (d.d_ino_softlimit && d.d_icount > d.d_ino_softlimit)
+ d.d_itimer = now + value;
+ if (d.d_rtb_softlimit && d.d_rtbcount > d.d_rtb_softlimit)
+ d.d_rtbtimer = now + value;
+ } else {
+ d.d_btimer = value;
+ d.d_itimer = value;
+ d.d_rtbtimer = value;
+ }
+
and then we set the quota via the quotactl as normal.
On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote: > So there's no real way to know that the grace period adjustment failed > on an older kernel. We could consider that a bug and fix it, or > consider it a change in behavior that we can't just make without > at least some form of versioning. Thoughts? I'd consider a bug, as applications are very unlikely to rely on these ignored calls (famous last words..).
On Mon, May 11, 2020 at 08:35:32AM -0700, Christoph Hellwig wrote: > On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote: > > So there's no real way to know that the grace period adjustment failed > > on an older kernel. We could consider that a bug and fix it, or > > consider it a change in behavior that we can't just make without > > at least some form of versioning. Thoughts? Well you /could/ roll this new functionality into a new revision of the ioctl that supports 64bit time values... > I'd consider a bug, as applications are very unlikely to rely on > these ignored calls (famous last words..). ...but OTOH that might be a better discussion to have when we start reviewing the bigtime series. In the meantime, seeing as xfs_quota never really let you push out soft grace period expiration anyway, I guess you could just teach userspace to try to make the change and declare success if an immediate re-query shows the value changed. --D
On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote: > vfs/ext3/4 quota allows the administrator to push out the grace time > for soft quota with the setquota -T command: > > setquota -T [ -u | -g ] [ -F quotaformat ] name block-grace inode-grace -a | filesystem... > > -T, --edit-times > Alter times for individual user/group when softlimit is enforced. > Times block-grace and inode-grace are specified in seconds or can > be string 'unset'. > > Essentially, if you do "setquota -T -u username 1d 1d" and "username" is > over their soft quotas and into their grace time, it will extend the > grace time expiry to 1d from now. > > xfs can't do this, today. The patch below is a first cut at allowing us > to do this, and userspace updates are needed as well (I have those in a > patch stack.) > > I'm not looking so much for patch review right now, though, what I'm > wondering is if this is a change we can make from the ABI perspective? > > Because today, if you try to pass in a UID other than 0 (i.e. the > default grace period) it just gets ignored by the kernel, not rejected. > > So there's no real way to know that the grace period adjustment failed > on an older kernel. We could consider that a bug and fix it, or > consider it a change in behavior that we can't just make without > at least some form of versioning. Thoughts? > > Anyway, the patch below moves the disk quota grace period adjustment out > from "if id == 0" and allows the change for any ID; it only sets the > default grace value in the "id == 0" case. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c > index f48561b7e947..e58ee98f938c 100644 > --- a/fs/xfs/xfs_qm_syscalls.c > +++ b/fs/xfs/xfs_qm_syscalls.c > @@ -555,32 +555,41 @@ xfs_qm_scall_setqlim( > ddq->d_rtbwarns = cpu_to_be16(newlim->d_rt_spc_warns); > > if (id == 0) { > - /* > - * Timelimits for the super user set the relative time > - * the other users can be over quota for this file system. > - * If it is zero a default is used. Ditto for the default > - * soft and hard limit values (already done, above), and > - * for warnings. > - */ > - if (newlim->d_fieldmask & QC_SPC_TIMER) { > - defq->btimelimit = newlim->d_spc_timer; > - ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer); > - } > - if (newlim->d_fieldmask & QC_INO_TIMER) { > - defq->itimelimit = newlim->d_ino_timer; > - ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer); > - } > - if (newlim->d_fieldmask & QC_RT_SPC_TIMER) { > - defq->rtbtimelimit = newlim->d_rt_spc_timer; > - ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer); > - } > if (newlim->d_fieldmask & QC_SPC_WARNS) > defq->bwarnlimit = newlim->d_spc_warns; > if (newlim->d_fieldmask & QC_INO_WARNS) > defq->iwarnlimit = newlim->d_ino_warns; > if (newlim->d_fieldmask & QC_RT_SPC_WARNS) > defq->rtbwarnlimit = newlim->d_rt_spc_warns; > - } else { > + } > + > + /* > + * Timelimits for the super user set the relative time the other users > + * can be over quota for this file system. If it is zero a default is > + * used. Ditto for the default soft and hard limit values (already > + * done, above), and for warnings. > + * > + * For other IDs, userspace can bump out the grace period if over > + * the soft limit. > + */ > + if (newlim->d_fieldmask & QC_SPC_TIMER) { > + if (!id) > + defq->btimelimit = newlim->d_spc_timer; > + ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer); > + } > + if (newlim->d_fieldmask & QC_INO_TIMER) { > + printk("setting inode timer to %d\n", newlim->d_ino_timer); Stray printk here. > + if (!id) > + defq->itimelimit = newlim->d_ino_timer; > + ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer); > + } > + if (newlim->d_fieldmask & QC_RT_SPC_TIMER) { > + if (!id) > + defq->rtbtimelimit = newlim->d_rt_spc_timer; > + ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer); > + } Otherwise I guess this looks reasonable. It might help to patchbomb all the kernel/userspace/fstests changes together so I don't have to go scurrying around to find the pieces(?) --D > + > + if (id != 0) { > /* > * If the user is now over quota, start the timelimit. > * The user will not be 'warned'. > >
On 5/12/20 6:29 PM, Darrick J. Wong wrote: > On Sat, May 09, 2020 at 02:47:02PM -0500, Eric Sandeen wrote: >> vfs/ext3/4 quota allows the administrator to push out the grace time >> for soft quota with the setquota -T command: >> >> setquota -T [ -u | -g ] [ -F quotaformat ] name block-grace inode-grace -a | filesystem... >> >> -T, --edit-times >> Alter times for individual user/group when softlimit is enforced. >> Times block-grace and inode-grace are specified in seconds or can >> be string 'unset'. >> >> Essentially, if you do "setquota -T -u username 1d 1d" and "username" is >> over their soft quotas and into their grace time, it will extend the >> grace time expiry to 1d from now. >> >> xfs can't do this, today. The patch below is a first cut at allowing us >> to do this, and userspace updates are needed as well (I have those in a >> patch stack.) >> >> I'm not looking so much for patch review right now, though, what I'm >> wondering is if this is a change we can make from the ABI perspective? ... >> + * For other IDs, userspace can bump out the grace period if over >> + * the soft limit. >> + */ >> + if (newlim->d_fieldmask & QC_SPC_TIMER) { >> + if (!id) >> + defq->btimelimit = newlim->d_spc_timer; >> + ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer); >> + } >> + if (newlim->d_fieldmask & QC_INO_TIMER) { >> + printk("setting inode timer to %d\n", newlim->d_ino_timer); > > Stray printk here. > >> + if (!id) >> + defq->itimelimit = newlim->d_ino_timer; >> + ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer); >> + } >> + if (newlim->d_fieldmask & QC_RT_SPC_TIMER) { >> + if (!id) >> + defq->rtbtimelimit = newlim->d_rt_spc_timer; >> + ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer); >> + } > > Otherwise I guess this looks reasonable. It might help to patchbomb all > the kernel/userspace/fstests changes together so I don't have to go > scurrying around to find the pieces(?) yeah I will do that, this was more akin to pseudocode fo the larger "can I even change this behavior?" question. Thanks, -Eric
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c index f48561b7e947..e58ee98f938c 100644 --- a/fs/xfs/xfs_qm_syscalls.c +++ b/fs/xfs/xfs_qm_syscalls.c @@ -555,32 +555,41 @@ xfs_qm_scall_setqlim( ddq->d_rtbwarns = cpu_to_be16(newlim->d_rt_spc_warns); if (id == 0) { - /* - * Timelimits for the super user set the relative time - * the other users can be over quota for this file system. - * If it is zero a default is used. Ditto for the default - * soft and hard limit values (already done, above), and - * for warnings. - */ - if (newlim->d_fieldmask & QC_SPC_TIMER) { - defq->btimelimit = newlim->d_spc_timer; - ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer); - } - if (newlim->d_fieldmask & QC_INO_TIMER) { - defq->itimelimit = newlim->d_ino_timer; - ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer); - } - if (newlim->d_fieldmask & QC_RT_SPC_TIMER) { - defq->rtbtimelimit = newlim->d_rt_spc_timer; - ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer); - } if (newlim->d_fieldmask & QC_SPC_WARNS) defq->bwarnlimit = newlim->d_spc_warns; if (newlim->d_fieldmask & QC_INO_WARNS) defq->iwarnlimit = newlim->d_ino_warns; if (newlim->d_fieldmask & QC_RT_SPC_WARNS) defq->rtbwarnlimit = newlim->d_rt_spc_warns; - } else { + } + + /* + * Timelimits for the super user set the relative time the other users + * can be over quota for this file system. If it is zero a default is + * used. Ditto for the default soft and hard limit values (already + * done, above), and for warnings. + * + * For other IDs, userspace can bump out the grace period if over + * the soft limit. + */ + if (newlim->d_fieldmask & QC_SPC_TIMER) { + if (!id) + defq->btimelimit = newlim->d_spc_timer; + ddq->d_btimer = cpu_to_be32(newlim->d_spc_timer); + } + if (newlim->d_fieldmask & QC_INO_TIMER) { + printk("setting inode timer to %d\n", newlim->d_ino_timer); + if (!id) + defq->itimelimit = newlim->d_ino_timer; + ddq->d_itimer = cpu_to_be32(newlim->d_ino_timer); + } + if (newlim->d_fieldmask & QC_RT_SPC_TIMER) { + if (!id) + defq->rtbtimelimit = newlim->d_rt_spc_timer; + ddq->d_rtbtimer = cpu_to_be32(newlim->d_rt_spc_timer); + } + + if (id != 0) { /* * If the user is now over quota, start the timelimit. * The user will not be 'warned'.
vfs/ext3/4 quota allows the administrator to push out the grace time for soft quota with the setquota -T command: setquota -T [ -u | -g ] [ -F quotaformat ] name block-grace inode-grace -a | filesystem... -T, --edit-times Alter times for individual user/group when softlimit is enforced. Times block-grace and inode-grace are specified in seconds or can be string 'unset'. Essentially, if you do "setquota -T -u username 1d 1d" and "username" is over their soft quotas and into their grace time, it will extend the grace time expiry to 1d from now. xfs can't do this, today. The patch below is a first cut at allowing us to do this, and userspace updates are needed as well (I have those in a patch stack.) I'm not looking so much for patch review right now, though, what I'm wondering is if this is a change we can make from the ABI perspective? Because today, if you try to pass in a UID other than 0 (i.e. the default grace period) it just gets ignored by the kernel, not rejected. So there's no real way to know that the grace period adjustment failed on an older kernel. We could consider that a bug and fix it, or consider it a change in behavior that we can't just make without at least some form of versioning. Thoughts? Anyway, the patch below moves the disk quota grace period adjustment out from "if id == 0" and allows the change for any ID; it only sets the default grace value in the "id == 0" case. Signed-off-by: Eric Sandeen <sandeen@redhat.com> ---