[Top] [All Lists]

Re: [REVIEW] User-space support for bad_features2 patch

To: "Eric Sandeen" <sandeen@xxxxxxxxxxx>
Subject: Re: [REVIEW] User-space support for bad_features2 patch
From: "Barry Naujok" <bnaujok@xxxxxxx>
Date: Mon, 25 Feb 2008 11:39:41 +1100
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
In-reply-to: <47BF0963.6020809@xxxxxxxxxxx>
Organization: SGI
References: <op.t6w1h5f53jf8g2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <47BF0963.6020809@xxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Opera Mail/9.24 (Win32)
On Sat, 23 Feb 2008 04:41:55 +1100, Eric Sandeen <sandeen@xxxxxxxxxxx> wrote:

Barry Naujok wrote:
The attached patch fixes mkfs.xfs writing the bad features2 in the first
place (the change to xfs_sb.h does this).

Next xfs_db support printing of this superblock field and xfs_check can
report the bad_features2 field is set.

xfs_repair can correct the error in the same fashion that David Chinner's
mount code does it.

This patch applies to the lazy-count xfs_repair conversion code that I
posted a short time before this patch.


+        * Check bad_features2, if set and features2 is zero, copy
+        * bad_features2 to features2 and zero bad_features2.
+        */
+       if (sb->sb_bad_features2 != 0) {
+               if (sb->sb_features2 == 0)
+                       sb->sb_features2 = sb->sb_bad_features2;
+               sb->sb_bad_features2 = 0;
+               primary_sb_modified = 1;
+               do_warn(_("superblock's features2 field is in the wrong "
+                       "location, correcting\n"));
+       }

My only thought here is that if you repair it, then use an older kernel
w/o the fix, suddenly your fs behavior changes, whereas before you often
got lucky, and both userspace & kernelspace swapped the same way, and
you found the bits you were looking for out of luck :)  (same goes for
the recent kernel fix too, I guess)

I believe the kernel code never tried to access "bad_features2" part
of the superblock, it always did the correct thing (correct me if I'm
wrong of course :).

But, as Dave pointed out, I should be OR'ing the two together before
zeroing the bad one.

Should we flag the bad_features2 as "already copied to features2" but
otherwise leave it?  Hmm I guess that would look like an unsupported
feature flag to old kernels, wouldn't it.  Urgh.


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