From owner-p4-projects@FreeBSD.ORG Mon Dec 18 03:17:48 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id BE1AE16A40F; Mon, 18 Dec 2006 03:17:48 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7EC3F16A403 for ; Mon, 18 Dec 2006 03:17:48 +0000 (UTC) (envelope-from mjacob@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [69.147.83.41]) by mx1.FreeBSD.org (Postfix) with ESMTP id 476F743CA2 for ; Mon, 18 Dec 2006 03:17:48 +0000 (GMT) (envelope-from mjacob@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id kBI3HmiM006853 for ; Mon, 18 Dec 2006 03:17:48 GMT (envelope-from mjacob@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id kBI3HmHf006850 for perforce@freebsd.org; Mon, 18 Dec 2006 03:17:48 GMT (envelope-from mjacob@freebsd.org) Date: Mon, 18 Dec 2006 03:17:48 GMT Message-Id: <200612180317.kBI3HmHf006850@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to mjacob@freebsd.org using -f From: Matt Jacob To: Perforce Change Reviews Cc: Subject: PERFORCE change 111872 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Dec 2006 03:17:49 -0000 http://perforce.freebsd.org/chv.cgi?CH=111872 Change 111872 by mjacob@mjexp on 2006/12/18 03:17:32 More toy cleanups that allow new GEOMs to be tasted and come alive even if the names are the same. That is, the UUID is the real distinguishor, but using UUIDs is a PITA as a 'friendly' name. So, if you are doing a taste and you have a new UUID, but it has a name that conflicts, gen up a (temp) *new* name and let the new GEOM come in. Generalize the geom stuff to just have a list of consumers waiting to become active rather than just a two-consumer case. Put lots more informative messaging in. Affected files ... .. //depot/projects/mjexp/sys/geom/multipath/g_multipath.c#5 edit .. //depot/projects/mjexp/sys/geom/multipath/g_multipath.h#4 edit Differences ... ==== //depot/projects/mjexp/sys/geom/multipath/g_multipath.c#5 (text+ko) ==== @@ -87,16 +87,19 @@ struct bio *cbp; gp = bp->bio_to->geom; + sc = gp->softc; + KASSERT(sc != NULL, ("NULL sc")); + cp = sc->cp_active; + if (cp == NULL) { + g_io_deliver(bp, ENXIO); + return; + } cbp = g_clone_bio(bp); if (cbp == NULL) { g_io_deliver(bp, ENOMEM); return; } cbp->bio_done = g_multipath_done; - sc = gp->softc; - KASSERT(sc != NULL, ("NULL sc")); - cp = sc->consumers[sc->cur_prov]; - KASSERT(cp != NULL, ("NULL cp")); g_io_request(cbp, cp); } @@ -109,10 +112,6 @@ int dofail; KASSERT(sc != NULL, ("NULL sc")); - if (sc->ready == 0) { - g_std_done(bp); - return; - } if (bp->bio_error == ENXIO || bp->bio_error == EIO) { dofail = 1; @@ -130,21 +129,39 @@ dofail = 0; } - /* XXX yes, this only handles single failures XXX */ if (dofail) { - if ((pbp->bio_pflags & G_MULTIPATH_BIO_PFLAG_ERROR) == 0) { - struct g_provider *pp0, *pp1; - pp0 = sc->providers[sc->cur_prov]; - sc->cur_prov++; - pp1 = sc->providers[sc->cur_prov]; - pbp->bio_pflags |= G_MULTIPATH_BIO_PFLAG_ERROR; - printf("error %d: switching from provider %s to" - " provider %s\n", bp->bio_error, - pp0->name, pp1->name); + struct g_consumer *cp = bp->bio_from; + + /* + * If we had a failure, we have to check first to see + * whether the consumer it failed on was the currently + * active consumer (i.e., this is the first in perhaps + * a number of failures). If so, we then switch consumers + * to the next available consumer. + */ + if (cp == sc->cp_active) { + printf("i/o failure is causing detach of %s from %s\n", + cp->provider->name, gp->name); +/* + * XXX: The following two lines are probably wrong due to inflights + */ + g_detach(cp); + g_destroy_consumer(cp); + sc->cp_active = LIST_FIRST(&gp->consumer); + } + + /* + * If we can fruitfully restart the I/O, do so. + */ + if (sc->cp_active) { + printf("switching to provider %s\n", + sc->cp_active->provider->name); g_destroy_bio(bp); pbp->bio_children--; g_multipath_start(pbp); return; + } else { + printf("out of providers to try\n"); } } g_std_done(bp); @@ -156,21 +173,26 @@ g_multipath_access(struct g_provider *pp, int dr, int dw, int de) { struct g_geom *gp; - struct g_multipath_softc *sc; + struct g_consumer *cp, *badcp = NULL; int error; gp = pp->geom; - sc = gp->softc; - KASSERT(sc != NULL, ("NULL sc")); - if (sc->ready == 0) { - return (ENXIO); + + LIST_FOREACH(cp, &gp->consumer, consumer) { + error = g_access(cp, dr, dw, de); + if (error) { + badcp = cp; + goto fail; + } } - error = g_access(sc->consumers[0], dr, dw, de); - if (error == 0) { - error = g_access(sc->consumers[1], dr, dw, de); - if (error) { - (void) g_access(sc->consumers[0], -dr, -dw, -de); + return (0); + +fail: + LIST_FOREACH(cp, &gp->consumer, consumer) { + if (cp == badcp) { + break; } + (void) g_access(cp, -dr, -dw, -de); } return (error); } @@ -180,18 +202,15 @@ { struct g_multipath_softc *sc; struct g_geom *gp; - struct g_provider *newpp; - struct g_consumer *cp0, *cp1; + struct g_provider *pp; char name[64]; g_topology_assert(); - gp = NULL; - newpp = NULL; - cp0 = cp1 = NULL; - LIST_FOREACH(gp, &mp->geom, geom) { if (strcmp(gp->name, md->md_name) == 0) { + printf("GEOM_MULTIPATH: name %s already exists\n", + md->md_name); return (NULL); } } @@ -214,15 +233,15 @@ memcpy(sc->sc_name, md->md_name, sizeof (sc->sc_name)); snprintf(name, sizeof(name), "multipath/%s", md->md_name); - newpp = g_new_providerf(gp, name); - if (newpp == NULL) { + pp = g_new_providerf(gp, name); + if (pp == NULL) { goto fail; } /* limit the provider to not have it stomp on metadata */ - newpp->mediasize = md->md_size - md->md_sectorsize; - newpp->sectorsize = md->md_sectorsize; - g_error_provider(newpp, 0); - sc->pp = newpp; + pp->mediasize = md->md_size - md->md_sectorsize; + pp->sectorsize = md->md_sectorsize; + g_error_provider(pp, 0); + sc->pp = pp; return (gp); fail: if (gp != NULL) { @@ -238,38 +257,46 @@ g_multipath_add_disk(struct g_geom *gp, struct g_provider *pp) { struct g_multipath_softc *sc; - struct g_consumer *cp; + struct g_consumer *cp, *nxtcp; int error; sc = gp->softc; KASSERT(sc, ("no softc")); - if (sc->nattached >= 2) { - printf("cannot attach %s to %s (%d disks already attached)\n", - pp->name, gp->name, sc->nattached); - return (EINVAL); - } - + nxtcp = LIST_FIRST(&gp->consumer); cp = g_new_consumer(gp); if (cp == NULL) { return (ENOMEM); } error = g_attach(cp, pp); if (error != 0) { - printf("Cannot attach provider %s", pp->name); + printf("cannot attach %s to %s", pp->name, sc->sc_name); g_destroy_consumer(cp); return (error); } cp->private = sc; - cp->index = 0; + cp->index = sc->index++; - sc->consumers[sc->nattached] = cp; - sc->providers[sc->nattached] = pp; - sc->nattached++; - if (sc->nattached == 2) { - printf("activating %s\n", sc->sc_name); - sc->ready = 1; + /* + * Set access permissions on new consumer to match other consumers + */ + if (nxtcp) { + error = g_access(cp, nxtcp->acr, nxtcp->acw, nxtcp->ace); + if (error) { + printf("GEOM_MULTIPATH: cannot set access in " + "attaching %s to %s/%s (%d)\n", + pp->name, sc->sc_name, sc->sc_uuid, error); + g_detach(cp); + g_destroy_consumer(cp); + return (error); + } + } + if (sc->cp_active == NULL) { + sc->cp_active = cp; + printf("GEOM_MULTIPATH: activating %s/%s\n", + sc->sc_name, sc->sc_uuid); } + printf("GEOM_MULTIPATH: adding %s to %s\n", pp->name, sc->sc_name); return (0); } @@ -332,7 +359,7 @@ struct g_multipath_metadata md; struct g_multipath_softc *sc; struct g_consumer *cp; - struct g_geom *gp; + struct g_geom *gp, *gp1; int error, isnew; g_topology_assert(); @@ -348,11 +375,15 @@ g_destroy_consumer(cp); g_destroy_geom(gp); if (error != 0) { + printf("%s had error %d reading metadata\n", pp->name, error); return (NULL); } gp = NULL; if (strcmp(md.md_magic, G_MULTIPATH_MAGIC) != 0) { + if (g_multipath_debug) { + printf("%s is not MULTIPATH\n", pp->name); + } return (NULL); } if (md.md_version != G_MULTIPATH_VERSION) { @@ -361,12 +392,20 @@ G_MULTIPATH_VERSION); return (NULL); } + if (g_multipath_debug) { + printf("MULTIPATH: %s/%s\n", md.md_name, md.md_uuid); + } /* * Let's check if such a device already is present. We check against - * uuid alone at first because that's the true distinguishor. After - * that test passes, we run through and make sure that the name is - * unique and if it isn't, we generate a name. + * uuid alone first because that's the true distinguishor. If that + * passes, then we check for name conflicts. If there are conflicts, + * modify the name. + * + * The whole purpose of this is to solve the problem that people don't + * pick good unique names, but good unique names (like uuids) are a + * pain to use. So, we allow people to build GEOMs with friendly names + * and uuids, and modify the names in case there's a collision. */ sc = NULL; LIST_FOREACH(gp, &mp->geom, geom) { @@ -379,31 +418,58 @@ } } + LIST_FOREACH(gp1, &mp->geom, geom) { + if (gp1 == gp) { + continue; + } + sc = gp1->softc; + if (sc == NULL) { + continue; + } + if (strncmp(md.md_name, sc->sc_name, sizeof(md.md_name)) == 0) { + break; + } + } + + /* + * If gp is NULL, we had no extant MULTIPATH geom with this uuid. + * + * If gp1 is *not* NULL, that means we have a MULTIPATH geom extant + * with the same name (but a different UUID). + * + * If gp is NULL, then modify the name with a random number and + * complain, but allow the creation of the geom to continue. + * + * If gp is *not* NULL, just use the geom's name as we're attaching + * this disk to the (previously generated) name. + */ + + if (gp1) { + sc = gp1->softc; + if (gp == NULL) { + char buf[16]; + u_long rand = random(); + + snprintf(buf, sizeof (buf), "%s-%lu", md.md_name, rand); + printf("GEOM_MULTIPATH: geom %s/%s exists already\n", + sc->sc_name, sc->sc_uuid); + printf("GEOM_MULTIPATH: %s will be (temporarily) %s\n", + md.md_uuid, buf); + strlcpy(md.md_name, buf, sizeof (md.md_name)); + } else { + strlcpy(md.md_name, sc->sc_name, sizeof (md.md_name)); + } + } + if (gp == NULL) { gp = g_multipath_create(mp, &md); if (gp == NULL) { - printf("cannot create geom %s\n", md.md_name); + printf("GEOM_MULTIPATH: cannot create geom %s/%s\n", + md.md_name, md.md_uuid); return (NULL); } isnew = 1; } else { - struct g_geom *gp1; - LIST_FOREACH(gp1, &mp->geom, geom) { - sc = gp1->softc; - if (sc == NULL) { - continue; - } - if (strncmp(md.md_name, sc->sc_name, - sizeof(md.md_name)) == 0) { - break; - } - } - if (gp1 != NULL) { - printf("cannot add %s to %s because %s already exists " - "with a different uuid\n", pp->name, gp1->name, - gp1->name); - return (NULL); - } isnew = 0; } ==== //depot/projects/mjexp/sys/geom/multipath/g_multipath.h#4 (text+ko) ==== @@ -42,17 +42,12 @@ #ifdef _KERNEL -#define G_MULTIPATH_BIO_PFLAG_ERROR 0x1 struct g_multipath_softc { struct g_provider * pp; - unsigned int : 28, - nattached : 2, - ready : 1, - cur_prov : 1; - struct g_consumer * consumers[2]; - struct g_provider * providers[2]; + struct g_consumer * cp_active; char sc_name[16]; char sc_uuid[40]; + int index; }; #endif /* _KERNEL */