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

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs