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@xxxxxxxxxx> > > >> --- > > > > > > 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@xxxxxxx> > > 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@xxxxxxxxxx> (Let's see what an fstests run spits out...) --D > Brian > > > -Eric > > >
