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>