multipathd: Fix miscounting active paths
authorBenjamin Marzinski <bmarzins@redhat.com>
Sat, 30 Mar 2019 06:05:55 +0000 (01:05 -0500)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Thu, 18 Apr 2019 11:03:32 +0000 (13:03 +0200)
When multipathd gets a change uevent, it calls pathinfo with DI_NOIO.
This sets the path state to the return value of path_offline(). If a
path is in the PATH_DOWN state but path_offline() returns PATH_UP, when
that path gets a change event, its state will get moved to PATH_UP
without either reinstating the path, or reloading the map.  The next
call to check_path() will move the path back to PATH_DOWN. Since
check_path() simply increments and decrements nr_active instead of
calculating it based on the actual number of active paths, nr_active
will get decremented a second time for this failed path, potentially
putting the multipath device into recovery mode.

This commit does two things to avoid this situation. It makes the
DI_NOIO flag only set pp->state in pathinfo() if DI_CHECKER is also set.
This isn't set in uev_update_path() to avoid changing the path state in
this case.  Also, to guard against pp->state getting changed in some
other code path without properly updating the map state, check_path()
now calls set_no_path_retry, which recalculates nr_active based on the
actual number of active paths, and makes sure that the queue_if_no_path
value in the features line is correct.

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
libmultipath/discovery.c
multipath/main.c
multipathd/main.c

index 10bd8cd..729bcb9 100644 (file)
@@ -1914,11 +1914,12 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
        if (path_state == PATH_REMOVED)
                goto blank;
        else if (mask & DI_NOIO) {
-               /*
-                * Avoid any IO on the device itself.
-                * Behave like DI_CHECKER in the "path unavailable" case.
-                */
-               pp->chkrstate = pp->state = path_state;
+               if (mask & DI_CHECKER)
+                       /*
+                        * Avoid any IO on the device itself.
+                        * simply use the path_offline() return as its state
+                        */
+                       pp->chkrstate = pp->state = path_state;
                return PATHINFO_OK;
        }
 
index 5abb118..69141db 100644 (file)
@@ -356,7 +356,7 @@ static int check_usable_paths(struct config *conf,
                        pp->udev = get_udev_device(pp->dev_t, DEV_DEVT);
                        if (pp->udev == NULL)
                                continue;
-                       if (pathinfo(pp, conf, DI_SYSFS|DI_NOIO) != PATHINFO_OK)
+                       if (pathinfo(pp, conf, DI_SYSFS|DI_NOIO|DI_CHECKER) != PATHINFO_OK)
                                continue;
 
                        if (pp->state == PATH_UP &&
index 43830e8..678ecf8 100644 (file)
@@ -392,7 +392,8 @@ static void set_no_path_retry(struct multipath *mpp)
        default:
                if (mpp->nr_active > 0) {
                        mpp->retry_tick = 0;
-                       dm_queue_if_no_path(mpp->alias, 1);
+                       if (!is_queueing)
+                               dm_queue_if_no_path(mpp->alias, 1);
                } else if (is_queueing && mpp->retry_tick == 0)
                        enter_recovery_mode(mpp);
                break;
@@ -2072,6 +2073,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
        /* if update_multipath_strings orphaned the path, quit early */
        if (!pp->mpp)
                return 0;
+       set_no_path_retry(pp->mpp);
 
        if ((newstate == PATH_UP || newstate == PATH_GHOST) &&
                        check_path_reinstate_state(pp)) {