xfs
[Top] [All Lists]

Re: [PATCH 2/4] xfsprogs: Add new field pquotaino to on disk superblock

To: Chandra Seetharaman <sekharan@xxxxxxxxxx>
Subject: Re: [PATCH 2/4] xfsprogs: Add new field pquotaino to on disk superblock
From: Ben Myers <bpm@xxxxxxx>
Date: Fri, 4 May 2012 11:09:53 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20120123173258.31735.97984.sendpatchset@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <20120123173243.31735.37262.sendpatchset@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20120123173258.31735.97984.sendpatchset@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Mon, Jan 23, 2012 at 11:32:58AM -0600, Chandra Seetharaman wrote:
> >From e0df4087facfd89e59183f4b8003196396d1a6d3 Mon Sep 17 00:00:00 2001
> From: Chandra Seetharaman <sekharan@xxxxxxxxxx>
> Date: Tue, 13 Dec 2011 16:06:33 -0600
> Subject: [PATCH 2/4] Add a new field sb_pquotino to the on-disk superblock 
> data structure
>  and add accompanying code.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@xxxxxxxxxx>

Looking pretty good.  Again, if you could be more specific about what the patch
does in your commit header it'd be helpful.

...

> @@ -174,11 +192,36 @@ xfs_sb_to_disk(
>       xfs_sb_field_t  f;
>       int             first;
>       int             size;
> +     __be16          saved_qflags = 0;
>  
>       ASSERT(fields);
>       if (!fields)
>               return;
>  
> +     if (!xfs_sb_version_hasnooquota(from) &&
> +                 (from->sb_qflags & (XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
> +                                     XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD))) {
> +
> +             if (from->sb_qflags & XFS_PQUOTA_ACCT) {
> +                     from->sb_gquotino = from->sb_pquotino;
> +                     from->sb_pquotino = 0;
> +             }
> +             /*
> +              * in-core version of qflags do not have XFS_OQUOTA.*, whereas
> +              * the on-disk version does. So, save the in-core sb_qflags
> +              * and restore it after we modify and copy it to the buffer
> +              * to be copied to disk.
> +              */
> +             saved_qflags = from->sb_qflags;
> +
> +             if (from->sb_qflags & (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
> +                     from->sb_qflags |= XFS_OQUOTA_ENFD;
> +             if (from->sb_qflags & (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
> +                     from->sb_qflags |= XFS_OQUOTA_CHKD;
> +             from->sb_qflags &= ~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
> +                                     XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
> +     }
> +
>       while (fields) {
>               f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
>               first = xfs_sb_info[f].offset;
> @@ -209,6 +252,14 @@ xfs_sb_to_disk(
>  
>               fields &= ~(1LL << f);
>       }
> +     /* Revert to the old saved values */
> +     if (saved_qflags) {
> +             from->sb_qflags = saved_qflags;
> +             if (from->sb_qflags & XFS_PQUOTA_ACCT) {
> +                     from->sb_pquotino = from->sb_gquotino;
> +                     from->sb_gquotino = 0;
> +             }
> +     }
>  }
>

I'd really prefer not modifying 'from' temporarily.  If possible use a temporary
variable and do the conversion manually for qflags as you have done in the
kernel.

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH 2/4] xfsprogs: Add new field pquotaino to on disk superblock, Ben Myers <=