Date: Mon, 5 Jul 2021 20:58:53 +0100 From: Jessica Clarke <jrtc27@freebsd.org> To: Cy Schubert <Cy.Schubert@cschubert.com> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: af433832f752 - main - geom_label: Remove an old sysinstall(8) workaround Message-ID: <11BB2019-EAE3-40A6-A3B1-EA3241AA91BF@freebsd.org> In-Reply-To: <202107051856.165IukLl003702@slippy.cwsent.com> References: <202107051517.165FHsfD012512@gitrepo.freebsd.org> <202107051836.165IaspD003460@slippy.cwsent.com> <E77F3B91-8B43-4936-AFFB-B96C8580467A@freebsd.org> <202107051856.165IukLl003702@slippy.cwsent.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 5 Jul 2021, at 19:56, Cy Schubert <Cy.Schubert@cschubert.com> wrote: >=20 > In message <E77F3B91-8B43-4936-AFFB-B96C8580467A@freebsd.org>, Jessica=20= > Clarke w > rites: >> On 5 Jul 2021, at 19:36, Cy Schubert <Cy.Schubert@cschubert.com> = wrote: >>>=20 >>> In message <202107051517.165FHsfD012512@gitrepo.freebsd.org>, = Jessica=20 >>> Clarke wr >>> ites: >>>> The branch main has been updated by jrtc27: >>>>=20 >>>> URL: = https://cgit.FreeBSD.org/src/commit/?id=3Daf433832f7520840c22edd1fe1266 >> c1a >>>> 5cb781ad >>>>=20 >>>> commit af433832f7520840c22edd1fe1266c1a5cb781ad >>>> Author: Jessica Clarke <jrtc27@FreeBSD.org> >>>> AuthorDate: 2021-07-05 15:15:32 +0000 >>>> Commit: Jessica Clarke <jrtc27@FreeBSD.org> >>>> CommitDate: 2021-07-05 15:15:32 +0000 >>>>=20 >>>> geom_label: Remove an old sysinstall(8) workaround >>>>=20 >>>> 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). >>>>=20 >>>> 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(-) >>>>=20 >>>> 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 >>>>=20 >>>> /* >>>> - * 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) =09 >>>> \ >>>> - ( 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 vo >> lum >>>> 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,=20 >> in >>>> t what) >>>> @@ -81,24 +70,10 @@ g_label_ufs_taste_common(struct g_consumer *cp, = char * >> lab >>>> el, size_t size, int wh >>>> return; >>>> } >>>>=20 >>>> - /* >>>> - * 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 =3D=3D 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 =3D=3D FS_UFS1_MAGIC && fs->fs_fsize > 0) { >>>> /* Valid UFS1. */ >>>> - } else if (fs->fs_magic =3D=3D 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 =3D=3D FS_UFS2_MAGIC && fs->fs_fsize > = 0) { >>>> /* Valid UFS2. */ >>>> } else { >>>> goto out; >>>>=20 >>>=20 >>> On one of my machines which uses UFS filesystems (sandbox) also = fails: >>>=20 >>> 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#=20 >>>=20 >>> After the mount of /alt/s11-amd64/root, s10-amd64 the s13-* = filesystems=20 >>> become unavailble They share the same fdisk slice, using different=20= >>> bsdlabel partitions within the slice. >>>=20 >>> My laptop zfs pool is on a partition in the same slice as its UFS=20 >>> filesystems. Looks like once a filesystem on a slice has been = mounted, the=20 >>> other partitions within the slice are no longer available following = this=20 >>> commit. >>=20 >> Hm, what does bsdlabel on the disk say? My guess is your disk is set = up >> in the bogus manner mentioned in that comment. Is it an old machine >> installed by sysinstall(8)? >=20 > A little more investigation shows that on the two affected machines, = UFS=20 > partitions are at offset 0 of their respective slices. For legacy=20 > environments this will require some kind of workaround or boot off a = rescue=20 > disk (which I do have) to dump/recreate/restore the partitions at an = offset=20 > that is not zero. The problem is quite logical. >=20 > Doing the work to recreate the partitions doesn't bother me but other=20= > people may trip across this. Your call. How about something like D31068? Jess
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?11BB2019-EAE3-40A6-A3B1-EA3241AA91BF>