On Tue 13-08-13 17:39:04, Chandra Seetharaman wrote: > On Tue, 2013-08-13 at 23:22 +0200, Jan Kara wrote: > > Hi, > > > > Neither me nor linux-fsdevel has been CCed on this change. Please do that > > Jan, > > All the CC in the email you got were from my original email. I did CC > you and linux-fsdevel when I sent this patchset a week ago > (http://oss.sgi.com/archives/xfs/2013-08/msg00171.html). > > I am confused on what happened and how you didn't get the original > email. Just now I checked linux-fsdevel archive. I do not see it there > either. Bizarre. (May be something wrong in the way I used > git-send-email) > > Sorry. No problem. > > next time. Now looking into the patch in xfs mailing list archive I have > > one comment: You declare: > > struct fs_quota_statv { > > __s8 qs_version; /* version for future changes */ > > __u8 qs_pad1; /* pad for 16bit alignment */ > > __u16 qs_flags; /* FS_QUOTA_.* flags */ > > __u32 qs_incoredqs; /* number of dquots incore */ > > struct fs_qfilestatv qs_uquota; /* user quota information */ > > struct fs_qfilestatv qs_gquota; /* group quota information */ > > struct fs_qfilestatv qs_pquota; /* project quota information */ > > __s32 qs_btimelimit; /* limit for blks timer */ > > __s32 qs_itimelimit; /* limit for inodes timer */ > > __s32 qs_rtbtimelimit;/* limit for rt blks timer */ > > __u16 qs_bwarnlimit; /* limit for num warnings */ > > __u16 qs_iwarnlimit; /* limit for num warnings */ > > __u64 qs_pad2[8]; /* for future proofing */ > > }; > > > > Now do you really need qs_pad2 field? Since the structure is properly > > versioned now, even its size can vary between versions, cannot it? > > Yes, it can. > > I added the pad based on Dave Chinner's suggestion: OK, makes sense. So I'm ok with the patch. You can add: Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > > ---------- > > > Dave: > > > > > future enhancements, maybe we should add 64 bytes of empty > > > > > space at the end of the structure.... > > > > Chandra: > > > > Since this version is fully backward compatible, I didn't think a > > > > future pad was needed. Do you want me to add ? > > > Dave: > > > 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.... > > Chandra: > > Ok. I will all 64 bytes of additional padding at the end. > > Otherwise the patch looks fine. > > > ---------- > > His argument convinced me to add the padding. What do you think ? > > > > Honza > > > > -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
