Re: [PATCH] quota-tools: Set FS_DQ_TIMER_MASK for individual xfs grace times

From: Eric Sandeen <sandeen@sandeen.net>
To: Jan Kara <jack@suse.cz>
Cc: "Eric Sandeen" <sandeen@redhat.com>,
	linux-xfs <linux-xfs@vger.kernel.org>,
	"Petr Písař" <ppisar@redhat.com>
Subject: Re: [PATCH] quota-tools: Set FS_DQ_TIMER_MASK for individual xfs grace times
Date: Wed, 20 May 2020 09:56:13 -0500	[thread overview]
Message-ID: <061d54e9-2364-1459-04cc-76279775dd0b@sandeen.net> (raw)
In-Reply-To: <20200520142752.GF30597@quack2.suse.cz>

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