Re: [RFC v3 PATCH 1/4] xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD

[Ben Myers <bpm@sgi.com>]

Hey Chandra,

On Mon, Feb 13, 2012 at 10:05:57AM -0600, Chandra Seetharaman wrote:
> On Fri, 2012-02-10 at 18:41 -0600, Ben Myers wrote:
> 
> <snip>
> 
> > > @@ -657,6 +686,7 @@ xfs_sb_to_disk(
> > >  
> > >  		fields &= ~(1LL << f);
> > >  	}
> > > +	from->sb_qflags = saved_qflags;
> > 
> > I am disgusted that you should have to save the sb_qflags like this.
> > After reading this 'fields' loop in xfs_sb_to_disk along with all the
> > related crap I am actually feeling a little nauseous.
> 
> Let me know if you have any other solutions. I am open for it.

How would you feel about something like this?

@@ -622,6 +636,7 @@ xfs_sb_to_disk(
        xfs_sb_field_t  f;
        int             first;
        int             size;
+       __uint16_t      tmp16;

        ASSERT(fields);
        if (!fields)
@@ -636,6 +651,27 @@ xfs_sb_to_disk(

                if (size == 1 || xfs_sb_info[f].type == 1) {
                        memcpy(to_ptr + first, from_ptr + first, size);
+               } else if (f == XFS_SBS_QFLAGS) {
+                       /*
+                        * The in-core version of sb_qflags do not have
+                        * XFS_OQUOTA_* flags, whereas the on-disk version
+                        * does.  Save the in-core sb_qflags temporarily,
+                        * removing the new XFS_{PG}QUOTA_* flags and re-apply
+                        * the old on-disk flags.  This is a temporary
+                        * situation until the on-disk flags are updated.
+                        */
+                       tmp16 = from->sb_qflags &
+                                       ~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
+                                         XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
+
+                       if (from->sb_qflags &
+                                       (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
+                               tmp16 |= XFS_OQUOTA_ENFD;
+                       if (from->sb_qflags &
+                                       (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
+                               tmp16 |= XFS_OQUOTA_CHKD;
+
+                       *(__be16 *)(to_ptr + first) = cpu_to_be16(tmp16);
                } else {
                        switch (size) {
                        case 2:

This way we needn't worry about any change (or locking) of the incore
superblock sb_qflags.

> > I don't know why in XFS we seem to feel the need to do everything in the
> > most complicated way possible.

I suppose they were trying to cut down on conditionals on fields... maybe
something that could be done better at compile time... someday.

Regards,
	Ben

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