multipath: sort all pathgroups by priority

Right now, the only path grouping policy that sorts pathgroups by priority is
group_by_prio.  For the others, the pathgroups are setup in the order that
their paths are discovered.  This can cause a problem when the kernel needs to
switch pathgroups, such as when a path fails.  The kernel will simply pick the
first valid pathgroup.  If failback isn't set to manual, this can cause
needless pathgroup switching.  Even worse, with failback set to manual, which
is the default, the kernel will just continue to use the non-optimal pathgroup
until it fails.  This patch makes all path grouping policies except multibus
sort the pathgroups by priority.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index cfaf1c3..f76ad60 100644 (file)
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -11,6 +11,7 @@
 #include "vector.h"
 #include "structs.h"
 #include "pgpolicies.h"
+#include "switchgroup.h"
 
 extern int
 get_pgpolicy_id (char * str)
@@ -57,6 +58,34 @@ get_pgpolicy_name (char * buff, int len, int id)
        return snprintf(buff, POLICY_NAME_SIZE, "%s", s);
 }
 
+
+void
+sort_pathgroups (struct multipath *mp) {
+       int i, j;
+       struct pathgroup * pgp1, * pgp2;
+
+       if (!mp->pg)
+               return;
+
+       vector_foreach_slot(mp->pg, pgp1, i) {
+               path_group_prio_update(pgp1);
+               for (j = i - 1; j >= 0; j--) {
+                       pgp2 = VECTOR_SLOT(mp->pg, j);
+                       if (!pgp2)
+                               continue;
+                       if (pgp2->priority > pgp1->priority ||
+                           (pgp2->priority == pgp1->priority &&
+                            pgp2->enabled_paths >= pgp1->enabled_paths)) {
+                               vector_move_up(mp->pg, i, j + 1);
+                               break;
+                       }
+               }
+               if (j < 0 && i != 0)
+               vector_move_up(mp->pg, i, 0);
+       }
+}
+
+
 /*
  * One path group per unique tgt_node_name present in the path vector
  */
@@ -119,6 +148,7 @@ group_by_node_name (struct multipath * mp) {
                }
        }
        FREE(bitmap);
+       sort_pathgroups(mp);
        free_pathvec(mp->paths, KEEP_PATHS);
        mp->paths = NULL;
        return 0;
@@ -191,6 +221,7 @@ group_by_serial (struct multipath * mp) {
                }
        }
        FREE(bitmap);
+       sort_pathgroups(mp);
        free_pathvec(mp->paths, KEEP_PATHS);
        mp->paths = NULL;
        return 0;
@@ -228,6 +259,7 @@ one_path_per_group (struct multipath * mp)
                if (store_path(pgp->paths, pp))
                        goto out;
        }
+       sort_pathgroups(mp);
        free_pathvec(mp->paths, KEEP_PATHS);
        mp->paths = NULL;
        return 0;

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 4279fe9..cf634ba 100644 (file)
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -379,7 +379,6 @@ snprint_pg_selector (char * buff, size_t len, struct pathgroup * pgp)
 static int
 snprint_pg_pri (char * buff, size_t len, struct pathgroup * pgp)
 {
-       int avg_priority = 0;
        /*
         * path group priority is not updated for every path prio change,
         * but only on switch group code path.
@@ -387,9 +386,7 @@ snprint_pg_pri (char * buff, size_t len, struct pathgroup * pgp)
         * Printing is another reason to update.
         */
        path_group_prio_update(pgp);
-       if (pgp->enabled_paths)
-               avg_priority = pgp->priority / pgp->enabled_paths;
-       return snprint_int(buff, len, avg_priority);
+       return snprint_int(buff, len, pgp->priority);
 }
 
 static int

diff --git a/libmultipath/switchgroup.c b/libmultipath/switchgroup.c
index d995cd3..025a95d 100644 (file)
--- a/libmultipath/switchgroup.c
+++ b/libmultipath/switchgroup.c
@@ -25,14 +25,17 @@ path_group_prio_update (struct pathgroup * pgp)
                        pgp->enabled_paths++;
                }
        }
-       pgp->priority = priority;
+       if (pgp->enabled_paths)
+               pgp->priority = priority / pgp->enabled_paths;
+       else
+               pgp->priority = 0;
 }
 
 extern int
 select_path_group (struct multipath * mpp)
 {
        int i;
-       int max_priority = 0, avg_priority;
+       int max_priority = 0;
        int bestpg = 1;
        int max_enabled_paths = 1;
        struct pathgroup * pgp;
@@ -46,12 +49,11 @@ select_path_group (struct multipath * mpp)
 
                path_group_prio_update(pgp);
                if (pgp->enabled_paths) {
-                       avg_priority = pgp->priority / pgp->enabled_paths;
-                       if (avg_priority > max_priority) {
-                               max_priority = avg_priority;
+                       if (pgp->priority > max_priority) {
+                               max_priority = pgp->priority;
                                max_enabled_paths = pgp->enabled_paths;
                                bestpg = i + 1;
-                       } else if (avg_priority == max_priority) {
+                       } else if (pgp->priority == max_priority) {
                                if (pgp->enabled_paths > max_enabled_paths) {
                                        max_enabled_paths = pgp->enabled_paths;
                                        bestpg = i + 1;

diff --git a/libmultipath/vector.c b/libmultipath/vector.c
index 67c7500..ea6f82c 100644 (file)
--- a/libmultipath/vector.c
+++ b/libmultipath/vector.c
@@ -50,6 +50,22 @@ vector_alloc_slot(vector v)
        return v->slot;
 }
 
+int
+vector_move_up(vector v, int src, int dest)
+{
+       void *value;
+       int i;
+       if (dest == src)
+               return 0;
+       if (dest > src || src >= v->allocated)
+               return -1;
+       value = v->slot[src];
+       for (i = src - 1; i >= dest; i--)
+               v->slot[i + 1] = v->slot[i];
+       v->slot[dest] = value;
+       return 0;
+}
+
 void *
 vector_insert_slot(vector v, int slot, void *value)
 {