Date: Mon, 18 Dec 2006 03:17:48 GMT From: Matt Jacob <mjacob@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 111872 for review Message-ID: <200612180317.kBI3HmHf006850@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200612180317.kBI3HmHf006850>