Re: [PATCH v8 4/5] xfs: Add proper versioning support to fs_quota_stat

[Chandra Seetharaman <sekharan@us.ibm.com>]

On Tue, 2013-06-11 at 09:27 +1000, Dave Chinner wrote:
> On Fri, May 24, 2013 at 05:17:22PM -0500, Chandra Seetharaman wrote:
> > On Fri, 2013-05-17 at 15:10 +1000, Dave Chinner wrote:
> > > On Fri, May 10, 2013 at 04:21:28PM -0500, Chandra Seetharaman wrote:
> > > > Added appropriate pads and code for backward compatibility.
> > > > 
> > > > Copied over the old version as it is different from the newer padded
> > > > version.
> > > > 
> > > > New callers of the system call have to just set the version of the data
> > > > structure being passed, and kernel will fill as much data as requested.
> > > > 
> > > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > > 
> > > This needs to be cc'd to Steve Whitehouse so he's aware that we are
> > > changing the quota interface...
> 
> > > > +/*
> > > > + * Since Version 1 did not have padding at appropriate places,
> > > > + * a new data structure has been defined for the older version to
> > > > + * provide backward compatibility.
> > > > + * Future extentions of this data structure won't require new
> > > > + * data structure definitions as the current one can be extended
> > > > + * with the logic and padding in place now.
> > > > + */
> > > > +#define FS_QSTAT_V1_SIZE	(sizeof(struct fs_quota_stat_v1))
> > > > +#define FS_QSTAT_V2_SIZE	(offsetof(struct fs_quota_stat, qs_pquota) + \
> > > > +					sizeof(fs_qfilestat_t))
> > > 
> > > Just use sizeof(struct fs_quota_stat) as the size. Indeed, for
> > 
> > My thinking w.r.t not making it sizeof (fs_quota_stat) is ...future
> > changes doesn't have to touch the macro definitions of earlier versions,
> > they can define a new macro just by copying the last V?_SIZE macro and
> > changing the last field name.
> 
> If we need a future revision, we can deal with the structure
> differences and versions at that point in time. Second guessing
> unknown future requirements is not necessary. ;)

sure.
> 
> > > future enhancements, maybe we should add 64 bytes of empty space at
> > > the end of the structure....
> > 
> > Since this version is fully backward compatible, I didn't think a future
> > pad was needed. Do you want me to add ?
> 
> We only really need to change the structure version when we change
> input parameters, the size or the shape of the structure being
> passed in from userspace. If we add padding now, then we can expand
> output of the call without needing to bump the version of the
> structure. Old code simply won't know (or care) about the new output
> in the region of the structure it considers empty padding....

Ok. I will all 64 bytes of additional padding at the end.

> 
> Cheers,
> 
> Dave.


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