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

Dave Chinner
2013-06-10T23:27:27Z

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. ;)

> > 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....

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com