xfs
[Top] [All Lists]

Re: [PATCH VER 4] Extend project quotas to support 32bit project identif

To: Arkadiusz Miskiewicz <arekm@xxxxxxxx>
Subject: Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators.
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 24 Sep 2010 10:37:32 +1000
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <201009222101.13985.arekm@xxxxxxxx>
References: <1285177343-11108-1-git-send-email-arekm@xxxxxxxx> <20100922184242.GA9911@xxxxxxxxxxxxx> <201009222101.13985.arekm@xxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Wed, Sep 22, 2010 at 09:01:13PM +0200, Arkadiusz Miskiewicz wrote:
> On Wednesday 22 of September 2010, Christoph Hellwig wrote:
> > On Wed, Sep 22, 2010 at 07:42:23PM +0200, Arkadiusz Mi??kiewicz wrote:
> > >   if (mask & FSX_PROJID) {
> > > 
> > > +         /*
> > > +          * Switch on the PROJID32BIT superblock bit when needed
> > > +          * (implies also FEATURES2)
> > > +          */
> > > +         if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) &&
> > > +                         fa->fsx_projid > (__uint16_t)-1)
> > > +                 xfs_addprojid32bit(tp, ip);
> > 
> > Didn't we agree that we want to enable this feature explicitly via
> > xfs_admin (or mkfs.xfs)?
> 
> Actually there was no agreement on this. Some think that it's good to do that 
> automaticly (so user doesn't have to do anything and has everything working) 
> and others think that it should be turned on explictly by xfs_admin/mkfs.xfs.
> 
> For me both methods are fine, both have some advantages and disadvantages.
> 
> (There was an agreement that separate projid (from group) quota should be 
> turned on manually but was no such agreement on projid32bit)

I think I eventually agreed with Christoph that xfs_admin/mkfs was
the way to go, simply from the principle of least surprise. i.e.
upgrading the kernel shouldn't make a projid set that previously
failed now succeed and prevent a kernel downgrade that is only
discovered during downgrade...


> > > - __u16           bs_projid;      /* project id                   */
> > > + __u16           bs_projid_lo;   /* lower part of project id     */
> > > 
> > >   __u16           bs_forkoff;     /* inode fork offset in bytes   */
> > > 
> > > - unsigned char   bs_pad[12];     /* pad space, unused            */
> > > + __u16           bs_projid_hi;   /* higher part of project id    */
> > > + unsigned char   bs_pad[10];     /* pad space, unused            */
> > 
> > Unlike in the inode we can't just rename the lo field here - that would
> > break the compilation of existing applications.
> 
> Ok but maybe breaking these is good?

No, there is never a good reason for breaking applications like
this.

> So these can be extended to support 32bit 
> projid. Otherwise these will get crap if 32bit projid is enabled (actually 
> already built binaries will still get crap for > 16bit values with 
> projid32bit 
> patch).

At which point a userspace upgrade is required to match the kernel
upgrade. Another reason so only allowing the feature to be switched
on via xfs-admin/mkfs - it means that userspace XFS utilities have
already been upgraded to support 32 bit projids before it can be
switched on.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>