libmultipath: drop mpp->nr_active field" patch v1 to v2
authorMartin Wilck <mwilck@suse.com>
Sat, 7 Mar 2020 10:57:53 +0000 (11:57 +0100)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Sat, 7 Mar 2020 10:57:53 +0000 (11:57 +0100)
v1 was merged accidentally as commit 60711766.  This patch merges
v1 to v2 changes.

Follow the v2 commit message, more detailled than v1's.

The tracking of nr_active has turned out to be error prone and hard
to verify. Calculating it on the fly is a quick operation, so
do this rather than trying to track nr_active. Use a boolean
field instead to track whether a map is currently in recovery mode.

Moreover, clean up the recovery logic by making set_no_path_retry()
the place that checks the current configuration and state, sets
"queue_if_no_path" if necessary, and enters or leaves recovery
mode if necessary. This ensures that consistent logic is applied.

In the client handlers, we can't be sure that mpp->features is
up-to-date. Also, users that change the queuing mode expect that
the requested action is performed, regardless of state. Therefore,
transform set_no_path_retry() into __set_no_path_retry(), which takes
an additional parameter indicating whether the current state should
be checked, and set this parameter to false when calling the function
from the client handler code, true otherwise. Moreover, in the very
unlikely case mpp->features is NULL, don't assume that queuing is off,
just make no assumption about the current state.

libmultipath/structs_vec.c
libmultipath/structs_vec.h

index 0c5a3a8..3dbbaa0 100644 (file)
@@ -334,22 +334,23 @@ static void leave_recovery_mode(struct multipath *mpp)
        }
 }
 
-void set_no_path_retry(struct multipath *mpp)
+void __set_no_path_retry(struct multipath *mpp, bool check_features)
 {
-       bool is_queueing = 0;
+       bool is_queueing;
 
-       if (mpp->features && strstr(mpp->features, "queue_if_no_path"))
-               is_queueing = 1;
+       check_features = check_features && mpp->features != NULL;
+       if (check_features)
+               is_queueing = strstr(mpp->features, "queue_if_no_path");
 
        switch (mpp->no_path_retry) {
        case NO_PATH_RETRY_UNDEF:
                break;
        case NO_PATH_RETRY_FAIL:
-               if (is_queueing)
+               if (!check_features || is_queueing)
                        dm_queue_if_no_path(mpp->alias, 0);
                break;
        case NO_PATH_RETRY_QUEUE:
-               if (!is_queueing)
+               if (!check_features || !is_queueing)
                        dm_queue_if_no_path(mpp->alias, 1);
                break;
        default:
@@ -358,7 +359,8 @@ void set_no_path_retry(struct multipath *mpp)
                         * If in_recovery is set, leave_recovery_mode() takes
                         * care of dm_queue_if_no_path. Otherwise, do it here.
                         */
-                       if (!is_queueing && !mpp->in_recovery)
+                       if ((!check_features || !is_queueing) &&
+                           !mpp->in_recovery)
                                dm_queue_if_no_path(mpp->alias, 1);
                        leave_recovery_mode(mpp);
                } else
index 678efe4..2a5e3d6 100644 (file)
@@ -11,7 +11,8 @@ struct vectors {
        vector mpvec;
 };
 
-void set_no_path_retry(struct multipath *mpp);
+void __set_no_path_retry(struct multipath *mpp, bool check_features);
+#define set_no_path_retry(mpp) __set_no_path_retry(mpp, true)
 
 int adopt_paths (vector pathvec, struct multipath * mpp);
 void orphan_paths(vector pathvec, struct multipath *mpp,