Re: fsid change of ZFS?

Pawel Jakub Dawidek wrote:
> On Wed, Aug 24, 2011 at 04:41:25PM +0300, Kostik Belousov wrote:
> > On Wed, Aug 24, 2011 at 09:36:37AM -0400, Rick Macklem wrote:
> > > Well, doesn't this result in the same issue as the fixed table?
> > > In other words, the developer has to supply the "suggested byte"
> > > for
> > > fsid and make sure that it doesn't conflict with other "suggested
> > > byte"
> > > values or suffer the same consequence as forgetting to update the
> > > fixed
> > > table. (ie. It just puts the fixed value in a different place,
> > > from what
> > > I see, for in-tree modules. Also, with a fixed table, they are all
> > > in
> > > one place, so it's easy to choose a non-colliding value?)
> > The reason for my proposal was Pawel note that a porter of the
> > filesystem
> > should be aware of some place in kern/ where to register, besides
> > writing
> > the module.
> 
> Well, he has to be aware, but we should do all we can to minimize the
> number of place he needs to update, as it is easy to forget some.
> 
> I agree with Rick that what you proposed is similar to fixed table of
> file system names and I'd prefer to avoid that. If we can have
> name-based hash that produces no collision for in-tree file systems
> and
> know current 3rd party file systems plus collision detection for the
> future then it is good enough, IMHO. And this is what Rick proposed
> with
> his patch.
> 
Ok, here is the list of file system names I've been checking and there
haven't been any collisions (either hash function). If you know of other
well known file type names, please email and I'll add them to the list
and check for collisions again.

"cd9660"
"ufs"
"procfs"
"devfs"
"msdosfs"
"nfs"
"xfs"
"reiserfs"
"tmpfs"
"hpfs"
"ntfs"
"ext2fs"
"udf"
"zfs"
"afs"

and here is my current rendition of the patch. (I took Gleb's suggestion
and switched to fnv_32_str(). I'll leave it that way unless there is a
collision after adding any names people post to the above list.)

It sounds like people have agreed that this is a reasonable solution.
If hrs_at_ can confirm that testing shows it fixes the original problem
(the ZFS file handles don't change when it's loaded at different times),
I'll pass it along to re_at_.

Thanks for the helpful suggestions, rick

--- kern/vfs_init.c.sav	2011-06-11 18:58:33.000000000 -0400
+++ kern/vfs_init.c	2011-08-24 16:15:24.000000000 -0400
_at__at_ -39,6 +39,7 _at__at_ __FBSDID("$FreeBSD: head/sys/kern/vfs_in
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/fnv_hash.h>
 #include <sys/kernel.h>
 #include <sys/linker.h>
 #include <sys/mount.h>
_at__at_ -138,6 +139,8 _at__at_ vfs_register(struct vfsconf *vfc)
 	struct sysctl_oid *oidp;
 	struct vfsops *vfsops;
 	static int once;
+	struct vfsconf *tvfc;
+	uint32_t hashval;
 
 	if (!once) {
 		vattr_null(&va_null);
_at__at_ -152,7 +155,26 _at__at_ vfs_register(struct vfsconf *vfc)
 	if (vfs_byname(vfc->vfc_name) != NULL)
 		return EEXIST;
 
-	vfc->vfc_typenum = maxvfsconf++;
+	/*
+	 * Calculate a hash on vfc_name to use for vfc_typenum. Unless
+	 * a collision occurs, it is limited to 8bits since that is
+	 * what ZFS uses from vfc_typenum and that also limits how sparsely
+	 * distributed vfc_typenum becomes.
+	 */
+	hashval = fnv_32_str(vfc->vfc_name, FNV1_32_INIT);
+	hashval &= 0xff;
+	do {
+		/* Look for and fix any collision. */
+		TAILQ_FOREACH(tvfc, &vfsconf, vfc_list) {
+			if (hashval == tvfc->vfc_typenum) {
+				hashval++; /* Can exceed 8bits, if needed. */
+				break;
+			}
+		}
+	} while (tvfc != NULL);
+	vfc->vfc_typenum = hashval;
+	if (vfc->vfc_typenum >= maxvfsconf)
+		maxvfsconf = vfc->vfc_typenum + 1;
 	TAILQ_INSERT_TAIL(&vfsconf, vfc, vfc_list);
 
 	/*