| Message ID | 72a454f1-c2ee-b777-90db-6bdfd4a8572c@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | quota-tools: Set FS_DQ_TIMER_MASK for individual xfs grace times | expand |
On Wed 13-05-20 22:45:32, Eric Sandeen wrote: > xfs quota code doesn't currently allow increasing an individual > user's grace time, but kernel patches are in development for this. > > In order for setquota to be able to send this update via > setquota -T, we need to add the FS_DQ_TIMER_MASK when we are trying > to update the grace times on an individual user's dquot. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> The patch looks good to me. I've added it to my tree. > I wonder if we should only be setting the LIMIT_MASK only if > (flags & COMMIT_LIMITS), but it doesn't seem to be a problem and > is unrelated to this change I'm leaving it alone for now, though if > anyone thinks it's better I can update the patch. > > I'm putting together xfstests cases for this, if you want to wait > for those, that's fine. Thanks! Yeah, that looks like a good thing to do. Also FS_DQ_LIMIT_MASK contains real-time limits bits which quota tools aren't able to manipulate in any way so maybe not setting those bits would be wiser... Will you send a patch or should I just fix it? Honza > > diff --git a/quotaio_xfs.c b/quotaio_xfs.c > index b22c7b4..a4d6f67 100644 > --- a/quotaio_xfs.c > +++ b/quotaio_xfs.c > @@ -166,6 +166,8 @@ static int xfs_commit_dquot(struct dquot *dquot, int flags) > xdqblk.d_fieldmask |= FS_DQ_BCOUNT; > } else { > xdqblk.d_fieldmask |= FS_DQ_LIMIT_MASK; > + if (flags & COMMIT_TIMES) /* indiv grace period */ > + xdqblk.d_fieldmask |= FS_DQ_TIMER_MASK; > } > > qcmd = QCMD(Q_XFS_SETQLIM, h->qh_type); >
On 5/14/20 5:20 AM, Jan Kara wrote: >> I'm putting together xfstests cases for this, if you want to wait >> for those, that's fine. Thanks! > Yeah, that looks like a good thing to do. Also FS_DQ_LIMIT_MASK contains > real-time limits bits which quota tools aren't able to manipulate in any > way so maybe not setting those bits would be wiser... Will you send a patch > or should I just fix it? I've sent those tests now, btw. I agree that the whole section of flag-setting is a bit odd, I hadn't intended to clean it up right now. I'd be happy to review though if you found the time. :) Thanks, -Eric
On Tue 19-05-20 12:19:14, Eric Sandeen wrote: > On 5/14/20 5:20 AM, Jan Kara wrote: > >> I'm putting together xfstests cases for this, if you want to wait > >> for those, that's fine. Thanks! > > Yeah, that looks like a good thing to do. Also FS_DQ_LIMIT_MASK contains > > real-time limits bits which quota tools aren't able to manipulate in any > > way so maybe not setting those bits would be wiser... Will you send a patch > > or should I just fix it? > > I've sent those tests now, btw. > > I agree that the whole section of flag-setting is a bit odd, I hadn't > intended to clean it up right now. I'd be happy to review though if you > found the time. :) Patch attached :) Honza
On 5/20/20 9:27 AM, Jan Kara wrote: > On Tue 19-05-20 12:19:14, Eric Sandeen wrote: >> On 5/14/20 5:20 AM, Jan Kara wrote: >>>> I'm putting together xfstests cases for this, if you want to wait >>>> for those, that's fine. Thanks! >>> Yeah, that looks like a good thing to do. Also FS_DQ_LIMIT_MASK contains >>> real-time limits bits which quota tools aren't able to manipulate in any >>> way so maybe not setting those bits would be wiser... Will you send a patch >>> or should I just fix it? >> >> I've sent those tests now, btw. >> >> I agree that the whole section of flag-setting is a bit odd, I hadn't >> intended to clean it up right now. I'd be happy to review though if you >> found the time. :) > > Patch attached :) Oh ok I see. I should have just sent this, sorry. Thanks, yes this looks right. I was more bothered by the gfs2/xfs distinction but this is obviously a proper fix for the xfs conditional. Reviewed-by: Eric Sandeen <sandeen@redhat.com> > From 1814341547753865bcbd92bbe62af51f3e6866dd Mon Sep 17 00:00:00 2001 > From: Jan Kara <jack@suse.cz> > Date: Wed, 20 May 2020 16:22:52 +0200 > Subject: [PATCH] Fix limits setting on XFS filesystem > > xfs_commit_dquot() always set FS_DQ_LIMIT_MASK when calling > Q_XFS_SETQLIM. So far this wasn't a problem since quota tools didn't > support setting of anything else for XFS but now that kernel will start > supporting setting of grace times for XFS, we need to be more careful > and set limits bits only if we really want to update them. Also > FS_DQ_LIMIT_MASK contains real-time limits as well. Quota tools > currently don't support them in any way so avoid telling kernel to set > them. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > quotaio_xfs.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/quotaio_xfs.c b/quotaio_xfs.c > index a4d6f67b0c5a..3333bb1645d9 100644 > --- a/quotaio_xfs.c > +++ b/quotaio_xfs.c > @@ -165,7 +165,9 @@ static int xfs_commit_dquot(struct dquot *dquot, int flags) > if (flags & COMMIT_USAGE) /* block usage */ > xdqblk.d_fieldmask |= FS_DQ_BCOUNT; > } else { > - xdqblk.d_fieldmask |= FS_DQ_LIMIT_MASK; > + if (flags & COMMIT_LIMITS) /* warn/limit */ > + xdqblk.d_fieldmask |= FS_DQ_BSOFT | FS_DQ_BHARD | > + FS_DQ_ISOFT | FS_DQ_IHARD; > if (flags & COMMIT_TIMES) /* indiv grace period */ > xdqblk.d_fieldmask |= FS_DQ_TIMER_MASK; > } > -- > 2.16.4
diff --git a/quotaio_xfs.c b/quotaio_xfs.c index b22c7b4..a4d6f67 100644 --- a/quotaio_xfs.c +++ b/quotaio_xfs.c @@ -166,6 +166,8 @@ static int xfs_commit_dquot(struct dquot *dquot, int flags) xdqblk.d_fieldmask |= FS_DQ_BCOUNT; } else { xdqblk.d_fieldmask |= FS_DQ_LIMIT_MASK; + if (flags & COMMIT_TIMES) /* indiv grace period */ + xdqblk.d_fieldmask |= FS_DQ_TIMER_MASK; } qcmd = QCMD(Q_XFS_SETQLIM, h->qh_type);
xfs quota code doesn't currently allow increasing an individual user's grace time, but kernel patches are in development for this. In order for setquota to be able to send this update via setquota -T, we need to add the FS_DQ_TIMER_MASK when we are trying to update the grace times on an individual user's dquot. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- I wonder if we should only be setting the LIMIT_MASK only if (flags & COMMIT_LIMITS), but it doesn't seem to be a problem and is unrelated to this change I'm leaving it alone for now, though if anyone thinks it's better I can update the patch. I'm putting together xfstests cases for this, if you want to wait for those, that's fine. Thanks!