* [PATCH 01/28] kpartx: no need to use FREE_CONST
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 02/28] libmultipath: fix memory leak in process_config_dir() Martin Wilck
` (27 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
A function that returns a result of strdup() doesn't need to declare that
result as const char*.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
kpartx/devmapper.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 8f68a246..f94d70e7 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -11,7 +11,6 @@
#include <sys/sysmacros.h>
#include "devmapper.h"
-#define FREE_CONST(p) do { free((void*)(long)p); p = NULL; } while(0)
#define _UUID_PREFIX "part"
#define UUID_PREFIX _UUID_PREFIX "%d-"
#define _UUID_PREFIX_LEN (sizeof(_UUID_PREFIX) - 1)
@@ -252,10 +251,11 @@ out:
return r;
}
-static const char *dm_find_uuid(const char *uuid)
+static char *dm_find_uuid(const char *uuid)
{
struct dm_task *dmt;
- const char *name = NULL, *tmp;
+ char *name = NULL;
+ const char *tmp;
if ((dmt = dm_task_create(DM_DEVICE_INFO)) == NULL)
return NULL;
@@ -642,7 +642,7 @@ int dm_find_part(const char *parent, const char *delim, int part,
{
int r;
char params[PARAMS_SIZE];
- const char *tmp;
+ char *tmp;
char *uuid;
int major, minor;
char dev_t[32];
@@ -696,7 +696,7 @@ int dm_find_part(const char *parent, const char *delim, int part,
} else
*part_uuid = uuid;
out:
- FREE_CONST(tmp);
+ free(tmp);
return r;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 02/28] libmultipath: fix memory leak in process_config_dir()
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
2018-06-08 10:20 ` [PATCH 01/28] kpartx: no need to use FREE_CONST Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 03/28] libmultipath: remove superfluous conditionals in load_config() Martin Wilck
` (26 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 085a3e12..8769441c 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -551,6 +551,11 @@ free_config (struct config * conf)
FREE(conf);
}
+static void free_namelist(void *nl)
+{
+ free(nl);
+}
+
/* if multipath fails to process the config directory, it should continue,
* with just a warning message */
static void
@@ -574,7 +579,9 @@ process_config_dir(struct config *conf, vector keywords, char *dir)
condlog(0, "couldn't open configuration dir '%s': %s",
dir, strerror(errno));
return;
- }
+ } else if (n == 0)
+ return;
+ pthread_cleanup_push(free_namelist, namelist);
for (i = 0; i < n; i++) {
if (!strstr(namelist[i]->d_name, ".conf"))
continue;
@@ -586,6 +593,7 @@ process_config_dir(struct config *conf, vector keywords, char *dir)
factorize_hwtable(conf->hwtable, old_hwtable_size);
}
+ pthread_cleanup_pop(1);
}
struct config *
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 03/28] libmultipath: remove superfluous conditionals in load_config()
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
2018-06-08 10:20 ` [PATCH 01/28] kpartx: no need to use FREE_CONST Martin Wilck
2018-06-08 10:20 ` [PATCH 02/28] libmultipath: fix memory leak in process_config_dir() Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 04/28] libmultipath/structs.c: constify some functions Martin Wilck
` (25 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
In load_config(), conf is freshly allocated, there's no point in checking
previously set values.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 8769441c..ce7bedac 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -607,8 +607,7 @@ load_config (char * file)
/*
* internal defaults
*/
- if (!conf->verbosity)
- conf->verbosity = DEFAULT_VERBOSITY;
+ conf->verbosity = DEFAULT_VERBOSITY;
get_sys_max_fds(&conf->max_fds);
conf->bindings_file = set_default(DEFAULT_BINDINGS_FILE);
@@ -634,12 +633,9 @@ load_config (char * file)
/*
* preload default hwtable
*/
- if (conf->hwtable == NULL) {
- conf->hwtable = vector_alloc();
-
- if (!conf->hwtable)
+ conf->hwtable = vector_alloc();
+ if (!conf->hwtable)
goto out;
- }
if (setup_default_hwtable(conf->hwtable))
goto out;
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 04/28] libmultipath/structs.c: constify some functions
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (2 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 03/28] libmultipath: remove superfluous conditionals in load_config() Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 05/28] libmultipath: some const usage in hwentry handling Martin Wilck
` (24 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/structs.c | 20 ++++++++++----------
libmultipath/structs.h | 24 ++++++++++++------------
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 991095cb..6df2f4ec 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -353,7 +353,7 @@ store_adaptergroup(vector adapters, struct adapter_group * agp)
}
struct multipath *
-find_mp_by_minor (vector mpvec, int minor)
+find_mp_by_minor (const struct _vector *mpvec, int minor)
{
int i;
struct multipath * mpp;
@@ -372,7 +372,7 @@ find_mp_by_minor (vector mpvec, int minor)
}
struct multipath *
-find_mp_by_wwid (vector mpvec, char * wwid)
+find_mp_by_wwid (const struct _vector *mpvec, const char * wwid)
{
int i;
struct multipath * mpp;
@@ -388,7 +388,7 @@ find_mp_by_wwid (vector mpvec, char * wwid)
}
struct multipath *
-find_mp_by_alias (vector mpvec, const char * alias)
+find_mp_by_alias (const struct _vector *mpvec, const char * alias)
{
int i;
int len;
@@ -411,7 +411,7 @@ find_mp_by_alias (vector mpvec, const char * alias)
}
struct multipath *
-find_mp_by_str (vector mpvec, char * str)
+find_mp_by_str (const struct _vector *mpvec, const char * str)
{
int minor;
@@ -422,7 +422,7 @@ find_mp_by_str (vector mpvec, char * str)
}
struct path *
-find_path_by_dev (vector pathvec, const char * dev)
+find_path_by_dev (const struct _vector *pathvec, const char * dev)
{
int i;
struct path * pp;
@@ -439,7 +439,7 @@ find_path_by_dev (vector pathvec, const char * dev)
}
struct path *
-find_path_by_devt (vector pathvec, const char * dev_t)
+find_path_by_devt (const struct _vector *pathvec, const char * dev_t)
{
int i;
struct path * pp;
@@ -455,7 +455,7 @@ find_path_by_devt (vector pathvec, const char * dev_t)
return NULL;
}
-int pathcountgr(struct pathgroup *pgp, int state)
+int pathcountgr(const struct pathgroup *pgp, int state)
{
struct path *pp;
int count = 0;
@@ -468,7 +468,7 @@ int pathcountgr(struct pathgroup *pgp, int state)
return count;
}
-int pathcount(struct multipath *mpp, int state)
+int pathcount(const struct multipath *mpp, int state)
{
struct pathgroup *pgp;
int count = 0;
@@ -481,7 +481,7 @@ int pathcount(struct multipath *mpp, int state)
return count;
}
-int pathcmp(struct pathgroup *pgp, struct pathgroup *cpgp)
+int pathcmp(const struct pathgroup *pgp, const struct pathgroup *cpgp)
{
int i, j;
struct path *pp, *cpp;
@@ -501,7 +501,7 @@ int pathcmp(struct pathgroup *pgp, struct pathgroup *cpgp)
}
struct path *
-first_path (struct multipath * mpp)
+first_path (const struct multipath * mpp)
{
struct pathgroup * pgp;
if (!mpp->pg)
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index e424b150..fef416b4 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -413,18 +413,18 @@ int store_path (vector pathvec, struct path * pp);
int store_pathgroup (vector pgvec, struct pathgroup * pgp);
int add_pathgroup(struct multipath*, struct pathgroup *);
-struct multipath * find_mp_by_alias (vector mp, const char * alias);
-struct multipath * find_mp_by_wwid (vector mp, char * wwid);
-struct multipath * find_mp_by_str (vector mp, char * wwid);
-struct multipath * find_mp_by_minor (vector mp, int minor);
-
-struct path * find_path_by_devt (vector pathvec, const char * devt);
-struct path * find_path_by_dev (vector pathvec, const char * dev);
-struct path * first_path (struct multipath * mpp);
-
-int pathcountgr (struct pathgroup *, int);
-int pathcount (struct multipath *, int);
-int pathcmp (struct pathgroup *, struct pathgroup *);
+struct multipath * find_mp_by_alias (const struct _vector *mp, const char *alias);
+struct multipath * find_mp_by_wwid (const struct _vector *mp, const char *wwid);
+struct multipath * find_mp_by_str (const struct _vector *mp, const char *wwid);
+struct multipath * find_mp_by_minor (const struct _vector *mp, int minor);
+
+struct path * find_path_by_devt (const struct _vector *pathvec, const char *devt);
+struct path * find_path_by_dev (const struct _vector *pathvec, const char *dev);
+struct path * first_path (const struct multipath *mpp);
+
+int pathcountgr (const struct pathgroup *, int);
+int pathcount (const struct multipath *, int);
+int pathcmp (const struct pathgroup *, const struct pathgroup *);
int add_feature (char **, const char *);
int remove_feature (char **, const char *);
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 05/28] libmultipath: some const usage in hwentry handling
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (3 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 04/28] libmultipath/structs.c: constify some functions Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 06/28] libmultipath: change prototypes of hwe_regmatch() and find_hwe() Martin Wilck
` (23 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Add some const qualifiers in in the code dealing with hwentries.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/blacklist.c | 6 ++++--
libmultipath/config.c | 8 ++++----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index ee396e26..34a7e717 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -126,7 +126,8 @@ _blacklist (vector blist, const char * str)
}
int
-_blacklist_exceptions_device(vector elist, char * vendor, char * product)
+_blacklist_exceptions_device(const struct _vector *elist, const char * vendor,
+ const char * product)
{
int i;
struct blentry_device * ble;
@@ -144,7 +145,8 @@ _blacklist_exceptions_device(vector elist, char * vendor, char * product)
}
int
-_blacklist_device (vector blist, char * vendor, char * product)
+_blacklist_device (const struct _vector *blist, const char * vendor,
+ const char * product)
{
int i;
struct blentry_device * ble;
diff --git a/libmultipath/config.c b/libmultipath/config.c
index ce7bedac..44f141ba 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -28,7 +28,7 @@
#include "propsel.h"
static int
-hwe_strmatch (struct hwentry *hwe1, struct hwentry *hwe2)
+hwe_strmatch (const struct hwentry *hwe1, const struct hwentry *hwe2)
{
if ((hwe2->vendor && !hwe1->vendor) ||
(hwe1->vendor && (!hwe2->vendor ||
@@ -49,7 +49,7 @@ hwe_strmatch (struct hwentry *hwe1, struct hwentry *hwe2)
}
static struct hwentry *
-find_hwe_strmatch (vector hwtable, struct hwentry *hwe)
+find_hwe_strmatch (const struct _vector *hwtable, const struct hwentry *hwe)
{
int i;
struct hwentry *tmp, *ret = NULL;
@@ -64,7 +64,7 @@ find_hwe_strmatch (vector hwtable, struct hwentry *hwe)
}
static int
-hwe_regmatch (struct hwentry *hwe1, struct hwentry *hwe2)
+hwe_regmatch (const struct hwentry *hwe1, const struct hwentry *hwe2)
{
regex_t vre, pre, rre;
int retval = 1;
@@ -283,7 +283,7 @@ alloc_hwe (void)
}
static char *
-set_param_str(char * str)
+set_param_str(const char * str)
{
char * dst;
int len;
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 06/28] libmultipath: change prototypes of hwe_regmatch() and find_hwe()
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (4 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 05/28] libmultipath: some const usage in hwentry handling Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 07/28] libmultipath/prio: constify simple getters Martin Wilck
` (22 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
hwentry structures are supposed to be initialized and destroyed
with alloc_hwe() and free_hwe(), respectively. The second
argument is of hwe_regmatch() isn't a real hwentry, just a
vendor/product/revision tuple. Clarify that by adapting the
prototype. This allows to use const arguments in find_hwe(), too.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 30 +++++++++++++++---------------
libmultipath/config.h | 4 +++-
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 44f141ba..d2812e4a 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -64,7 +64,8 @@ find_hwe_strmatch (const struct _vector *hwtable, const struct hwentry *hwe)
}
static int
-hwe_regmatch (const struct hwentry *hwe1, const struct hwentry *hwe2)
+hwe_regmatch (const struct hwentry *hwe1, const char *vendor,
+ const char *product, const char *revision)
{
regex_t vre, pre, rre;
int retval = 1;
@@ -81,13 +82,13 @@ hwe_regmatch (const struct hwentry *hwe1, const struct hwentry *hwe2)
regcomp(&rre, hwe1->revision, REG_EXTENDED|REG_NOSUB))
goto out_pre;
- if ((hwe2->vendor || hwe2->product || hwe2->revision) &&
- (!hwe1->vendor || !hwe2->vendor ||
- !regexec(&vre, hwe2->vendor, 0, NULL, 0)) &&
- (!hwe1->product || !hwe2->product ||
- !regexec(&pre, hwe2->product, 0, NULL, 0)) &&
- (!hwe1->revision || !hwe2->revision ||
- !regexec(&rre, hwe2->revision, 0, NULL, 0)))
+ if ((vendor || product || revision) &&
+ (!hwe1->vendor || !vendor ||
+ !regexec(&vre, vendor, 0, NULL, 0)) &&
+ (!hwe1->product || !product ||
+ !regexec(&pre, product, 0, NULL, 0)) &&
+ (!hwe1->revision || !revision ||
+ !regexec(&rre, revision, 0, NULL, 0)))
retval = 0;
if (hwe1->revision)
@@ -103,14 +104,12 @@ out:
}
struct hwentry *
-find_hwe (vector hwtable, char * vendor, char * product, char * revision)
+find_hwe (const struct _vector *hwtable,
+ const char * vendor, const char * product, const char * revision)
{
int i;
- struct hwentry hwe, *tmp, *ret = NULL;
+ struct hwentry *tmp, *ret = NULL;
- hwe.vendor = vendor;
- hwe.product = product;
- hwe.revision = revision;
/*
* Search backwards here.
* User modified entries are attached at the end of
@@ -118,7 +117,7 @@ find_hwe (vector hwtable, char * vendor, char * product, char * revision)
* continuing to the generic entries
*/
vector_foreach_slot_backwards (hwtable, tmp, i) {
- if (hwe_regmatch(tmp, &hwe))
+ if (hwe_regmatch(tmp, vendor, product, revision))
continue;
ret = tmp;
break;
@@ -454,7 +453,8 @@ restart:
free_hwe(hwe2);
continue;
}
- if (hwe_regmatch(hwe1, hwe2))
+ if (hwe_regmatch(hwe1, hwe2->vendor,
+ hwe2->product, hwe2->revision))
continue;
/* dup */
merge_hwe(hwe2, hwe1);
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 6e69a37e..83eaf62f 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -213,7 +213,9 @@ struct config {
extern struct udev * udev;
-struct hwentry * find_hwe (vector hwtable, char * vendor, char * product, char *revision);
+struct hwentry * find_hwe (const struct _vector *hwtable,
+ const char * vendor, const char * product,
+ const char *revision);
struct mpentry * find_mpe (vector mptable, char * wwid);
char * get_mpe_wwid (vector mptable, char * alias);
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 07/28] libmultipath/prio: constify simple getters
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (5 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 06/28] libmultipath: change prototypes of hwe_regmatch() and find_hwe() Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 08/28] tests/Makefile: autogenerate list of symbols to be wrapped Martin Wilck
` (21 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
One day we should also constify getprio(), but that requires
touching all prioritizers.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/prio.c | 8 ++++----
libmultipath/prio.h | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index 7fce9215..17acfd05 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -81,7 +81,7 @@ static struct prio * prio_lookup (char * name)
return NULL;
}
-int prio_set_args (struct prio * p, char * args)
+int prio_set_args (struct prio * p, const char * args)
{
return snprintf(p->args, PRIO_ARGS_LEN, "%s", args);
}
@@ -130,19 +130,19 @@ int prio_getprio (struct prio * p, struct path * pp, unsigned int timeout)
return p->getprio(pp, p->args, timeout);
}
-int prio_selected (struct prio * p)
+int prio_selected (const struct prio * p)
{
if (!p)
return 0;
return (p->getprio) ? 1 : 0;
}
-char * prio_name (struct prio * p)
+const char * prio_name (const struct prio * p)
{
return p->name;
}
-char * prio_args (struct prio * p)
+const char * prio_args (const struct prio * p)
{
return p->args;
}
diff --git a/libmultipath/prio.h b/libmultipath/prio.h
index c97fe397..aa587ccd 100644
--- a/libmultipath/prio.h
+++ b/libmultipath/prio.h
@@ -60,10 +60,10 @@ struct prio * add_prio (char *, char *);
int prio_getprio (struct prio *, struct path *, unsigned int);
void prio_get (char *, struct prio *, char *, char *);
void prio_put (struct prio *);
-int prio_selected (struct prio *);
-char * prio_name (struct prio *);
-char * prio_args (struct prio *);
-int prio_set_args (struct prio *, char *);
+int prio_selected (const struct prio *);
+const char * prio_name (const struct prio *);
+const char * prio_args (const struct prio *);
+int prio_set_args (struct prio *, const char *);
/* The only function exported by prioritizer dynamic libraries (.so) */
int getprio(struct path *, char *, unsigned int);
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 08/28] tests/Makefile: autogenerate list of symbols to be wrapped
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (6 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 07/28] libmultipath/prio: constify simple getters Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 09/28] tests/test-lib: cmocka helpers to simulate path and map discovery Martin Wilck
` (20 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Rather than manually listing all the -Wl,--wrap=... options, autogenerate
a list of functions to be wrapped in a cmocka test. Allowing this not only
for the test file itself but also for additional "test library" source files
requires some Makefile incantations.
Moreover, no need to list globals.c in dependencies, automatic dependency
tracking takes care for this (and avoids recompilation of tests that don't
pull in global.c).
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
tests/Makefile | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/tests/Makefile b/tests/Makefile
index 1f364110..7439c8c3 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -10,23 +10,39 @@ TESTS := uevent parser util dmevents
all: $(TESTS:%=%.out)
-dmevents-test: dmevents.o ../multipathd/dmevents.c globals.c $(multipathdir)/libmultipath.so
- @$(CC) -o $@ $< $(LDFLAGS) $(LIBDEPS) -lpthread -ldevmapper -lurcu -Wl,--wrap=open -Wl,--wrap=close -Wl,--wrap=dm_is_mpath -Wl,--wrap=dm_geteventnr -Wl,--wrap=ioctl -Wl,--wrap=libmp_dm_task_create -Wl,--wrap=dm_task_no_open_count -Wl,--wrap=dm_task_run -Wl,--wrap=dm_task_get_names -Wl,--wrap=dm_task_destroy -Wl,--wrap=poll -Wl,--wrap=remove_map_by_alias -Wl,--wrap=update_multipath
+# test-specific linker flags
+# XYZ-test-TESTDEPS: test libraries containing __wrap_xyz functions
+# XYZ-test_OBJDEPS: object files from libraries to link in explicitly
+# That may be necessary if functions called from the object file are wrapped
+# (wrapping works only for symbols which are undefined after processing a
+# linker input file).
+# XYZ-test_LIBDEPS: Additional libs to link for this test
-%-test: %.o globals.c $(multipathdir)/libmultipath.so
- @$(CC) -o $@ $< $(LDFLAGS) $(LIBDEPS)
+dmevents-test_LIBDEPS = -lpthread -ldevmapper -lurcu
%.out: %-test
@echo == running $< ==
@LD_LIBRARY_PATH=$(multipathdir):$(mpathcmddir) ./$< >$@
clean: dep_clean
- rm -f $(TESTS:%=%-test) $(TESTS:%=%.out) $(TESTS:%=%.o)
+ $(RM) $(TESTS:%=%-test) $(TESTS:%=%.out) $(OBJS)
-OBJS = $(TESTS:%=%.o)
.SECONDARY: $(OBJS)
include $(wildcard $(OBJS:.o=.d))
dep_clean:
$(RM) $(OBJS:.o=.d)
+
+%.o.wrap: %.c
+ @sed -n 's/^.*__wrap_\([a-zA-Z0-9_]*\).*$$/-Wl,--wrap=\1/p' $< | \
+ sort -u | tr '\n' ' ' >$@
+
+# COLON will get expanded during second expansion below
+COLON:=:
+.SECONDEXPANSION:
+%-test: %.o %.o.wrap $$($$@_OBJDEPS) $$($$@_TESTDEPS) $$($$@_TESTDEPS$$(COLON).o=.o.wrap) \
+ $(multipathdir)/libmultipath.so Makefile
+ $(CC) $(CFLAGS) -o $@ $(LDFLAGS) $< $($@_TESTDEPS) $($@_OBJDEPS) \
+ $(LIBDEPS) $($@_LIBDEPS) \
+ $(file <$<.wrap) $(foreach dep,$($@_TESTDEPS),$(file <$(dep).wrap))
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 09/28] tests/test-lib: cmocka helpers to simulate path and map discovery
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (7 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 08/28] tests/Makefile: autogenerate list of symbols to be wrapped Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 10/28] tests/hwtable: tests for config file handling and hwentry merging Martin Wilck
` (19 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
These functions simulate path discovery and multipath setup with cmocka. They will be
used in the hwtable test case, but may be useful for other future test cases,
too.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
tests/test-lib.c | 359 +++++++++++++++++++++++++++++++++++++++++++++++
tests/test-lib.h | 67 +++++++++
2 files changed, 426 insertions(+)
create mode 100644 tests/test-lib.c
create mode 100644 tests/test-lib.h
diff --git a/tests/test-lib.c b/tests/test-lib.c
new file mode 100644
index 00000000..cff7a5d1
--- /dev/null
+++ b/tests/test-lib.c
@@ -0,0 +1,359 @@
+#include <string.h>
+#include <setjmp.h>
+#include <stdarg.h>
+#include <cmocka.h>
+#include <libudev.h>
+#include <sys/sysmacros.h>
+#include "debug.h"
+#include "util.h"
+#include "vector.h"
+#include "structs.h"
+#include "structs_vec.h"
+#include "config.h"
+#include "discovery.h"
+#include "propsel.h"
+#include "test-lib.h"
+
+const int default_mask = (DI_SYSFS|DI_BLACKLIST|DI_WWID|DI_CHECKER|DI_PRIO);
+const char default_devnode[] = "sdTEST";
+const char default_wwid[] = "TEST-WWID";
+/* default_wwid should be a substring of default_wwid_1! */
+const char default_wwid_1[] = "TEST-WWID-1";
+
+/*
+ * Helper wrappers for mock_path().
+ *
+ * We need to make pathinfo() think it has detected a device with
+ * certain vendor/product/rev. This requires faking lots of udev
+ * and sysfs function responses.
+ *
+ * This requires hwtable-test_OBJDEPS = ../libmultipath/discovery.o
+ * in the Makefile in order to wrap calls from discovery.o.
+ *
+ * Note that functions that are called and defined in discovery.o can't
+ * be wrapped this way (e.g. sysfs_get_vendor), because symbols are
+ * resolved by the assembler before the linking stage.
+ */
+
+int __real_open(const char *path, int flags, int mode);
+
+static const char _mocked_filename[] = "mocked_path";
+int __wrap_open(const char *path, int flags, int mode)
+{
+ condlog(4, "%s: %s", __func__, path);
+
+ if (!strcmp(path, _mocked_filename))
+ return 111;
+ return __real_open(path, flags, mode);
+}
+
+int __wrap_execute_program(char *path, char *value, int len)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s", __func__, val);
+ strlcpy(value, val, len);
+ return 0;
+}
+
+bool __wrap_is_claimed_by_foreign(struct udev_device *ud)
+{
+ condlog(5, "%s: %p", __func__, ud);
+ return false;
+}
+
+struct udev_list_entry
+*__wrap_udev_device_get_properties_list_entry(struct udev_device *ud)
+{
+ void *p = (void*)0x12345678;
+ condlog(5, "%s: %p", __func__, p);
+
+ return p;
+}
+
+struct udev_list_entry
+*__wrap_udev_list_entry_get_next(struct udev_list_entry *udle)
+{
+ void *p = NULL;
+ condlog(5, "%s: %p", __func__, p);
+
+ return p;
+}
+
+const char *__wrap_udev_list_entry_get_name(struct udev_list_entry *udle)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s", __func__, val);
+ return val;
+}
+
+struct udev_device *__wrap_udev_device_ref(struct udev_device *ud)
+{
+ return ud;
+}
+
+struct udev_device *__wrap_udev_device_unref(struct udev_device *ud)
+{
+ return ud;
+}
+
+char *__wrap_udev_device_get_subsystem(struct udev_device *ud)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s", __func__, val);
+ return val;
+}
+
+char *__wrap_udev_device_get_sysname(struct udev_device *ud)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s", __func__, val);
+ return val;
+}
+
+char *__wrap_udev_device_get_devnode(struct udev_device *ud)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s", __func__, val);
+ return val;
+}
+
+dev_t __wrap_udev_device_get_devnum(struct udev_device *ud)
+{
+ condlog(5, "%s: %p", __func__, ud);
+ return makedev(17, 17);
+}
+
+char *__wrap_udev_device_get_sysattr_value(struct udev_device *ud,
+ const char *attr)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s->%s", __func__, attr, val);
+ return val;
+}
+
+char *__wrap_udev_device_get_property_value(struct udev_device *ud,
+ const char *attr)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s->%s", __func__, attr, val);
+ return val;
+}
+
+int __wrap_sysfs_get_size(struct path *pp, unsigned long long *sz)
+{
+ *sz = 12345678UL;
+ return 0;
+}
+
+void *__wrap_udev_device_get_parent_with_subsystem_devtype(
+ struct udev_device *ud, const char *subsys, char *type)
+{
+ /* return non-NULL for sysfs_get_tgt_nodename */
+ return type;
+}
+
+void *__wrap_udev_device_get_parent(struct udev_device *ud)
+{
+ char *val = mock_ptr_type(void *);
+
+ condlog(5, "%s: %p", __func__, val);
+ return val;
+}
+
+ssize_t __wrap_sysfs_attr_get_value(struct udev_device *dev,
+ const char *attr_name,
+ char *value, size_t sz)
+{
+ char *val = mock_ptr_type(char *);
+
+ condlog(5, "%s: %s", __func__, val);
+ strlcpy(value, val, sz);
+ return strlen(value);
+}
+
+int __wrap_checker_check(struct checker *c, int st)
+{
+ condlog(5, "%s: %d", __func__, st);
+ return st;
+}
+
+int __wrap_prio_getprio(struct prio *p, struct path *pp, unsigned int tmo)
+{
+ int pr = 5;
+
+ condlog(5, "%s: %d", __func__, pr);
+ return pr;
+}
+
+struct mocked_path *fill_mocked_path(struct mocked_path *mp,
+ const char *vendor, const char *product,
+ const char *rev, const char *wwid,
+ const char *devnode, unsigned int flags)
+{
+ mp->vendor = (vendor ? vendor : "noname");
+ mp->product = (product ? product : "noprod");
+ mp->rev = (rev ? rev : "0");
+ mp->wwid = (wwid ? wwid : default_wwid);
+ mp->devnode = (devnode ? devnode : default_devnode);
+ mp->flags = flags|NEED_SELECT_PRIO|NEED_FD;
+ return mp;
+}
+
+struct mocked_path *mocked_path_from_path(struct mocked_path *mp,
+ const struct path *pp)
+{
+ mp->vendor = pp->vendor_id;
+ mp->product = pp->product_id;
+ mp->rev = pp->rev;
+ mp->wwid = pp->wwid;
+ mp->devnode = pp->dev;
+ mp->flags = (prio_selected(&pp->prio) ? 0 : NEED_SELECT_PRIO) |
+ (pp->fd < 0 ? NEED_FD : 0) |
+ (pp->getuid ? USE_GETUID : 0);
+ return mp;
+}
+
+static void mock_sysfs_pathinfo(const struct mocked_path *mp)
+{
+ static const char hbtl[] = "4:0:3:1";
+
+ will_return(__wrap_udev_device_get_subsystem, "scsi");
+ will_return(__wrap_udev_device_get_sysname, hbtl);
+ will_return(__wrap_udev_device_get_sysname, hbtl);
+ will_return(__wrap_udev_device_get_sysattr_value, mp->vendor);
+ will_return(__wrap_udev_device_get_sysname, hbtl);
+ will_return(__wrap_udev_device_get_sysattr_value, mp->product);
+ will_return(__wrap_udev_device_get_sysname, hbtl);
+ will_return(__wrap_udev_device_get_sysattr_value, mp->rev);
+
+ /* sysfs_get_tgt_nodename */
+ will_return(__wrap_udev_device_get_sysattr_value, NULL);
+ will_return(__wrap_udev_device_get_parent, NULL);
+ will_return(__wrap_udev_device_get_parent, NULL);
+ will_return(__wrap_udev_device_get_sysname, "nofibre");
+ will_return(__wrap_udev_device_get_sysname, "noiscsi");
+ will_return(__wrap_udev_device_get_parent, NULL);
+ will_return(__wrap_udev_device_get_sysname, "ata25");
+}
+
+/*
+ * Pretend we detected a SCSI device with given vendor/prod/rev
+ */
+void mock_pathinfo(int mask, const struct mocked_path *mp)
+{
+ /* filter_property */
+ will_return(__wrap_udev_device_get_sysname, mp->devnode);
+ if (mp->flags & BL_BY_PROPERTY) {
+ will_return(__wrap_udev_list_entry_get_name, "BAZ");
+ return;
+ } else
+ will_return(__wrap_udev_list_entry_get_name,
+ "SCSI_IDENT_LUN_NAA_EXT");
+ if (mask & DI_SYSFS)
+ mock_sysfs_pathinfo(mp);
+
+ if (mp->flags & BL_BY_DEVICE &&
+ (mask & DI_BLACKLIST && mask & DI_SYSFS))
+ return;
+
+ /* path_offline */
+ will_return(__wrap_udev_device_get_subsystem, "scsi");
+ will_return(__wrap_sysfs_attr_get_value, "running");
+
+ if (mask & DI_NOIO)
+ return;
+
+ /* fake open() in pathinfo() */
+ if (mp->flags & NEED_FD)
+ will_return(__wrap_udev_device_get_devnode, _mocked_filename);
+ /* DI_SERIAL is unsupported */
+ assert_false(mask & DI_SERIAL);
+
+ if (mask & DI_WWID) {
+ if (mp->flags & USE_GETUID)
+ will_return(__wrap_execute_program, mp->wwid);
+ else
+ /* get_udev_uid() */
+ will_return(__wrap_udev_device_get_property_value,
+ mp->wwid);
+ }
+
+ if (mask & DI_CHECKER) {
+ /* get_state -> sysfs_get_timeout */
+ will_return(__wrap_udev_device_get_subsystem, "scsi");
+ will_return(__wrap_udev_device_get_sysattr_value, "180");
+ }
+
+ if (mask & DI_PRIO && mp->flags & NEED_SELECT_PRIO) {
+
+ /* sysfs_get_timeout, again (!?) */
+ will_return(__wrap_udev_device_get_subsystem, "scsi");
+ will_return(__wrap_udev_device_get_sysattr_value, "180");
+
+ }
+}
+
+void mock_store_pathinfo(int mask, const struct mocked_path *mp)
+{
+ will_return(__wrap_udev_device_get_sysname, mp->devnode);
+ mock_pathinfo(mask, mp);
+}
+
+struct path *__mock_path(vector pathvec,
+ const char *vnd, const char *prd,
+ const char *rev, const char *wwid,
+ const char *dev,
+ unsigned int flags, int mask)
+{
+ struct mocked_path mop;
+ struct path *pp;
+ struct config *conf;
+ int r;
+
+ fill_mocked_path(&mop, vnd, prd, rev, wwid, dev, flags);
+ mock_store_pathinfo(mask, &mop);
+
+ conf = get_multipath_config();
+ r = store_pathinfo(pathvec, conf, (void *)&mop, mask, &pp);
+ put_multipath_config(conf);
+
+ if (flags & BL_MASK) {
+ assert_int_equal(r, PATHINFO_SKIPPED);
+ return NULL;
+ }
+ assert_int_equal(r, PATHINFO_OK);
+ assert_non_null(pp);
+ return pp;
+}
+
+
+struct multipath *__mock_multipath(struct vectors *vecs, struct path *pp)
+{
+ struct multipath *mp;
+ struct config *conf;
+ struct mocked_path mop;
+
+ mocked_path_from_path(&mop, pp);
+ /* pathinfo() call in adopt_paths */
+ mock_pathinfo(DI_CHECKER|DI_PRIO, &mop);
+
+ mp = add_map_with_path(vecs, pp, 1);
+ assert_ptr_not_equal(mp, NULL);
+
+ /* TBD: mock setup_map() ... */
+ conf = get_multipath_config();
+ select_pgpolicy(conf, mp);
+ select_no_path_retry(conf, mp);
+ select_retain_hwhandler(conf, mp);
+ select_minio(conf, mp);
+ put_multipath_config(conf);
+
+ return mp;
+}
diff --git a/tests/test-lib.h b/tests/test-lib.h
new file mode 100644
index 00000000..d2745979
--- /dev/null
+++ b/tests/test-lib.h
@@ -0,0 +1,67 @@
+#ifndef __LIB_H
+#define __LIB_H
+
+extern const int default_mask;
+extern const char default_devnode[];
+extern const char default_wwid[];
+extern const char default_wwid_1[];
+
+enum {
+ BL_BY_DEVNODE = (1 << 0),
+ BL_BY_DEVICE = (1 << 1),
+ BL_BY_WWID = (1 << 2),
+ BL_BY_PROPERTY = (1 << 3),
+ BL_MASK = BL_BY_DEVNODE|BL_BY_DEVICE|BL_BY_WWID|BL_BY_PROPERTY,
+ NEED_SELECT_PRIO = (1 << 8),
+ NEED_FD = (1 << 9),
+ USE_GETUID = (1 << 10)
+};
+
+struct mocked_path {
+ const char *vendor;
+ const char *product;
+ const char *rev;
+ const char *wwid;
+ const char *devnode;
+ unsigned int flags;
+};
+
+struct mocked_path *fill_mocked_path(struct mocked_path *mp,
+ const char *vendor,
+ const char *product,
+ const char *rev,
+ const char *wwid,
+ const char *devnode,
+ unsigned int flags);
+
+struct mocked_path *mocked_path_from_path(struct mocked_path *mp,
+ const struct path *pp);
+
+void mock_pathinfo(int mask, const struct mocked_path *mp);
+void mock_store_pathinfo(int mask, const struct mocked_path *mp);
+struct path *__mock_path(vector pathvec,
+ const char *vnd, const char *prd,
+ const char *rev, const char *wwid,
+ const char *dev,
+ unsigned int flags, int mask);
+
+#define mock_path(v, p) \
+ __mock_path(hwt->vecs->pathvec, (v), (p), "0", NULL, NULL, \
+ 0, default_mask)
+#define mock_path_flags(v, p, f) \
+ __mock_path(hwt->vecs->pathvec, (v), (p), "0", NULL, NULL, \
+ (f), default_mask)
+#define mock_path_blacklisted(v, p) \
+ __mock_path(hwt->vecs->pathvec, (v), (p), "0", NULL, NULL, \
+ BL_BY_DEVICE, default_mask)
+#define mock_path_wwid(v, p, w) \
+ __mock_path(hwt->vecs->pathvec, (v), (p), "0", (w), NULL, \
+ 0, default_mask)
+#define mock_path_wwid_flags(v, p, w, f) \
+ __mock_path(hwt->vecs->pathvec, (v), (p), "0", (w), \
+ NULL, (f), default_mask)
+
+struct multipath *__mock_multipath(struct vectors *vecs, struct path *pp);
+#define mock_multipath(pp) __mock_multipath(hwt->vecs, (pp))
+
+#endif
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 10/28] tests/hwtable: tests for config file handling and hwentry merging
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (8 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 09/28] tests/test-lib: cmocka helpers to simulate path and map discovery Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 11/28] libmultipath: add debug messages to hwentry lookup/merging code Martin Wilck
` (18 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Add unit tests for parsing "device" and "blacklist" configuration entries and
hwtable entry merging. The tests work by creating a multipath.conf file and one
or more config_dir/*.conf files, reading device entries from both, simulating
discovery of fake paths and multipaths, and verifying these are configured as
expected. The focus is on the behavior if several hwtable or blacklist entries
match a single device.
Most of these tests in this file serve as regression test for follow-up changes.
But Some of the tests are meant to illustrate problems problems with
the current code, which behaves inconsistently in certain cases. By
changing the BROKEN macro at the top of the file, the user can control if broken
behavior causes test failure, or succeeds with a warning. Subsequent patches
will remove the BROKEN tests. Reviewers are encouraged to focus on the BROKEN
tests, which annotate current behavior that follow-up patches are going to change.
Problems with the current code are mainly found in two areas:
1. It currently makes a difference whether several entries matching the same
device are in in the same configuration file (default "multipath.conf") or
distributed over "multipath.conf" and other files under config_dir. This is
highly unexpected behavior.
Moreover, in this context, the built-in hwtable can be viewed as yet another
"configuration file" which is parsed before "multipath.conf". The same problem
arises: adding a hwentry to "multipath.conf" doesn't necessarily have the same
effect as adding the same entry to the built-in hwtable and recompiling,
because different merging logic applies (in short: entries from different
"configuration files" are merged, but entries from the same "file" aren't).
This is a major blocker for using the configuration dumped with
"multipath -t" as input for multipath-tools, because a built-in entry which
is dumped ends up in "multipath.conf" or a config_dir file, which may change
the way it's merged with other entries.
2. The vendor/product/rev IDs are regular expressions, and merging of entries
by regular expressions simply doesn't work. Entries are merged if a later
regex, interpreted as a string, matches an earlier added regex. But that's
incorrect. Except in trivial cases, matching of a regex r2 by another regex
r1 doesn't say anything about whether strings matched by r2 will be matched
by r1 as well.
Another, minor issue is the treatment of double quotes.
The way these tests are implemented using the test_driver() function may seem
weird in the first place. The reason is that in a later patch, when we have
configuration-dumping-and-reloading basically working, test_driver() will be
extended to run the same test both with the initial and dumped configuration
and ensure that there are no differences. If I didn't introduce test_driver()
right away, I'd have major, confusing changes in the test code when this
functionality is added.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
tests/Makefile | 8 +-
tests/hwtable.c | 1689 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 1696 insertions(+), 1 deletion(-)
create mode 100644 tests/hwtable.c
diff --git a/tests/Makefile b/tests/Makefile
index 7439c8c3..78755edd 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,7 +3,7 @@ include ../Makefile.inc
CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
-TESTS := uevent parser util dmevents
+TESTS := uevent parser util dmevents hwtable
.SILENT: $(TESTS:%=%.o)
.PRECIOUS: $(TESTS:%=%-test)
@@ -19,11 +19,17 @@ all: $(TESTS:%=%.out)
# XYZ-test_LIBDEPS: Additional libs to link for this test
dmevents-test_LIBDEPS = -lpthread -ldevmapper -lurcu
+hwtable-test_TESTDEPS := test-lib.o
+hwtable-test_OBJDEPS := ../libmultipath/discovery.o ../libmultipath/blacklist.o \
+ ../libmultipath/prio.o ../libmultipath/callout.o ../libmultipath/structs.o
+hwtable-test_LIBDEPS := -ludev -lpthread -ldl
%.out: %-test
@echo == running $< ==
@LD_LIBRARY_PATH=$(multipathdir):$(mpathcmddir) ./$< >$@
+OBJS = $(TESTS:%=%.o) test-lib.o
+
clean: dep_clean
$(RM) $(TESTS:%=%-test) $(TESTS:%=%.out) $(OBJS)
diff --git a/tests/hwtable.c b/tests/hwtable.c
new file mode 100644
index 00000000..b2a0511a
--- /dev/null
+++ b/tests/hwtable.c
@@ -0,0 +1,1689 @@
+/* Set BROKEN to 1 to treat broken behavior as success */
+#define BROKEN 1
+#define VERBOSITY 2
+
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <cmocka.h>
+#include <libudev.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <errno.h>
+#include <limits.h>
+#include <sys/sysmacros.h>
+#include "structs.h"
+#include "structs_vec.h"
+#include "config.h"
+#include "debug.h"
+#include "defaults.h"
+#include "pgpolicies.h"
+#include "test-lib.h"
+
+#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
+#define N_CONF_FILES 2
+
+static const char tmplate[] = "/tmp/hwtable-XXXXXX";
+/* pretend new dm, use minio_rq */
+static const unsigned int dm_tgt_version[3] = { 1, 1, 1 };
+
+struct key_value {
+ const char *key;
+ const char *value;
+};
+
+struct hwt_state {
+ char *tmpname;
+ char *dirname;
+ FILE *config_file;
+ FILE *conf_dir_file[N_CONF_FILES];
+ struct vectors *vecs;
+ void (*test)(const struct hwt_state *);
+ const char *test_name;
+};
+
+#define SET_TEST_FUNC(hwt, func) do { \
+ hwt->test = func; \
+ hwt->test_name = #func; \
+ } while (0)
+
+static struct config *_conf;
+struct udev *udev;
+int logsink;
+
+struct config *get_multipath_config(void)
+{
+ return _conf;
+}
+
+void put_multipath_config(void *arg)
+{}
+
+void make_config_file_path(char *buf, int buflen,
+ const struct hwt_state *hwt, int i)
+{
+ static const char fn_template[] = "%s/test-%02d.conf";
+
+ if (i == -1)
+ /* main config file */
+ snprintf(buf, buflen, fn_template, hwt->tmpname, 0);
+ else
+ snprintf(buf, buflen, fn_template, hwt->dirname, i);
+}
+
+static void reset_vecs(struct vectors *vecs)
+{
+ remove_maps(vecs);
+ free_pathvec(vecs->pathvec, FREE_PATHS);
+
+ vecs->pathvec = vector_alloc();
+ assert_ptr_not_equal(vecs->pathvec, NULL);
+ vecs->mpvec = vector_alloc();
+ assert_ptr_not_equal(vecs->mpvec, NULL);
+}
+
+static void free_hwt(struct hwt_state *hwt)
+{
+ char buf[PATH_MAX];
+ int i;
+
+ if (hwt->config_file != NULL)
+ fclose(hwt->config_file);
+ for (i = 0; i < N_CONF_FILES; i++) {
+ if (hwt->conf_dir_file[i] != NULL)
+ fclose(hwt->conf_dir_file[i]);
+ }
+
+ if (hwt->tmpname != NULL) {
+ make_config_file_path(buf, sizeof(buf), hwt, -1);
+ unlink(buf);
+ rmdir(hwt->tmpname);
+ free(hwt->tmpname);
+ }
+
+ if (hwt->dirname != NULL) {
+ for (i = 0; i < N_CONF_FILES; i++) {
+ make_config_file_path(buf, sizeof(buf), hwt, i);
+ unlink(buf);
+ }
+ rmdir(hwt->dirname);
+ free(hwt->dirname);
+ }
+
+ if (hwt->vecs != NULL) {
+ if (hwt->vecs->mpvec != NULL)
+ remove_maps(hwt->vecs);
+ if (hwt->vecs->pathvec != NULL)
+ free_pathvec(hwt->vecs->pathvec, FREE_PATHS);
+ pthread_mutex_destroy(&hwt->vecs->lock.mutex);
+ free(hwt->vecs);
+ }
+ free(hwt);
+}
+
+static int setup(void **state)
+{
+ struct hwt_state *hwt;
+ char buf[PATH_MAX];
+ int i;
+
+ *state = NULL;
+ hwt = calloc(1, sizeof(*hwt));
+ if (hwt == NULL)
+ return -1;
+
+ snprintf(buf, sizeof(buf), "%s", tmplate);
+ if (mkdtemp(buf) == NULL) {
+ condlog(0, "mkdtemp: %s", strerror(errno));
+ goto err;
+ }
+ hwt->tmpname = strdup(buf);
+
+ snprintf(buf, sizeof(buf), "%s", tmplate);
+ if (mkdtemp(buf) == NULL) {
+ condlog(0, "mkdtemp (2): %s", strerror(errno));
+ goto err;
+ }
+ hwt->dirname = strdup(buf);
+
+ make_config_file_path(buf, sizeof(buf), hwt, -1);
+ hwt->config_file = fopen(buf, "w+");
+ if (hwt->config_file == NULL)
+ goto err;
+
+ for (i = 0; i < N_CONF_FILES; i++) {
+ make_config_file_path(buf, sizeof(buf), hwt, i);
+ hwt->conf_dir_file[i] = fopen(buf, "w+");
+ if (hwt->conf_dir_file[i] == NULL)
+ goto err;
+ }
+
+ hwt->vecs = calloc(1, sizeof(*hwt->vecs));
+ if (hwt->vecs == NULL)
+ goto err;
+ pthread_mutex_init(&hwt->vecs->lock.mutex, NULL);
+ hwt->vecs->pathvec = vector_alloc();
+ hwt->vecs->mpvec = vector_alloc();
+ if (hwt->vecs->pathvec == NULL || hwt->vecs->mpvec == NULL)
+ goto err;
+
+ *state = hwt;
+ return 0;
+
+err:
+ free_hwt(hwt);
+ return -1;
+}
+
+static int teardown(void **state)
+{
+ if (state == NULL || *state == NULL)
+ return -1;
+
+ free_hwt(*state);
+ *state = NULL;
+
+ return 0;
+}
+
+/*
+ * Helpers for creating the config file(s)
+ */
+
+static void reset_config(FILE *ff)
+{
+ if (ff == NULL)
+ return;
+ rewind(ff);
+ if (ftruncate(fileno(ff), 0) == -1)
+ condlog(1, "ftruncate: %s", strerror(errno));
+}
+
+static void reset_configs(const struct hwt_state *hwt)
+{
+ int i;
+
+ reset_config(hwt->config_file);
+ for (i = 0; i < N_CONF_FILES; i++)
+ reset_config(hwt->conf_dir_file[i]);
+}
+
+static void write_key_values(FILE *ff, int nkv, const struct key_value *kv)
+{
+ int i;
+
+ for (i = 0; i < nkv; i++) {
+ if (strchr(kv[i].value, ' ') == NULL &&
+ strchr(kv[i].value, '\"') == NULL)
+ fprintf(ff, "\t%s %s\n", kv[i].key, kv[i].value);
+ else
+ fprintf(ff, "\t%s \"%s\"\n", kv[i].key, kv[i].value);
+ }
+}
+
+static void begin_section(FILE *ff, const char *section)
+{
+ fprintf(ff, "%s {\n", section);
+}
+
+static void end_section(FILE *ff)
+{
+ fprintf(ff, "}\n");
+}
+
+static void write_section(FILE *ff, const char *section,
+ int nkv, const struct key_value *kv)
+{
+ begin_section(ff, section);
+ write_key_values(ff, nkv, kv);
+ end_section(ff);
+}
+
+static void write_defaults(const struct hwt_state *hwt)
+{
+ static const char bindings_name[] = "bindings";
+ static struct key_value defaults[] = {
+ { "config_dir", NULL },
+ { "bindings_file", NULL },
+ { "detect_prio", "no" },
+ { "detect_checker", "no" },
+ };
+ char buf[sizeof(tmplate) + sizeof(bindings_name)];
+
+ snprintf(buf, sizeof(buf), "%s/%s", hwt->tmpname, bindings_name);
+ defaults[0].value = hwt->dirname;
+ defaults[1].value = buf;
+ write_section(hwt->config_file, "defaults",
+ ARRAY_SIZE(defaults), defaults);
+}
+
+static void begin_config(const struct hwt_state *hwt)
+{
+ reset_configs(hwt);
+ write_defaults(hwt);
+}
+
+static void begin_section_all(const struct hwt_state *hwt, const char *section)
+{
+ int i;
+
+ begin_section(hwt->config_file, section);
+ for (i = 0; i < N_CONF_FILES; i++)
+ begin_section(hwt->conf_dir_file[i], section);
+}
+
+static void end_section_all(const struct hwt_state *hwt)
+{
+ int i;
+
+ end_section(hwt->config_file);
+ for (i = 0; i < N_CONF_FILES; i++)
+ end_section(hwt->conf_dir_file[i]);
+}
+
+static void finish_config(const struct hwt_state *hwt)
+{
+ int i;
+
+ fflush(hwt->config_file);
+ for (i = 0; i < N_CONF_FILES; i++) {
+ fflush(hwt->conf_dir_file[i]);
+ }
+}
+
+static void write_device(FILE *ff, int nkv, const struct key_value *kv)
+{
+ write_section(ff, "device", nkv, kv);
+}
+
+/*
+ * Some macros to avoid boilerplace code
+ */
+
+#define CHECK_STATE(state) ({ \
+ assert_ptr_not_equal(state, NULL); \
+ assert_ptr_not_equal(*(state), NULL); \
+ *state; })
+
+#define WRITE_EMPTY_CONF(hwt) do { \
+ begin_config(hwt); \
+ finish_config(hwt); \
+ } while (0)
+
+#define WRITE_ONE_DEVICE(hwt, kv) do { \
+ begin_config(hwt); \
+ begin_section_all(hwt, "devices"); \
+ write_device(hwt->config_file, ARRAY_SIZE(kv), kv); \
+ end_section_all(hwt); \
+ finish_config(hwt); \
+ } while (0)
+
+#define WRITE_TWO_DEVICES(hwt, kv1, kv2) do { \
+ begin_config(hwt); \
+ begin_section_all(hwt, "devices"); \
+ write_device(hwt->config_file, ARRAY_SIZE(kv1), kv1); \
+ write_device(hwt->config_file, ARRAY_SIZE(kv2), kv2); \
+ end_section_all(hwt); \
+ finish_config(hwt); \
+ } while (0)
+
+#define WRITE_TWO_DEVICES_W_DIR(hwt, kv1, kv2) do { \
+ begin_config(hwt); \
+ begin_section_all(hwt, "devices"); \
+ write_device(hwt->config_file, ARRAY_SIZE(kv1), kv1); \
+ write_device(hwt->conf_dir_file[0], \
+ ARRAY_SIZE(kv2), kv2); \
+ end_section_all(hwt); \
+ finish_config(hwt); \
+ } while (0)
+
+#define LOAD_CONFIG(hwt) ({ \
+ char buf[PATH_MAX]; \
+ struct config *__cf; \
+ \
+ make_config_file_path(buf, sizeof(buf), hwt, -1); \
+ __cf = load_config(buf); \
+ assert_ptr_not_equal(__cf, NULL); \
+ assert_ptr_not_equal(__cf->hwtable, NULL); \
+ __cf->verbosity = VERBOSITY; \
+ memcpy(&__cf->version, dm_tgt_version, sizeof(__cf->version)); \
+ __cf; })
+
+#define FREE_CONFIG(conf) do { \
+ free_config(conf); \
+ conf = NULL; \
+ } while (0)
+
+#define TEST_PROP(prop, val) do { \
+ if (val == NULL) \
+ assert_ptr_equal(prop, NULL); \
+ else { \
+ assert_ptr_not_equal(prop, NULL); \
+ assert_string_equal(prop, val); \
+ } \
+ } while (0)
+
+#if BROKEN
+#define TEST_PROP_BROKEN(name, prop, bad, good) do { \
+ condlog(1, "%s: WARNING: Broken test for %s == \"%s\" on line %d, should be \"%s\"", \
+ __func__, name, bad ? bad : "NULL", \
+ __LINE__, good ? good : "NULL"); \
+ TEST_PROP(prop, bad); \
+ } while (0)
+#else
+#define TEST_PROP_BROKEN(name, prop, bad, good) TEST_PROP(prop, good)
+#endif
+
+/*
+ * Some predefined key/value pairs
+ */
+
+static const char _wwid[] = "wwid";
+static const char _vendor[] = "vendor";
+static const char _product[] = "product";
+static const char _prio[] = "prio";
+static const char _checker[] = "path_checker";
+static const char _getuid[] = "getuid_callout";
+static const char _uid_attr[] = "uid_attribute";
+static const char _bl_product[] = "product_blacklist";
+static const char _minio[] = "rr_min_io_rq";
+static const char _no_path_retry[] = "no_path_retry";
+
+/* Device identifiers */
+static const struct key_value vnd_foo = { _vendor, "foo" };
+static const struct key_value prd_bar = { _product, "bar" };
+static const struct key_value prd_bam = { _product, "bam" };
+static const struct key_value prd_baq = { _product, "\"bar\"" };
+static const struct key_value prd_baqq = { _product, "\"\"bar\"\"" };
+static const struct key_value prd_barz = { _product, "barz" };
+static const struct key_value vnd_boo = { _vendor, "boo" };
+static const struct key_value prd_baz = { _product, "baz" };
+static const struct key_value wwid_test = { _wwid, default_wwid };
+
+/* Regular expresssions */
+static const struct key_value vnd__oo = { _vendor, ".oo" };
+static const struct key_value vnd_t_oo = { _vendor, "^.oo" };
+static const struct key_value prd_ba_ = { _product, "ba." };
+static const struct key_value prd_ba_s = { _product, "(bar|baz|ba\\.)$" };
+/* Pathological cases, see below */
+static const struct key_value prd_barx = { _product, "ba[[rxy]" };
+static const struct key_value prd_bazy = { _product, "ba[zy]" };
+static const struct key_value prd_bazy1 = { _product, "ba(z|y)" };
+
+/* Properties */
+static const struct key_value prio_emc = { _prio, "emc" };
+static const struct key_value prio_hds = { _prio, "hds" };
+static const struct key_value prio_rdac = { _prio, "rdac" };
+static const struct key_value chk_hp = { _checker, "hp_sw" };
+static const struct key_value gui_foo = { _getuid, "/tmp/foo" };
+static const struct key_value uid_baz = { _uid_attr, "BAZ_ATTR" };
+static const struct key_value bl_bar = { _bl_product, "bar" };
+static const struct key_value bl_baz = { _bl_product, "baz" };
+static const struct key_value bl_barx = { _bl_product, "ba[[rxy]" };
+static const struct key_value bl_bazy = { _bl_product, "ba[zy]" };
+static const struct key_value minio_99 = { _minio, "99" };
+static const struct key_value npr_37 = { _no_path_retry, "37" };
+static const struct key_value npr_queue = { _no_path_retry, "queue" };
+
+/***** BEGIN TESTS SECTION *****/
+
+/*
+ * Run the given test case. This will later be extended
+ * to run the test several times in a row.
+ */
+static void test_driver(void **state)
+{
+ const struct hwt_state *hwt;
+
+ hwt = CHECK_STATE(state);
+ _conf = LOAD_CONFIG(hwt);
+ hwt->test(hwt);
+
+ reset_vecs(hwt->vecs);
+ FREE_CONFIG(_conf);
+}
+
+/*
+ * Sanity check for the test itself, because defaults may be changed
+ * in libmultipath.
+ *
+ * Our checking for match or non-match relies on the defaults being
+ * different from what our device sections contain.
+ */
+static void test_sanity_globals(void **state)
+{
+ assert_string_not_equal(prio_emc.value, DEFAULT_PRIO);
+ assert_string_not_equal(prio_hds.value, DEFAULT_PRIO);
+ assert_string_not_equal(chk_hp.value, DEFAULT_CHECKER);
+ assert_int_not_equal(MULTIBUS, DEFAULT_PGPOLICY);
+ assert_int_not_equal(NO_PATH_RETRY_QUEUE, DEFAULT_NO_PATH_RETRY);
+ assert_int_not_equal(atoi(minio_99.value), DEFAULT_MINIO_RQ);
+ assert_int_not_equal(atoi(npr_37.value), DEFAULT_NO_PATH_RETRY);
+}
+
+/*
+ * Regression test for internal hwtable. NVME is an example of two entries
+ * in the built-in hwtable, one if which matches a subset of the other.
+ */
+static void test_internal_nvme(const struct hwt_state *hwt)
+{
+ struct path *pp;
+ struct multipath *mp;
+
+ /*
+ * Generic NVMe: expect defaults for pgpolicy and no_path_retry
+ */
+ pp = mock_path("NVME", "NoName");
+ mp = mock_multipath(pp);
+ assert_ptr_not_equal(mp, NULL);
+ TEST_PROP(pp->checker.name, NONE);
+ TEST_PROP(pp->uid_attribute, "ID_WWN");
+ assert_int_equal(mp->pgpolicy, DEFAULT_PGPOLICY);
+ assert_int_equal(mp->no_path_retry, DEFAULT_NO_PATH_RETRY);
+ assert_int_equal(mp->retain_hwhandler, RETAIN_HWHANDLER_OFF);
+
+ /*
+ * NetApp NVMe: expect special values for pgpolicy and no_path_retry
+ */
+ pp = mock_path_wwid("NVME", "NetApp ONTAP Controller",
+ default_wwid_1);
+ mp = mock_multipath(pp);
+ assert_ptr_not_equal(mp, NULL);
+ TEST_PROP(pp->checker.name, NONE);
+ TEST_PROP(pp->uid_attribute, "ID_WWN");
+ assert_int_equal(mp->pgpolicy, MULTIBUS);
+ assert_int_equal(mp->no_path_retry, NO_PATH_RETRY_QUEUE);
+ assert_int_equal(mp->retain_hwhandler, RETAIN_HWHANDLER_OFF);
+}
+
+static int setup_internal_nvme(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_EMPTY_CONF(hwt);
+ SET_TEST_FUNC(hwt, test_internal_nvme);
+
+ return 0;
+}
+
+/*
+ * Device section with a simple entry qith double quotes ('foo:"bar"')
+ */
+static void test_quoted_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+ /* foo:"bar" matches */
+ pp = mock_path(vnd_foo.value, prd_baq.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+
+ /* foo:bar doesn't match */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+}
+
+static int setup_quoted_hwe(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv[] = { vnd_foo, prd_baqq, prio_emc };
+
+ WRITE_ONE_DEVICE(hwt, kv);
+ SET_TEST_FUNC(hwt, test_quoted_hwe);
+ return 0;
+}
+
+/*
+ * Device section with a single simple entry ("foo:bar")
+ */
+static void test_string_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:bar matches */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+
+ /* boo:bar doesn't match */
+ pp = mock_path(vnd_boo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+}
+
+static int setup_string_hwe(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv[] = { vnd_foo, prd_bar, prio_emc };
+
+ WRITE_ONE_DEVICE(hwt, kv);
+ SET_TEST_FUNC(hwt, test_string_hwe);
+ return 0;
+}
+
+/*
+ * Device section with a single regex entry ("^.foo:(bar|baz|ba\.)$")
+ */
+static void test_regex_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:bar matches */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+
+ /* foo:baz matches */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+
+ /* boo:baz matches */
+ pp = mock_path(vnd_boo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+
+ /* foo:BAR doesn't match */
+ pp = mock_path(vnd_foo.value, "BAR");
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+
+ /* bboo:bar doesn't match */
+ pp = mock_path("bboo", prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+}
+
+static int setup_regex_hwe(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv[] = { vnd_t_oo, prd_ba_s, prio_emc };
+
+ WRITE_ONE_DEVICE(hwt, kv);
+ SET_TEST_FUNC(hwt, test_regex_hwe);
+ return 0;
+}
+
+/*
+ * Two device entries, kv1 is a regex match ("^.foo:(bar|baz|ba\.)$"),
+ * kv2 a string match (foo:bar) which matches a subset of the regex.
+ * Both are added to the main config file.
+ *
+ * Expected: Devices matching both get properties from both, kv2 taking
+ * precedence. Devices matching kv1 only just get props from kv1.
+ *
+ * Current: These entries are currently _NOT_ merged, therefore getuid is
+ * default for kv1 matches, and checker is default on kv2 matches.
+ */
+static void test_regex_string_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz matches kv1 */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* boo:baz matches kv1 */
+ pp = mock_path(vnd_boo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* .oo:ba. matches kv1 */
+ pp = mock_path(vnd__oo.value, prd_ba_.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* .foo:(bar|baz|ba\.) doesn't match */
+ pp = mock_path(vnd__oo.value, prd_ba_s.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches kv2 and kv1 */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ /*
+ * You'd expect that the two entries above be merged,
+ * but that isn't the case if they're in the same input file.
+ */
+ TEST_PROP_BROKEN(_checker, pp->checker.name,
+ DEFAULT_CHECKER, chk_hp.value);
+}
+
+static int setup_regex_string_hwe(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv1[] = { vnd_t_oo, prd_ba_s, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+
+ WRITE_TWO_DEVICES(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_regex_string_hwe);
+ return 0;
+}
+
+/*
+ * Two device entries, kv1 is a regex match ("^.foo:(bar|baz|ba\.)$"),
+ * kv2 a string match (foo:bar) which matches a subset of the regex.
+ * kv1 is added to the main config file, kv2 to a config_dir file.
+ * This case is more important as you may think, because it's equivalent
+ * to kv1 being in the built-in hwtable and kv2 in multipath.conf.
+ *
+ * Expected: Devices matching kv2 (and thus, both) get properties
+ * from both, kv2 taking precedence.
+ * Devices matching kv1 only just get props from kv1.
+ *
+ * Current: behaves as expected.
+ */
+static void test_regex_string_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz matches kv1 */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* boo:baz matches kv1 */
+ pp = mock_path(vnd_boo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* .oo:ba. matches kv1 */
+ pp = mock_path(vnd__oo.value, prd_ba_.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* .oo:(bar|baz|ba\.)$ doesn't match */
+ pp = mock_path(vnd__oo.value, prd_ba_s.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches kv2 */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ /* Later match takes prio */
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ /* This time it's merged */
+ TEST_PROP(pp->checker.name, chk_hp.value);
+}
+
+static int setup_regex_string_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_t_oo, prd_ba_s, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES_W_DIR(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_regex_string_hwe_dir);
+ return 0;
+}
+
+/*
+ * Three device entries, kv1 is a regex match and kv2 and kv3 string
+ * matches, where kv3 is a substring of kv2. All in different config
+ * files.
+ *
+ * Expected: Devices matching kv3 get props from all, devices matching
+ * kv2 from kv2 and kv1, and devices matching kv1 only just from kv1.
+ */
+static void test_regex_2_strings_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz matches kv1 */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* boo:baz doesn't match */
+ pp = mock_path(vnd_boo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches kv2 and kv1 */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->uid_attribute, uid_baz.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* foo:barz matches kv3 and kv2 and kv1 */
+ pp = mock_path_flags(vnd_foo.value, prd_barz.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_rdac.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP(pp->uid_attribute, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+}
+
+static int setup_regex_2_strings_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_ba_, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, uid_baz };
+ const struct key_value kv3[] = { vnd_foo, prd_barz,
+ prio_rdac, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "devices");
+ write_device(hwt->config_file, ARRAY_SIZE(kv1), kv1);
+ write_device(hwt->conf_dir_file[0], ARRAY_SIZE(kv2), kv2);
+ write_device(hwt->conf_dir_file[1], ARRAY_SIZE(kv3), kv3);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_regex_2_strings_hwe_dir);
+ return 0;
+}
+
+/*
+ * Like test_regex_string_hwe_dir, but the order of kv1 and kv2 is exchanged.
+ *
+ * Expected: Devices matching kv1 (and thus, both) get properties
+ * from both, kv1 taking precedence.
+ * Devices matching kv1 only just get props from kv1.
+ *
+ * Current: kv2 never matches, because kv1 is more generic and encountered
+ * first; thus properties from kv2 aren't used.
+ */
+static void test_string_regex_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:bar matches kv2 and kv1 */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value,
+ BROKEN == 1 ? 0 : USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP_BROKEN(_getuid, pp->getuid, (char *)NULL, gui_foo.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* foo:baz matches kv1 */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* boo:baz matches kv1 */
+ pp = mock_path(vnd_boo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* .oo:ba. matches kv1 */
+ pp = mock_path(vnd__oo.value, prd_ba_.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* .oo:(bar|baz|ba\.)$ doesn't match */
+ pp = mock_path(vnd__oo.value, prd_ba_s.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+}
+
+static int setup_string_regex_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_t_oo, prd_ba_s, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES_W_DIR(hwt, kv2, kv1);
+ SET_TEST_FUNC(hwt, test_string_regex_hwe_dir);
+ return 0;
+}
+
+/*
+ * Two identical device entries kv1 and kv2, trival regex ("string").
+ * Both are added to the main config file.
+ * These entries are NOT merged.
+ * This could happen in a large multipath.conf file.
+ *
+ * Expected: matching devices get props from both, kv2 taking precedence.
+ *
+ * Current: devices get props from kv2 only.
+ */
+static void test_2_ident_strings_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches both, but only kv2 is seen */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
+ chk_hp.value);
+}
+
+static int setup_2_ident_strings_hwe(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_2_ident_strings_hwe);
+ return 0;
+}
+
+/*
+ * Two identical device entries kv1 and kv2, trival regex ("string").
+ * Both are added to an extra config file.
+ * This could happen in a large multipath.conf file.
+ *
+ * Expected: matching devices get props from both, kv2 taking precedence.
+ */
+static void test_2_ident_strings_both_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches both */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
+ chk_hp.value);
+}
+
+static int setup_2_ident_strings_both_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "devices");
+ write_device(hwt->conf_dir_file[1], ARRAY_SIZE(kv1), kv1);
+ write_device(hwt->conf_dir_file[1], ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_2_ident_strings_both_dir);
+ return 0;
+}
+
+/*
+ * Two identical device entries kv1 and kv2, trival regex ("string").
+ * Both are added to an extra config file.
+ * An empty entry kv0 with the same string exists in the main config file.
+ *
+ * Expected: matching devices get props from both, kv2 taking precedence.
+ */
+static void test_2_ident_strings_both_dir_w_prev(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches both */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
+ chk_hp.value);
+}
+
+static int setup_2_ident_strings_both_dir_w_prev(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ const struct key_value kv0[] = { vnd_foo, prd_bar };
+ const struct key_value kv1[] = { vnd_foo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+
+ begin_config(hwt);
+ begin_section_all(hwt, "devices");
+ write_device(hwt->config_file, ARRAY_SIZE(kv0), kv0);
+ write_device(hwt->conf_dir_file[1], ARRAY_SIZE(kv1), kv1);
+ write_device(hwt->conf_dir_file[1], ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_2_ident_strings_both_dir_w_prev);
+ return 0;
+}
+
+/*
+ * Two identical device entries kv1 and kv2, trival regex ("string").
+ * kv1 is added to the main config file, kv2 to a config_dir file.
+ * These entries are merged.
+ * This case is more important as you may think, because it's equivalent
+ * to kv1 being in the built-in hwtable and kv2 in multipath.conf.
+ *
+ * Expected: matching devices get props from both, kv2 taking precedence.
+ *
+ * Current: behaves as expected.
+ */
+static void test_2_ident_strings_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches both */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+}
+
+static int setup_2_ident_strings_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES_W_DIR(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_2_ident_strings_hwe_dir);
+ return 0;
+}
+
+/*
+ * Like test_2_ident_strings_hwe_dir, but this time the config_dir file
+ * contains an additional, empty entry (kv0).
+ *
+ * Expected: matching devices get props from kv1 and kv2, kv2 taking precedence.
+ *
+ * Current: kv0 and kv1 are merged into kv0, and then ignored because kv2 takes
+ * precedence. Thus the presence of the empty kv0 changes how kv1 is treated.
+ */
+static void test_3_ident_strings_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches both */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
+ chk_hp.value);
+}
+
+static int setup_3_ident_strings_hwe_dir(void **state)
+{
+ const struct key_value kv0[] = { vnd_foo, prd_bar };
+ const struct key_value kv1[] = { vnd_foo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "devices");
+ write_device(hwt->config_file, ARRAY_SIZE(kv1), kv1);
+ write_device(hwt->conf_dir_file[1], ARRAY_SIZE(kv0), kv0);
+ write_device(hwt->conf_dir_file[1], ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_3_ident_strings_hwe_dir);
+ return 0;
+}
+
+/*
+ * Two identical device entries kv1 and kv2, non-trival regex that matches
+ * itself (string ".oo" matches regex ".oo").
+ * kv1 is added to the main config file, kv2 to a config_dir file.
+ * This case is more important as you may think, because it's equivalent
+ * to kv1 being in the built-in hwtable and kv2 in multipath.conf.
+ *
+ * Expected: matching devices get props from both, kv2 taking precedence.
+ *
+ * Current: behaves as expected.
+ */
+static void test_2_ident_self_matching_re_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches both */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+}
+
+static int setup_2_ident_self_matching_re_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd__oo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd__oo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES_W_DIR(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_2_ident_self_matching_re_hwe_dir);
+ return 0;
+}
+
+/*
+ * Two identical device entries kv1 and kv2, non-trival regex that matches
+ * itself (string ".oo" matches regex ".oo").
+ * kv1 and kv2 are added to the main config file.
+ *
+ * Expected: matching devices get props from both, kv2 taking precedence.
+ *
+ * Current: Devices get properties from kv2 only (kv1 and kv2 are not merged).
+ */
+static void test_2_ident_self_matching_re_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name,
+ DEFAULT_CHECKER, chk_hp.value);
+}
+
+static int setup_2_ident_self_matching_re_hwe(void **state)
+{
+ const struct key_value kv1[] = { vnd__oo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd__oo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_2_ident_self_matching_re_hwe);
+ return 0;
+}
+
+/*
+ * Two identical device entries kv1 and kv2, non-trival regex that doesn't
+ * match itself (string "^.oo" doesn't match regex "^.oo").
+ * kv1 is added to the main config file, kv2 to a config_dir file.
+ * This case is more important as you may think, see above.
+ *
+ * Expected: matching devices get props from both, kv2 taking precedence.
+ *
+ * Current: devices get props from kv2 only.
+ */
+static void
+test_2_ident_not_self_matching_re_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:baz doesn't match */
+ pp = mock_path(vnd_foo.value, prd_baz.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /* foo:bar matches both, but only kv2 is seen */
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name,
+ DEFAULT_CHECKER, chk_hp.value);
+}
+
+static int setup_2_ident_not_self_matching_re_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_t_oo, prd_bar, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_t_oo, prd_bar, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES_W_DIR(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_2_ident_not_self_matching_re_hwe_dir);
+ return 0;
+}
+
+/*
+ * Two different non-trivial regexes kv1, kv2. The 1st one matches the 2nd, but
+ * it doesn't match all possible strings matching the second.
+ * ("ba[zy]" matches regex "ba[[rxy]", but "baz" does not).
+ * This causes the first entry to be merged into the second, but both entries
+ * to be kept.
+ *
+ * Expected: Devices matching both regexes get properties from both, kv2
+ * taking precedence. Devices matching just one regex get properties from
+ * that one regex only.
+ *
+ * Current: behaves as expected, except for devices that match only kv2.
+ * Those get properties from kv1, too.
+ */
+static void test_2_matching_res_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:bar matches k1 only */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* foo:bay matches k1 and k2 */
+ pp = mock_path_flags(vnd_foo.value, "bay", USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
+
+ /* foo:baz matches k2 only. */
+ pp = mock_path_flags(vnd_foo.value, prd_baz.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name,
+ chk_hp.value, DEFAULT_CHECKER);
+}
+
+static int setup_2_matching_res_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_barx, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bazy, prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES_W_DIR(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_2_matching_res_hwe_dir);
+ return 0;
+}
+
+/*
+ * Two different non-trivial regexes which match the same set of strings.
+ * But they don't match each other.
+ * "baz" matches both regex "ba[zy]" and "ba(z|y)"
+ *
+ * Expected: matching devices get properties from both, kv2 taking precedence.
+ *
+ * Current: matching devices get properties from kv2 only.
+ */
+static void test_2_nonmatching_res_hwe_dir(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:bar doesn't match */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+ TEST_PROP(pp->getuid, NULL);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+
+ /*
+ * foo:baz matches k2 and k1. Yet it sees the value from k2 only.
+ */
+ pp = mock_path_flags(vnd_foo.value, prd_baz.value, USE_GETUID);
+ TEST_PROP(prio_name(&pp->prio), prio_hds.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
+ TEST_PROP_BROKEN(_checker, pp->checker.name,
+ DEFAULT_CHECKER, chk_hp.value);
+}
+
+static int setup_2_nonmatching_res_hwe_dir(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_bazy, prio_emc, chk_hp };
+ const struct key_value kv2[] = { vnd_foo, prd_bazy1,
+ prio_hds, gui_foo };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES_W_DIR(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_2_nonmatching_res_hwe_dir);
+ return 0;
+}
+
+/*
+ * Simple blacklist test.
+ *
+ * NOTE: test failures in blacklisting tests will manifest as cmocka errors
+ * "Could not get value to mock function XYZ", because pathinfo() takes
+ * different code paths for blacklisted devices.
+ */
+static void test_blacklist(const struct hwt_state *hwt)
+{
+ mock_path_flags(vnd_foo.value, prd_bar.value, BL_BY_DEVICE);
+ mock_path(vnd_foo.value, prd_baz.value);
+}
+
+static int setup_blacklist(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_bar };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "blacklist");
+ write_device(hwt->config_file, ARRAY_SIZE(kv1), kv1);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_blacklist);
+ return 0;
+}
+
+/*
+ * Simple blacklist test with regex and exception
+ */
+static void test_blacklist_regex(const struct hwt_state *hwt)
+{
+ mock_path(vnd_foo.value, prd_bar.value);
+ mock_path_flags(vnd_foo.value, prd_baz.value, BL_BY_DEVICE);
+ mock_path(vnd_foo.value, prd_bam.value);
+}
+
+static int setup_blacklist_regex(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_ba_s };
+ const struct key_value kv2[] = { vnd_foo, prd_bar };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ hwt = CHECK_STATE(state);
+ begin_config(hwt);
+ begin_section_all(hwt, "blacklist");
+ write_device(hwt->config_file, ARRAY_SIZE(kv1), kv1);
+ end_section_all(hwt);
+ begin_section_all(hwt, "blacklist_exceptions");
+ write_device(hwt->conf_dir_file[0], ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_blacklist_regex);
+ return 0;
+}
+
+/*
+ * Simple blacklist test with regex and exception
+ * config file order inverted wrt test_blacklist_regex
+ */
+static int setup_blacklist_regex_inv(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_ba_s };
+ const struct key_value kv2[] = { vnd_foo, prd_bar };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "blacklist");
+ write_device(hwt->conf_dir_file[0], ARRAY_SIZE(kv1), kv1);
+ end_section_all(hwt);
+ begin_section_all(hwt, "blacklist_exceptions");
+ write_device(hwt->config_file, ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_blacklist_regex);
+ return 0;
+}
+
+/*
+ * Simple blacklist test with regex and exception
+ * config file order inverted wrt test_blacklist_regex
+ */
+static void test_blacklist_regex_matching(const struct hwt_state *hwt)
+{
+ mock_path_flags(vnd_foo.value, prd_bar.value, BL_BY_DEVICE);
+ mock_path_flags(vnd_foo.value, prd_baz.value, BL_BY_DEVICE);
+ mock_path(vnd_foo.value, prd_bam.value);
+}
+
+static int setup_blacklist_regex_matching(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_barx };
+ const struct key_value kv2[] = { vnd_foo, prd_bazy };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "blacklist");
+ write_device(hwt->config_file, ARRAY_SIZE(kv1), kv1);
+ write_device(hwt->conf_dir_file[0], ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_blacklist_regex_matching);
+ return 0;
+}
+
+/*
+ * Test for blacklisting by WWID
+ *
+ * Note that default_wwid is a substring of default_wwid_1. Because
+ * matching is done by regex, both paths are blacklisted.
+ */
+static void test_blacklist_wwid(const struct hwt_state *hwt)
+{
+ mock_path_flags(vnd_foo.value, prd_bar.value, BL_BY_WWID);
+ mock_path_wwid_flags(vnd_foo.value, prd_baz.value, default_wwid_1,
+ BL_BY_WWID);
+}
+
+static int setup_blacklist_wwid(void **state)
+{
+ const struct key_value kv[] = { wwid_test };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ write_section(hwt->config_file, "blacklist", ARRAY_SIZE(kv), kv);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_blacklist_wwid);
+ return 0;
+}
+
+/*
+ * Test for blacklisting by WWID
+ *
+ * Here the blacklist contains only default_wwid_1. Thus the path
+ * with default_wwid is NOT blacklisted.
+ */
+static void test_blacklist_wwid_1(const struct hwt_state *hwt)
+{
+ mock_path(vnd_foo.value, prd_bar.value);
+ mock_path_wwid_flags(vnd_foo.value, prd_baz.value, default_wwid_1,
+ BL_BY_WWID);
+}
+
+static int setup_blacklist_wwid_1(void **state)
+{
+ const struct key_value kv[] = { { _wwid, default_wwid_1 }, };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ write_section(hwt->config_file, "blacklist", ARRAY_SIZE(kv), kv);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_blacklist_wwid_1);
+ return 0;
+}
+
+/*
+ * Test for product_blacklist. Two entries blacklisting each other.
+ *
+ * Expected: Both are blacklisted.
+ */
+static void test_product_blacklist(const struct hwt_state *hwt)
+{
+ mock_path_flags(vnd_foo.value, prd_baz.value, BL_BY_DEVICE);
+ mock_path_flags(vnd_foo.value, prd_bar.value, BL_BY_DEVICE);
+ mock_path(vnd_foo.value, prd_bam.value);
+}
+
+static int setup_product_blacklist(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_bar, bl_baz };
+ const struct key_value kv2[] = { vnd_foo, prd_baz, bl_bar };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_product_blacklist);
+ return 0;
+}
+
+/*
+ * Test for product_blacklist. The second regex "matches" the first.
+ * This is a pathological example.
+ *
+ * Expected: "foo:bar", "foo:baz" are blacklisted.
+ */
+static void test_product_blacklist_matching(const struct hwt_state *hwt)
+{
+ mock_path_flags(vnd_foo.value, prd_bar.value, BL_BY_DEVICE);
+#if BROKEN == 1
+ condlog(1, "%s: WARNING: broken blacklist test on line %d",
+ __func__, __LINE__ + 1);
+ mock_path(vnd_foo.value, prd_baz.value);
+#else
+ mock_path_blacklisted(vnd_foo.value, prd_baz.value);
+#endif
+ mock_path(vnd_foo.value, prd_bam.value);
+}
+
+static int setup_product_blacklist_matching(void **state)
+{
+ const struct key_value kv1[] = { vnd_foo, prd_bar, bl_barx };
+ const struct key_value kv2[] = { vnd_foo, prd_baz, bl_bazy };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ WRITE_TWO_DEVICES(hwt, kv1, kv2);
+ SET_TEST_FUNC(hwt, test_product_blacklist_matching);
+ return 0;
+}
+
+/*
+ * Basic test for multipath-based configuration.
+ *
+ * Expected: properties, including pp->prio, are taken from multipath
+ * section.
+ */
+static void test_multipath_config(const struct hwt_state *hwt)
+{
+ struct path *pp;
+ struct multipath *mp;
+
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ mp = mock_multipath(pp);
+ assert_ptr_not_equal(mp->mpe, NULL);
+ TEST_PROP(prio_name(&pp->prio), prio_rdac.value);
+ assert_int_equal(mp->minio, atoi(minio_99.value));
+ TEST_PROP(pp->uid_attribute, uid_baz.value);
+
+ /* test different wwid */
+ pp = mock_path_wwid(vnd_foo.value, prd_bar.value, default_wwid_1);
+ mp = mock_multipath(pp);
+ // assert_ptr_equal(mp->mpe, NULL);
+ TEST_PROP(prio_name(&pp->prio), prio_emc.value);
+ assert_int_equal(mp->minio, DEFAULT_MINIO_RQ);
+ TEST_PROP(pp->uid_attribute, uid_baz.value);
+}
+
+static int setup_multipath_config(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kvm[] = { wwid_test, prio_rdac, minio_99 };
+ const struct key_value kvp[] = { vnd_foo, prd_bar, prio_emc, uid_baz };
+
+ begin_config(hwt);
+ begin_section_all(hwt, "devices");
+ write_section(hwt->conf_dir_file[0], "device", ARRAY_SIZE(kvp), kvp);
+ end_section_all(hwt);
+ begin_section_all(hwt, "multipaths");
+ write_section(hwt->config_file, "multipath", ARRAY_SIZE(kvm), kvm);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_multipath_config);
+ return 0;
+}
+
+/*
+ * Basic test for multipath-based configuration. Two sections for the same wwid.
+ *
+ * Expected: properties are taken from both multipath sections, later taking
+ * precedence
+ *
+ * Current: gets properties from first entry only.
+ */
+static void test_multipath_config_2(const struct hwt_state *hwt)
+{
+ struct path *pp;
+ struct multipath *mp;
+
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ mp = mock_multipath(pp);
+ assert_ptr_not_equal(mp, NULL);
+ assert_ptr_not_equal(mp->mpe, NULL);
+ TEST_PROP(prio_name(&pp->prio), prio_rdac.value);
+#if BROKEN
+ condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
+ assert_int_equal(mp->minio, DEFAULT_MINIO_RQ);
+ condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
+ assert_int_equal(mp->no_path_retry, NO_PATH_RETRY_QUEUE);
+#else
+ assert_int_equal(mp->minio, atoi(minio_99.value));
+ assert_int_equal(mp->no_path_retry, atoi(npr_37.value));
+#endif
+}
+
+static int setup_multipath_config_2(void **state)
+{
+ const struct key_value kv1[] = { wwid_test, prio_rdac, npr_queue };
+ const struct key_value kv2[] = { wwid_test, minio_99, npr_37 };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "multipaths");
+ write_section(hwt->config_file, "multipath", ARRAY_SIZE(kv1), kv1);
+ write_section(hwt->conf_dir_file[1], "multipath", ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_multipath_config_2);
+ return 0;
+}
+
+/*
+ * Same as test_multipath_config_2, both entries in the same config file.
+ *
+ * Expected: properties are taken from both multipath sections.
+ */
+static void test_multipath_config_3(const struct hwt_state *hwt)
+{
+ struct path *pp;
+ struct multipath *mp;
+
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ mp = mock_multipath(pp);
+ assert_ptr_not_equal(mp, NULL);
+ assert_ptr_not_equal(mp->mpe, NULL);
+ TEST_PROP(prio_name(&pp->prio), prio_rdac.value);
+#if BROKEN
+ condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
+ assert_int_equal(mp->minio, DEFAULT_MINIO_RQ);
+ condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
+ assert_int_equal(mp->no_path_retry, NO_PATH_RETRY_QUEUE);
+#else
+ assert_int_equal(mp->minio, atoi(minio_99.value));
+ assert_int_equal(mp->no_path_retry, atoi(npr_37.value));
+#endif
+}
+
+static int setup_multipath_config_3(void **state)
+{
+ const struct key_value kv1[] = { wwid_test, prio_rdac, npr_queue };
+ const struct key_value kv2[] = { wwid_test, minio_99, npr_37 };
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ begin_config(hwt);
+ begin_section_all(hwt, "multipaths");
+ write_section(hwt->config_file, "multipath", ARRAY_SIZE(kv1), kv1);
+ write_section(hwt->config_file, "multipath", ARRAY_SIZE(kv2), kv2);
+ end_section_all(hwt);
+ finish_config(hwt);
+ SET_TEST_FUNC(hwt, test_multipath_config_3);
+ return 0;
+}
+
+/*
+ * Create wrapper functions around test_driver() to avoid that cmocka
+ * always uses the same test name. That makes it easier to read test results.
+ */
+
+#define define_test(x) \
+ static void run_##x(void **state) \
+ { \
+ return test_driver(state); \
+ }
+
+define_test(string_hwe)
+define_test(quoted_hwe)
+define_test(internal_nvme)
+define_test(regex_hwe)
+define_test(regex_string_hwe)
+define_test(regex_string_hwe_dir)
+define_test(regex_2_strings_hwe_dir)
+define_test(string_regex_hwe_dir)
+define_test(2_ident_strings_hwe)
+define_test(2_ident_strings_both_dir)
+define_test(2_ident_strings_both_dir_w_prev)
+define_test(2_ident_strings_hwe_dir)
+define_test(3_ident_strings_hwe_dir)
+define_test(2_ident_self_matching_re_hwe_dir)
+define_test(2_ident_self_matching_re_hwe)
+define_test(2_ident_not_self_matching_re_hwe_dir)
+define_test(2_matching_res_hwe_dir)
+define_test(2_nonmatching_res_hwe_dir)
+define_test(blacklist)
+define_test(blacklist_wwid)
+define_test(blacklist_wwid_1)
+define_test(blacklist_regex)
+define_test(blacklist_regex_inv)
+define_test(blacklist_regex_matching)
+define_test(product_blacklist)
+define_test(product_blacklist_matching)
+define_test(multipath_config)
+define_test(multipath_config_2)
+define_test(multipath_config_3)
+
+#define test_entry(x) \
+ cmocka_unit_test_setup(run_##x, setup_##x)
+
+static int test_hwtable(void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_sanity_globals),
+ test_entry(internal_nvme),
+ test_entry(string_hwe),
+ test_entry(quoted_hwe),
+ test_entry(regex_hwe),
+ test_entry(regex_string_hwe),
+ test_entry(regex_string_hwe_dir),
+ test_entry(regex_2_strings_hwe_dir),
+ test_entry(string_regex_hwe_dir),
+ test_entry(2_ident_strings_hwe),
+ test_entry(2_ident_strings_both_dir),
+ test_entry(2_ident_strings_both_dir_w_prev),
+ test_entry(2_ident_strings_hwe_dir),
+ test_entry(3_ident_strings_hwe_dir),
+ test_entry(2_ident_self_matching_re_hwe_dir),
+ test_entry(2_ident_self_matching_re_hwe),
+ test_entry(2_ident_not_self_matching_re_hwe_dir),
+ test_entry(2_matching_res_hwe_dir),
+ test_entry(2_nonmatching_res_hwe_dir),
+ test_entry(blacklist),
+ test_entry(blacklist_wwid),
+ test_entry(blacklist_wwid_1),
+ test_entry(blacklist_regex),
+ test_entry(blacklist_regex_inv),
+ test_entry(blacklist_regex_matching),
+ test_entry(product_blacklist),
+ test_entry(product_blacklist_matching),
+ test_entry(multipath_config),
+ test_entry(multipath_config_2),
+ test_entry(multipath_config_3),
+ };
+
+ return cmocka_run_group_tests(tests, setup, teardown);
+}
+
+int main(void)
+{
+ int ret = 0;
+
+ ret += test_hwtable();
+ return ret;
+}
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 11/28] libmultipath: add debug messages to hwentry lookup/merging code
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (9 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 10/28] tests/hwtable: tests for config file handling and hwentry merging Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 12/28] libmultipath: use vector for for pp->hwe and mp->hwe Martin Wilck
` (17 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index d2812e4a..9e2f166f 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -103,6 +103,16 @@ out:
return retval;
}
+static void _log_match(const char *fn, const struct hwentry *h,
+ const char *vendor, const char *product,
+ const char *revision)
+{
+ condlog(4, "%s: found match /%s:%s:%s/ for '%s:%s:%s'", fn,
+ h->vendor, h->product, h->revision,
+ vendor, product, revision);
+}
+#define log_match(h, v, p, r) _log_match(__func__, (h), (v), (p), (r))
+
struct hwentry *
find_hwe (const struct _vector *hwtable,
const char * vendor, const char * product, const char * revision)
@@ -120,6 +130,7 @@ find_hwe (const struct _vector *hwtable,
if (hwe_regmatch(tmp, vendor, product, revision))
continue;
ret = tmp;
+ log_match(tmp, vendor, product, revision);
break;
}
return ret;
@@ -457,8 +468,13 @@ restart:
hwe2->product, hwe2->revision))
continue;
/* dup */
+ log_match(hwe1, hwe2->vendor,
+ hwe2->product, hwe2->revision);
merge_hwe(hwe2, hwe1);
if (hwe_strmatch(hwe2, hwe1) == 0) {
+ condlog(4, "%s: removing hwentry %s:%s:%s",
+ __func__, hwe1->vendor, hwe1->product,
+ hwe1->revision);
vector_del_slot(hw, i);
free_hwe(hwe1);
n -= 1;
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 12/28] libmultipath: use vector for for pp->hwe and mp->hwe
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (10 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 11/28] libmultipath: add debug messages to hwentry lookup/merging code Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 13/28] libmultipath: allow more than one hwentry Martin Wilck
` (16 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Change the data structure of "struct path" and "struct multipath"
such that the "hwe" entry is a vector of hwentry structures rather
than a single hwentry structure. Add respective code to the
constructors and destructors (note that mp->hwe is never allocated,
it's always a pointer to a path hwe).
Change find_hwe() to fill in the passed vector rather than returning
a hwentry pointer. Change the propsel code to look through vectors
of hwentries to determine a given property.
This patch just creates the new data structure and the functions to
deal with them, it doesn't introduce semantic changes.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 21 ++++++++++------
libmultipath/config.h | 6 ++---
libmultipath/discovery.c | 10 ++++----
libmultipath/propsel.c | 53 ++++++++++++++++++++++++++++++++++------
libmultipath/structs.c | 6 +++++
libmultipath/structs.h | 4 +--
6 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 9e2f166f..95a71447 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -113,27 +113,34 @@ static void _log_match(const char *fn, const struct hwentry *h,
}
#define log_match(h, v, p, r) _log_match(__func__, (h), (v), (p), (r))
-struct hwentry *
+int
find_hwe (const struct _vector *hwtable,
- const char * vendor, const char * product, const char * revision)
+ const char * vendor, const char * product, const char * revision,
+ vector result)
{
- int i;
- struct hwentry *tmp, *ret = NULL;
+ int i, n = 0;
+ struct hwentry *tmp;
/*
- * Search backwards here.
+ * Search backwards here, and add forward.
* User modified entries are attached at the end of
* the list, so we have to check them first before
* continuing to the generic entries
*/
+ vector_reset(result);
vector_foreach_slot_backwards (hwtable, tmp, i) {
if (hwe_regmatch(tmp, vendor, product, revision))
continue;
- ret = tmp;
+ if (vector_alloc_slot(result) != NULL) {
+ vector_set_slot(result, tmp);
+ n++;
+ }
log_match(tmp, vendor, product, revision);
break;
}
- return ret;
+ condlog(n > 1 ? 3 : 4, "%s: found %d hwtable matches for %s:%s:%s",
+ __func__, n, vendor, product, revision);
+ return n;
}
struct mpentry *find_mpe(vector mptable, char *wwid)
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 83eaf62f..e1cbd59b 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -213,9 +213,9 @@ struct config {
extern struct udev * udev;
-struct hwentry * find_hwe (const struct _vector *hwtable,
- const char * vendor, const char * product,
- const char *revision);
+int find_hwe (const struct _vector *hwtable,
+ const char * vendor, const char * product, const char *revision,
+ vector result);
struct mpentry * find_mpe (vector mptable, char * wwid);
char * get_mpe_wwid (vector mptable, char * alias);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 1ef1dfa7..b974d6cd 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1176,7 +1176,7 @@ scsi_sysfs_pathinfo (struct path * pp, vector hwtable)
/*
* set the hwe configlet pointer
*/
- pp->hwe = find_hwe(hwtable, pp->vendor_id, pp->product_id, pp->rev);
+ find_hwe(hwtable, pp->vendor_id, pp->product_id, pp->rev, pp->hwe);
/*
* host / bus / target / lun
@@ -1240,7 +1240,7 @@ nvme_sysfs_pathinfo (struct path * pp, vector hwtable)
condlog(3, "%s: serial = %s", pp->dev, pp->serial);
condlog(3, "%s: rev = %s", pp->dev, pp->rev);
- pp->hwe = find_hwe(hwtable, pp->vendor_id, pp->product_id, NULL);
+ find_hwe(hwtable, pp->vendor_id, pp->product_id, NULL, pp->hwe);
return 0;
}
@@ -1256,7 +1256,7 @@ rbd_sysfs_pathinfo (struct path * pp, vector hwtable)
/*
* set the hwe configlet pointer
*/
- pp->hwe = find_hwe(hwtable, pp->vendor_id, pp->product_id, NULL);
+ find_hwe(hwtable, pp->vendor_id, pp->product_id, NULL, pp->hwe);
return 0;
}
@@ -1297,7 +1297,7 @@ ccw_sysfs_pathinfo (struct path * pp, vector hwtable)
/*
* set the hwe configlet pointer
*/
- pp->hwe = find_hwe(hwtable, pp->vendor_id, pp->product_id, NULL);
+ find_hwe(hwtable, pp->vendor_id, pp->product_id, NULL, pp->hwe);
/*
* host / bus / target / lun
@@ -1360,7 +1360,7 @@ cciss_sysfs_pathinfo (struct path * pp, vector hwtable)
/*
* set the hwe configlet pointer
*/
- pp->hwe = find_hwe(hwtable, pp->vendor_id, pp->product_id, pp->rev);
+ find_hwe(hwtable, pp->vendor_id, pp->product_id, pp->rev, pp->hwe);
/*
* host / bus / target / lun
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 018f4879..d645507e 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -44,6 +44,23 @@ do { \
} \
} while(0)
+#define do_set_from_vec(type, var, src, dest, msg) \
+do { \
+ type *_p; \
+ int i; \
+ \
+ vector_foreach_slot(src, _p, i) { \
+ if (_p->var) { \
+ dest = _p->var; \
+ origin = msg; \
+ goto out; \
+ } \
+ } \
+} while (0)
+
+#define do_set_from_hwe(var, src, dest, msg) \
+ do_set_from_vec(struct hwentry, var, src->hwe, dest, msg)
+
static const char default_origin[] = "(setting: multipath internal)";
static const char hwe_origin[] =
"(setting: storage device configuration)";
@@ -67,7 +84,7 @@ do { \
#define mp_set_mpe(var) \
do_set(var, mp->mpe, mp->var, multipaths_origin)
#define mp_set_hwe(var) \
-do_set(var, mp->hwe, mp->var, hwe_origin)
+do_set_from_hwe(var, mp, mp->var, hwe_origin)
#define mp_set_ovr(var) \
do_set(var, conf->overrides, mp->var, overrides_origin)
#define mp_set_conf(var) \
@@ -78,7 +95,7 @@ do_default(mp->var, value)
#define pp_set_mpe(var) \
do_set(var, mpe, pp->var, multipaths_origin)
#define pp_set_hwe(var) \
-do_set(var, pp->hwe, pp->var, hwe_origin)
+do_set_from_hwe(var, pp, pp->var, hwe_origin)
#define pp_set_conf(var) \
do_set(var, conf, pp->var, conf_origin)
#define pp_set_ovr(var) \
@@ -250,8 +267,8 @@ want_user_friendly_names(struct config *conf, struct multipath * mp)
multipaths_origin);
do_set(user_friendly_names, conf->overrides, user_friendly_names,
overrides_origin);
- do_set(user_friendly_names, mp->hwe, user_friendly_names,
- hwe_origin);
+ do_set_from_hwe(user_friendly_names, mp, user_friendly_names,
+ hwe_origin);
do_set(user_friendly_names, conf, user_friendly_names,
conf_origin);
do_default(user_friendly_names, DEFAULT_USER_FRIENDLY_NAMES);
@@ -471,9 +488,9 @@ int select_checker(struct config *conf, struct path *pp)
checker_name = TUR;
goto out;
}
- }
+ }
do_set(checker_name, conf->overrides, checker_name, overrides_origin);
- do_set(checker_name, pp->hwe, checker_name, hwe_origin);
+ do_set_from_hwe(checker_name, pp, checker_name, hwe_origin);
do_set(checker_name, conf, checker_name, conf_origin);
do_default(checker_name, DEFAULT_CHECKER);
out:
@@ -547,6 +564,25 @@ do { \
} \
} while(0)
+#define set_prio_from_vec(type, dir, src, msg, p) \
+do { \
+ type *_p; \
+ int i; \
+ char *prio_name = NULL, *prio_args = NULL; \
+ \
+ vector_foreach_slot(src, _p, i) { \
+ if (prio_name == NULL && _p->prio_name) \
+ prio_name = _p->prio_name; \
+ if (prio_args == NULL && _p->prio_args) \
+ prio_args = _p->prio_args; \
+ } \
+ if (prio_name != NULL) { \
+ prio_get(dir, p, prio_name, prio_args); \
+ origin = msg; \
+ goto out; \
+ } \
+} while (0)
+
int select_prio(struct config *conf, struct path *pp)
{
const char *origin;
@@ -563,7 +599,8 @@ int select_prio(struct config *conf, struct path *pp)
mpe = find_mpe(conf->mptable, pp->wwid);
set_prio(conf->multipath_dir, mpe, multipaths_origin);
set_prio(conf->multipath_dir, conf->overrides, overrides_origin);
- set_prio(conf->multipath_dir, pp->hwe, hwe_origin);
+ set_prio_from_vec(struct hwentry, conf->multipath_dir,
+ pp->hwe, hwe_origin, p);
set_prio(conf->multipath_dir, conf, conf_origin);
prio_get(conf->multipath_dir, p, DEFAULT_PRIO, DEFAULT_PRIO_ARGS);
origin = default_origin;
@@ -616,7 +653,7 @@ select_minio_rq (struct config *conf, struct multipath * mp)
do_set(minio_rq, mp->mpe, mp->minio, multipaths_origin);
do_set(minio_rq, conf->overrides, mp->minio, overrides_origin);
- do_set(minio_rq, mp->hwe, mp->minio, hwe_origin);
+ do_set_from_hwe(minio_rq, mp, mp->minio, hwe_origin);
do_set(minio_rq, conf, mp->minio, conf_origin);
do_default(mp->minio, DEFAULT_MINIO_RQ);
out:
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 6df2f4ec..ae847d61 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -102,6 +102,11 @@ alloc_path (void)
pp->priority = PRIO_UNDEF;
checker_clear(&pp->checker);
dm_path_to_gen(pp)->ops = &dm_gen_path_ops;
+ pp->hwe = vector_alloc();
+ if (pp->hwe == NULL) {
+ free(pp);
+ return NULL;
+ }
}
return pp;
}
@@ -125,6 +130,7 @@ free_path (struct path * pp)
udev_device_unref(pp->udev);
pp->udev = NULL;
}
+ vector_free(pp->hwe);
FREE(pp);
}
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index fef416b4..a801cd92 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -283,7 +283,7 @@ struct path {
int io_err_pathfail_starttime;
int find_multipaths_timeout;
/* configlet pointers */
- struct hwentry * hwe;
+ vector hwe;
struct gen_path generic_path;
};
@@ -342,7 +342,7 @@ struct multipath {
char * features;
char * hwhandler;
struct mpentry * mpe;
- struct hwentry * hwe;
+ vector hwe;
/* threads */
pthread_t waiter;
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 13/28] libmultipath: allow more than one hwentry
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (11 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 12/28] libmultipath: use vector for for pp->hwe and mp->hwe Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 14/28] libmultipath: don't merge hwentries by regex Martin Wilck
` (15 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
The previous patch "use vector for for pp->hwe and mp->hwe" changed the
data structure for hwentries. This patch changes actual behavior by
allowing more than one hwentry to match a given path (or multipath).
This fixes several currently broken test cases. The test code is adapted
accordingly.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 1 -
tests/hwtable.c | 64 ++++++++-----------------------------------
2 files changed, 12 insertions(+), 53 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 95a71447..89aad15a 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -136,7 +136,6 @@ find_hwe (const struct _vector *hwtable,
n++;
}
log_match(tmp, vendor, product, revision);
- break;
}
condlog(n > 1 ? 3 : 4, "%s: found %d hwtable matches for %s:%s:%s",
__func__, n, vendor, product, revision);
diff --git a/tests/hwtable.c b/tests/hwtable.c
index b2a0511a..8b2ed95d 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -610,9 +610,6 @@ static int setup_regex_hwe(void **state)
*
* Expected: Devices matching both get properties from both, kv2 taking
* precedence. Devices matching kv1 only just get props from kv1.
- *
- * Current: These entries are currently _NOT_ merged, therefore getuid is
- * default for kv1 matches, and checker is default on kv2 matches.
*/
static void test_regex_string_hwe(const struct hwt_state *hwt)
{
@@ -646,12 +643,7 @@ static void test_regex_string_hwe(const struct hwt_state *hwt)
pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- /*
- * You'd expect that the two entries above be merged,
- * but that isn't the case if they're in the same input file.
- */
- TEST_PROP_BROKEN(_checker, pp->checker.name,
- DEFAULT_CHECKER, chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_regex_string_hwe(void **state)
@@ -675,8 +667,6 @@ static int setup_regex_string_hwe(void **state)
* Expected: Devices matching kv2 (and thus, both) get properties
* from both, kv2 taking precedence.
* Devices matching kv1 only just get props from kv1.
- *
- * Current: behaves as expected.
*/
static void test_regex_string_hwe_dir(const struct hwt_state *hwt)
{
@@ -711,7 +701,6 @@ static void test_regex_string_hwe_dir(const struct hwt_state *hwt)
/* Later match takes prio */
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- /* This time it's merged */
TEST_PROP(pp->checker.name, chk_hp.value);
}
@@ -792,19 +781,15 @@ static int setup_regex_2_strings_hwe_dir(void **state)
* Expected: Devices matching kv1 (and thus, both) get properties
* from both, kv1 taking precedence.
* Devices matching kv1 only just get props from kv1.
- *
- * Current: kv2 never matches, because kv1 is more generic and encountered
- * first; thus properties from kv2 aren't used.
*/
static void test_string_regex_hwe_dir(const struct hwt_state *hwt)
{
struct path *pp;
/* foo:bar matches kv2 and kv1 */
- pp = mock_path_flags(vnd_foo.value, prd_bar.value,
- BROKEN == 1 ? 0 : USE_GETUID);
+ pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_emc.value);
- TEST_PROP_BROKEN(_getuid, pp->getuid, (char *)NULL, gui_foo.value);
+ TEST_PROP(pp->getuid, gui_foo.value);
TEST_PROP(pp->checker.name, chk_hp.value);
/* foo:baz matches kv1 */
@@ -850,8 +835,6 @@ static int setup_string_regex_hwe_dir(void **state)
* This could happen in a large multipath.conf file.
*
* Expected: matching devices get props from both, kv2 taking precedence.
- *
- * Current: devices get props from kv2 only.
*/
static void test_2_ident_strings_hwe(const struct hwt_state *hwt)
{
@@ -863,12 +846,11 @@ static void test_2_ident_strings_hwe(const struct hwt_state *hwt)
TEST_PROP(pp->getuid, NULL);
TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
- /* foo:bar matches both, but only kv2 is seen */
+ /* foo:bar matches both */
pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
- chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_2_ident_strings_hwe(void **state)
@@ -903,8 +885,7 @@ static void test_2_ident_strings_both_dir(const struct hwt_state *hwt)
pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
- chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_2_ident_strings_both_dir(void **state)
@@ -944,8 +925,7 @@ static void test_2_ident_strings_both_dir_w_prev(const struct hwt_state *hwt)
pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
- chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_2_ident_strings_both_dir_w_prev(void **state)
@@ -975,8 +955,6 @@ static int setup_2_ident_strings_both_dir_w_prev(void **state)
* to kv1 being in the built-in hwtable and kv2 in multipath.conf.
*
* Expected: matching devices get props from both, kv2 taking precedence.
- *
- * Current: behaves as expected.
*/
static void test_2_ident_strings_hwe_dir(const struct hwt_state *hwt)
{
@@ -1011,9 +989,6 @@ static int setup_2_ident_strings_hwe_dir(void **state)
* contains an additional, empty entry (kv0).
*
* Expected: matching devices get props from kv1 and kv2, kv2 taking precedence.
- *
- * Current: kv0 and kv1 are merged into kv0, and then ignored because kv2 takes
- * precedence. Thus the presence of the empty kv0 changes how kv1 is treated.
*/
static void test_3_ident_strings_hwe_dir(const struct hwt_state *hwt)
{
@@ -1029,8 +1004,7 @@ static void test_3_ident_strings_hwe_dir(const struct hwt_state *hwt)
pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name, DEFAULT_CHECKER,
- chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_3_ident_strings_hwe_dir(void **state)
@@ -1059,8 +1033,6 @@ static int setup_3_ident_strings_hwe_dir(void **state)
* to kv1 being in the built-in hwtable and kv2 in multipath.conf.
*
* Expected: matching devices get props from both, kv2 taking precedence.
- *
- * Current: behaves as expected.
*/
static void test_2_ident_self_matching_re_hwe_dir(const struct hwt_state *hwt)
{
@@ -1096,8 +1068,6 @@ static int setup_2_ident_self_matching_re_hwe_dir(void **state)
* kv1 and kv2 are added to the main config file.
*
* Expected: matching devices get props from both, kv2 taking precedence.
- *
- * Current: Devices get properties from kv2 only (kv1 and kv2 are not merged).
*/
static void test_2_ident_self_matching_re_hwe(const struct hwt_state *hwt)
{
@@ -1113,8 +1083,7 @@ static void test_2_ident_self_matching_re_hwe(const struct hwt_state *hwt)
pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name,
- DEFAULT_CHECKER, chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_2_ident_self_matching_re_hwe(void **state)
@@ -1135,8 +1104,6 @@ static int setup_2_ident_self_matching_re_hwe(void **state)
* This case is more important as you may think, see above.
*
* Expected: matching devices get props from both, kv2 taking precedence.
- *
- * Current: devices get props from kv2 only.
*/
static void
test_2_ident_not_self_matching_re_hwe_dir(const struct hwt_state *hwt)
@@ -1149,12 +1116,11 @@ test_2_ident_not_self_matching_re_hwe_dir(const struct hwt_state *hwt)
TEST_PROP(pp->getuid, NULL);
TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
- /* foo:bar matches both, but only kv2 is seen */
+ /* foo:bar matches both */
pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name,
- DEFAULT_CHECKER, chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_2_ident_not_self_matching_re_hwe_dir(void **state)
@@ -1223,8 +1189,6 @@ static int setup_2_matching_res_hwe_dir(void **state)
* "baz" matches both regex "ba[zy]" and "ba(z|y)"
*
* Expected: matching devices get properties from both, kv2 taking precedence.
- *
- * Current: matching devices get properties from kv2 only.
*/
static void test_2_nonmatching_res_hwe_dir(const struct hwt_state *hwt)
{
@@ -1236,14 +1200,10 @@ static void test_2_nonmatching_res_hwe_dir(const struct hwt_state *hwt)
TEST_PROP(pp->getuid, NULL);
TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
- /*
- * foo:baz matches k2 and k1. Yet it sees the value from k2 only.
- */
pp = mock_path_flags(vnd_foo.value, prd_baz.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name,
- DEFAULT_CHECKER, chk_hp.value);
+ TEST_PROP(pp->checker.name, chk_hp.value);
}
static int setup_2_nonmatching_res_hwe_dir(void **state)
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 14/28] libmultipath: don't merge hwentries by regex
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (12 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 13/28] libmultipath: allow more than one hwentry Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 15/28] libmultipath: merge hwentries inside a conf file Martin Wilck
` (14 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Merging by regular expression is wrong, because regular expressions can't be
matched against each other. Unexpected results for hardware properties may
result. Don't do it any more.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 8 +-------
tests/hwtable.c | 8 +-------
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 89aad15a..713ac7f3 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -470,18 +470,12 @@ restart:
free_hwe(hwe2);
continue;
}
- if (hwe_regmatch(hwe1, hwe2->vendor,
- hwe2->product, hwe2->revision))
- continue;
- /* dup */
- log_match(hwe1, hwe2->vendor,
- hwe2->product, hwe2->revision);
- merge_hwe(hwe2, hwe1);
if (hwe_strmatch(hwe2, hwe1) == 0) {
condlog(4, "%s: removing hwentry %s:%s:%s",
__func__, hwe1->vendor, hwe1->product,
hwe1->revision);
vector_del_slot(hw, i);
+ merge_hwe(hwe2, hwe1);
free_hwe(hwe1);
n -= 1;
/*
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 8b2ed95d..15f364e4 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -1138,15 +1138,10 @@ static int setup_2_ident_not_self_matching_re_hwe_dir(void **state)
* Two different non-trivial regexes kv1, kv2. The 1st one matches the 2nd, but
* it doesn't match all possible strings matching the second.
* ("ba[zy]" matches regex "ba[[rxy]", but "baz" does not).
- * This causes the first entry to be merged into the second, but both entries
- * to be kept.
*
* Expected: Devices matching both regexes get properties from both, kv2
* taking precedence. Devices matching just one regex get properties from
* that one regex only.
- *
- * Current: behaves as expected, except for devices that match only kv2.
- * Those get properties from kv1, too.
*/
static void test_2_matching_res_hwe_dir(const struct hwt_state *hwt)
{
@@ -1168,8 +1163,7 @@ static void test_2_matching_res_hwe_dir(const struct hwt_state *hwt)
pp = mock_path_flags(vnd_foo.value, prd_baz.value, USE_GETUID);
TEST_PROP(prio_name(&pp->prio), prio_hds.value);
TEST_PROP(pp->getuid, gui_foo.value);
- TEST_PROP_BROKEN(_checker, pp->checker.name,
- chk_hp.value, DEFAULT_CHECKER);
+ TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
}
static int setup_2_matching_res_hwe_dir(void **state)
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 15/28] libmultipath: merge hwentries inside a conf file
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (13 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 14/28] libmultipath: don't merge hwentries by regex Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-15 18:03 ` Benjamin Marzinski
2018-06-08 10:20 ` [PATCH 16/28] libmultipath/hwtable: remove inherited props from ONTAP NVMe Martin Wilck
` (13 subsequent siblings)
28 siblings, 1 reply; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Merging hwentries with identical vendor/product/revision is still useful,
although it's not strictly necessary any more. But such identical entries
should always be merged, not only if they appear in different configuration
files. This requires changing the logic of factorize_hwtable.
By setting -DCHECK_BUILTIN_HWTABLE at compile time, the built-in hwtable
can be checked against duplicates as well.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 43 +++++++++++++++++++++----------------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 713ac7f3..fb41d620 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -452,7 +452,7 @@ out:
}
static void
-factorize_hwtable (vector hw, int n)
+factorize_hwtable (vector hw, int n, const char *table_desc)
{
struct hwentry *hwe1, *hwe2;
int i, j;
@@ -462,22 +462,26 @@ restart:
if (i == n)
break;
j = n;
+ /* drop invalid device configs */
+ if (i >= n && (!hwe1->vendor || !hwe1->product)) {
+ condlog(0, "device config in %s missing vendor or product parameter",
+ table_desc);
+ vector_del_slot(hw, i--);
+ free_hwe(hwe1);
+ continue;
+ }
+ j = n > i + 1 ? n : i + 1;
vector_foreach_slot_after(hw, hwe2, j) {
- /* drop invalid device configs */
- if (!hwe2->vendor || !hwe2->product) {
- condlog(0, "device config missing vendor or product parameter");
- vector_del_slot(hw, j--);
- free_hwe(hwe2);
- continue;
- }
if (hwe_strmatch(hwe2, hwe1) == 0) {
- condlog(4, "%s: removing hwentry %s:%s:%s",
+ condlog(i >= n ? 1 : 3,
+ "%s: duplicate device section for %s:%s:%s in %s",
__func__, hwe1->vendor, hwe1->product,
- hwe1->revision);
+ hwe1->revision, table_desc);
vector_del_slot(hw, i);
merge_hwe(hwe2, hwe1);
free_hwe(hwe1);
- n -= 1;
+ if (i < n)
+ n -= 1;
/*
* Play safe here; we have modified
* the original vector so the outer
@@ -605,9 +609,8 @@ process_config_dir(struct config *conf, vector keywords, char *dir)
snprintf(path, LINE_MAX, "%s/%s", dir, namelist[i]->d_name);
path[LINE_MAX-1] = '\0';
process_file(conf, path);
- if (VECTOR_SIZE(conf->hwtable) > old_hwtable_size)
- factorize_hwtable(conf->hwtable, old_hwtable_size);
-
+ factorize_hwtable(conf->hwtable, old_hwtable_size,
+ namelist[i]->d_name);
}
pthread_cleanup_pop(1);
}
@@ -655,6 +658,9 @@ load_config (char * file)
if (setup_default_hwtable(conf->hwtable))
goto out;
+#ifdef CHECK_BUILTIN_HWTABLE
+ factorize_hwtable(conf->hwtable, 0, "builtin");
+#endif
/*
* read the config file
*/
@@ -668,14 +674,7 @@ load_config (char * file)
condlog(0, "error parsing config file");
goto out;
}
- if (VECTOR_SIZE(conf->hwtable) > builtin_hwtable_size) {
- /*
- * remove duplica in hwtable. config file
- * takes precedence over build-in hwtable
- */
- factorize_hwtable(conf->hwtable, builtin_hwtable_size);
- }
-
+ factorize_hwtable(conf->hwtable, builtin_hwtable_size, file);
}
conf->processed_main_config = 1;
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 15/28] libmultipath: merge hwentries inside a conf file
2018-06-08 10:20 ` [PATCH 15/28] libmultipath: merge hwentries inside a conf file Martin Wilck
@ 2018-06-15 18:03 ` Benjamin Marzinski
2018-06-18 9:33 ` Martin Wilck
2018-06-18 9:54 ` [PATCH 29/30] tests/hwtable: add test for broken hwentry filtering Martin Wilck
0 siblings, 2 replies; 34+ messages in thread
From: Benjamin Marzinski @ 2018-06-15 18:03 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel, Xose Vazquez Perez
On Fri, Jun 08, 2018 at 12:20:28PM +0200, Martin Wilck wrote:
> Merging hwentries with identical vendor/product/revision is still useful,
> although it's not strictly necessary any more. But such identical entries
> should always be merged, not only if they appear in different configuration
> files. This requires changing the logic of factorize_hwtable.
Sorry for my slowness in reviewing these. This one looks wrong to me
> By setting -DCHECK_BUILTIN_HWTABLE at compile time, the built-in hwtable
> can be checked against duplicates as well.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> libmultipath/config.c | 43 +++++++++++++++++++++----------------------
> 1 file changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index 713ac7f3..fb41d620 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -452,7 +452,7 @@ out:
> }
>
> static void
> -factorize_hwtable (vector hw, int n)
> +factorize_hwtable (vector hw, int n, const char *table_desc)
> {
> struct hwentry *hwe1, *hwe2;
> int i, j;
> @@ -462,22 +462,26 @@ restart:
> if (i == n)
> break;
> j = n;
> + /* drop invalid device configs */
> + if (i >= n && (!hwe1->vendor || !hwe1->product)) {
How can i >= n? vector_foreach_slot() starts i at 0, and then next
lines are
if (i == n)
break;
> + condlog(0, "device config in %s missing vendor or product parameter",
> + table_desc);
> + vector_del_slot(hw, i--);
shouldn't we also decrement n here. I realize that doesn't work well for
the CHECK_BUILTIN_HWTABLE check, where n is 0, but as I said above, I'm
pretty sure that will never get here.
> + free_hwe(hwe1);
> + continue;
> + }
> + j = n > i + 1 ? n : i + 1;
again, n must always be at least i + 1 to get here, so j is always n.
> vector_foreach_slot_after(hw, hwe2, j) {
> - /* drop invalid device configs */
> - if (!hwe2->vendor || !hwe2->product) {
> - condlog(0, "device config missing vendor or product parameter");
> - vector_del_slot(hw, j--);
> - free_hwe(hwe2);
> - continue;
> - }
> if (hwe_strmatch(hwe2, hwe1) == 0) {
> - condlog(4, "%s: removing hwentry %s:%s:%s",
> + condlog(i >= n ? 1 : 3,
this check also looks wrong
> + "%s: duplicate device section for %s:%s:%s in %s",
> __func__, hwe1->vendor, hwe1->product,
> - hwe1->revision);
> + hwe1->revision, table_desc);
> vector_del_slot(hw, i);
> merge_hwe(hwe2, hwe1);
> free_hwe(hwe1);
> - n -= 1;
> + if (i < n)
and this one looks unnecessary.
> + n -= 1;
> /*
> * Play safe here; we have modified
> * the original vector so the outer
> @@ -605,9 +609,8 @@ process_config_dir(struct config *conf, vector keywords, char *dir)
> snprintf(path, LINE_MAX, "%s/%s", dir, namelist[i]->d_name);
> path[LINE_MAX-1] = '\0';
> process_file(conf, path);
> - if (VECTOR_SIZE(conf->hwtable) > old_hwtable_size)
> - factorize_hwtable(conf->hwtable, old_hwtable_size);
> -
> + factorize_hwtable(conf->hwtable, old_hwtable_size,
> + namelist[i]->d_name);
> }
> pthread_cleanup_pop(1);
> }
> @@ -655,6 +658,9 @@ load_config (char * file)
> if (setup_default_hwtable(conf->hwtable))
> goto out;
>
> +#ifdef CHECK_BUILTIN_HWTABLE
> + factorize_hwtable(conf->hwtable, 0, "builtin");
> +#endif
> /*
> * read the config file
> */
> @@ -668,14 +674,7 @@ load_config (char * file)
> condlog(0, "error parsing config file");
> goto out;
> }
> - if (VECTOR_SIZE(conf->hwtable) > builtin_hwtable_size) {
> - /*
> - * remove duplica in hwtable. config file
> - * takes precedence over build-in hwtable
> - */
> - factorize_hwtable(conf->hwtable, builtin_hwtable_size);
> - }
> -
> + factorize_hwtable(conf->hwtable, builtin_hwtable_size, file);
> }
>
> conf->processed_main_config = 1;
> --
> 2.17.0
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 15/28] libmultipath: merge hwentries inside a conf file
2018-06-15 18:03 ` Benjamin Marzinski
@ 2018-06-18 9:33 ` Martin Wilck
2018-06-18 9:54 ` [PATCH 29/30] tests/hwtable: add test for broken hwentry filtering Martin Wilck
1 sibling, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-18 9:33 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: dm-devel, Xose Vazquez Perez
Hi Ben,
On Fri, 2018-06-15 at 13:03 -0500, Benjamin Marzinski wrote:
> On Fri, Jun 08, 2018 at 12:20:28PM +0200, Martin Wilck wrote:
> > Merging hwentries with identical vendor/product/revision is still
> > useful,
> > although it's not strictly necessary any more. But such identical
> > entries
> > should always be merged, not only if they appear in different
> > configuration
> > files. This requires changing the logic of factorize_hwtable.
>
> Sorry for my slowness in reviewing these. This one looks wrong to me
Thanks for the thorough review. Given the size of the series, I don't
think this was slow.
> > By setting -DCHECK_BUILTIN_HWTABLE at compile time, the built-in
> > hwtable
> > can be checked against duplicates as well.
> >
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> > libmultipath/config.c | 43 +++++++++++++++++++++----------------
> > ------
> > 1 file changed, 21 insertions(+), 22 deletions(-)
> >
> > diff --git a/libmultipath/config.c b/libmultipath/config.c
> > index 713ac7f3..fb41d620 100644
> > --- a/libmultipath/config.c
> > +++ b/libmultipath/config.c
> > @@ -452,7 +452,7 @@ out:
> > }
> >
> > static void
> > -factorize_hwtable (vector hw, int n)
> > +factorize_hwtable (vector hw, int n, const char *table_desc)
> > {
> > struct hwentry *hwe1, *hwe2;
> > int i, j;
> > @@ -462,22 +462,26 @@ restart:
> > if (i == n)
> > break;
> > j = n;
> > + /* drop invalid device configs */
> > + if (i >= n && (!hwe1->vendor || !hwe1->product)) {
>
> How can i >= n? vector_foreach_slot() starts i at 0, and then next
> lines are
>
> if (i == n)
> break;
The above two lines, and the line "j = n" after that, were meant to be
deleted. That slipped through in my rebasing work, sorry about that.
When these lines are gone, I believe your issues are resolved, see
below.
n denotes the number of hwentries present before the current config
file was parsed. Before my patch series, "factorize_hwtable" would only
check for duplicates between the "old" (i < n) and "new" (i >= n)
sections, and would not remove the dups inside either section. Now
duplicates are also merged if they occur in the same section (except
for the built-in hwtable, for which the "inside" check is only done
with -DCHECK_BUILTIN_HWTABLE).
Well - that's what I intended to do, but by by forgetting to delete the
above lines, I failed :-(
In practice, this error only breaks the "broken hwentry" and "dup
removal inside a section" features - application of "good" hwentries to
paths and multipaths isn't affected, and thus the test cases pass
despite the bug. I'll post a fix, plus new test cases covering it.
> > + condlog(0, "device config in %s missing
> > vendor or product parameter",
> > + table_desc);
> > + vector_del_slot(hw, i--);
>
> shouldn't we also decrement n here. I realize that doesn't work well
> for
> the CHECK_BUILTIN_HWTABLE check, where n is 0, but as I said above,
> I'm
> pretty sure that will never get here.
No, see above. The check for broken entries is only done for "new" ones
(i > n), as we assume the entries up to n to be checked already. So
when we reach this condition, is is really always above n.
>
> > + free_hwe(hwe1);
> > + continue;
> > + }
> > + j = n > i + 1 ? n : i + 1;
>
> again, n must always be at least i + 1 to get here, so j is always n.
See above.
>
> > vector_foreach_slot_after(hw, hwe2, j) {
> > - /* drop invalid device configs */
> > - if (!hwe2->vendor || !hwe2->product) {
> > - condlog(0, "device config missing
> > vendor or product parameter");
> > - vector_del_slot(hw, j--);
> > - free_hwe(hwe2);
> > - continue;
> > - }
> > if (hwe_strmatch(hwe2, hwe1) == 0) {
> > - condlog(4, "%s: removing hwentry
> > %s:%s:%s",
> > + condlog(i >= n ? 1 : 3,
>
> this check also looks wrong
The intention is not to log at prio 1 if a new section (read in most
cases: multipath.conf) contains a duplicate to a previous section (read
in most cases: built-in hwtable). Such use is perfectly valid, whereas
two hwentries with the same vendor and product in one config file are
suspicious and should be logged.
>
> > + "%s: duplicate device
> > section for %s:%s:%s in %s",
> > __func__, hwe1->vendor,
> > hwe1->product,
> > - hwe1->revision);
> > + hwe1->revision,
> > table_desc);
> > vector_del_slot(hw, i);
> > merge_hwe(hwe2, hwe1);
> > free_hwe(hwe1);
> > - n -= 1;
> > + if (i < n)
>
> and this one looks unnecessary.
See above.
I order to avoid spamming dm-devel with the whole series again, I'll
send patches on top of it. If you prefer that I send the full rebased
series, I'll do of course, but I'll wait for more reviews.
Thanks,
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 29/30] tests/hwtable: add test for broken hwentry filtering
2018-06-15 18:03 ` Benjamin Marzinski
2018-06-18 9:33 ` Martin Wilck
@ 2018-06-18 9:54 ` Martin Wilck
2018-06-18 9:54 ` [PATCH 30/30] fixup "libmultipath: merge hwentries inside a conf file" Martin Wilck
1 sibling, 1 reply; 34+ messages in thread
From: Martin Wilck @ 2018-06-18 9:54 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Broken hwentries weren't correctly filtered after patch
"libmultipath: merge hwentries inside a conf file". Add a test
case for that.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
tests/hwtable.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 9db79391..3488640d 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -651,6 +651,52 @@ static int setup_string_hwe(void **state)
return 0;
}
+/*
+ * Device section with a broken entry (no product)
+ * It should be ignored.
+ */
+static void test_broken_hwe(const struct hwt_state *hwt)
+{
+ struct path *pp;
+
+ /* foo:bar doesn't match, as hwentry is ignored */
+ pp = mock_path(vnd_foo.value, prd_bar.value);
+ TEST_PROP_BROKEN("prio", prio_name(&pp->prio), prio_emc.value,
+ DEFAULT_PRIO);
+
+ /* boo:bar doesn't match */
+ pp = mock_path(vnd_boo.value, prd_bar.value);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
+}
+
+static int setup_broken_hwe(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv[] = { vnd_foo, prio_emc };
+
+ WRITE_ONE_DEVICE(hwt, kv);
+ SET_TEST_FUNC(hwt, test_broken_hwe);
+ return 0;
+}
+
+/*
+ * Like test_broken_hwe, but in config_dir file.
+ */
+static int setup_broken_hwe_dir(void **state)
+{
+ struct hwt_state *hwt = CHECK_STATE(state);
+ const struct key_value kv[] = { vnd_foo, prio_emc };
+
+ begin_config(hwt);
+ begin_section_all(hwt, "devices");
+ write_device(hwt->conf_dir_file[0], ARRAY_SIZE(kv), kv);
+ end_section_all(hwt);
+ finish_config(hwt);
+ hwt->test = test_broken_hwe;
+ hwt->test_name = "test_broken_hwe_dir";
+ return 0;
+}
+
/*
* Device section with a single regex entry ("^.foo:(bar|baz|ba\.)$")
*/
@@ -1628,6 +1674,8 @@ static int setup_multipath_config_3(void **state)
}
define_test(string_hwe)
+define_test(broken_hwe)
+define_test(broken_hwe_dir)
define_test(quoted_hwe)
define_test(internal_nvme)
define_test(regex_hwe)
@@ -1666,6 +1714,8 @@ static int test_hwtable(void)
cmocka_unit_test(test_sanity_globals),
test_entry(internal_nvme),
test_entry(string_hwe),
+ test_entry(broken_hwe),
+ test_entry(broken_hwe_dir),
test_entry(quoted_hwe),
test_entry(regex_hwe),
test_entry(regex_string_hwe),
--
2.17.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 30/30] fixup "libmultipath: merge hwentries inside a conf file"
2018-06-18 9:54 ` [PATCH 29/30] tests/hwtable: add test for broken hwentry filtering Martin Wilck
@ 2018-06-18 9:54 ` Martin Wilck
0 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-18 9:54 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
The previous patch "libmultipath: merge hwentries inside a conf file"
meant to enable checking for duplicate entries not only between different
"sections" of the configuration (i.e. built-in hwtable, multipath.conf,
config dir files), but also inside every section except the built-in
table. This patch fixes a bug to actually implement this new behavior.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 3 ---
tests/hwtable.c | 3 +--
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 28b76dd2..50826a1d 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -527,9 +527,6 @@ factorize_hwtable (vector hw, int n, const char *table_desc)
restart:
vector_foreach_slot(hw, hwe1, i) {
- if (i == n)
- break;
- j = n;
/* drop invalid device configs */
if (i >= n && (!hwe1->vendor || !hwe1->product)) {
condlog(0, "device config in %s missing vendor or product parameter",
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 3488640d..42127adf 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -661,8 +661,7 @@ static void test_broken_hwe(const struct hwt_state *hwt)
/* foo:bar doesn't match, as hwentry is ignored */
pp = mock_path(vnd_foo.value, prd_bar.value);
- TEST_PROP_BROKEN("prio", prio_name(&pp->prio), prio_emc.value,
- DEFAULT_PRIO);
+ TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
/* boo:bar doesn't match */
pp = mock_path(vnd_boo.value, prd_bar.value);
--
2.17.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 16/28] libmultipath/hwtable: remove inherited props from ONTAP NVMe
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (14 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 15/28] libmultipath: merge hwentries inside a conf file Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 17/28] libmultipath: don't merge by regex in setup_default_blist() Martin Wilck
` (12 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
With the new hwtable code, properties inherited from generic NVMe
don't need to be duplicated.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/hwtable.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 148f0ba2..d20ed194 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -746,11 +746,8 @@ static struct hwentry default_hw[] = {
*/
.vendor = "NVME",
.product = "^NetApp ONTAP Controller",
- .uid_attribute = "ID_WWN",
- .checker_name = NONE,
.pgpolicy = MULTIBUS,
.no_path_retry = NO_PATH_RETRY_QUEUE,
- .retain_hwhandler = RETAIN_HWHANDLER_OFF,
},
/*
* Nexenta
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 17/28] libmultipath: don't merge by regex in setup_default_blist()
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (15 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 16/28] libmultipath/hwtable: remove inherited props from ONTAP NVMe Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 18/28] multipath, multipathd: consolidate config dumping Martin Wilck
` (11 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
The merging of blacklist entries by regular expression leads
to similar problems as the merging of hwentries. Only merge
blacklist entries if they're exactly equal.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/blacklist.c | 23 +++++++++++++++++++++--
tests/hwtable.c | 8 +-------
2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index 34a7e717..91ed7ddf 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -163,6 +163,25 @@ _blacklist_device (const struct _vector *blist, const char * vendor,
return 0;
}
+static int
+find_blacklist_device (const struct _vector *blist, const char * vendor,
+ const char * product)
+{
+ int i;
+ struct blentry_device * ble;
+
+ vector_foreach_slot (blist, ble, i) {
+ if (((!vendor && !ble->vendor) ||
+ (vendor && ble->vendor &&
+ !strcmp(vendor, ble->vendor))) &&
+ ((!product && !ble->product) ||
+ (product && ble->product &&
+ !strcmp(product, ble->product))))
+ return 1;
+ }
+ return 0;
+}
+
int
setup_default_blist (struct config * conf)
{
@@ -191,8 +210,8 @@ setup_default_blist (struct config * conf)
vector_foreach_slot (conf->hwtable, hwe, i) {
if (hwe->bl_product) {
- if (_blacklist_device(conf->blist_device, hwe->vendor,
- hwe->bl_product))
+ if (find_blacklist_device(conf->blist_device,
+ hwe->vendor, hwe->bl_product))
continue;
if (alloc_ble_device(conf->blist_device))
return 1;
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 15f364e4..85215946 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -1399,13 +1399,7 @@ static int setup_product_blacklist(void **state)
static void test_product_blacklist_matching(const struct hwt_state *hwt)
{
mock_path_flags(vnd_foo.value, prd_bar.value, BL_BY_DEVICE);
-#if BROKEN == 1
- condlog(1, "%s: WARNING: broken blacklist test on line %d",
- __func__, __LINE__ + 1);
- mock_path(vnd_foo.value, prd_baz.value);
-#else
- mock_path_blacklisted(vnd_foo.value, prd_baz.value);
-#endif
+ mock_path_flags(vnd_foo.value, prd_baz.value, BL_BY_DEVICE);
mock_path(vnd_foo.value, prd_bam.value);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 18/28] multipath, multipathd: consolidate config dumping
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (16 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 17/28] libmultipath: don't merge by regex in setup_default_blist() Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 19/28] tests/hwtable: implement configuration dump + reload test Martin Wilck
` (10 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
"multipath -t" and "multipathd show config" use very similar code.
Consolidate it into a libmultipath function. Simplify it a bit in
the process, and do some "const" and "static" cleanup.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/print.c | 76 ++++++++++++++++++++++++++++++++++-----
libmultipath/print.h | 7 +---
multipath/main.c | 63 ++++----------------------------
multipathd/cli_handlers.c | 52 ++-------------------------
4 files changed, 78 insertions(+), 120 deletions(-)
diff --git a/libmultipath/print.c b/libmultipath/print.c
index b1844c98..7d4e2ace 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1342,7 +1342,8 @@ snprint_multipath_topology_json (char * buff, int len, const struct vectors * ve
}
static int
-snprint_hwentry (struct config *conf, char * buff, int len, const struct hwentry * hwe)
+snprint_hwentry (const struct config *conf,
+ char * buff, int len, const struct hwentry * hwe)
{
int i;
int fwd = 0;
@@ -1374,7 +1375,8 @@ snprint_hwentry (struct config *conf, char * buff, int len, const struct hwentry
return fwd;
}
-int snprint_hwtable(struct config *conf, char *buff, int len, vector hwtable)
+static int snprint_hwtable(const struct config *conf,
+ char *buff, int len, vector hwtable)
{
int fwd = 0;
int i;
@@ -1400,7 +1402,8 @@ int snprint_hwtable(struct config *conf, char *buff, int len, vector hwtable)
}
static int
-snprint_mpentry (struct config *conf, char * buff, int len, const struct mpentry * mpe)
+snprint_mpentry (const struct config *conf, char * buff, int len,
+ const struct mpentry * mpe)
{
int i;
int fwd = 0;
@@ -1426,7 +1429,8 @@ snprint_mpentry (struct config *conf, char * buff, int len, const struct mpentry
return fwd;
}
-int snprint_mptable(struct config *conf, char *buff, int len, vector mptable)
+static int snprint_mptable(const struct config *conf,
+ char *buff, int len, vector mptable)
{
int fwd = 0;
int i;
@@ -1451,8 +1455,8 @@ int snprint_mptable(struct config *conf, char *buff, int len, vector mptable)
return fwd;
}
-int snprint_overrides(struct config *conf, char * buff, int len,
- const struct hwentry *overrides)
+static int snprint_overrides(const struct config *conf, char * buff, int len,
+ const struct hwentry *overrides)
{
int fwd = 0;
int i;
@@ -1481,7 +1485,7 @@ out:
return fwd;
}
-int snprint_defaults(struct config *conf, char *buff, int len)
+static int snprint_defaults(const struct config *conf, char *buff, int len)
{
int fwd = 0;
int i;
@@ -1624,7 +1628,7 @@ int snprint_blacklist_report(struct config *conf, char *buff, int len)
return fwd;
}
-int snprint_blacklist(struct config *conf, char *buff, int len)
+static int snprint_blacklist(const struct config *conf, char *buff, int len)
{
int i;
struct blentry * ble;
@@ -1700,7 +1704,8 @@ int snprint_blacklist(struct config *conf, char *buff, int len)
return fwd;
}
-int snprint_blacklist_except(struct config *conf, char *buff, int len)
+static int snprint_blacklist_except(const struct config *conf,
+ char *buff, int len)
{
int i;
struct blentry * ele;
@@ -1776,6 +1781,59 @@ int snprint_blacklist_except(struct config *conf, char *buff, int len)
return fwd;
}
+char *snprint_config(const struct config *conf, int *len)
+{
+ char *reply;
+ /* built-in config is >20kB already */
+ unsigned int maxlen = 32768;
+
+ for (reply = NULL; maxlen <= UINT_MAX/2; maxlen *= 2) {
+ char *c, *tmp = reply;
+
+ reply = REALLOC(reply, maxlen);
+ if (!reply) {
+ if (tmp)
+ free(tmp);
+ return NULL;
+ }
+
+ c = reply + snprint_defaults(conf, reply, maxlen);
+ if ((c - reply) == maxlen)
+ continue;
+
+ c += snprint_blacklist(conf, c, reply + maxlen - c);
+ if ((c - reply) == maxlen)
+ continue;
+
+ c += snprint_blacklist_except(conf, c, reply + maxlen - c);
+ if ((c - reply) == maxlen)
+ continue;
+
+ c += snprint_hwtable(conf, c, reply + maxlen - c,
+ conf->hwtable);
+ if ((c - reply) == maxlen)
+ continue;
+
+ c += snprint_overrides(conf, c, reply + maxlen - c,
+ conf->overrides);
+ if ((c - reply) == maxlen)
+ continue;
+
+ if (VECTOR_SIZE(conf->mptable) > 0)
+ c += snprint_mptable(conf, c, reply + maxlen - c,
+ conf->mptable);
+
+ if ((c - reply) < maxlen) {
+ if (len)
+ *len = c - reply;
+ return reply;
+ }
+ }
+
+ free(reply);
+ return NULL;
+}
+
int snprint_status(char *buff, int len, const struct vectors *vecs)
{
int fwd = 0;
diff --git a/libmultipath/print.h b/libmultipath/print.h
index 9b5a23aa..fed80d55 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -119,18 +119,13 @@ int _snprint_multipath_topology (const struct gen_multipath *, char *, int,
_snprint_multipath_topology (dm_multipath_to_gen(mpp), buf, len, v)
int snprint_multipath_topology_json (char * buff, int len,
const struct vectors * vecs);
+char *snprint_config(const struct config *conf, int *len);
int snprint_multipath_map_json (char * buff, int len,
const struct multipath * mpp, int last);
-int snprint_defaults (struct config *, char *, int);
-int snprint_blacklist (struct config *, char *, int);
-int snprint_blacklist_except (struct config *, char *, int);
int snprint_blacklist_report (struct config *, char *, int);
int snprint_wildcards (char *, int);
int snprint_status (char *, int, const struct vectors *);
int snprint_devices (struct config *, char *, int, const struct vectors *);
-int snprint_hwtable (struct config *, char *, int, const vector);
-int snprint_mptable (struct config *, char *, int, const vector);
-int snprint_overrides (struct config *, char *, int, const struct hwentry *);
int snprint_path_serial (char *, size_t, const struct path *);
int snprint_host_wwnn (char *, size_t, const struct path *);
int snprint_host_wwpn (char *, size_t, const struct path *);
diff --git a/multipath/main.c b/multipath/main.c
index 3f0a6aa7..288251c3 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -753,63 +753,14 @@ out:
static int
dump_config (struct config *conf)
{
- char * c, * tmp = NULL;
- char * reply;
- unsigned int maxlen = 256;
- int again = 1;
-
- reply = MALLOC(maxlen);
-
- while (again) {
- if (!reply) {
- if (tmp)
- free(tmp);
- return 1;
- }
- c = tmp = reply;
- c += snprint_defaults(conf, c, reply + maxlen - c);
- again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen *= 2);
- continue;
- }
- c += snprint_blacklist(conf, c, reply + maxlen - c);
- again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen *= 2);
- continue;
- }
- c += snprint_blacklist_except(conf, c, reply + maxlen - c);
- again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen *= 2);
- continue;
- }
- c += snprint_hwtable(conf, c, reply + maxlen - c, conf->hwtable);
- again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen *= 2);
- continue;
- }
- c += snprint_overrides(conf, c, reply + maxlen - c,
- conf->overrides);
- again = ((c - reply) == maxlen);
- if (again) {
- reply = REALLOC(reply, maxlen *= 2);
- continue;
- }
- if (VECTOR_SIZE(conf->mptable) > 0) {
- c += snprint_mptable(conf, c, reply + maxlen - c,
- conf->mptable);
- again = ((c - reply) == maxlen);
- if (again)
- reply = REALLOC(reply, maxlen *= 2);
- }
- }
+ char * reply = snprint_config(conf, NULL);
- printf("%s", reply);
- FREE(reply);
- return 0;
+ if (reply != NULL) {
+ printf("%s", reply);
+ FREE(reply);
+ return 0;
+ } else
+ return 1;
}
static int
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index ba50fb8f..6f043d7f 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -248,62 +248,16 @@ show_map_json (char ** r, int * len, struct multipath * mpp,
int
show_config (char ** r, int * len)
{
- char * c;
- char * reply;
- unsigned int maxlen = INITIAL_REPLY_LEN;
- int again = 1;
struct config *conf;
- int fail = 0;
-
- c = reply = MALLOC(maxlen);
+ char *reply;
conf = get_multipath_config();
pthread_cleanup_push(put_multipath_config, conf);
- while (again) {
- if (!reply) {
- fail = 1;
- break;
- }
- c = reply;
- c += snprint_defaults(conf, c, reply + maxlen - c);
- again = ((c - reply) == maxlen);
- REALLOC_REPLY(reply, again, maxlen);
- if (again)
- continue;
- c += snprint_blacklist(conf, c, reply + maxlen - c);
- again = ((c - reply) == maxlen);
- REALLOC_REPLY(reply, again, maxlen);
- if (again)
- continue;
- c += snprint_blacklist_except(conf, c, reply + maxlen - c);
- again = ((c - reply) == maxlen);
- REALLOC_REPLY(reply, again, maxlen);
- if (again)
- continue;
- c += snprint_hwtable(conf, c, reply + maxlen - c,
- conf->hwtable);
- again = ((c - reply) == maxlen);
- REALLOC_REPLY(reply, again, maxlen);
- if (again)
- continue;
- c += snprint_overrides(conf, c, reply + maxlen - c,
- conf->overrides);
- again = ((c - reply) == maxlen);
- REALLOC_REPLY(reply, again, maxlen);
- if (again)
- continue;
- if (VECTOR_SIZE(conf->mptable) > 0) {
- c += snprint_mptable(conf, c, reply + maxlen - c,
- conf->mptable);
- again = ((c - reply) == maxlen);
- REALLOC_REPLY(reply, again, maxlen);
- }
- }
+ reply = snprint_config(conf, len);
pthread_cleanup_pop(1);
- if (fail)
+ if (reply == NULL)
return 1;
*r = reply;
- *len = (int)(c - reply + 1);
return 0;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 19/28] tests/hwtable: implement configuration dump + reload test
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (17 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 18/28] multipath, multipathd: consolidate config dumping Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 20/28] libmultipath: allow dumping only "local" hwtable in snprint_config Martin Wilck
` (9 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
In order to test dumping and re-reading the configuration, we modify the
test_driver() function so that runs the same test three twice:
1) with "user-supplied" configuration,
2) using the config dump from 1) (equivalent with "multipath -t" output) as input
configuration.
The expectation is that actual paths and maps have same options applied.
Furthermore, the "multipath -T" output from 2) should be the same as from 1),
IOW, "multipath -T" in 2) reproduces its input config file exactly.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
tests/hwtable.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 86 insertions(+), 2 deletions(-)
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 85215946..85924fb7 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -23,6 +23,7 @@
#include "defaults.h"
#include "pgpolicies.h"
#include "test-lib.h"
+#include "print.h"
#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
#define N_CONF_FILES 2
@@ -358,6 +359,16 @@ static void write_device(FILE *ff, int nkv, const struct key_value *kv)
conf = NULL; \
} while (0)
+static void replace_config(const struct hwt_state *hwt,
+ const char *conf_str)
+{
+ FREE_CONFIG(_conf);
+ reset_configs(hwt);
+ fprintf(hwt->config_file, "%s", conf_str);
+ fflush(hwt->config_file);
+ _conf = LOAD_CONFIG(hwt);
+}
+
#define TEST_PROP(prop, val) do { \
if (val == NULL) \
assert_ptr_equal(prop, NULL); \
@@ -432,8 +443,58 @@ static const struct key_value npr_queue = { _no_path_retry, "queue" };
/***** BEGIN TESTS SECTION *****/
/*
- * Run the given test case. This will later be extended
- * to run the test several times in a row.
+ * Dump the configuration, subistitute the dumped configuration
+ * for the current one, and verify that the result is identical.
+ */
+static void replicate_config(const struct hwt_state *hwt)
+{
+ char *cfg1, *cfg2;
+ struct config *conf;
+
+ condlog(1, "--- %s: replicating configuration", __func__);
+
+ conf = get_multipath_config();
+ cfg1 = snprint_config(conf, NULL);
+
+ assert_non_null(cfg1);
+ put_multipath_config(conf);
+
+ replace_config(hwt, cfg1);
+
+ conf = get_multipath_config();
+ cfg2 = snprint_config(conf, NULL);
+ assert_non_null(cfg2);
+ put_multipath_config(conf);
+
+// #define DBG_CONFIG 1
+#ifdef DBG_CONFIG
+#define DUMP_CFG_STR(x) do { \
+ FILE *tmp = fopen("/tmp/hwtable-" #x ".txt", "w"); \
+ fprintf(tmp, "%s", x); \
+ fclose(tmp); \
+ } while (0)
+
+ DUMP_CFG_STR(cfg1);
+ DUMP_CFG_STR(cfg2);
+#endif
+
+#if BROKEN
+ condlog(1, "%s: WARNING: skipping tests for same configuration after dump/reload on %d",
+ __func__, __LINE__);
+#else
+ assert_int_equal(strlen(cfg2), strlen(cfg1));
+ assert_string_equal(cfg2, cfg1);
+#endif
+ free(cfg1);
+ free(cfg2);
+}
+
+/*
+ * Run hwt->test three times; once with the constructed configuration,
+ * once after re-reading the full dumped configuration, and once with the
+ * dumped local configuration.
+ *
+ * Expected: test passes every time.
*/
static void test_driver(void **state)
{
@@ -443,6 +504,10 @@ static void test_driver(void **state)
_conf = LOAD_CONFIG(hwt);
hwt->test(hwt);
+ replicate_config(hwt);
+ reset_vecs(hwt->vecs);
+ hwt->test(hwt);
+
reset_vecs(hwt->vecs);
FREE_CONFIG(_conf);
}
@@ -513,9 +578,18 @@ static int setup_internal_nvme(void **state)
/*
* Device section with a simple entry qith double quotes ('foo:"bar"')
*/
+#if BROKEN
+static void test_quoted_hwe(void **state)
+#else
static void test_quoted_hwe(const struct hwt_state *hwt)
+#endif
{
struct path *pp;
+#if BROKEN
+ struct hwt_state *hwt = CHECK_STATE(state);
+
+ _conf = LOAD_CONFIG(hwt);
+#endif
/* foo:"bar" matches */
pp = mock_path(vnd_foo.value, prd_baq.value);
TEST_PROP(prio_name(&pp->prio), prio_emc.value);
@@ -531,7 +605,11 @@ static int setup_quoted_hwe(void **state)
const struct key_value kv[] = { vnd_foo, prd_baqq, prio_emc };
WRITE_ONE_DEVICE(hwt, kv);
+#if BROKEN
+ condlog(0, "%s: WARNING: skipping conf reload test", __func__);
+#else
SET_TEST_FUNC(hwt, test_quoted_hwe);
+#endif
return 0;
}
@@ -1558,7 +1636,9 @@ static int setup_multipath_config_3(void **state)
}
define_test(string_hwe)
+#if !BROKEN
define_test(quoted_hwe)
+#endif
define_test(internal_nvme)
define_test(regex_hwe)
define_test(regex_string_hwe)
@@ -1596,7 +1676,11 @@ static int test_hwtable(void)
cmocka_unit_test(test_sanity_globals),
test_entry(internal_nvme),
test_entry(string_hwe),
+#if BROKEN
+ cmocka_unit_test_setup(test_quoted_hwe, setup_quoted_hwe),
+#else
test_entry(quoted_hwe),
+#endif
test_entry(regex_hwe),
test_entry(regex_string_hwe),
test_entry(regex_string_hwe_dir),
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 20/28] libmultipath: allow dumping only "local" hwtable in snprint_config
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (18 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 19/28] tests/hwtable: implement configuration dump + reload test Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 21/28] tests/hwtable: add test for local configuration dump Martin Wilck
` (8 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
This change is key for allowing to dump the "local" multipath configuration
only, i.e. those entries of the hardware table that match existing devices
in the system. By passing the output of the new helper function
get_used_hwes() as the last argument for snprint_config(), only the local
entries will be dumped.
The rationale is that "multipath -t" output is overwhelmingly long for most
use cases; in particular it's not useful as a template for creating a local
configuration.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/print.c | 8 +++++---
libmultipath/print.h | 3 ++-
libmultipath/structs_vec.c | 19 +++++++++++++++++++
libmultipath/structs_vec.h | 1 +
libmultipath/vector.c | 12 ++++++++++++
libmultipath/vector.h | 1 +
multipath/main.c | 2 +-
multipathd/cli_handlers.c | 8 ++++----
tests/hwtable.c | 4 ++--
9 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/libmultipath/print.c b/libmultipath/print.c
index 7d4e2ace..af4c00e2 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1376,7 +1376,8 @@ snprint_hwentry (const struct config *conf,
}
static int snprint_hwtable(const struct config *conf,
- char *buff, int len, vector hwtable)
+ char *buff, int len,
+ const struct _vector *hwtable)
{
int fwd = 0;
int i;
@@ -1781,7 +1782,8 @@ static int snprint_blacklist_except(const struct config *conf,
return fwd;
}
-char *snprint_config(const struct config *conf, int *len)
+char *snprint_config(const struct config *conf, int *len,
+ const struct _vector *hwtable)
{
char *reply;
/* built-in config is >20kB already */
@@ -1810,7 +1812,7 @@ char *snprint_config(const struct config *conf, int *len)
continue;
c += snprint_hwtable(conf, c, reply + maxlen - c,
- conf->hwtable);
+ hwtable ? hwtable : conf->hwtable);
if ((c - reply) == maxlen)
continue;
diff --git a/libmultipath/print.h b/libmultipath/print.h
index fed80d55..c63e0547 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -119,7 +119,8 @@ int _snprint_multipath_topology (const struct gen_multipath *, char *, int,
_snprint_multipath_topology (dm_multipath_to_gen(mpp), buf, len, v)
int snprint_multipath_topology_json (char * buff, int len,
const struct vectors * vecs);
-char *snprint_config(const struct config *conf, int *len);
+char *snprint_config(const struct config *conf, int *len,
+ const struct _vector *hwtable);
int snprint_multipath_map_json (char * buff, int len,
const struct multipath * mpp, int last);
int snprint_blacklist_report (struct config *, char *, int);
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 38f04387..f87d69d4 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -445,3 +445,22 @@ void update_queue_mode_add_path(struct multipath *mpp)
}
condlog(2, "%s: remaining active paths: %d", mpp->alias, mpp->nr_active);
}
+
+vector get_used_hwes(const struct _vector *pathvec)
+{
+ int i, j;
+ struct path *pp;
+ struct hwentry *hwe;
+ vector v = vector_alloc();
+
+ if (v == NULL)
+ return NULL;
+
+ vector_foreach_slot(pathvec, pp, i) {
+ vector_foreach_slot_backwards(pp->hwe, hwe, j) {
+ vector_find_or_add_slot(v, hwe);
+ }
+ }
+
+ return v;
+}
diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index 4220ea33..f7777aaf 100644
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -38,5 +38,6 @@ void update_queue_mode_add_path(struct multipath *mpp);
int update_multipath_table (struct multipath *mpp, vector pathvec,
int is_daemon);
int update_multipath_status (struct multipath *mpp);
+vector get_used_hwes(const struct _vector *pathvec);
#endif /* _STRUCTS_VEC_H */
diff --git a/libmultipath/vector.c b/libmultipath/vector.c
index f741ae08..501cf4c5 100644
--- a/libmultipath/vector.c
+++ b/libmultipath/vector.c
@@ -196,3 +196,15 @@ vector_set_slot(vector v, void *value)
i = VECTOR_SIZE(v) - 1;
v->slot[i] = value;
}
+
+int vector_find_or_add_slot(vector v, void *value)
+{
+ int n = find_slot(v, value);
+
+ if (n >= 0)
+ return n;
+ if (vector_alloc_slot(v) == NULL)
+ return -1;
+ vector_set_slot(v, value);
+ return VECTOR_SIZE(v) - 1;
+}
diff --git a/libmultipath/vector.h b/libmultipath/vector.h
index b9450ac8..41d2b896 100644
--- a/libmultipath/vector.h
+++ b/libmultipath/vector.h
@@ -82,6 +82,7 @@ extern void vector_set_slot(vector v, void *value);
extern void vector_del_slot(vector v, int slot);
extern void *vector_insert_slot(vector v, int slot, void *value);
int find_slot(vector v, void * addr);
+int vector_find_or_add_slot(vector v, void *value);
extern void vector_repack(vector v);
extern void vector_dump(vector v);
extern void dump_strvec(vector strvec);
diff --git a/multipath/main.c b/multipath/main.c
index 288251c3..87d80dd3 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -753,7 +753,7 @@ out:
static int
dump_config (struct config *conf)
{
- char * reply = snprint_config(conf, NULL);
+ char * reply = snprint_config(conf, NULL, NULL);
if (reply != NULL) {
printf("%s", reply);
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 6f043d7f..75cc8a9e 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -245,15 +245,15 @@ show_map_json (char ** r, int * len, struct multipath * mpp,
return 0;
}
-int
-show_config (char ** r, int * len)
+static int
+show_config (char ** r, int * len, const struct _vector *hwtable)
{
struct config *conf;
char *reply;
conf = get_multipath_config();
pthread_cleanup_push(put_multipath_config, conf);
- reply = snprint_config(conf, len);
+ reply = snprint_config(conf, len, hwtable);
pthread_cleanup_pop(1);
if (reply == NULL)
return 1;
@@ -277,7 +277,7 @@ cli_list_config (void * v, char ** reply, int * len, void * data)
{
condlog(3, "list config (operator)");
- return show_config(reply, len);
+ return show_config(reply, len, NULL);
}
int
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 85924fb7..3c3caddd 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -454,7 +454,7 @@ static void replicate_config(const struct hwt_state *hwt)
condlog(1, "--- %s: replicating configuration", __func__);
conf = get_multipath_config();
- cfg1 = snprint_config(conf, NULL);
+ cfg1 = snprint_config(conf, NULL, NULL);
assert_non_null(cfg1);
put_multipath_config(conf);
@@ -462,7 +462,7 @@ static void replicate_config(const struct hwt_state *hwt)
replace_config(hwt, cfg1);
conf = get_multipath_config();
- cfg2 = snprint_config(conf, NULL);
+ cfg2 = snprint_config(conf, NULL, NULL);
assert_non_null(cfg2);
put_multipath_config(conf);
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 21/28] tests/hwtable: add test for local configuration dump
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (19 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 20/28] libmultipath: allow dumping only "local" hwtable in snprint_config Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 22/28] libmultipath: allow printing local maps in snprint_config Martin Wilck
` (7 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
This adds another test iteration in test_driver(): It now tests:
1) with "user-supplied" configuration,
2) using the full config dump (equivalent with "multipath -t" output),
3) using the local config dump as implemented in the previous patch.
Again, the properties of paths and maps should be the same for all 3
scenarios. For 3), the "multipath -t" output can't be expected to be
exactly equal to the output in 1), because merging of hwentries may move the
position of a hwentry down in the hwentry list, and empty "multipath"
sections are being added.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
tests/hwtable.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 3c3caddd..08ed67d3 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -446,21 +446,42 @@ static const struct key_value npr_queue = { _no_path_retry, "queue" };
* Dump the configuration, subistitute the dumped configuration
* for the current one, and verify that the result is identical.
*/
-static void replicate_config(const struct hwt_state *hwt)
+static void replicate_config(const struct hwt_state *hwt, bool local)
{
char *cfg1, *cfg2;
+ vector hwtable;
struct config *conf;
- condlog(1, "--- %s: replicating configuration", __func__);
+ condlog(1, "--- %s: replicating %s configuration", __func__,
+ local ? "local" : "full");
conf = get_multipath_config();
- cfg1 = snprint_config(conf, NULL, NULL);
+ if (!local)
+ /* "full" configuration */
+ cfg1 = snprint_config(conf, NULL, NULL);
+ else {
+ /* "local" configuration */
+ hwtable = get_used_hwes(hwt->vecs->pathvec);
+ cfg1 = snprint_config(conf, NULL, hwtable);
+ }
assert_non_null(cfg1);
put_multipath_config(conf);
replace_config(hwt, cfg1);
+ /*
+ * The local configuration adds multipath entries, and may move device
+ * entries for local devices to the end of the list. Identical config
+ * strings therefore can't be expected in the "local" case.
+ * That doesn't matter. The important thing is that, with the reloaded
+ * configuration, the test case still passes.
+ */
+ if (local) {
+ free(cfg1);
+ return;
+ }
+
conf = get_multipath_config();
cfg2 = snprint_config(conf, NULL, NULL);
assert_non_null(cfg2);
@@ -504,7 +525,11 @@ static void test_driver(void **state)
_conf = LOAD_CONFIG(hwt);
hwt->test(hwt);
- replicate_config(hwt);
+ replicate_config(hwt, false);
+ reset_vecs(hwt->vecs);
+ hwt->test(hwt);
+
+ replicate_config(hwt, true);
reset_vecs(hwt->vecs);
hwt->test(hwt);
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 22/28] libmultipath: allow printing local maps in snprint_config
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (20 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 21/28] tests/hwtable: add test for local configuration dump Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 23/28] multipathd: implement "show config local" Martin Wilck
` (6 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
"mulitpatd show config" only dumps multipath sections for maps which are
present in local configuration files. This patch adds the ability to dump
"multipath" subsections for every locally detected map, even for those
that have no local configuration, and at the same time, skip dumping
multipath configurations for maps that aree not present locally.
This makes it possible for users to generate a multipath.conf template
matching local devices, and e.g. add explicit alias entries for existing
maps. If user_friendly_names is in use, a commented-out config line is added
to the dump output showing the implicitly configured alias, simplifying
identification of the devices.
This facility is optional and will only be used in new commands added
in follow-up patches.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/print.c | 60 ++++++++++++++++++++++++++++++++++-----
libmultipath/print.h | 3 +-
multipath/main.c | 2 +-
multipathd/cli_handlers.c | 2 +-
tests/hwtable.c | 6 ++--
5 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/libmultipath/print.c b/libmultipath/print.c
index af4c00e2..222d2701 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1404,12 +1404,16 @@ static int snprint_hwtable(const struct config *conf,
static int
snprint_mpentry (const struct config *conf, char * buff, int len,
- const struct mpentry * mpe)
+ const struct mpentry * mpe, const struct _vector *mpvec)
{
int i;
int fwd = 0;
struct keyword * kw;
struct keyword * rootkw;
+ struct multipath *mpp = NULL;
+
+ if (mpvec != NULL && (mpp = find_mp_by_wwid(mpvec, mpe->wwid)) == NULL)
+ return 0;
rootkw = find_keyword(conf->keywords, NULL, "multipath");
if (!rootkw)
@@ -1424,6 +1428,15 @@ snprint_mpentry (const struct config *conf, char * buff, int len,
if (fwd >= len)
return len;
}
+ /*
+ * This mpp doesn't have alias defined. Add the alias in a comment.
+ */
+ if (mpp != NULL && strcmp(mpp->alias, mpp->wwid)) {
+ fwd += snprintf(buff + fwd, len - fwd, "\t\t# alias \"%s\"\n",
+ mpp->alias);
+ if (fwd >= len)
+ return len;
+ }
fwd += snprintf(buff + fwd, len - fwd, "\t}\n");
if (fwd >= len)
return len;
@@ -1431,7 +1444,7 @@ snprint_mpentry (const struct config *conf, char * buff, int len,
}
static int snprint_mptable(const struct config *conf,
- char *buff, int len, vector mptable)
+ char *buff, int len, const struct _vector *mpvec)
{
int fwd = 0;
int i;
@@ -1445,11 +1458,43 @@ static int snprint_mptable(const struct config *conf,
fwd += snprintf(buff + fwd, len - fwd, "multipaths {\n");
if (fwd >= len)
return len;
- vector_foreach_slot (mptable, mpe, i) {
- fwd += snprint_mpentry(conf, buff + fwd, len - fwd, mpe);
+ vector_foreach_slot (conf->mptable, mpe, i) {
+ fwd += snprint_mpentry(conf, buff + fwd, len - fwd, mpe, mpvec);
if (fwd >= len)
return len;
}
+ if (mpvec != NULL) {
+ struct multipath *mpp;
+
+ vector_foreach_slot(mpvec, mpp, i) {
+ if (find_mpe(conf->mptable, mpp->wwid) != NULL)
+ continue;
+
+ fwd += snprintf(buff + fwd, len - fwd,
+ "\tmultipath {\n");
+ if (fwd >= len)
+ return len;
+ fwd += snprintf(buff + fwd, len - fwd,
+ "\t\twwid \"%s\"\n", mpp->wwid);
+ if (fwd >= len)
+ return len;
+ /*
+ * This mpp doesn't have alias defined in
+ * multipath.conf - otherwise find_mpe would have
+ * found it. Add the alias in a comment.
+ */
+ if (strcmp(mpp->alias, mpp->wwid)) {
+ fwd += snprintf(buff + fwd, len - fwd,
+ "\t\t# alias \"%s\"\n",
+ mpp->alias);
+ if (fwd >= len)
+ return len;
+ }
+ fwd += snprintf(buff + fwd, len - fwd, "\t}\n");
+ if (fwd >= len)
+ return len;
+ }
+ }
fwd += snprintf(buff + fwd, len - fwd, "}\n");
if (fwd >= len)
return len;
@@ -1783,7 +1828,7 @@ static int snprint_blacklist_except(const struct config *conf,
}
char *snprint_config(const struct config *conf, int *len,
- const struct _vector *hwtable)
+ const struct _vector *hwtable, const struct _vector *mpvec)
{
char *reply;
/* built-in config is >20kB already */
@@ -1821,9 +1866,10 @@ char *snprint_config(const struct config *conf, int *len,
if ((c - reply) == maxlen)
continue;
- if (VECTOR_SIZE(conf->mptable) > 0)
+ if (VECTOR_SIZE(conf->mptable) > 0 ||
+ (mpvec != NULL && VECTOR_SIZE(mpvec) > 0))
c += snprint_mptable(conf, c, reply + maxlen - c,
- conf->mptable);
+ mpvec);
if ((c - reply) < maxlen) {
if (len)
diff --git a/libmultipath/print.h b/libmultipath/print.h
index c63e0547..608b7d5f 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -120,7 +120,8 @@ int _snprint_multipath_topology (const struct gen_multipath *, char *, int,
int snprint_multipath_topology_json (char * buff, int len,
const struct vectors * vecs);
char *snprint_config(const struct config *conf, int *len,
- const struct _vector *hwtable);
+ const struct _vector *hwtable,
+ const struct _vector *mpvec);
int snprint_multipath_map_json (char * buff, int len,
const struct multipath * mpp, int last);
int snprint_blacklist_report (struct config *, char *, int);
diff --git a/multipath/main.c b/multipath/main.c
index 87d80dd3..54a98026 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -753,7 +753,7 @@ out:
static int
dump_config (struct config *conf)
{
- char * reply = snprint_config(conf, NULL, NULL);
+ char * reply = snprint_config(conf, NULL, NULL, NULL);
if (reply != NULL) {
printf("%s", reply);
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 75cc8a9e..0ac00155 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -253,7 +253,7 @@ show_config (char ** r, int * len, const struct _vector *hwtable)
conf = get_multipath_config();
pthread_cleanup_push(put_multipath_config, conf);
- reply = snprint_config(conf, len, hwtable);
+ reply = snprint_config(conf, len, hwtable, NULL);
pthread_cleanup_pop(1);
if (reply == NULL)
return 1;
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 08ed67d3..1a6b3188 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -458,11 +458,11 @@ static void replicate_config(const struct hwt_state *hwt, bool local)
conf = get_multipath_config();
if (!local)
/* "full" configuration */
- cfg1 = snprint_config(conf, NULL, NULL);
+ cfg1 = snprint_config(conf, NULL, NULL, NULL);
else {
/* "local" configuration */
hwtable = get_used_hwes(hwt->vecs->pathvec);
- cfg1 = snprint_config(conf, NULL, hwtable);
+ cfg1 = snprint_config(conf, NULL, hwtable, hwt->vecs->mpvec);
}
assert_non_null(cfg1);
@@ -483,7 +483,7 @@ static void replicate_config(const struct hwt_state *hwt, bool local)
}
conf = get_multipath_config();
- cfg2 = snprint_config(conf, NULL, NULL);
+ cfg2 = snprint_config(conf, NULL, NULL, NULL);
assert_non_null(cfg2);
put_multipath_config(conf);
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 23/28] multipathd: implement "show config local"
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (21 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 22/28] libmultipath: allow printing local maps in snprint_config Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 24/28] multipath: implement "multipath -T" Martin Wilck
` (5 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
This new command is like "show config", but only those "device" sections are
dumped that match actually present devices in the system. Furthermore, empty
"multipath" sections for all detected multipath maps are dumped. This way,
the output is suitable as a template for creating "multipath.conf".
"multipathd show config local" should produce output that,
when used as configuration file, creates the same configuration
as was in place while it was dumped. Add a test for this to the
test suite.
Some minor differences in the configuration dump can't be avoided
because the ordering of hwtable entries may change (e.g. if a user
adds a device entry for a device which is also in the built-in hwtable,
these entries will be merged end the merged entry will be at the position
of the new entry, i.e. after all built-in hwtable entries), and multipath
sections are added to the configuration. But by all means, path
and mulitpath objects should have the same properties than before.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipathd/cli.c | 2 ++
multipathd/cli.h | 2 ++
multipathd/cli_handlers.c | 28 +++++++++++++++++++++++++---
multipathd/cli_handlers.h | 1 +
multipathd/main.c | 1 +
multipathd/multipathd.8 | 5 +++++
6 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/multipathd/cli.c b/multipathd/cli.c
index d5ee4ff0..a75afe3f 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -212,6 +212,7 @@ load_keys (void)
r += add_key(keys, "setprkey", SETPRKEY, 0);
r += add_key(keys, "unsetprkey", UNSETPRKEY, 0);
r += add_key(keys, "key", KEY, 1);
+ r += add_key(keys, "local", LOCAL, 0);
if (r) {
@@ -549,6 +550,7 @@ cli_init (void) {
add_handler(LIST+MAP+FMT, NULL);
add_handler(LIST+MAP+RAW+FMT, NULL);
add_handler(LIST+CONFIG, NULL);
+ add_handler(LIST+CONFIG+LOCAL, NULL);
add_handler(LIST+BLACKLIST, NULL);
add_handler(LIST+DEVICES, NULL);
add_handler(LIST+WILDCARDS, NULL);
diff --git a/multipathd/cli.h b/multipathd/cli.h
index 021f25b7..7cc7e4be 100644
--- a/multipathd/cli.h
+++ b/multipathd/cli.h
@@ -44,6 +44,7 @@ enum {
__SETPRKEY,
__UNSETPRKEY,
__KEY,
+ __LOCAL,
};
#define LIST (1 << __LIST)
@@ -87,6 +88,7 @@ enum {
#define SETPRKEY (1ULL << __SETPRKEY)
#define UNSETPRKEY (1ULL << __UNSETPRKEY)
#define KEY (1ULL << __KEY)
+#define LOCAL (1ULL << __LOCAL)
#define INITIAL_REPLY_LEN 1200
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 0ac00155..a6ee92b1 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -246,14 +246,15 @@ show_map_json (char ** r, int * len, struct multipath * mpp,
}
static int
-show_config (char ** r, int * len, const struct _vector *hwtable)
+show_config (char ** r, int * len, const struct _vector *hwtable,
+ const struct _vector *mpvec)
{
struct config *conf;
char *reply;
conf = get_multipath_config();
pthread_cleanup_push(put_multipath_config, conf);
- reply = snprint_config(conf, len, hwtable, NULL);
+ reply = snprint_config(conf, len, hwtable, mpvec);
pthread_cleanup_pop(1);
if (reply == NULL)
return 1;
@@ -277,7 +278,28 @@ cli_list_config (void * v, char ** reply, int * len, void * data)
{
condlog(3, "list config (operator)");
- return show_config(reply, len, NULL);
+ return show_config(reply, len, NULL, NULL);
+}
+
+static void v_free(void *x)
+{
+ vector_free(x);
+}
+
+int
+cli_list_config_local (void * v, char ** reply, int * len, void * data)
+{
+ struct vectors * vecs = (struct vectors *)data;
+ vector hwes;
+ int ret;
+
+ condlog(3, "list config local (operator)");
+
+ hwes = get_used_hwes(vecs->pathvec);
+ pthread_cleanup_push(v_free, hwes);
+ ret = show_config(reply, len, hwes, vecs->mpvec);
+ pthread_cleanup_pop(1);
+ return ret;
}
int
diff --git a/multipathd/cli_handlers.h b/multipathd/cli_handlers.h
index 78a3a435..edbdf063 100644
--- a/multipathd/cli_handlers.h
+++ b/multipathd/cli_handlers.h
@@ -16,6 +16,7 @@ int cli_list_maps_topology (void * v, char ** reply, int * len, void * data);
int cli_list_map_json (void * v, char ** reply, int * len, void * data);
int cli_list_maps_json (void * v, char ** reply, int * len, void * data);
int cli_list_config (void * v, char ** reply, int * len, void * data);
+int cli_list_config_local (void * v, char ** reply, int * len, void * data);
int cli_list_blacklist (void * v, char ** reply, int * len, void * data);
int cli_list_devices (void * v, char ** reply, int * len, void * data);
int cli_list_wildcards (void * v, char ** reply, int * len, void * data);
diff --git a/multipathd/main.c b/multipathd/main.c
index 0db88eea..4d891508 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1494,6 +1494,7 @@ uxlsnrloop (void * ap)
set_handler_callback(LIST+MAP+FMT, cli_list_map_fmt);
set_handler_callback(LIST+MAP+RAW+FMT, cli_list_map_fmt);
set_handler_callback(LIST+MAP+JSON, cli_list_map_json);
+ set_handler_callback(LIST+CONFIG+LOCAL, cli_list_config_local);
set_handler_callback(LIST+CONFIG, cli_list_config);
set_handler_callback(LIST+BLACKLIST, cli_list_blacklist);
set_handler_callback(LIST+DEVICES, cli_list_devices);
diff --git a/multipathd/multipathd.8 b/multipathd/multipathd.8
index e78ac9ed..94c3f973 100644
--- a/multipathd/multipathd.8
+++ b/multipathd/multipathd.8
@@ -135,6 +135,11 @@ Show the currently used configuration, derived from default values and values
specified within the configuration file \fI/etc/multipath.conf\fR.
.
.TP
+.B list|show config local
+Show the currently used configuration like \fIshow config\fR, but limiting
+the devices section to those devices that are actually present in the system.
+.
+.TP
.B list|show blacklist
Show the currently used blacklist rules, derived from default values and values
specified within the configuration file \fI/etc/multipath.conf\fR.
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 24/28] multipath: implement "multipath -T"
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (22 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 23/28] multipathd: implement "show config local" Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 25/28] libmultipath: merge "multipath" config sections by wwid Martin Wilck
` (4 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
This does the same as "multipathd show config local", and is
provided for convenience.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.h | 1 +
multipath/main.c | 43 +++++++++++++++++++++++++++----------------
multipath/multipath.8 | 8 +++++++-
3 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/libmultipath/config.h b/libmultipath/config.h
index e1cbd59b..e2fa1852 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -37,6 +37,7 @@ enum mpath_cmds {
CMD_RESET_WWIDS,
CMD_ADD_WWID,
CMD_USABLE_PATHS,
+ CMD_DUMP_CONFIG,
};
enum force_reload_types {
diff --git a/multipath/main.c b/multipath/main.c
index 54a98026..ed4f40ef 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -80,6 +80,19 @@ void put_multipath_config(void *arg)
/* Noop for now */
}
+static int
+dump_config (struct config *conf, vector hwes, vector mpvec)
+{
+ char * reply = snprint_config(conf, NULL, hwes, mpvec);
+
+ if (reply != NULL) {
+ printf("%s", reply);
+ FREE(reply);
+ return 0;
+ } else
+ return 1;
+}
+
void rcu_register_thread_memb(void) {}
void rcu_unregister_thread_memb(void) {}
@@ -112,7 +125,7 @@ usage (char * progname)
fprintf (stderr, " %s [-a|-c|-w|-W] [-d] [-r] [-i] [-v lvl] [-p pol] [-b fil] [-q] [dev]\n", progname);
fprintf (stderr, " %s -l|-ll|-f [-v lvl] [-b fil] [-R num] [dev]\n", progname);
fprintf (stderr, " %s -F [-v lvl] [-R num]\n", progname);
- fprintf (stderr, " %s -t\n", progname);
+ fprintf (stderr, " %s [-t|-T]\n", progname);
fprintf (stderr, " %s -h\n", progname);
fprintf (stderr,
"\n"
@@ -128,6 +141,7 @@ usage (char * progname)
" -q allow queue_if_no_path when multipathd is not running\n"
" -d dry run, do not create or update devmaps\n"
" -t display the currently used multipathd configuration\n"
+ " -T display the multipathd configuration without builtin defaults\n"
" -r force devmap reload\n"
" -i ignore wwids file\n"
" -B treat the bindings file as read only\n"
@@ -671,6 +685,13 @@ configure (struct config *conf, enum mpath_cmds cmd,
filter_pathvec(pathvec, refwwid);
+ if (cmd == CMD_DUMP_CONFIG) {
+ vector hwes = get_used_hwes(pathvec);
+
+ dump_config(conf, hwes, curmp);
+ vector_free(hwes);
+ goto out;
+ }
if (cmd == CMD_VALID_PATH) {
struct path *pp;
@@ -750,19 +771,6 @@ out:
return r;
}
-static int
-dump_config (struct config *conf)
-{
- char * reply = snprint_config(conf, NULL, NULL, NULL);
-
- if (reply != NULL) {
- printf("%s", reply);
- FREE(reply);
- return 0;
- } else
- return 1;
-}
-
static int
get_dev_type(char *dev) {
struct stat buf;
@@ -858,7 +866,7 @@ main (int argc, char *argv[])
exit(1);
multipath_conf = conf;
conf->retrigger_tries = 0;
- while ((arg = getopt(argc, argv, ":adcChl::FfM:v:p:b:BrR:itquUwW")) != EOF ) {
+ while ((arg = getopt(argc, argv, ":adcChl::FfM:v:p:b:BrR:itTquUwW")) != EOF ) {
switch(arg) {
case 1: printf("optarg : %s\n",optarg);
break;
@@ -923,8 +931,11 @@ main (int argc, char *argv[])
conf->find_multipaths |= _FIND_MULTIPATHS_I;
break;
case 't':
- r = dump_config(conf);
+ r = dump_config(conf, NULL, NULL);
goto out_free_config;
+ case 'T':
+ cmd = CMD_DUMP_CONFIG;
+ break;
case 'h':
usage(argv[0]);
exit(0);
diff --git a/multipath/multipath.8 b/multipath/multipath.8
index 914a8cb2..b5e5292f 100644
--- a/multipath/multipath.8
+++ b/multipath/multipath.8
@@ -25,7 +25,7 @@ multipath \- Device mapper target autoconfig.
.RB [\| \-b\ \c
.IR bindings_file \|]
.RB [\| \-d \|]
-.RB [\| \-h | \-l | \-ll | \-f | \-t | \-F | \-B | \-c | \-C | \-q | \-r | \-i | \-a | \-u | \-U | \-w | \-W \|]
+.RB [\| \-h | \-l | \-ll | \-f | \-t | \-T | \-F | \-B | \-c | \-C | \-q | \-r | \-i | \-a | \-u | \-U | \-w | \-W \|]
.RB [\| \-p\ \c
.IR failover | multibus | group_by_serial | group_by_prio | group_by_node_name \|]
.RB [\| \-R\ \c
@@ -89,6 +89,12 @@ Flush all unused multipath device maps.
Display the currently used multipathd configuration.
.
.TP
+.B \-T
+Display the currently used multipathd configuration, limiting the output to
+those devices actually present in the system. This can be used a template for
+creating \fImultipath.conf\fR.
+.
+.TP
.B \-r
Force devmap reload.
.
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 25/28] libmultipath: merge "multipath" config sections by wwid
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (23 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 24/28] multipath: implement "multipath -T" Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 26/28] libmultipath: implement and use blacklist merging Martin Wilck
` (3 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
If more than one "multipath" section exists for a given wwid,
only the properties from the first section are applied, and
those of the later ones are silently discarded.
Fix this by merging mpentries in the same way we do it for hwentries.
Actually, later entries should take precedence, for consistency with
hwentry handling.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.c | 70 +++++++++++++++++++++++++++++++++++++++++++
tests/hwtable.c | 16 ----------
2 files changed, 70 insertions(+), 16 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index fb41d620..c8378f87 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -376,6 +376,74 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
return 0;
}
+static int
+merge_mpe(struct mpentry *dst, struct mpentry *src)
+{
+ if (!dst || !src)
+ return 1;
+
+ merge_str(alias);
+ merge_str(uid_attribute);
+ merge_str(getuid);
+ merge_str(selector);
+ merge_str(features);
+ merge_str(prio_name);
+ merge_str(prio_args);
+
+ if (dst->prkey_source == PRKEY_SOURCE_NONE &&
+ src->prkey_source != PRKEY_SOURCE_NONE) {
+ dst->prkey_source = src->prkey_source;
+ memcpy(&dst->reservation_key, &src->reservation_key,
+ sizeof(dst->reservation_key));
+ }
+
+ merge_num(pgpolicy);
+ merge_num(pgfailback);
+ merge_num(rr_weight);
+ merge_num(no_path_retry);
+ merge_num(minio);
+ merge_num(minio_rq);
+ merge_num(flush_on_last_del);
+ merge_num(attribute_flags);
+ merge_num(user_friendly_names);
+ merge_num(deferred_remove);
+ merge_num(delay_watch_checks);
+ merge_num(delay_wait_checks);
+ merge_num(marginal_path_err_sample_time);
+ merge_num(marginal_path_err_rate_threshold);
+ merge_num(marginal_path_err_recheck_gap_time);
+ merge_num(marginal_path_double_failed_time);
+ merge_num(skip_kpartx);
+ merge_num(max_sectors_kb);
+ merge_num(ghost_delay);
+ merge_num(uid);
+ merge_num(gid);
+ merge_num(mode);
+
+ return 0;
+}
+
+void merge_mptable(vector mptable)
+{
+ struct mpentry *mp1, *mp2;
+ int i, j;
+
+ vector_foreach_slot(mptable, mp1, i) {
+ j = i + 1;
+ vector_foreach_slot_after(mptable, mp2, j) {
+ if (strcmp(mp1->wwid, mp2->wwid))
+ continue;
+ condlog(1, "%s: duplicate multipath config section for %s",
+ __func__, mp1->wwid);
+ merge_mpe(mp2, mp1);
+ free_mpe(mp1);
+ vector_del_slot(mptable, i);
+ i--;
+ break;
+ }
+ }
+}
+
int
store_hwe (vector hwtable, struct hwentry * dhwe)
{
@@ -747,6 +815,8 @@ load_config (char * file)
if (!conf->mptable)
goto out;
}
+
+ merge_mptable(conf->mptable);
if (conf->bindings_file == NULL)
conf->bindings_file = set_default(DEFAULT_BINDINGS_FILE);
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 1a6b3188..f6fba14d 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -1567,8 +1567,6 @@ static int setup_multipath_config(void **state)
*
* Expected: properties are taken from both multipath sections, later taking
* precedence
- *
- * Current: gets properties from first entry only.
*/
static void test_multipath_config_2(const struct hwt_state *hwt)
{
@@ -1580,15 +1578,8 @@ static void test_multipath_config_2(const struct hwt_state *hwt)
assert_ptr_not_equal(mp, NULL);
assert_ptr_not_equal(mp->mpe, NULL);
TEST_PROP(prio_name(&pp->prio), prio_rdac.value);
-#if BROKEN
- condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
- assert_int_equal(mp->minio, DEFAULT_MINIO_RQ);
- condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
- assert_int_equal(mp->no_path_retry, NO_PATH_RETRY_QUEUE);
-#else
assert_int_equal(mp->minio, atoi(minio_99.value));
assert_int_equal(mp->no_path_retry, atoi(npr_37.value));
-#endif
}
static int setup_multipath_config_2(void **state)
@@ -1622,15 +1613,8 @@ static void test_multipath_config_3(const struct hwt_state *hwt)
assert_ptr_not_equal(mp, NULL);
assert_ptr_not_equal(mp->mpe, NULL);
TEST_PROP(prio_name(&pp->prio), prio_rdac.value);
-#if BROKEN
- condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
- assert_int_equal(mp->minio, DEFAULT_MINIO_RQ);
- condlog(1, "%s: WARNING: broken test on %d", __func__, __LINE__ + 1);
- assert_int_equal(mp->no_path_retry, NO_PATH_RETRY_QUEUE);
-#else
assert_int_equal(mp->minio, atoi(minio_99.value));
assert_int_equal(mp->no_path_retry, atoi(npr_37.value));
-#endif
}
static int setup_multipath_config_3(void **state)
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 26/28] libmultipath: implement and use blacklist merging
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (24 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 25/28] libmultipath: merge "multipath" config sections by wwid Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 27/28] libmultipath: escape '"' chars while dumping config Martin Wilck
` (2 subsequent siblings)
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Implement removal of invalid and duplicate entries in blacklists.
With this fix, we can fully enable the config dump/reload test,
which doesn't fail any more.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/blacklist.c | 96 +++++++++++++++++++++++++++++++++-------
libmultipath/blacklist.h | 2 +
libmultipath/config.c | 9 ++++
tests/hwtable.c | 5 ---
4 files changed, 91 insertions(+), 21 deletions(-)
diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index 91ed7ddf..361c6032 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -416,6 +416,15 @@ filter_property(struct config * conf, struct udev_device * udev)
return MATCH_PROPERTY_BLIST_MISSING;
}
+static void free_ble(struct blentry *ble)
+{
+ if (!ble)
+ return;
+ regfree(&ble->regex);
+ FREE(ble->str);
+ FREE(ble);
+}
+
void
free_blacklist (vector blist)
{
@@ -426,15 +435,45 @@ free_blacklist (vector blist)
return;
vector_foreach_slot (blist, ble, i) {
- if (ble) {
- regfree(&ble->regex);
- FREE(ble->str);
- FREE(ble);
- }
+ free_ble(ble);
}
vector_free(blist);
}
+void merge_blacklist(vector blist)
+{
+ struct blentry *bl1, *bl2;
+ int i, j;
+
+ vector_foreach_slot(blist, bl1, i) {
+ j = i + 1;
+ vector_foreach_slot_after(blist, bl2, j) {
+ if (!bl1->str || !bl2->str || strcmp(bl1->str, bl2->str))
+ continue;
+ condlog(3, "%s: duplicate blist entry section for %s",
+ __func__, bl1->str);
+ free_ble(bl2);
+ vector_del_slot(blist, j);
+ j--;
+ }
+ }
+}
+
+static void free_ble_device(struct blentry_device *ble)
+{
+ if (ble) {
+ if (ble->vendor) {
+ regfree(&ble->vendor_reg);
+ FREE(ble->vendor);
+ }
+ if (ble->product) {
+ regfree(&ble->product_reg);
+ FREE(ble->product);
+ }
+ FREE(ble);
+ }
+}
+
void
free_blacklist_device (vector blist)
{
@@ -445,17 +484,42 @@ free_blacklist_device (vector blist)
return;
vector_foreach_slot (blist, ble, i) {
- if (ble) {
- if (ble->vendor) {
- regfree(&ble->vendor_reg);
- FREE(ble->vendor);
- }
- if (ble->product) {
- regfree(&ble->product_reg);
- FREE(ble->product);
- }
- FREE(ble);
- }
+ free_ble_device(ble);
}
vector_free(blist);
}
+
+void merge_blacklist_device(vector blist)
+{
+ struct blentry_device *bl1, *bl2;
+ int i, j;
+
+ vector_foreach_slot(blist, bl1, i) {
+ if (!bl1->vendor && !bl1->product) {
+ free_ble_device(bl1);
+ vector_del_slot(blist, i);
+ i--;
+ }
+ }
+
+ vector_foreach_slot(blist, bl1, i) {
+ j = i + 1;
+ vector_foreach_slot_after(blist, bl2, j) {
+ if ((!bl1->vendor && bl2->vendor) ||
+ (bl1->vendor && !bl2->vendor) ||
+ (bl1->vendor && bl2->vendor &&
+ strcmp(bl1->vendor, bl2->vendor)))
+ continue;
+ if ((!bl1->product && bl2->product) ||
+ (bl1->product && !bl2->product) ||
+ (bl1->product && bl2->product &&
+ strcmp(bl1->product, bl2->product)))
+ continue;
+ condlog(3, "%s: duplicate blist entry section for %s:%s",
+ __func__, bl1->vendor, bl1->product);
+ free_ble_device(bl2);
+ vector_del_slot(blist, j);
+ j--;
+ }
+ }
+}
diff --git a/libmultipath/blacklist.h b/libmultipath/blacklist.h
index 443025d2..0b028d46 100644
--- a/libmultipath/blacklist.h
+++ b/libmultipath/blacklist.h
@@ -40,5 +40,7 @@ int store_ble (vector, char *, int);
int set_ble_device (vector, char *, char *, int);
void free_blacklist (vector);
void free_blacklist_device (vector);
+void merge_blacklist(vector);
+void merge_blacklist_device(vector);
#endif /* _BLACKLIST_H */
diff --git a/libmultipath/config.c b/libmultipath/config.c
index c8378f87..28b76dd2 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -817,6 +817,15 @@ load_config (char * file)
}
merge_mptable(conf->mptable);
+ merge_blacklist(conf->blist_devnode);
+ merge_blacklist(conf->blist_property);
+ merge_blacklist(conf->blist_wwid);
+ merge_blacklist_device(conf->blist_device);
+ merge_blacklist(conf->elist_devnode);
+ merge_blacklist(conf->elist_property);
+ merge_blacklist(conf->elist_wwid);
+ merge_blacklist_device(conf->elist_device);
+
if (conf->bindings_file == NULL)
conf->bindings_file = set_default(DEFAULT_BINDINGS_FILE);
diff --git a/tests/hwtable.c b/tests/hwtable.c
index f6fba14d..7daf71eb 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -499,13 +499,8 @@ static void replicate_config(const struct hwt_state *hwt, bool local)
DUMP_CFG_STR(cfg2);
#endif
-#if BROKEN
- condlog(1, "%s: WARNING: skipping tests for same configuration after dump/reload on %d",
- __func__, __LINE__);
-#else
assert_int_equal(strlen(cfg2), strlen(cfg1));
assert_string_equal(cfg2, cfg1);
-#endif
free(cfg1);
free(cfg2);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 27/28] libmultipath: escape '"' chars while dumping config
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (25 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 26/28] libmultipath: implement and use blacklist merging Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-08 10:20 ` [PATCH 28/28] multipath.conf(5): various corrections and clarifications Martin Wilck
2018-06-18 21:20 ` [PATCH 00/28] multipath-tools: improve config file handling Benjamin Marzinski
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
Since 044c5d6b "libmultipath: config parser: Allow '"' in strings",
double quotes can be part of string values in multipath.conf by escaping
them as double-double quotes ('""'). When dumping the configuration,
the un-escaping done during config file parsing must be reverted by replacing
'"' with '""' again.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/dict.c | 40 ++++++++++++++++++++++++++++++++++++++--
tests/hwtable.c | 18 ------------------
2 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 3e7c5d6d..b5958980 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -109,9 +109,45 @@ print_nonzero (char *buff, int len, long v)
static int
print_str (char *buff, int len, const char *ptr)
{
- if (!ptr)
+ char *p;
+ char *last;
+ const char *q;
+
+ if (!ptr || len <= 0)
return 0;
- return snprintf(buff, len, "\"%s\"", ptr);
+
+ q = strchr(ptr, '"');
+ if (q == NULL)
+ return snprintf(buff, len, "\"%s\"", ptr);
+
+ last = buff + len - 1;
+ p = buff;
+ if (p >= last)
+ goto out;
+ *p++ = '"';
+ if (p >= last)
+ goto out;
+ for (; q; q = strchr(ptr, '"')) {
+ if (q + 1 - ptr < last - p)
+ p = mempcpy(p, ptr, q + 1 - ptr);
+ else {
+ p = mempcpy(p, ptr, last - p);
+ goto out;
+ }
+ *p++ = '"';
+ if (p >= last)
+ goto out;
+ ptr = q + 1;
+ }
+ p += strlcpy(p, ptr, last - p);
+ if (p >= last)
+ goto out;
+ *p++ = '"';
+ *p = '\0';
+ return p - buff;
+out:
+ *p = '\0';
+ return len;
}
static int
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 7daf71eb..9db79391 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -598,18 +598,10 @@ static int setup_internal_nvme(void **state)
/*
* Device section with a simple entry qith double quotes ('foo:"bar"')
*/
-#if BROKEN
-static void test_quoted_hwe(void **state)
-#else
static void test_quoted_hwe(const struct hwt_state *hwt)
-#endif
{
struct path *pp;
-#if BROKEN
- struct hwt_state *hwt = CHECK_STATE(state);
- _conf = LOAD_CONFIG(hwt);
-#endif
/* foo:"bar" matches */
pp = mock_path(vnd_foo.value, prd_baq.value);
TEST_PROP(prio_name(&pp->prio), prio_emc.value);
@@ -625,11 +617,7 @@ static int setup_quoted_hwe(void **state)
const struct key_value kv[] = { vnd_foo, prd_baqq, prio_emc };
WRITE_ONE_DEVICE(hwt, kv);
-#if BROKEN
- condlog(0, "%s: WARNING: skipping conf reload test", __func__);
-#else
SET_TEST_FUNC(hwt, test_quoted_hwe);
-#endif
return 0;
}
@@ -1640,9 +1628,7 @@ static int setup_multipath_config_3(void **state)
}
define_test(string_hwe)
-#if !BROKEN
define_test(quoted_hwe)
-#endif
define_test(internal_nvme)
define_test(regex_hwe)
define_test(regex_string_hwe)
@@ -1680,11 +1666,7 @@ static int test_hwtable(void)
cmocka_unit_test(test_sanity_globals),
test_entry(internal_nvme),
test_entry(string_hwe),
-#if BROKEN
- cmocka_unit_test_setup(test_quoted_hwe, setup_quoted_hwe),
-#else
test_entry(quoted_hwe),
-#endif
test_entry(regex_hwe),
test_entry(regex_string_hwe),
test_entry(regex_string_hwe_dir),
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 28/28] multipath.conf(5): various corrections and clarifications
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (26 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 27/28] libmultipath: escape '"' chars while dumping config Martin Wilck
@ 2018-06-08 10:20 ` Martin Wilck
2018-06-18 21:20 ` [PATCH 00/28] multipath-tools: improve config file handling Benjamin Marzinski
28 siblings, 0 replies; 34+ messages in thread
From: Martin Wilck @ 2018-06-08 10:20 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: dm-devel, Xose Vazquez Perez, Martin Wilck
... related to the topics of option precedence, and config file syntax
in general.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/multipath.conf.5 | 174 +++++++++++++++++++++++--------------
1 file changed, 111 insertions(+), 63 deletions(-)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index f6897954..044204a1 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -85,6 +85,16 @@ the indentation shown in the above example is helpful for human readers but
not mandatory.
.LP
.
+.LP
+.B Note on regular expressions:
+The \fImultipath.conf\fR syntax allows many attribute values to be specified as POSIX
+Extended Regular Expressions (see \fBregex\fR(7)). These regular expressions
+are \fBcase sensitive\fR and \fBnot anchored\fR, thus the expression "bar" matches "barbie",
+"rhabarber", and "wunderbar", but not "Barbie". To avoid unwanted substring
+matches, standard regular expression syntax using the special characters "^" and "$" can be used.
+.
+.LP
+.
The following \fIsection\fP keywords are recognized:
.TP 17
.B defaults
@@ -104,10 +114,12 @@ multipath topology discovery, despite being listed in the
.B multipaths
This section defines the multipath topologies. They are indexed by a
\fIWorld Wide Identifier\fR(WWID). For details on the WWID generation
-see section \fIWWID generation\fR below.
+see section \fIWWID generation\fR below. Attributes set in this section take
+precedence over all others.
.TP
.B devices
-This section defines the device-specific settings.
+This section defines the device-specific settings. Devices are identified by
+vendor, product, and revision.
.TP
.B overrides
This section defines values for attributes that should override the
@@ -1114,100 +1126,112 @@ The default is \fBno\fR
.
.
.\" ----------------------------------------------------------------------------
-.SH "blacklist section"
+.SH "blacklist and blacklist_exceptions sections"
.\" ----------------------------------------------------------------------------
.
-The \fIblacklist\fR section is used to exclude specific device from inclusion in
-the multipath topology. It is most commonly used to exclude local disks or LUNs
-for the array controller.
+The \fIblacklist\fR section is used to exclude specific devices from
+the multipath topology. It is most commonly used to exclude local disks or
+non-disk devices (such as LUNs for the storage array controller) from
+being handled by multipath-tools.
.LP
.
.
-The following keywords are recognized:
+The \fIblacklist_exceptions\fR section is used to revert the actions of the
+\fIblacklist\fR section. This allows one to selectively include ("whitelist") devices which
+would normally be excluded via the \fIblacklist\fR section. A common usage is
+to blacklist "everything" using a catch-all regular expression, and create
+specific blacklist_exceptions entries for those devices that should be handled
+by multipath-tools.
+.LP
+.
+.
+The following keywords are recognized in both sections. The defaults are empty
+unless explicitly stated.
.TP 17
.B devnode
-Regular expression of the device nodes to be excluded.
+Regular expression matching the device nodes to be excluded/included.
.RS
-.TP
-The default is: \fB^(ram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]\fR and \fB^(td|hd|vd)[a-z]\fR
+.PP
+The default \fIblacklist\fR consists of the regular expressions
+"^(ram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]" and
+"^(td|hd|vd)[a-z]". This causes virtual devices, non-disk devices, and some other
+device types to be excluded from multipath handling by default.
.RE
.TP
.B wwid
-The \fIWorld Wide Identification\fR of a device.
-.TP
-.B property
-Regular expression of the udev property to be excluded.
+Regular expression for the \fIWorld Wide Identifier\fR of a device to be excluded/included.
+.
.TP
.B device
Subsection for the device description. This subsection recognizes the
.B vendor
and
.B product
-keywords. For a full description of these keywords please see the
+keywords. Both are regular expressions. For a full description of these keywords please see the
\fIdevices\fR section description.
-.
-.
-.\" ----------------------------------------------------------------------------
-.SH "blacklist_exceptions section"
-.\" ----------------------------------------------------------------------------
-.
-The \fIblacklist_exceptions\fR section is used to revert the actions of the
-\fIblacklist\fR section. For example to include specific device in the
-multipath topology. This allows one to selectively include devices which
-would normally be excluded via the \fIblacklist\fR section.
-.LP
-.
-.
-The following keywords are recognized:
-.TP 17
-.B devnode
-Regular expression of the device nodes to be whitelisted.
-.TP
-.B wwid
-The \fIWorld Wide Identification\fR of a device.
.TP
.B property
-Regular expression of the udev property to be whitelisted.
+Regular expression for an udev property. All
+devices that have matching udev properties will be excluded/included.
+The handling of the \fIproperty\fR keyword is special,
+because devices \fBmust\fR have at least one whitelisted udev property;
+otherwise they're treated as blacklisted, and the message
+"\fIblacklisted, udev property missing\fR" is displayed in the logs.
+.
.RS
-.TP
-The default is: \fB(SCSI_IDENT_|ID_WWN)\fR
+.PP
+The default \fIblacklist exception\fR is: \fB(SCSI_IDENT_|ID_WWN)\fR, causing
+well-behaved SCSI devices and devices that provide a WWN (World Wide Number)
+to be included, and all others to be excluded.
.RE
-.TP
-.B device
-Subsection for the device description. This subsection recognizes the
-.B vendor
-and
-.B product
-keywords. For a full description of these keywords please see the \fIdevices\fR
-section description.
.LP
-The \fIproperty\fR blacklist and whitelist handling is different from the usual
-handling in the sense that the whitelist \fIhas\fR to be set, otherwise the
-device will be blacklisted. In these cases the message \fIblacklisted, udev
-property missing\fR will be displayed.
+For every device, these 4 blacklist criteria are evaluated in the the order
+"property, dev\%node, device, wwid". If a device turns out to be
+blacklisted by any criterion, it's excluded from handling by multipathd, and
+the later criteria aren't evaluated any more. For each
+criterion, the whitelist takes precedence over the blacklist if a device
+matches both.
+.LP
+.B
+Note:
+Besides the blacklist and whitelist, other configuration options such as
+\fIfind_multipaths\fR have an impact on
+whether or not a given device is handled by multipath-tools.
.
.
.\" ----------------------------------------------------------------------------
.SH "multipaths section"
.\" ----------------------------------------------------------------------------
.
+The \fImultipaths\fR section allows setting attributes of multipath maps. The
+attributes that are set via the multipaths section (see list below) take
+precedence over all other configuration settings, including those from the
+\fIoverrides\fR section.
+.LP
The only recognized attribute for the \fImultipaths\fR section is the
-\fImultipath\fR subsection.
+\fImultipath\fR subsection. If there are multiple \fImultipath\fR subsections
+matching a given WWID, the contents of these sections are merged, and settings
+from later entries take precedence.
.LP
.
.
The \fImultipath\fR subsection recognizes the following attributes:
.TP 17
.B wwid
-(Mandatory) Index of the container.
+(Mandatory) World Wide Identifier. Detected multipath maps are matched agains this attribute.
+Note that, unlike the \fIwwid\fR attribute in the \fIblacklist\fR section,
+this is \fBnot\fR a regular expression or a substring; WWIDs must match
+exactly inside the multipaths section.
.TP
.B alias
-Symbolic name for the multipath map.
+Symbolic name for the multipath map. This takes precedence over a an entry for
+the same WWID in the \fIbindings_file\fR.
.LP
.
.
The following attributes are optional; if not set the default values
-are taken from the \fIdefaults\fR or \fIdevices\fR section:
+are taken from the \fIoverrides\fR, \fIdevices\fR, or \fIdefaults\fR
+section:
.sp 1
.PD .1v
.RS
@@ -1266,26 +1290,46 @@ are taken from the \fIdefaults\fR or \fIdevices\fR section:
.SH "devices section"
.\" ----------------------------------------------------------------------------
.
+\fImultipath-tools\fR have a built-in device table with reasonable defaults
+for more than 100 known multipath-capable storage devices. The devices section
+can be used to override these settings. If there are multiple matches for a
+given device, the attributes of all matching entries are applied to it.
+If an attribute is specified in several matching device subsections,
+later entries take precedence. Thus, entries in files under \fIconfig_dir\fR (in
+reverse alphabetical order) have the highest precedence, followed by entries
+in \fImultipath.conf\fR; the built-in hardware table has the lowest
+precedence. Inside a configuration file, later entries have higher precedence
+than earlier ones.
+.LP
The only recognized attribute for the \fIdevices\fR section is the \fIdevice\fR
-subsection.
+subsection. Devices detected in the system are matched against the device entries
+using the \fBvendor\fR, \fBproduct\fR, and \fBrevision\fR fields, which are
+all POSIX Extended regular expressions (see \fBregex\fR(7)).
+.LP
+The vendor, product, and revision fields that multipath or multipathd detect for
+devices in a system depend on the device type. For SCSI devices, they correspond to the
+respective fields of the SCSI INQUIRY page. In general, the command '\fImultipathd show
+paths format "%d %s"\fR' command can be used to see the detected properties
+for all devices in the system.
.LP
-.
.
The \fIdevice\fR subsection recognizes the following attributes:
-.TP
-vendor, product, revision and product_blacklist are POSIX Extended regex.
.TP 17
.B vendor
-(Mandatory) Vendor identifier.
+(Mandatory) Regular expression to match the vendor name.
.TP
.B product
-(Mandatory) Product identifier.
+(Mandatory) Regular expression to match the product name.
.TP
.B revision
-Revision identfier.
+Regular expression to match the product revision. If not specified, any
+revision matches.
.TP
.B product_blacklist
-Product strings to blacklist for this vendor.
+Products with the given \fBvendor\fR matching this string are
+blacklisted. This is equivalent to a \fBdevice\fR entry in the \fIblacklist\fR
+section with the \fIvendor\fR attribute set to this entry's \fIvendor\fR, and
+the \fIproduct\fR attribute set to the value of \fIproduct_blacklist\fR.
.TP
.B alias_prefix
The user_friendly_names prefix to use for this
@@ -1475,8 +1519,12 @@ Multipath uses a \fIWorld Wide Identification\fR (WWID) to determine
which paths belong to the same device. Each path presenting the same
WWID is assumed to point to the same device.
.LP
-The WWID is generated by three methods (in the order of preference):
+The WWID is generated by four methods (in the order of preference):
.TP 17
+.B uid_attrs
+The WWID is derived from udev attributes by matching the device node name. See
+description of \fIuid_attrs\fR in the defaults section above.
+.TP
.B getuid_callout
Use the specified external program; cf \fIgetuid_callout\fR above.
Care should be taken when using this method; the external program
--
2.17.0
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 00/28] multipath-tools: improve config file handling
2018-06-08 10:20 [PATCH 00/28] multipath-tools: improve config file handling Martin Wilck
` (27 preceding siblings ...)
2018-06-08 10:20 ` [PATCH 28/28] multipath.conf(5): various corrections and clarifications Martin Wilck
@ 2018-06-18 21:20 ` Benjamin Marzinski
28 siblings, 0 replies; 34+ messages in thread
From: Benjamin Marzinski @ 2018-06-18 21:20 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel, Xose Vazquez Perez
On Fri, Jun 08, 2018 at 12:20:13PM +0200, Martin Wilck wrote:
Thanks for cleaning this up.
ACK for the whole series 00-30
-Ben
> This patch series fixes a number of small but important issues in
> the way multipath-tools handle configuration files, in particular the
> handling of multiple "device" and "blacklist" sections matching the
> same device. Besides eliminating inconsistencies, the goal of the series is
> to be able to use "multipath -t" or "multipathd show config" ouput as
> configuration for multipath-tools, and obtain exactly the same results
> as with the original configuration.
>
> Late in the series, two new commands "multipath -T" and "multipath show config
> local" are added, which avoid the lengthyness of the full configuration
> dump and produce output more suitable as a local configuration template.
>
> By far the largest part of the series is a new unit test (hwtable.c) for
> configuration file handling. The commit message of the patch introducing
> this unit test ("tests/hwtable: tests for config file handling and hwentry
> merging") explains the problems / inconsistencies that the series is supposed
> to fix. Tests that fail in the first place are marked with the BROKEN macro.
> While reviewing the 2000 LOC of the new unit test is certainly a pain, I'd
> like to ask reviewers to *pay special attention to the tests marked BROKEN*,
> because that's where follow-up patches will change the behavior of
> multipath-tools in user-noticeable ways. I believe it's for the better, but
> I'd like to make sure we all agree. For the rest of the unit test, reviewers
> might appreciate the fact that tests succeed for the current code, and are
> intended as regeression tests for the follow-up patches.
>
> Patch 01-07 are just simple fixes and, as usual, "const" additions.
> Patch 08-10 introduce the new unit test.
> Patch 11-17 fix some basic problems which need to be fixed for dump/reload.
> Patch 18-24 add the ability for dumping the configuration, using the output
> as new configuration input ("reload"), and testing / comparing the results.
> Patch 23-24 implement "multipathd show config local" and "multipath -T" on these
> grounds.
> Patch 23-27 fix more problems that surfaced in the dump/reload test.
> Patch 28 clarifies the documentation of multipath.conf.
>
> Martin Wilck (28):
> kpartx: no need to use FREE_CONST
> libmultipath: fix memory leak in process_config_dir()
> libmultipath: remove superfluous conditionals in load_config()
> libmultipath/structs.c: constify some functions
> libmultipath: some const usage in hwentry handling
> libmultipath: change prototypes of hwe_regmatch() and find_hwe()
> libmultipath/prio: constify simple getters
> tests/Makefile: autogenerate list of symbols to be wrapped
> tests/test-lib: cmocka helpers to simulate path and map discovery
> tests/hwtable: tests for config file handling and hwentry merging
> libmultipath: add debug messages to hwentry lookup/merging code
> libmultipath: use vector for for pp->hwe and mp->hwe
> libmultipath: allow more than one hwentry
> libmultipath: don't merge hwentries by regex
> libmultipath: merge hwentries inside a conf file
> libmultipath/hwtable: remove inherited props from ONTAP NVMe
> libmultipath: don't merge by regex in setup_default_blist()
> multipath, multipathd: consolidate config dumping
> tests/hwtable: implement configuration dump + reload test
> libmultipath: allow dumping only "local" hwtable in snprint_config
> tests/hwtable: add test for local configuration dump
> libmultipath: allow printing local maps in snprint_config
> multipathd: implement "show config local"
> multipath: implement "multipath -T"
> libmultipath: merge "multipath" config sections by wwid
> libmultipath: implement and use blacklist merging
> libmultipath: escape '"' chars while dumping config
> multipath.conf(5): various corrections and clarifications
>
> kpartx/devmapper.c | 10 +-
> libmultipath/blacklist.c | 125 ++-
> libmultipath/blacklist.h | 2 +
> libmultipath/config.c | 208 +++--
> libmultipath/config.h | 5 +-
> libmultipath/dict.c | 40 +-
> libmultipath/discovery.c | 10 +-
> libmultipath/hwtable.c | 3 -
> libmultipath/print.c | 128 ++-
> libmultipath/print.h | 9 +-
> libmultipath/prio.c | 8 +-
> libmultipath/prio.h | 8 +-
> libmultipath/propsel.c | 53 +-
> libmultipath/structs.c | 26 +-
> libmultipath/structs.h | 24 +-
> libmultipath/structs_vec.c | 19 +
> libmultipath/structs_vec.h | 1 +
> libmultipath/vector.c | 12 +
> libmultipath/vector.h | 1 +
> multipath/main.c | 92 +-
> multipath/multipath.8 | 8 +-
> multipath/multipath.conf.5 | 174 ++--
> multipathd/cli.c | 2 +
> multipathd/cli.h | 2 +
> multipathd/cli_handlers.c | 80 +-
> multipathd/cli_handlers.h | 1 +
> multipathd/main.c | 1 +
> multipathd/multipathd.8 | 5 +
> tests/Makefile | 36 +-
> tests/hwtable.c | 1707 ++++++++++++++++++++++++++++++++++++
> tests/test-lib.c | 359 ++++++++
> tests/test-lib.h | 67 ++
> 32 files changed, 2892 insertions(+), 334 deletions(-)
> create mode 100644 tests/hwtable.c
> create mode 100644 tests/test-lib.c
> create mode 100644 tests/test-lib.h
>
> --
> 2.17.0
^ permalink raw reply [flat|nested] 34+ messages in thread