Date: Sat, 6 Jun 2020 01:17:07 -0700 From: Xin Li <delphij@delphij.net> To: Conrad Meyer <cem@FreeBSD.org> Cc: imp@freebsd.org, freebsd-current@freebsd.org Subject: HEADSUP: GEOM label may be broken [Was Re: svn commit: r361838 - in head/sys/geom: . label] Message-ID: <d630a3fd-32a2-dba5-757c-c7445d407855@delphij.net> In-Reply-To: <202006051612.055GCL11009491@repo.freebsd.org> References: <202006051612.055GCL11009491@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9LBAapg0ZvlvkHQyC6aCJMhvZL2l8PuIk Content-Type: multipart/mixed; boundary="DxvZX30G7JrhyBSDs5Xgmn4F5IlebGJ9w"; protected-headers="v1" From: Xin Li <delphij@delphij.net> Reply-To: d@delphij.net To: Conrad Meyer <cem@FreeBSD.org> Cc: imp@freebsd.org, freebsd-current@freebsd.org Message-ID: <d630a3fd-32a2-dba5-757c-c7445d407855@delphij.net> Subject: HEADSUP: GEOM label may be broken [Was Re: svn commit: r361838 - in head/sys/geom: . label] References: <202006051612.055GCL11009491@repo.freebsd.org> In-Reply-To: <202006051612.055GCL11009491@repo.freebsd.org> --DxvZX30G7JrhyBSDs5Xgmn4F5IlebGJ9w Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable I just spent quite some time to revive my laptop. TL;DR: if you are using /dev/diskid or /dev/gptid labels and GELI, please wait until things settled. =3D=3D=3D On 6/5/20 9:12 AM, Conrad Meyer wrote: > Author: cem > Date: Fri Jun 5 16:12:21 2020 > New Revision: 361838 > URL: https://svnweb.freebsd.org/changeset/base/361838 >=20 > Log: > geom_label: Use provider aliasing to alias upstream geoms > =20 > For synthetic aliases (just pseudonyms inferred from metadata like GP= T or > UFS labels, GPT UUIDs, etc), use the GEOM provider aliasing system to= create > a symlink to the real device instead of creating an independent devic= e. > This makes it more clear which labels and devices correspond, and we = can > safely have multiple labels to a single device accessed at once. > =20 > The confusingly named geom_label on-disk construct continues to behav= e > identically to how it did before. > =20 > This requires teaching GEOM's provider aliasing about the possibility= > that aliases might be added later in time, and GEOM's devfs interacti= on > layer not to worry about existing aliases during retaste. > =20 > Discussed with: imp > Relnotes: sure, if we don't end up reverting it This would break (and the effect can be quite persistent and hard to repair, see explanation below) existing configuration as some GEOM classes are not converted to support the new way of device representation, so I'd like to request this change be reverted for now until these are fixed. Consider the following configuration, where one have a hard drive partitioned with GPT, like: =3D> 40 1953525088 ada1 GPT (932G) 40 262144 1 efi (128M) 262184 8388608 2 freebsd-zfs (4.0G) 8650792 67108864 3 freebsd-swap (32G) 75759656 1877765472 4 freebsd-zfs (895G) Now, the first ZFS pool is created as root pool. ZFS gets an exclusive hold of 'ada1p2' despite the pool is carefully created to use /dev/diskid/<DISKSERIAL>p2 instead of ada1p2. ZFS writes the new device path to vdev label. For ZFS, this doesn't matter much as it always checks the label. However, this will prevent GEOM from properly creating /dev/diskid/<DISKSERIAL>. In order to prevent accidentally writing data to wrong disk, for "raw" disk partitions it's usually a good idea to reference them with labels, either by /dev/diskid or /dev/gptid. With /dev/ada1p2 exclusively accessed by ZFS, the /dev/diskid/<DISKSERIAL> representing the disk is now gone. And to make the situation even worse, simply changing the partition reference to the corresponding /dev/gptid/<uuid> doesn't really work either, when one is encrypting partitions individually. In the example above, adap4 is an encrypted partition and with the alias change, one can no longer "geli attach" them via /dev/gptid/<uuid>. They can still be attached by the "canonical" path (/dev/ada1p4), but for the swap partition that would completely defeat the purpose of using label (to prevent accidentally writing to the wrong disk). For my case, reverting to an older kernel is not sufficient to fix the configuration, because ZFS is recording the "canonical" device path (/dev/ada1p2) and /dev/diskid label for this disk disappeared somewhat permanently (I would have to find a USB drive somewhere to fix the root pool to use the "right" device path). > Differential Revision: https://reviews.freebsd.org/D24968 [...] > Modified: head/sys/geom/label/g_label.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/sys/geom/label/g_label.c Fri Jun 5 16:05:09 2020 (r361837) > +++ head/sys/geom/label/g_label.c Fri Jun 5 16:12:21 2020 (r361838) > @@ -344,18 +345,16 @@ g_label_taste(struct g_class *mp, struct g_provid= er *p > { > struct g_label_metadata md; > struct g_consumer *cp; > + struct g_class *clsp; > struct g_geom *gp; > int i; > + bool changed; > =20 > g_trace(G_T_TOPOLOGY, "%s(%s, %s)", __func__, mp->name, pp->name); > g_topology_assert(); > =20 > G_LABEL_DEBUG(2, "Tasting %s.", pp->name); > =20 > - /* Skip providers that are already open for writing. */ > - if (pp->acw > 0) > - return (NULL); > - > if (strcmp(pp->geom->class->name, mp->name) =3D=3D 0) > return (NULL); > =20 > @@ -391,9 +390,16 @@ g_label_taste(struct g_class *mp, struct g_provide= r *p > if (md.md_provsize !=3D pp->mediasize) > break; > =20 > + /* Skip providers that are already open for writing. */ > + if (pp->acw > 0) { > + g_access(cp, -1, 0, 0); > + goto end; > + } > + (Is this still necessary when the eventual provider would be the real one= ?) I think symlink aliasing is a the right direction but we need to be really careful to not break existing and legitimate usage. Since this also breaks GELI when using with labels, I'd like to request that this change be backed out for now until the consumers are correctly fixed. Cheers, --DxvZX30G7JrhyBSDs5Xgmn4F5IlebGJ9w-- --9LBAapg0ZvlvkHQyC6aCJMhvZL2l8PuIk Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.2.20 (FreeBSD) iQIzBAEBCgAdFiEEceNg5NEMZIki80nQQHl/fJX0g08FAl7bUQoACgkQQHl/fJX0 g09Xkw/5Aa1Q78SoPuVON2Y3gJ1IzsHOewY/rt5o9lkw07zdQCC53Xm/+xi2Te6+ RRll3ZBBJflUKU5W9KHG7drHnofDZso8rGw0cddLhP90hd7IqYtrRHaCLLHgKmOF 9b3bLQHQycGuU7PI2DDt7R3DmhXoUYc830qIk8pkQQselGEhcwo72sF2WEbNOk2A BrMkYWZ/5VMFOYKmewfc4+LGa7EDNHL8o4IytOxIkmzYKH+kUBHHEj9ssZR9MBNn xxJxRrJGzHPbDHME3b5RcNoZNrtjU1KoS92jIKck3dEaRa2cgz8W5qKvrBIKhDp/ RluRTyZb8dGr4TyDTUrkdJ6dyBvU/SrzKCRAxikaWkoTlP/xVznBAwkNvoLwuDZ/ RXBLUy3xAsoxkv2xJGccZsj9lsj+HoKokoHgT7gnA5nK8NX6+JyIiFncZQYSUC/v ryg2WG2cg2mqXhqZD9i5OwrasvQ6ZUu7E/lgrNwOUcGSC6SX+I/eFHj2GND2lYd4 SvGJOa5bv23f3umDCq6Lf0F0D+fWHlzYdhikR3rKc5lqGy+nd7QKq3VDWGB4LECe ANrKx1xBQuzPsqS8JyG6nObRqPoHevmb06EywA4MWtp3aY2Iqc0LvosnBnTkqEKM VzVTn9J0+19EZIkIQad9LRIYKKfJil5gKkvfKh/zY6c6fr79Y5I= =y1xp -----END PGP SIGNATURE----- --9LBAapg0ZvlvkHQyC6aCJMhvZL2l8PuIk--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?d630a3fd-32a2-dba5-757c-c7445d407855>