git: af433832f752 - main - geom_label: Remove an old sysinstall(8) workaround

[Cy Schubert]

In message <202107051836.165IaspD003460 at slippy.cwsent.com>, Cy Schubert 
writes:
> In message <202107051517.165FHsfD012512 at gitrepo.freebsd.org>, Jessica 
> Clarke wr
> ites:
> > The branch main has been updated by jrtc27:
> >
> > URL: https://cgit.FreeBSD.org/src/commit/?id=af433832f7520840c22edd1fe1266c
> 1a
> > 5cb781ad
> >
> > commit af433832f7520840c22edd1fe1266c1a5cb781ad
> > Author:     Jessica Clarke <jrtc27 at FreeBSD.org>
> > AuthorDate: 2021-07-05 15:15:32 +0000
> > Commit:     Jessica Clarke <jrtc27 at FreeBSD.org>
> > CommitDate: 2021-07-05 15:15:32 +0000
> >
> >     geom_label: Remove an old sysinstall(8) workaround
> >     
> >     We removed sysinstall(8) back in 2011, so this workaround should be lon
> g
> >     since unnecessary. This workaround can end up breaking cases that are
> >     hit in the real world, such as dd'ing a small pre-built disk image to a
> >     large partition that you intend to grow on first boot and uses a UFS
> >     disk label for / in its /etc/fstab (as the only reliable thing a raw UF
> S
> >     image can reference).
> >     
> >     Reviewed by:    imp, mckusick
> >     Differential Revision:  https://reviews.freebsd.org/D30825
> > ---
> >  sys/geom/label/g_label_ufs.c | 35 +++++------------------------------
> >  1 file changed, 5 insertions(+), 30 deletions(-)
> >
> > diff --git a/sys/geom/label/g_label_ufs.c b/sys/geom/label/g_label_ufs.c
> > index ababbaa4b43a..70d59488d7b6 100644
> > --- a/sys/geom/label/g_label_ufs.c
> > +++ b/sys/geom/label/g_label_ufs.c
> > @@ -49,19 +49,8 @@ __FBSDID("$FreeBSD$");
> >  #define	G_LABEL_UFS_ID		1
> >  
> >  /*
> > - * G_LABEL_UFS_CMP returns true if difference between provider mediasize
> > - * and filesystem size is less than G_LABEL_UFS_MAXDIFF sectors
> > - */
> > -#define	G_LABEL_UFS_CMP(prov, fsys, size) 			
> > 	   \
> > -	( abs( ((fsys)->size) - ( (prov)->mediasize / (fsys)->fs_fsize ))  \
> > -				< G_LABEL_UFS_MAXDIFF )
> > -#define	G_LABEL_UFS_MAXDIFF	0x100
> > -
> > -/*
> > - * Try to find a superblock on the provider. If successful, then
> > - * check that the size in the superblock corresponds to the size
> > - * of the underlying provider. Finally, look for a volume label
> > - * and create an appropriate provider based on that.
> > + * Try to find a superblock on the provider. If successful, look for a vol
> um
> > e
> > + * label and create an appropriate provider based on that.
> >   */
> >  static void
> >  g_label_ufs_taste_common(struct g_consumer *cp, char *label, size_t size, 
> in
> > t what)
> > @@ -81,24 +70,10 @@ g_label_ufs_taste_common(struct g_consumer *cp, char *l
> ab
> > el, size_t size, int wh
> >  		return;
> >  	}
> >  
> > -	/*
> > -	 * Check for magic. We also need to check if file system size
> > -	 * is almost equal to providers size, because sysinstall(8)
> > -	 * used to bogusly put first partition at offset 0
> > -	 * instead of 16, and glabel/ufs would find file system on slice
> > -	 * instead of partition.
> > -	 *
> > -	 * In addition, media size can be a bit bigger than file system
> > -	 * size. For instance, mkuzip can append bytes to align data
> > -	 * to large sector size (it improves compression rates).
> > -	 */
> > -	if (fs->fs_magic == FS_UFS1_MAGIC && fs->fs_fsize > 0 &&
> > -	    ( G_LABEL_UFS_CMP(pp, fs, fs_old_size)
> > -		|| G_LABEL_UFS_CMP(pp, fs, fs_providersize))) {
> > +	/* Check for magic. */
> > +	if (fs->fs_magic == FS_UFS1_MAGIC && fs->fs_fsize > 0) {
> >  		/* Valid UFS1. */
> > -	} else if (fs->fs_magic == FS_UFS2_MAGIC && fs->fs_fsize > 0 &&
> > -	    ( G_LABEL_UFS_CMP(pp, fs, fs_size)
> > -		|| G_LABEL_UFS_CMP(pp, fs, fs_providersize))) {
> > +	} else if (fs->fs_magic == FS_UFS2_MAGIC && fs->fs_fsize > 0) {
> >  		/* Valid UFS2. */
> >  	} else {
> >  		goto out;
> >
>
> On one of my machines which uses UFS filesystems (sandbox) also fails:
>
> bob# ls /dev/ufs
> s10-amd64	s13-amd64	s13-amd64e	test
> s11-amd64	s13-amd64a	s13-amd64f	testa
> s12-amd64	s13-amd64d	s13-i386
> bob# mount /alt/s11-amd64/root
> bob# ls /dev/ufs
> s10-amd64	s12-amd64	test
> s11-amd64	s13-i386	testa
> bob# 
>
> After the mount of /alt/s11-amd64/root, s10-amd64 the s13-* filesystems 
> become unavailble  They share the same fdisk slice, using different 
> bsdlabel partitions within the slice.
>
> My laptop zfs pool is on a partition in the same slice as its UFS 
> filesystems. Looks like once a filesystem on a slice has been mounted, the 
> other partitions within the slice are no longer available following this 
> commit.

Ok, I see the problem. The two affected machines have UFS partitions at 
offset 0 of their respective slices.


-- 
Cheers,
Cy Schubert <Cy.Schubert at cschubert.com>
FreeBSD UNIX:  <cy at FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy at nwtime.org>    Web:  https://nwtime.org

	The need of the many outweighs the greed of the few.