On Wed, Jul 15, 2020 at 02:20:32PM -0700, Darrick J. Wong wrote:
> On Wed, Jul 15, 2020 at 07:39:48PM +0100, Christoph Hellwig wrote:
> > On Wed, Jul 15, 2020 at 11:38:38AM -0700, Darrick J. Wong wrote:
> > > On Wed, Jul 15, 2020 at 06:43:40PM +0100, Christoph Hellwig wrote:
> > > > On Tue, Jul 14, 2020 at 11:05:02AM -0700, Darrick J. Wong wrote:
> > > > > On Tue, Jul 14, 2020 at 08:57:56AM +0100, Christoph Hellwig wrote:
> > > > > > On Mon, Jul 13, 2020 at 06:32:00PM -0700, Darrick J. Wong wrote:
> > > > > > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > > > > >
> > > > > > > Create a new type (xfs_dqtype_t) to represent the type of an incore
> > > > > > > dquot. Break the type field out from the dq_flags field of the incore
> > > > > > > dquot.
> > > > > >
> > > > > > I don't understand why we need separate in-core vs on-disk values for
> > > > > > the type. Why not something like this on top of the whole series:
> > > > >
> > > > > I want to keep the ondisk d_type values separate from the incore q_type
> > > > > values because they don't describe exactly the same concepts:
> > > > >
> > > > > First, the incore qtype has a NONE value that we can pass to the dquot
> > > > > core verifier when we don't actually know if this is a user, group, or
> > > > > project dquot. This should never end up on disk.
> > > >
> > > > Which we can trivially verify. Or just get rid of NONE, which actually
> > > > cleans things up a fair bit (patch on top of my previous one below)
> > >
> > > Ok, I'll get rid of that usage.
> > >
> > > > > Second, xfs_dqtype_t is a (barely concealed) enumeration type for quota
> > > > > callers to tell us that they want to perform an action on behalf of
> > > > > user, group, or project quotas. The incore q_flags and the ondisk
> > > > > d_type contain internal state that should not be exposed to quota
> > > > > callers.
> > > >
> > > > I don't think that is an argument, as we do the same elsewhere.
> > > >
> > > > >
> > > > > I feel a need to reiterate that I'm about to start adding more flags to
> > > > > d_type (for y2038+ time support), for which it will be very important to
> > > > > keep d_type and q_{type,flags} separate.
> > > >
> > > > Why? We'll just OR the bigtime flag in before writing to disk.
> > >
> > > Ugh, fine, I'll rework the whole series yet again, since it doesn't look
> > > like anyone else is going to have the time to review a 27 patch cleanup
> > > series.
> >
> > Let's just get your series in and I'll send an incremental patch
> > after the bigtime series..
>
> Seeing as I already refactored yesterday to produce v4 and am now midway
> through refactoring to produce v5 I just going to keep going with even
> more ******* cleanups.
To elaborate on that, I've split all the patches that /have/ been
reviewed into one branch, and all the flags cleanups into a second
branch. If that second branch passes testing I'll mail it out in the
morning.
--D
> --D
