Correctly update feature strings

The 'features' string might contain arbitrary features,
so we shouldn't assume anything here.
This patch implements a parser for properly adding
and removing selected features whilst keeping the
'features' string syntactically correct.

Signed-off-by: Hannes Reinecke <hare@suse.de>

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 520b5eb..69ef420 100644 (file)
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -407,8 +407,6 @@ domap (struct multipath * mpp, char * params)
                if (!conf->daemon) {
                        /* multipath client mode */
                        dm_switchgroup(mpp->alias, mpp->bestpg);
-                       if (mpp->action != ACT_NOTHING)
-                               print_multipath_topology(mpp, conf->verbosity);
                } else  {
                        /* multipath daemon mode */
                        mpp->stat_map_loads++;
@@ -558,10 +556,15 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
                        continue;
 
                if (mpp->no_path_retry != NO_PATH_RETRY_UNDEF) {
-                       if (mpp->no_path_retry == NO_PATH_RETRY_FAIL)
-                               dm_queue_if_no_path(mpp->alias, 0);
-                       else
-                               dm_queue_if_no_path(mpp->alias, 1);
+                       if (mpp->no_path_retry == NO_PATH_RETRY_FAIL) {
+                               if (!dm_queue_if_no_path(mpp->alias, 0))
+                                       remove_feature(&mpp->features,
+                                                      "queue_if_no_path");
+                       } else {
+                               if (!dm_queue_if_no_path(mpp->alias, 1))
+                                       add_feature(&mpp->features,
+                                                   "queue_if_no_path");
+                       }
                }
                if (mpp->pg_timeout != PGTIMEOUT_UNDEF) {
                        if (mpp->pg_timeout == -PGTIMEOUT_NONE)
@@ -570,6 +573,9 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
                                dm_set_pg_timeout(mpp->alias, mpp->pg_timeout);
                }
 
+               if (!conf->daemon && mpp->action != ACT_NOTHING)
+                       print_multipath_topology(mpp, conf->verbosity);
+
                if (newmp) {
                        if (mpp->action != ACT_REJECT) {
                                if (!vector_alloc_slot(newmp))

diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 027254a..a792dbc 100644 (file)
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -53,21 +53,40 @@ assemble_map (struct multipath * mp, char * params, int len)
        int shift, freechar;
        int minio;
        int nr_priority_groups, initial_pg_nr;
-       char * p;
+       char * p, * f;
+       char no_path_retry[] = "queue_if_no_path";
        struct pathgroup * pgp;
        struct path * pp;
 
        minio = mp->minio;
        p = params;
-       freechar = sizeof(params);
+       freechar = len;
 
        nr_priority_groups = VECTOR_SIZE(mp->pg);
        initial_pg_nr = (nr_priority_groups ? mp->bestpg : 0);
 
+       f = STRDUP(mp->features);
+
+       /*
+        * We have to set 'queue_if_no_path' here even
+        * to avoid path failures during map reload.
+        */
+       if (mp->no_path_retry == NO_PATH_RETRY_UNDEF ||
+           mp->no_path_retry == NO_PATH_RETRY_FAIL) {
+               /* remove queue_if_no_path settings */
+               condlog(3, "%s: remove queue_if_no_path from '%s'",
+                       mp->alias, mp->features);
+               remove_feature(&f, no_path_retry);
+       } else {
+               add_feature(&f, no_path_retry);
+       }
+
        shift = snprintf(p, freechar, "%s %s %i %i",
-                        mp->features, mp->hwhandler,
+                        f, mp->hwhandler,
                         nr_priority_groups, initial_pg_nr);
 
+       FREE(f);
+
        if (shift >= freechar) {
                fprintf(stderr, "mp->params too small\n");
                return 1;
@@ -124,6 +143,7 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
        int num_paths = 0;
        int num_paths_args = 0;
        int def_minio = 0;
+       int no_path_retry = NO_PATH_RETRY_UNDEF;
        struct path * pp;
        struct pathgroup * pgp;
 
@@ -138,6 +158,8 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
                return 1;
 
        num_features = atoi(mpp->features);
+       no_path_retry = mpp->no_path_retry;
+       mpp->no_path_retry = NO_PATH_RETRY_UNDEF;
 
        for (i = 0; i < num_features; i++) {
                p += get_word(p, &word);
@@ -158,6 +180,17 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
        }
 
        /*
+        * Reset no_path_retry.
+        * - if not set from features
+        * - if queue_if_no_path is set from features but
+        *   no_path_retry > 0 is selected.
+        */
+       if ((mpp->no_path_retry == NO_PATH_RETRY_UNDEF ||
+            mpp->no_path_retry == NO_PATH_RETRY_QUEUE) &&
+           mpp->no_path_retry != no_path_retry)
+               mpp->no_path_retry = no_path_retry;
+
+       /*
         * hwhandler
         */
        p += get_word(p, &mpp->hwhandler);

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index b0f3ceb..83f1006 100644 (file)
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -261,30 +261,29 @@ extern int
 select_features (struct multipath * mp)
 {
        struct mpentry * mpe;
+       char *origin;
 
-       if ((mpe = find_mpe(mp->wwid))) {
-               if (mpe->features) {
-                       mp->features = mpe->features;
-                       condlog(3, "%s: features = %s (LUN setting)",
-                               mp->alias, mp->features);
-                       return 0;
+       if ((mpe = find_mpe(mp->wwid)) && mpe->features) {
+               mp->features = STRDUP(mpe->features);
+               origin = "LUN setting";
+       } else if (mp->hwe && mp->hwe->features) {
+               mp->features = STRDUP(mp->hwe->features);
+               origin = "controller setting";
+       } else {
+               mp->features = STRDUP(conf->features);
+               origin = "internal default";
+       }
+       condlog(3, "%s: features = %s (%s)",
+               mp->alias, mp->features, origin);
+       if (strstr(mp->features, "queue_if_no_path")) {
+               if (mp->no_path_retry == NO_PATH_RETRY_UNDEF)
+                       mp->no_path_retry = NO_PATH_RETRY_QUEUE;
+               else if (mp->no_path_retry == NO_PATH_RETRY_FAIL) {
+                       condlog(1, "%s: config error, overriding 'no_path_retry' value",
+                               mp->alias);
+                       mp->no_path_retry = NO_PATH_RETRY_QUEUE;
                }
        }
-       if (mp->hwe && mp->hwe->features) {
-               mp->features = mp->hwe->features;
-               condlog(3, "%s: features = %s (controller setting)",
-                       mp->alias, mp->features);
-               return 0;
-       }
-       if (conf->features) {
-               mp->features = conf->features;
-               condlog(3, "%s: features = %s (config file default)",
-                       mp->alias, mp->features);
-               return 0;
-       }
-       mp->features = set_default(DEFAULT_FEATURES);
-       condlog(3, "%s: features = %s (internal default)",
-               mp->alias, mp->features);
        return 0;
 }
 
@@ -422,9 +421,12 @@ select_no_path_retry(struct multipath *mp)
                        mp->alias, mp->no_path_retry);
                return 0;
        }
-       mp->no_path_retry = NO_PATH_RETRY_UNDEF;
-       condlog(3, "%s: no_path_retry = NONE (internal default)",
-               mp->alias);
+       if (mp->no_path_retry != NO_PATH_RETRY_UNDEF)
+               condlog(3, "%s: no_path_retry = %i (inherited setting)",
+                       mp->alias, mp->no_path_retry);
+       else
+               condlog(3, "%s: no_path_retry = %i (internal default)",
+                       mp->alias, mp->no_path_retry);
        return 0;
 }
 

diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index a84c509..78e1727 100644 (file)
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -122,6 +122,7 @@ alloc_multipath (void)
        if (mpp) {
                mpp->bestpg = 1;
                mpp->mpcontext = NULL;
+               mpp->no_path_retry = NO_PATH_RETRY_UNDEF;
        }
        return mpp;
 }
@@ -140,9 +141,7 @@ free_multipath_attributes (struct multipath * mpp)
                mpp->selector = NULL;
        }
 
-       if (mpp->features &&
-           mpp->features != conf->features &&
-           (!mpp->hwe || (mpp->hwe && mpp->features != mpp->hwe->features))) {
+       if (mpp->features) {
                FREE(mpp->features);
                mpp->features = NULL;
        }
@@ -405,6 +404,187 @@ first_path (struct multipath * mpp)
 extern void
 setup_feature(struct multipath * mpp, char *feature)
 {
-       if (!strncmp(feature, "queue_if_no_path", 16))
-               mpp->no_path_retry = NO_PATH_RETRY_QUEUE;
+       if (!strncmp(feature, "queue_if_no_path", 16)) {
+               if (mpp->no_path_retry <= NO_PATH_RETRY_UNDEF)
+                       mpp->no_path_retry = NO_PATH_RETRY_QUEUE;
+       }
+}
+
+extern int
+add_feature (char **f, char *n)
+{
+       int c = 0, d, l;
+       char *e, *p, *t;
+
+       if (!f)
+               return 1;
+
+       /* Nothing to do */
+       if (!n || *n == '0')
+               return 0;
+
+       /* Check if feature is already present */
+       if (strstr(*f, n))
+               return 0;
+
+       /* Get feature count */
+       c = strtoul(*f, &e, 10);
+       if (*f == e)
+               /* parse error */
+               return 1;
+
+       /* Check if we need to increase feature count space */
+       l = strlen(*f) + strlen(n) + 1;
+
+       /* Count new features */
+       if ((c % 10) == 9)
+               l++;
+       c++;
+       p = n;
+       while (*p != '\0') {
+               if (*p == ' ' && p[1] != '\0' && p[1] != ' ') {
+                       if ((c % 10) == 9)
+                               l++;
+                       c++;
+               }
+               p++;
+       }
+
+       t = MALLOC(l + 1);
+       if (!t)
+               return 1;
+
+       memset(t, 0, l + 1);
+
+       /* Update feature count */
+       d = c;
+       l = 1;
+       while (d > 9) {
+               d /= 10;
+               l++;
+       }
+       p = t;
+       snprintf(p, l + 2, "%0d ", c);
+
+       /* Copy the feature string */
+       p = strchr(*f, ' ');
+       if (p) {
+               while (*p == ' ')
+                       p++;
+               strcat(t, p);
+               strcat(t, " ");
+       } else {
+               p = t + strlen(t);
+       }
+       strcat(t, n);
+
+       FREE(*f);
+       *f = t;
+
+       return 0;
 }
+
+extern int
+remove_feature(char **f, char *o)
+{
+       int c = 0, d, l;
+       char *e, *p, *n;
+
+       if (!f || !*f)
+               return 1;
+
+       /* Nothing to do */
+       if (!o || *o == '\0')
+               return 0;
+
+       /* Check if not present */
+       if (!strstr(*f, o))
+               return 0;
+
+       /* Get feature count */
+       c = strtoul(*f, &e, 10);
+       if (*f == e)
+               /* parse error */
+               return 1;
+
+       /* Normalize features */
+       while (*o == ' ') {
+               o++;
+       }
+       /* Just spaces, return */
+       if (*o == '\0')
+               return 0;
+       e = o + strlen(o);
+       while (*e == ' ')
+               e--;
+       d = (int)(e - o);
+
+       /* Update feature count */
+       c--;
+       p = o;
+       while (p[0] != '\0') {
+               if (p[0] == ' ' && p[1] != ' ' && p[1] != '\0')
+                       c--;
+               p++;
+       }
+
+       /* Quick exit if all features have been removed */
+       if (c == 0) {
+               n = MALLOC(2);
+               if (!n)
+                       return 1;
+               strcpy(n, "0");
+               goto out;
+       }
+
+       /* Search feature to be removed */
+       e = strstr(*f, o);
+       if (!e)
+               /* Not found, return */
+               return 0;
+
+       /* Update feature count space */
+       l = strlen(*f) - d;
+       n =  MALLOC(l + 1);
+       if (!n)
+               return 1;
+
+       /* Copy the feature count */
+       sprintf(n, "%0d", c);
+       /*
+        * Copy existing features up to the feature
+        * about to be removed
+        */
+       p = strchr(*f, ' ');
+       while (*p == ' ')
+               p++;
+       p--;
+       if (e != p) {
+               do {
+                       e--;
+                       d++;
+               } while (*e == ' ');
+               e++; d--;
+               strncat(n, p, (size_t)(e - p));
+               p += (size_t)(e - p);
+       }
+       /* Skip feature to be removed */
+       p += d;
+
+       /* Copy remaining features */
+       if (strlen(p)) {
+               while (*p == ' ')
+                       p++;
+               if (strlen(p)) {
+                       p--;
+                       strcat(n, p);
+               }
+       }
+
+out:
+       FREE(*f);
+       *f = n;
+
+       return 0;
+}
+

diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index cab9828..cbbf8a6 100644 (file)
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -253,6 +253,8 @@ int pathcountgr (struct pathgroup *, int);
 int pathcount (struct multipath *, int);
 int pathcmp (struct pathgroup *, struct pathgroup *);
 void setup_feature(struct multipath *, char *);
+int add_feature (char **, char *);
+int remove_feature (char **, char *);
 
 extern char sysfs_path[PATH_SIZE];