[Top] [All Lists]

Re: [PATCH 1/3] quota: Add a new quotactl command Q_XGETQSTATV

To: Jan Kara <jack@xxxxxxx>
Subject: Re: [PATCH 1/3] quota: Add a new quotactl command Q_XGETQSTATV
From: Rich Johnston <rjohnston@xxxxxxx>
Date: Tue, 13 Aug 2013 17:22:25 -0500
Cc: Chandra Seetharaman <sekharan@xxxxxxxxxx>, <xfs@xxxxxxxxxxx>, linux-fsdevel <linux-fsdevel@xxxxxxxxxxxxxxx>, Steven Whitehouse <swhiteho@xxxxxxxxxx>, Abhijith Das <adas@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130813212242.GA9158@xxxxxxxxxxxxx>
References: <1375828029-26360-1-git-send-email-sekharan@xxxxxxxxxx> <1375828029-26360-2-git-send-email-sekharan@xxxxxxxxxx> <520A9A2F.5090009@xxxxxxx> <20130813212242.GA9158@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120714 Thunderbird/14.0
On 08/13/2013 04:22 PM, Jan Kara wrote:

   Neither me nor linux-fsdevel has been CCed on this change. Please do that
next time. Now looking into the patch in xfs mailing list archive I have
Did you mean the email or the commit header?

As far as I can see you and linux-fsdevel were CCed on this entire email thread.


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?
Otherwise the patch looks fine.


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