Date: Tue, 27 Feb 2007 10:20:52 +0100 From: des@des.no (Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?=) To: Brooks Davis <brooks@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@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: <86vehohs7f.fsf@dwp.des.no> In-Reply-To: <20070226172130.GB21095@lor.one-eyed-alien.net> (Brooks Davis's message of "Mon, 26 Feb 2007 11:21:30 -0600") References: <200702091903.l19J3Ik5099479@repoman.freebsd.org> <86k5y6p9t2.fsf@dwp.des.no> <20070226172130.GB21095@lor.one-eyed-alien.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
The problem isn't just with the RAID implementations. It goes deeper
than that.
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:
/* 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();
The new implementation has the same bug / feature, but does not
document it:
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 exists=
.",
name);
}
return (NULL);
}
}
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')
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.
(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)
DES
--=20
Dag-Erling Sm=F8rgrav - des@des.no
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86vehohs7f.fsf>