Date: Tue, 27 Feb 2007 21:52:02 +0100 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: Dag-Erling Sm?rgrav <des@des.no> Cc: cvs-src@FreeBSD.org, Brooks Davis <brooks@FreeBSD.org>, cvs-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/amd64/conf GENERIC src/sys/i386/conf GENERIC src/sys/ia64/conf GENERIC src/sys/pc98/conf GENERIC src/sys/powerpc/conf GENERIC src/sys/sparc64/conf GENERIC src/sys/sun4v/conf GENERIC Message-ID: <20070227205202.GA32651@garage.freebsd.pl> In-Reply-To: <86vehohs7f.fsf@dwp.des.no> References: <200702091903.l19J3Ik5099479@repoman.freebsd.org> <86k5y6p9t2.fsf@dwp.des.no> <20070226172130.GB21095@lor.one-eyed-alien.net> <86vehohs7f.fsf@dwp.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
--Qxx1br4bt0+wmkIi Content-Type: text/plain; charset=iso-8859-2 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 27, 2007 at 10:20:52AM +0100, Dag-Erling Sm?rgrav wrote: > Brooks Davis <brooks@FreeBSD.org> writes: > > While I agree there are serious problems with glabel and software RAID1 > > configurations, I don't think that warrants continuing to hide it from > > the rest of us. We should probably add more warnings to the appropriate > > manpages and fix the RAID implementations. >=20 > The problem isn't just with the RAID implementations. It goes deeper > than that. >=20 > First, when geom_label sees multiple identical labels, it ignores all > but the first one. The old implementation (geom_vol_ffs) had comments > in the source code pointing this out: >=20 > /* XXX We need to check for namespace conflicts. */ > /* XXX How do you handle a mirror set? */ > /* XXX We don't validate the volume name. */ > g_topology_lock(); > /* Alright, we have a label and a volume name, reconfig. = */ > g_slice_config(gp, 0, G_SLICE_CONFIG_SET, (off_t) 0, > pp->mediasize, pp->sectorsize, "vol/%s", > fs->fs_volname); > g_free(fs); > g_topology_unlock(); >=20 > The new implementation has the same bug / feature, but does not > document it: >=20 > snprintf(name, sizeof(name), "%s/%s", dir, label); > LIST_FOREACH(gp, &mp->geom, geom) { > pp2 =3D LIST_FIRST(&gp->provider); > if (pp2 =3D=3D NULL) > continue; > if (strcmp(pp2->name, name) =3D=3D 0) { > G_LABEL_DEBUG(1, "Label %s(%s) already exists (%s= ).", > label, name, pp->name); > if (req !=3D NULL) { > gctl_error(req, "Provider %s already exis= ts.", > name); > } > return (NULL); > } > } >=20 > In addition, the issue is never logged; the debugging message is > normally disabled, and the error message is ignored when req is NULL > (req is always NULL when tasting existing labels; it is non-NULL only > when creating a new label using 'glabel create') >=20 > This is exacerbated by the fact that ataraid does not hide the > underlying devices when an array is configured, and they are usually > tasted before the array, so you are pretty much guaranteed that > geom_label attaches to the wrong provider. >=20 > (this same fact also leads to spurious and confusing error messages > from other GEOM classes, such as "corrupt or invalid GPT detected" > when tasting the first component of a RAID 0 array that contains a > GPT) Dag-Erling, you're proposing removing it from GENERIC, because ataraid doesn't work nicely in the current world order. From what I looked some time ago ataraid isn't using GEOM to access components. AFAIR at some point ataraid was hidding components. Gmirror/graid3 for example opens all components for write+exclusive which prevents such mistakes. Anyway, if we decide to remove glabel from GENERIC, I'd at least like to make the consensus clear - ataraid should be changed to fit better in what we currently have. --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --Qxx1br4bt0+wmkIi Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (FreeBSD) iD8DBQFF5JnyForvXbEpPzQRAnIxAJ0YVvYh2+TLxchTzZJTQf1RP/g6DQCeP/Ur xQMnAqYkylyqmT4wy9lVHqU= =PjEQ -----END PGP SIGNATURE----- --Qxx1br4bt0+wmkIi--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070227205202.GA32651>