On Fri, Sep 26, 2003 at 08:35:30AM +1000, Luke Mewburn wrote:
> On Sat, Sep 06, 2003 at 12:30:26AM +0200, Manuel Bouyer wrote:
>   | since ffsv2 was introduced, we have a problem where an old NetBSD
>   | kernel fails to use filesystems which have been mounted R/W by a new
>   | kernel (PR 21283) I tracked this down to the new kernel setting the
>   | FS_FLAGS_UPDATED flag in the superblock, which unfortunably happens
>   | to be the old FS_SWAPPED flag. And old kernel will keep this flag
>   | after mount, and swap the filesystem when it shouldn't.
>   | 
>   | I agree that the (old) kernel should have filtered this flag at
>   | mount time, but unfortunably it doesn't and it's too late to fix it.
> 
> Yes.  However, your fix doesn't work correctly either...
> 
> Recently I experienced some lossage with a new kernel on an older
> -current system (looked like FS corruption), and upon further

Was it current before or after ufs2 integration ?

What kind of fs corruption ? At kernel or fsck level ?

> investigation, determined that a -current GENERIC kernel works
> but my custom cut-down kernel with FFS_EI did not.
> 
> I also noticed that dumpfs was printing the wrong results.
> See below for more info...
> 
> 
>   | FS_FLAGS_UPDATED is used to tell the kernel that the superblock has been
>   | updated to the new world. It's used in 3 places:
> 
> Actually, it's used in more than just the kernel.
> 
> Further investigation reveals that FS_SWAPPED and FS_FLAGS_UPDATED
> are used in more than just src/sys/ufs ...
> 
>    FS_FLAGS_UPDATED
> 	sys/ufs/ffs/ffs_vfsops.c
> 		set in fs_old_flags
> 		tested in fs_old_flags
> 		fs_flags gets all fs_old_flags except FS_FLAGS_UPDATED
> 	sys/ufs/ffs/fs.h
> 		defined
> 	sbin/fsck_ffs/utilities.c
> 		tested in fs_old_flags

This shouldn't matter; it'll always get the old values, but current make sure
the old and new fields will match.

> 	sbin/newfs/mkfs.c
> 		set in fs_old_flags

We should probably not set it if none of the new flags are used.

> 	usr.sbin/dumpfs/dumpfs.c
> 		tested in fs_old_flags

Same comment as for fsck

> 	usr.sbin/makefs/ffs/mkfs.c
> 		set in fs_old_flags

same comment as for newfs

> 
>    FS_SWAPPED
> 	sys/ufs/ffs/ffs_vfsops.c
> 		set in fs_flags
> 		cleared in fs_flags
> 	sys/ufs/ffs/fs.h
> 		defined
> 	sys/ufs/ufs/ufs_bswap.h
> 		tested as part of UFS_FSNEEDSWAP()
> 	usr.sbin/makefs/ffs/mkfs.c
> 		set in fs_flags (for ffs_write_superblock() ?)

We should make sure it's not written on disk.

> 
> 
>   [...]
> 
> 
> Given that fsck_ffs & dumpfs currently test FS_FLAGS_UPDATED,

They do but shouldn't.
At the current state, it should't matter if FS_FLAGS_UPDATED is set or not,
current will make sure the old and new field matches.

I'm not sure why you ran into this problem; but I don't think the lack of
FS_FLAGS_UPDATED on disk could cause it. It would be good to be able to
investigate further.
The date of your old current kernel would help.


> we need to do one of:
> 
>    a)	Renumber FS_FLAGS_UPDATED to 0x40 and reinstate FS_FLAGS_UPDATED
>     	setting in fs_old_flags in the kernel.
> 
> 	This should retain backwards compat with NetBSD 1.6.x prior
> 	to your recent ``fs_flags &= ~FS_SWAPPED'' fix & pullup.
> 
> 	This isn't compatible with the "other BSDs", but neither
> 	is our current situation anyway...

It should, other BSDs will think the fs has not been updated and will use
the old fields, which are up to date.

> 
> 	We can ask the "other BSDs" (i.e, Kirk :) to reserve 0x40 in
> 	their fs_flags and not use it going forward.
> 
> 
>     b)	Implement a new macro, something like:
> 
> 		#define FS_UPDATED_UFS1(fs) \
> 			(((fs)->fs_magic == FS_UFS1_MAGIC)
> 			&& ((fs)->fs_maxbsize == (fs)->fs_bsize))
> 
> 	Use that instead of "(foo.fs_old_flags & FS_FLAGS_UPDATED) == 0"
> 
> 	Deprecate FS_FLAGS_UPDATED except as a place-holder (maybe
> 	rename it in fs.h ?)

We need to do that anyway.
FS_FLAGS_UPDATED will still be set, in case new flags are used, so we can't
deprecate it.

> 
> 
>     c)	Revert the recent changes and set FS_FLAGS_UPDATED in the
> 	kernel, with FS_FLAGS_UPDATED being 0x80.
> 
> 	Leave the FS_SWAPPED fixes in -current and the head of the
> 	-1-6 branch.

No, I don't think this is not acceptable. 

> 
> 
> I'm leaning towards `a)' at this point.

I'd prefer to avoid it if possible. We need to find the exact cause of the
corruption first.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 24 ans d'experience feront toujours la difference
--