From owner-svn-src-all@FreeBSD.ORG Fri Apr 23 19:35:07 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D2080106566B; Fri, 23 Apr 2010 19:35:07 +0000 (UTC) (envelope-from mjacob@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id BF9C48FC12; Fri, 23 Apr 2010 19:35:07 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o3NJZ7Gf094044; Fri, 23 Apr 2010 19:35:07 GMT (envelope-from mjacob@svn.freebsd.org) Received: (from mjacob@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o3NJZ7Zt094042; Fri, 23 Apr 2010 19:35:07 GMT (envelope-from mjacob@svn.freebsd.org) Message-Id: <201004231935.o3NJZ7Zt094042@svn.freebsd.org> From: Matt Jacob Date: Fri, 23 Apr 2010 19:35:07 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r207133 - stable/7/sys/geom/multipath X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Apr 2010 19:35:07 -0000 Author: mjacob Date: Fri Apr 23 19:35:07 2010 New Revision: 207133 URL: http://svn.freebsd.org/changeset/base/207133 Log: This is an MFC of 205847, 204071, 196580 and 196579 ------------------------------------------------------------------------ Change how multipath labels are created and managed. This makes it easier to support various storage boxes which really aren't active-active. We only write the label on the *first* provider. For all other providers we just "add" the disk. This also allows for an "add" verb. A usage implication is that you should specificy the currently active storage path as the first provider. Note that this does not add RDAC-like functionality, but better allows for autovolumefailover configurations (additional checkins elsewhere will support this). ------------------------------------------------------------------------ - Style fixes. - Prefer strlcpy() over strncpy(). ------------------------------------------------------------------------ There's no need for checking result of M_WAITOK allocation. ------------------------------------------------------------------------ Fix an obvious topology lock leak. Modified: stable/7/sys/geom/multipath/g_multipath.c Directory Properties: stable/7/sys/ (props changed) stable/7/sys/cddl/contrib/opensolaris/ (props changed) stable/7/sys/contrib/dev/acpica/ (props changed) stable/7/sys/contrib/pf/ (props changed) Modified: stable/7/sys/geom/multipath/g_multipath.c ============================================================================== --- stable/7/sys/geom/multipath/g_multipath.c Fri Apr 23 19:26:03 2010 (r207132) +++ stable/7/sys/geom/multipath/g_multipath.c Fri Apr 23 19:35:07 2010 (r207133) @@ -97,9 +97,8 @@ g_mpd(void *arg, int flags __unused) g_topology_assert(); cp = arg; - if (cp->acr > 0 || cp->acw > 0 || cp->ace > 0) { + if (cp->acr > 0 || cp->acw > 0 || cp->ace > 0) g_access(cp, -cp->acr, -cp->acw, -cp->ace); - } if (cp->provider) { printf("GEOM_MULTIPATH: %s removed from %s\n", cp->provider->name, cp->geom->name); @@ -223,15 +222,16 @@ g_multipath_done_error(struct bio *bp) static void g_multipath_kt(void *arg) { + g_multipath_kt_state = GKT_RUN; mtx_lock(&gmtbq_mtx); while (g_multipath_kt_state == GKT_RUN) { for (;;) { struct bio *bp; + bp = bioq_takefirst(&gmtbq); - if (bp == NULL) { + if (bp == NULL) break; - } mtx_unlock(&gmtbq_mtx); g_multipath_done_error(bp); mtx_lock(&gmtbq_mtx); @@ -265,9 +265,8 @@ g_multipath_access(struct g_provider *pp fail: LIST_FOREACH(cp, &gp->consumer, consumer) { - if (cp == badcp) { + if (cp == badcp) break; - } (void) g_access(cp, -dr, -dw, -de); } return (error); @@ -291,9 +290,8 @@ g_multipath_create(struct g_class *mp, s } gp = g_new_geomf(mp, md->md_name); - if (gp == NULL) { + if (gp == NULL) goto fail; - } sc = g_malloc(sizeof(*sc), M_WAITOK | M_ZERO); if (sc == NULL) { @@ -308,9 +306,8 @@ g_multipath_create(struct g_class *mp, s memcpy(sc->sc_name, md->md_name, sizeof (sc->sc_name)); pp = g_new_providerf(gp, "multipath/%s", md->md_name); - if (pp == NULL) { + if (pp == NULL) goto fail; - } /* limit the provider to not have it stomp on metadata */ pp->mediasize = md->md_size - md->md_sectorsize; pp->sectorsize = md->md_sectorsize; @@ -319,9 +316,8 @@ g_multipath_create(struct g_class *mp, s return (gp); fail: if (gp != NULL) { - if (gp->softc != NULL) { + if (gp->softc != NULL) g_free(gp->softc); - } g_destroy_geom(gp); } return (NULL); @@ -343,9 +339,8 @@ g_multipath_add_disk(struct g_geom *gp, * Make sure that the passed provider isn't already attached */ LIST_FOREACH(cp, &gp->consumer, consumer) { - if (cp->provider == pp) { + if (cp->provider == pp) break; - } } if (cp) { printf("GEOM_MULTIPATH: provider %s already attached to %s\n", @@ -354,9 +349,8 @@ g_multipath_add_disk(struct g_geom *gp, } nxtcp = LIST_FIRST(&gp->consumer); cp = g_new_consumer(gp); - if (cp == NULL) { + if (cp == NULL) return (ENOMEM); - } error = g_attach(cp, pp); if (error != 0) { printf("GEOM_MULTIPATH: cannot attach %s to %s", @@ -397,13 +391,11 @@ g_multipath_destroy(struct g_geom *gp) struct g_provider *pp; g_topology_assert(); - if (gp->softc == NULL) { + if (gp->softc == NULL) return (ENXIO); - } pp = LIST_FIRST(&gp->provider); - if (pp != NULL && (pp->acr != 0 || pp->acw != 0 || pp->ace != 0)) { + if (pp != NULL && (pp->acr != 0 || pp->acw != 0 || pp->ace != 0)) return (EBUSY); - } printf("GEOM_MULTIPATH: destroying %s\n", gp->name); g_free(gp->softc); gp->softc = NULL; @@ -415,6 +407,7 @@ static int g_multipath_destroy_geom(struct gctl_req *req, struct g_class *mp, struct g_geom *gp) { + return (g_multipath_destroy(gp)); } @@ -447,9 +440,8 @@ g_multipath_init(struct g_class *mp) { bioq_init(&gmtbq); mtx_init(&gmtbq_mtx, "gmtbq", NULL, MTX_DEF); - if (kthread_create(g_multipath_kt, mp, NULL, 0, 0, "g_mp_kt") == 0) { + if (kthread_create(g_multipath_kt, mp, NULL, 0, 0, "g_mp_kt") == 0) g_multipath_kt_state = GKT_RUN; - } } static void @@ -475,18 +467,16 @@ g_multipath_read_metadata(struct g_consu g_topology_assert(); error = g_access(cp, 1, 0, 0); - if (error != 0) { + if (error != 0) return (error); - } pp = cp->provider; g_topology_unlock(); buf = g_read_data(cp, pp->mediasize - pp->sectorsize, pp->sectorsize, &error); g_topology_lock(); g_access(cp, -1, 0, 0); - if (buf == NULL) { + if (buf == NULL) return (error); - } multipath_metadata_decode(buf, md); g_free(buf); return (0); @@ -513,15 +503,13 @@ g_multipath_taste(struct g_class *mp, st g_detach(cp); g_destroy_consumer(cp); g_destroy_geom(gp); - if (error != 0) { + if (error != 0) return (NULL); - } gp = NULL; if (strcmp(md.md_magic, G_MULTIPATH_MAGIC) != 0) { - if (g_multipath_debug) { + if (g_multipath_debug) printf("%s is not MULTIPATH\n", pp->name); - } return (NULL); } if (md.md_version != G_MULTIPATH_VERSION) { @@ -530,9 +518,8 @@ g_multipath_taste(struct g_class *mp, st G_MULTIPATH_VERSION); return (NULL); } - if (g_multipath_debug) { + 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 @@ -548,25 +535,20 @@ g_multipath_taste(struct g_class *mp, st sc = NULL; LIST_FOREACH(gp, &mp->geom, geom) { sc = gp->softc; - if (sc == NULL) { + if (sc == NULL) continue; - } - if (strncmp(md.md_uuid, sc->sc_uuid, sizeof(md.md_uuid)) == 0) { + if (strncmp(md.md_uuid, sc->sc_uuid, sizeof(md.md_uuid)) == 0) break; - } } LIST_FOREACH(gp1, &mp->geom, geom) { - if (gp1 == gp) { + if (gp1 == gp) continue; - } sc = gp1->softc; - if (sc == NULL) { + if (sc == NULL) continue; - } - if (strncmp(md.md_name, sc->sc_name, sizeof(md.md_name)) == 0) { + if (strncmp(md.md_name, sc->sc_name, sizeof(md.md_name)) == 0) break; - } } /* @@ -593,9 +575,9 @@ g_multipath_taste(struct g_class *mp, st 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)); + strlcpy(md.md_name, buf, sizeof(md.md_name)); } else { - strlcpy(md.md_name, sc->sc_name, sizeof (md.md_name)); + strlcpy(md.md_name, sc->sc_name, sizeof(md.md_name)); } } @@ -615,9 +597,8 @@ g_multipath_taste(struct g_class *mp, st KASSERT(sc != NULL, ("sc is NULL")); error = g_multipath_add_disk(gp, pp); if (error != 0) { - if (isnew) { + if (isnew) g_multipath_destroy(gp); - } return (NULL); } return (gp); @@ -656,9 +637,8 @@ g_multipath_ctl_create(struct gctl_req * gctl_error(req, "No 'arg1' argument"); return; } - if (strncmp(name, devpf, 5) == 0) { + if (strncmp(name, devpf, 5) == 0) name += 5; - } pp0 = g_provider_by_name(name); if (pp0 == NULL) { gctl_error(req, "Provider %s is invalid", name); @@ -670,9 +650,8 @@ g_multipath_ctl_create(struct gctl_req * gctl_error(req, "No 'arg2' argument"); return; } - if (strncmp(name, devpf, 5) == 0) { + if (strncmp(name, devpf, 5) == 0) name += 5; - } pp1 = g_provider_by_name(name); if (pp1 == NULL) { gctl_error(req, "Provider %s is invalid", name); @@ -716,13 +695,12 @@ g_multipath_ctl_create(struct gctl_req * memset(&md, 0, sizeof(md)); md.md_size = pp0->mediasize; md.md_sectorsize = pp0->sectorsize; - strncpy(md.md_name, mpname, sizeof (md.md_name)); - strncpy(md.md_uuid, uuid, sizeof (md.md_uuid)); + strlcpy(md.md_name, mpname, sizeof(md.md_name)); + strlcpy(md.md_uuid, uuid, sizeof(md.md_uuid)); gp = g_multipath_create(mp, &md); - if (gp == NULL) { + if (gp == NULL) return; - } error = g_multipath_add_disk(gp, pp0); if (error) { g_multipath_destroy(gp);