Subject: Re: ffs compat problem since ffs2
To: None <tech-kern@NetBSD.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 09/06/2003 17:36:41
--HcAYCG3uE/tztfnV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Sep 06, 2003 at 12:30:26AM +0200, Manuel Bouyer wrote:
> Hi,
> 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. This is a real
> backward-compat problem which can cause damage to users (start a sysinst
> boot floppy, start upgrade, change your mind, bummer! now your old
> kernel can't use its root any more !).
> This needs to be fixed in current.
> 
> One obvious way of doing it would be to change FS_FLAGS_UPDATED to 0x40,
> but this will likely cause compat problems with the others BSDs.
> 
> 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:
> - ffs_mountfs():
>                 if ((fsblockloc == sblockloc ||
> 		     (fs->fs_old_flags & FS_FLAGS_UPDATED) == 0)
> 		      && sbsize <= MAXBSIZE && sbsize >= sizeof(struct fs))
>   If I read it properly, we always have fsblockloc == sblockloc for
>   FS_UFS1_MAGIC filesystems, so we could check FS_FLAGS_UPDATED for v2
>   filesystems only.
> - ffs_oldfscompat_read():
>   we could probably move it under the
>   if (fs->fs_maxbsize != fs->fs_bsize || fs->fs_time < fs->fs_old_time) 
>   check. We probably want to update the flags anyway if the filesystem
>   was mounted R/W by an old kernel (maybe fs->fs_flags |= fs->fs_old_flags
>   instead of =, and clear any new flags if fs->fs_maxbsize != fs->fs_bsize) ?
> - ffs_sbupdate(): update fs_sblockloc. Unless I missed something,
>   this should already have been done by ffs_oldfscompat_read() ?
> 
> So it looks like we don't need to set FS_FLAGS_UPDATED for v1 filesystem on
> NetBSD. But unfortunably, not doing this may cause compatibility problems with
> others BSD.
> 
> I think we could remove checks of FS_FLAGS_UPDATED in our kernel, and
> not set FS_FLAGS_UPDATED if we're not using any of the new features
> (basically, if fs->fs_flags contains only FS_UNCLEAN and FS_DOSOFTDEP)
> 
> comments ?


Attached is a patch I'll request to get commited to the release branches,
to improve compatibility with FFS from other OSes.
The NetBSD backward-compat needs to be solved in a different way.

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

--HcAYCG3uE/tztfnV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ffsei_16.diff"

Index: ffs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.98.4.1
diff -u -r1.98.4.1 ffs_vfsops.c
--- ffs_vfsops.c	2002/06/10 16:46:17	1.98.4.1
+++ ffs_vfsops.c	2003/09/06 15:30:48
@@ -476,8 +476,9 @@
 	if (VFSTOUFS(mountp)->um_flags & UFS_NEEDSWAP) {
 		ffs_sb_swap((struct fs*)bp->b_data, newfs);
 		fs->fs_flags |= FS_SWAPPED;
-	}
+	} else
 #endif
+		fs->fs_flags &= ~FS_SWAPPED;
 	if (newfs->fs_magic != FS_MAGIC || newfs->fs_bsize > MAXBSIZE ||
 	    newfs->fs_bsize < sizeof(struct fs)) {
 		brelse(bp);
@@ -680,8 +681,9 @@
 	if (needswap) {
 		ffs_sb_swap((struct fs*)bp->b_data, fs);
 		fs->fs_flags |= FS_SWAPPED;
-	}
+	} else
 #endif
+		fs->fs_flags &= ~FS_SWAPPED;
 	ffs_oldfscompat(fs);
 
 	if (fs->fs_bsize > MAXBSIZE || fs->fs_bsize < sizeof(struct fs)) {

--HcAYCG3uE/tztfnV--