Hi Matthew,
Please take a look at the patch I'm attaching to this mail. It fixes
couple of issues with cp->active being left pointing to detached
device. I have as well tried to remove code duplication a bit by using
g_multipath_rotate() in g_mpd() and g_multipath_done_error() instead
of identical loops.
One thing which I'm not 100% sure about is this part (this is within
g_multipath_access()):
@@ -258,7 +268,7 @@
LIST_FOREACH(cp, &gp->consumer, consumer) {
error = g_access(cp, dr, dw, de);
- if (error) {
+ if (error && error != ENXIO) {
badcp = cp;
goto fail;
}
This change fixes undesired behavior in the following case:
While running
while true; do
echo
date
dd if=/dev/multipath/test of=/dev/null bs=512 count=1
sleep 1
done
disconnect currently active path of /dev/multipath/test.
Without "&& error != ENXIO" I have exactly one failed dd. Whith it -
none of dd fails.
Shouldn't the overall logic of g_multipath_access() be as follows: try
to do what we're asked to do while there are at least one available
path, and remove from g_multipath any path for which g_access() call
has failed? I would suppose that multipath device should try to work
as long as there is at least one workable path, and any path which has
failed (no matter if it was a physical failure or, f.e. ENOMEM in
device driver) should be detached from multipath device.
--
Oleg Sharoyko
Index: sys/geom/multipath/g_multipath.c
===================================================================
--- sys/geom/multipath/g_multipath.c (revision 215098)
+++ sys/geom/multipath/g_multipath.c (working copy)
@@ -71,7 +71,7 @@
g_multipath_destroy_geom(struct gctl_req *, struct g_class *, struct g_geom *);
static struct g_geom *g_multipath_find_geom(struct g_class *, const char *);
-static int g_multipath_rotate(struct g_geom *);
+static void g_multipath_rotate(struct g_geom *);
static g_taste_t g_multipath_taste;
static g_ctl_req_t g_multipath_config;
@@ -95,11 +95,30 @@
g_mpd(void *arg, int flags __unused)
{
struct g_consumer *cp;
+ struct g_geom *gp;
+ struct g_multipath_softc *sc;
+ struct g_provider *pp;
g_topology_assert();
cp = arg;
if (cp->acr > 0 || cp->acw > 0 || cp->ace > 0)
g_access(cp, -cp->acr, -cp->acw, -cp->ace);
+
+ gp = cp->geom;
+ sc = gp->softc;
+ pp = cp->provider;
+
+ if (cp == sc->cp_active) {
+ printf("GEOM_MULTIPATH: %s failed in %s\n",
+ pp->name, sc->sc_name);
+ sc->cp_active = NULL;
+ g_multipath_rotate(gp);
+ if (sc->cp_active == NULL || sc->cp_active->provider == NULL) {
+ printf("GEOM_MULTIPATH: out of providers for %s\n",
+ sc->sc_name);
+ }
+ }
+
if (cp->provider) {
printf("GEOM_MULTIPATH: %s removed from %s\n",
cp->provider->name, cp->geom->name);
@@ -187,24 +206,15 @@
g_post_event(g_mpd, cp, M_NOWAIT, NULL);
}
if (cp == sc->cp_active) {
- struct g_consumer *lcp;
printf("GEOM_MULTIPATH: %s failed in %s\n",
pp->name, sc->sc_name);
sc->cp_active = NULL;
- LIST_FOREACH(lcp, &gp->consumer, consumer) {
- if ((lcp->index & MP_BAD) == 0) {
- sc->cp_active = lcp;
- break;
- }
- }
+ g_multipath_rotate(gp);
if (sc->cp_active == NULL || sc->cp_active->provider == NULL) {
printf("GEOM_MULTIPATH: out of providers for %s\n",
sc->sc_name);
g_topology_unlock();
return;
- } else {
- printf("GEOM_MULTIPATH: %s now active path in %s\n",
- sc->cp_active->provider->name, sc->sc_name);
}
}
g_topology_unlock();
@@ -258,7 +268,7 @@
LIST_FOREACH(cp, &gp->consumer, consumer) {
error = g_access(cp, dr, dw, de);
- if (error) {
+ if (error && error != ENXIO) {
badcp = cp;
goto fail;
}
@@ -409,17 +419,16 @@
return (g_multipath_destroy(gp));
}
-static int
+static void
g_multipath_rotate(struct g_geom *gp)
{
struct g_consumer *lcp;
struct g_multipath_softc *sc = gp->softc;
g_topology_assert();
- if (sc == NULL)
- return (ENXIO);
+ KASSERT(sc != NULL, ("NULL sc"));
LIST_FOREACH(lcp, &gp->consumer, consumer) {
- if ((lcp->index & MP_BAD) == 0) {
+ if ((lcp->index & (MP_BAD|MP_POSTED)) == 0) {
if (sc->cp_active != lcp) {
break;
}
@@ -430,7 +439,6 @@
printf("GEOM_MULTIPATH: %s now active path in %s\n",
lcp->provider->name, sc->sc_name);
}
- return (0);
}
static void
@@ -715,7 +723,6 @@
{
struct g_geom *gp;
const char *name;
- int error;
g_topology_assert();
@@ -729,10 +736,11 @@
gctl_error(req, "Device %s is invalid", name);
return;
}
- error = g_multipath_rotate(gp);
- if (error != 0) {
- gctl_error(req, "failed to rotate %s (err=%d)", name, error);
+ if (gp->softc == NULL) {
+ gctl_error(req, "Device %s not fully initialized", name);
+ return;
}
+ g_multipath_rotate(gp);
}
static void
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-geom
To unsubscribe, send any mail to "[email protected]"
