[checkers]

checker_lookup() silently returns the default checker if the requested
one can't be loaded. Even worse, it the default checker can't be loaded,
it gets stuck in an infinite loop.

It seems to me that if the requested checker can't be loaded, we should
just throw an error. That's what this patch does.

multipath.conf should be order independent, and with the existing
checker code, it isn't.  The multipath_dir had to be set before
you could set a checker, otherwise the checker load failed (since the
multipath_dir wasn't specified).

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index 3aa1f9d..5889ad7 100644 (file)
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -47,10 +47,7 @@ struct checker * checker_lookup (char * name)
                if (!strncmp(name, c->name, CHECKER_NAME_LEN))
                        return c;
        }
-       c = add_checker(name);
-       if (c)
-               return c;
-       return checker_default();
+       return add_checker(name);
 }
 
 struct checker * add_checker (char * name)
@@ -172,13 +169,14 @@ char * checker_message (struct checker * c)
        return c->message;
 }
 
-struct checker * checker_default (void)
+void checker_get (struct checker * dst, char * name)
 {
-       return checker_lookup(DEFAULT_CHECKER);
-}
+       struct checker * src = checker_lookup(name);
 
-void checker_get (struct checker * dst, struct checker * src)
-{
+       if (!src) {
+               dst->check = NULL;
+               return;
+       }
        dst->fd = src->fd;
        dst->sync = src->sync;
        strncpy(dst->name, src->name, CHECKER_NAME_LEN);

diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h
index 4ff475d..a65aaf9 100644 (file)
--- a/libmultipath/checkers.h
+++ b/libmultipath/checkers.h
@@ -117,12 +117,10 @@ void checker_set_async (struct checker *);
 void checker_set_fd (struct checker *, int);
 void checker_enable (struct checker *);
 void checker_disable (struct checker *);
-struct checker * checker_lookup (char *);
 int checker_check (struct checker *);
 int checker_selected (struct checker *);
 char * checker_name (struct checker *);
 char * checker_message (struct checker *);
-struct checker * checker_default (void);
-void checker_get (struct checker *, struct checker *);
+void checker_get (struct checker *, char *);
 
 #endif /* _CHECKERS_H */

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 4548ad1..0518e3a 100644 (file)
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -270,13 +270,15 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
 
        if (dhwe->selector && !(hwe->selector = set_param_str(dhwe->selector)))
                goto out;
+
+       if (dhwe->checker_name && !(hwe->checker_name = set_param_str(dhwe->checker_name)))
+               goto out;
                                
        hwe->pgpolicy = dhwe->pgpolicy;
        hwe->pgfailback = dhwe->pgfailback;
        hwe->rr_weight = dhwe->rr_weight;
        hwe->no_path_retry = dhwe->no_path_retry;
        hwe->minio = dhwe->minio;
-       hwe->checker = dhwe->checker;
        hwe->prio = dhwe->prio;
 
        if (dhwe->bl_product && !(hwe->bl_product = set_param_str(dhwe->bl_product)))
@@ -455,8 +457,8 @@ load_config (char * file)
        if (!conf->prio)
                conf->prio = prio_default();
 
-       if (!conf->checker)
-               conf->checker = checker_lookup(DEFAULT_CHECKER);
+       if (!conf->checker_name)
+               conf->checker_name = set_default(DEFAULT_CHECKER);
 
        return 0;
 out:

diff --git a/libmultipath/config.h b/libmultipath/config.h
index 8a97799..b644544 100644 (file)
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -29,7 +29,6 @@ struct hwentry {
        int minio;
        int pg_timeout;
        struct prio * prio;
-       struct checker * checker;
        char * bl_product;
 };
 
@@ -55,7 +54,6 @@ struct config {
        int with_sysfs;
        int pgpolicy;
        struct prio * prio;
-       struct checker * checker;
        enum devtypes dev_type;
        int minio;
        int checkint;
@@ -77,6 +75,7 @@ struct config {
        char * features;
        char * hwhandler;
        char * bindings_file;
+       char * checker_name;
 
        vector keywords;
        vector mptable;

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index b6f7845..c9edcc4 100644 (file)
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -120,16 +120,11 @@ def_features_handler(vector strvec)
 static int
 def_path_checker_handler(vector strvec)
 {
-       char * buff;
-
-       buff = set_value(strvec);
+       conf->checker_name = set_value(strvec);
 
-       if (!buff)
+       if (!conf->checker_name)
                return 1;
        
-       conf->checker = checker_lookup(buff);
-       FREE(buff);
-
        return 0;
 }
 
@@ -549,20 +544,16 @@ hw_selector_handler(vector strvec)
 static int
 hw_path_checker_handler(vector strvec)
 {
-       char * buff;
        struct hwentry * hwe = VECTOR_LAST_SLOT(conf->hwtable);
 
        if (!hwe)
                return 1;
 
-       buff = set_value(strvec);
+       hwe->checker_name = set_value(strvec);
 
-       if (!buff)
+       if (!hwe->checker_name)
                return 1;
        
-       hwe->checker = checker_lookup(buff);
-       FREE(buff);
-
        return 0;
 }
 
@@ -1312,14 +1303,12 @@ snprint_hw_path_checker (char * buff, int len, void * data)
 {
        struct hwentry * hwe = (struct hwentry *)data;
 
-       if (!hwe->checker)
-               return 0;
-       if (!checker_selected(hwe->checker))
+       if (!hwe->checker_name)
                return 0;
-       if (hwe->checker == conf->checker)
+       if (!strcmp(hwe->checker_name, conf->checker_name))
                return 0;
        
-       return snprintf(buff, len, "%s", checker_name(hwe->checker));
+       return snprintf(buff, len, "%s", hwe->checker_name);
 }
 
 static int
@@ -1417,12 +1406,12 @@ snprint_def_features (char * buff, int len, void * data)
 static int
 snprint_def_path_checker (char * buff, int len, void * data)
 {
-       if (!conf->checker)
+       if (!conf->checker_name)
                return 0;
-       if (conf->checker == checker_default())
+       if (conf->checker_name == DEFAULT_CHECKER)
                return 0;
        
-       return snprintf(buff, len, "%s", checker_name(conf->checker));
+       return snprintf(buff, len, "%s", conf->checker_name);
 }
 
 static int

diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 5ed7090..d879cb1 100644 (file)
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -732,7 +732,6 @@ setup_default_hwtable (vector hw)
        struct hwentry * hwe = default_hw;
 
        while (hwe->vendor) {
-               hwe->checker = checker_lookup(hwe->checker_name);
                hwe->prio = prio_lookup(hwe->prio_name);
                r += store_hwe(hw, hwe);
                hwe++;

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 0c7fb99..071c913 100644 (file)
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -217,19 +217,19 @@ select_checker(struct path *pp)
 {
        struct checker * c = &pp->checker;
 
-       if (pp->hwe && pp->hwe->checker) {
-               checker_get(c, pp->hwe->checker);
+       if (pp->hwe && pp->hwe->checker_name) {
+               checker_get(c, pp->hwe->checker_name);
                condlog(3, "%s: path checker = %s (controller setting)",
                        pp->dev, checker_name(c));
                return 0;
        }
-       if (conf->checker) {
-               checker_get(c, conf->checker);
+       if (conf->checker_name) {
+               checker_get(c, conf->checker_name);
                condlog(3, "%s: path checker = %s (config file default)",
                        pp->dev, checker_name(c));
                return 0;
        }
-       checker_get(c, checker_default());
+       checker_get(c, DEFAULT_CHECKER);
        condlog(3, "%s: path checker = %s (internal default)",
                pp->dev, checker_name(c));
        return 0;