Re: [PATCH 2/2] xfs: remove XFS_QMOPT_ENOSPC flag

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Fri, May 08, 2020 at 12:21:29PM -0400, Brian Foster wrote:
> On Fri, May 08, 2020 at 10:45:48AM -0500, Eric Sandeen wrote:
> > On 5/8/20 8:01 AM, Brian Foster wrote:
> > > On Thu, May 07, 2020 at 11:00:34PM -0500, Eric Sandeen wrote:
> > >> The only place we return -EDQUOT, and therefore need to make a decision
> > >> about returning -ENOSPC for project quota instead, is in xfs_trans_dqresv().
> > >>
> > >> So there's no reason to be setting and clearing XFS_QMOPT_ENOSPC at higher
> > >> levels; if xfs_trans_dqresv has failed, test if the dqp we were were handed
> > >> is a project quota and if so, return -ENOSPC instead of EDQUOT.  The
> > >> complexity is just a leftover from when project & group quota were mutually
> > >> exclusive and shared some codepaths.
> > >>
> > >> The prior patch was the trivial bugfix, this is the slightly more involved
> > >> cleanup.
> > >>
> > >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > >> ---
> > > 
> > > Hmm so what about callers that don't pass QMOPT_ENOSPC? For example, it
> > > looks like various xfs_trans_reserve_quota() calls pass a pdqp but don't
> > > set the flag.
> > 
> > Oh, interesting.  I bet that was an oversight, tbh, but let's see.
> > 
> > <rewinds 14 years>
> > 
> > commit 9a2a7de268f67fea0c450ed3e99a2d31f43d7166
> > Author: Nathan Scott <nathans@sgi.com>
> > Date:   Fri Mar 31 13:04:49 2006 +1000
> > 
> >     [XFS] Make project quota enforcement return an error code consistent with
> >     its use.
> > 
> > so yeah, even back then, stuff like xfs_symlink returned EDQUOT not ENOSPC.
> > 
> > Today, these call the reservation w/o the special ENOSPC flag:
> > 
> > xfs_unmap_extent
> > xfs_create
> > xfs_create_tmpfile
> > xfs_symlink
> > 
> > and so will return EDQUOT instead of ENOSPC even for project quota.
> > 
> > You're right that my patch changes these to ENOSPC.
> > 
> > > Is the intent to change behavior such that -ENOSPC is
> > > unconditional for project quota reservation failures?
> > 
> > Now it's a conundrum.  I /think/ the current behavior is due to an oversight, but 
> > 
> > a) I'm not certain, and
> > b) can we change it now?
> > 
> 
> Heh, I can't really tell what the intended/expected behavior is. For
> whatever it's worth, it seems reasonable enough to me to change it based
> on the fact that project quotas have been expected to return -ENOSPC in
> at least some common cases for many years. It seems unlikely that users
> would know or care about the change in behavior in the subset noted
> above, but who knows. It might be good to get some other opinions. :P

"I bet you a beer at the next conference (if they ever happen again)
that nobody will notice"? :P

TBH while I find it a little odd that project quota gets to return
ENOSPC instead of EDQUOT, I find it more odd that sometimes it doesn't.
This at least gets us to consistent behavior (EDQUOT for user/group,
ENOSPC for project) so for the series:

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

(Let's see what an fstests run spits out...)

--D

> Brian
> 
> > -Eric
> > 
>