From owner-cvs-src@FreeBSD.ORG Tue Feb 27 20:53:28 2007 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 4384616A400; Tue, 27 Feb 2007 20:53:28 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id 892B213C428; Tue, 27 Feb 2007 20:53:27 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id CCCC34569A; Tue, 27 Feb 2007 21:53:22 +0100 (CET) Received: from localhost (154.81.datacomsa.pl [195.34.81.154]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 29F1745684; Tue, 27 Feb 2007 21:53:13 +0100 (CET) Date: Tue, 27 Feb 2007 21:52:02 +0100 From: Pawel Jakub Dawidek To: Dag-Erling Sm?rgrav Message-ID: <20070227205202.GA32651@garage.freebsd.pl> References: <200702091903.l19J3Ik5099479@repoman.freebsd.org> <86k5y6p9t2.fsf@dwp.des.no> <20070226172130.GB21095@lor.one-eyed-alien.net> <86vehohs7f.fsf@dwp.des.no> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Qxx1br4bt0+wmkIi" Content-Disposition: inline In-Reply-To: <86vehohs7f.fsf@dwp.des.no> X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 User-Agent: mutt-ng/devel-r804 (FreeBSD) X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=BAYES_00 autolearn=ham version=3.0.4 Cc: cvs-src@FreeBSD.org, Brooks Davis , 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 X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Feb 2007 20:53:28 -0000 --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 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--