[dm-devel] [PATCH v2 00/12] multipath-tools: add linker version scripts

[dm-devel] [PATCH v2 00/12] multipath-tools: add linker version scripts

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Hi Christophe, hi Ben,

Patch 1-5 are small fixes, the first two resent from an earlier
submission. Patch 6ff. add version scripts for the linker to
libmultipath, libmpathpersist, and libmpathcmd.
(new in v2: versions start at 1.0.0 for all 3 libraries).

Is it useful to do this for libmultipath? We have always said that this is
not a public, stable ABI. However, I still believe it has merits. First of
all, it's a description of the ABI we use. It turns out that it cuts the
size of the exported symbol list of libmultipath roughly in half, which is
better than I'd expected. It leads to ld.so-time failure rather than weird
crashes in the unlikely case that non-matching binaries are used
together. It allows packaging scripts to check compatibility of binaries
and libraries without resorting to version and release. It will help us
stabilize the ABI, albeit only in the long run. Finally, it's a step
towards modernizing our code base in general.

To avoid misunderstanding, my intention is not to provide a stable or even
backward-compatible ABI in libmultipath.so.0. We're still allowed to make
changes to globally visible data structures like "struct config", and to
remove symbols from the ABI, like no serious shared library would do.
We just need to bump the ABI version when we do so.

Changes v1->v2:

08/12: "libmultipath: create separate .so for unit tests"
       fix test_clean rule (Ben)
9/12-11/12: Changed library versions from 0.8.4 to 1.0.0 as discussed with Ben,
       kept Reviewed-by: trailer
12/12 "libmpathpersist: initialize mpp->hwe in get_mpvec()"
      Fix for the issue mentioned by Ben in his review for 02/12
      "libmultipath: copy mpp->hwe from pp->hwe"

Regards
Martin

Martin Wilck (12):
  libmultipath: find_mpe(): don't match with empty WWID
  libmultipath: copy mpp->hwe from pp->hwe
  libmultipath: dm_map_present_by_uuid(): fix dm_task_create() call
  libdmmp tests: fix compilation
  libmultipath: prio: constify some function parameters
  libmultipath: checkers/prio: allow non-lazy .so loading
  multipath-tools Makefiles: separate rules for .so and man pages
  libmultipath: create separate .so for unit tests
  libmultipath: add linker version script
  libmpathpersist: add linker version script
  libmpathcmd: add linker version script
  libmpathpersist: initialize mpp->hwe in get_mpvec()

 libdmmp/test/libdmmp_speed_test.c       |   2 +-
 libdmmp/test/libdmmp_test.c             |   2 +-
 libmpathcmd/Makefile                    |  14 +-
 libmpathcmd/libmpathcmd.version         |  25 +++
 libmpathpersist/Makefile                |  16 +-
 libmpathpersist/libmpathpersist.version |  32 +++
 libmpathpersist/mpath_persist.c         |   6 +-
 libmultipath/Makefile                   |  22 ++-
 libmultipath/checkers.c                 |  17 ++
 libmultipath/config.c                   |   2 +-
 libmultipath/configure.c                |   7 +
 libmultipath/devmapper.c                |   2 +-
 libmultipath/libmultipath.version       | 248 ++++++++++++++++++++++++
 libmultipath/prio.c                     |  26 ++-
 libmultipath/prio.h                     |   4 +-
 libmultipath/propsel.c                  |   4 +-
 libmultipath/structs.c                  |  15 ++
 libmultipath/structs.h                  |   1 +
 libmultipath/structs_vec.c              |  54 +++---
 multipathd/main.c                       |  10 -
 tests/Makefile                          |  10 +-
 21 files changed, 445 insertions(+), 74 deletions(-)
 create mode 100644 libmpathcmd/libmpathcmd.version
 create mode 100644 libmpathpersist/libmpathpersist.version
 create mode 100644 libmultipath/libmultipath.version

-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 01/12] libmultipath: find_mpe(): don't match with empty WWID

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index b9bdbdb..df0f8f4 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -147,7 +147,7 @@ struct mpentry *find_mpe(vector mptable, char *wwid)
 	int i;
 	struct mpentry * mpe;
 
-	if (!wwid)
+	if (!wwid || !*wwid)
 		return NULL;
 
 	vector_foreach_slot (mptable, mpe, i)
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 02/12] libmultipath: copy mpp->hwe from pp->hwe

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Since f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe"),
we've been trying to fix issues caused by paths getting freed and mpp->hwe
dangling. This approach couldn't work because we need mpp->hwe to persist,
even if all paths are removed from the map. Before f0462f0, a simple
assignment worked, because the lifetime of the hwe wasn't bound to the
path. But now, we need to copy the vector. It turns out that we need to set
mpp->hwe only in two places, add_map_with_path() and setup_map(), and
that the code is simplified overall.

Even now, it can happen that a map is added with add_map_without_paths(),
and has no paths. In that case, calling do_set_from_hwe() with a NULL
pointer is not a bug, so remove the message.

Fixes: f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe")
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c   |  7 +++++
 libmultipath/propsel.c     |  4 +--
 libmultipath/structs.c     | 15 +++++++++++
 libmultipath/structs.h     |  1 +
 libmultipath/structs_vec.c | 54 ++++++++++++++++----------------------
 multipathd/main.c          | 10 -------
 6 files changed, 46 insertions(+), 45 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 6fb477f..d7afc91 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -311,6 +311,13 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
 	if (mpp->disable_queueing && VECTOR_SIZE(mpp->paths) != 0)
 		mpp->disable_queueing = 0;
 
+	/*
+	 * If this map was created with add_map_without_path(),
+	 * mpp->hwe might not be set yet.
+	 */
+	if (!mpp->hwe)
+		extract_hwe_from_path(mpp);
+
 	/*
 	 * properties selectors
 	 *
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 7e6e0d6..4020134 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -65,9 +65,7 @@ do {									\
 	__do_set_from_vec(struct hwentry, var, (src)->hwe, dest)
 
 #define do_set_from_hwe(var, src, dest, msg)				\
-	if (!src->hwe) {						\
-		condlog(0, "BUG: do_set_from_hwe called with hwe == NULL"); \
-	} else if (__do_set_from_hwe(var, src, dest)) {			\
+	if (src->hwe && __do_set_from_hwe(var, src, dest)) {		\
 		origin = msg;						\
 		goto out;						\
 	}
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 464596f..2efad6f 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -234,6 +234,17 @@ alloc_multipath (void)
 	return mpp;
 }
 
+void *set_mpp_hwe(struct multipath *mpp, const struct path *pp)
+{
+	if (!mpp || !pp || !pp->hwe)
+		return NULL;
+	if (mpp->hwe)
+		return mpp->hwe;
+	mpp->hwe = vector_convert(NULL, pp->hwe,
+				  struct hwentry, identity);
+	return mpp->hwe;
+}
+
 void free_multipath_attributes(struct multipath *mpp)
 {
 	if (!mpp)
@@ -290,6 +301,10 @@ free_multipath (struct multipath * mpp, enum free_path_mode free_paths)
 
 	free_pathvec(mpp->paths, free_paths);
 	free_pgvec(mpp->pg, free_paths);
+	if (mpp->hwe) {
+		vector_free(mpp->hwe);
+		mpp->hwe = NULL;
+	}
 	FREE_PTR(mpp->mpcontext);
 	FREE(mpp);
 }
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 7de93d6..4ce3055 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -422,6 +422,7 @@ struct host_group {
 struct path * alloc_path (void);
 struct pathgroup * alloc_pathgroup (void);
 struct multipath * alloc_multipath (void);
+void *set_mpp_hwe(struct multipath *mpp, const struct path *pp);
 void uninitialize_path(struct path *pp);
 void free_path (struct path *);
 void free_pathvec (vector vec, enum free_path_mode free_paths);
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 8895fa7..f7f45f1 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -294,11 +294,6 @@ err:
 void orphan_path(struct path *pp, const char *reason)
 {
 	condlog(3, "%s: orphan path, %s", pp->dev, reason);
-	if (pp->mpp && pp->hwe && pp->mpp->hwe == pp->hwe) {
-		condlog(0, "BUG: orphaning path %s that holds hwe of %s",
-			pp->dev, pp->mpp->alias);
-		pp->mpp->hwe = NULL;
-	}
 	pp->mpp = NULL;
 	uninitialize_path(pp);
 }
@@ -308,8 +303,6 @@ void orphan_paths(vector pathvec, struct multipath *mpp, const char *reason)
 	int i;
 	struct path * pp;
 
-	/* Avoid BUG message from orphan_path() */
-	mpp->hwe = NULL;
 	vector_foreach_slot (pathvec, pp, i) {
 		if (pp->mpp == mpp) {
 			if (pp->initialized == INIT_REMOVED) {
@@ -397,24 +390,26 @@ extract_hwe_from_path(struct multipath * mpp)
 	if (mpp->hwe || !mpp->paths)
 		return;
 
-	condlog(3, "%s: searching paths for valid hwe", mpp->alias);
+	condlog(4, "%s: searching paths for valid hwe", mpp->alias);
 	/* doing this in two passes seems like paranoia to me */
 	vector_foreach_slot(mpp->paths, pp, i) {
-		if (pp->state != PATH_UP)
-			continue;
-		if (pp->hwe) {
-			mpp->hwe = pp->hwe;
-			return;
-		}
+		if (pp->state == PATH_UP &&
+		    pp->initialized != INIT_REMOVED && pp->hwe)
+			goto done;
 	}
 	vector_foreach_slot(mpp->paths, pp, i) {
-		if (pp->state == PATH_UP)
-			continue;
-		if (pp->hwe) {
-			mpp->hwe = pp->hwe;
-			return;
-		}
+		if (pp->state != PATH_UP &&
+		    pp->initialized != INIT_REMOVED && pp->hwe)
+			goto done;
 	}
+done:
+	if (i < VECTOR_SIZE(mpp->paths))
+		(void)set_mpp_hwe(mpp, pp);
+
+	if (mpp->hwe)
+		condlog(3, "%s: got hwe from path %s", mpp->alias, pp->dev);
+	else
+		condlog(2, "%s: no hwe found", mpp->alias);
 }
 
 int
@@ -514,8 +509,6 @@ void sync_paths(struct multipath *mpp, vector pathvec)
 		}
 		if (!found) {
 			condlog(3, "%s dropped path %s", mpp->alias, pp->dev);
-			if (mpp->hwe == pp->hwe)
-				mpp->hwe = NULL;
 			vector_del_slot(mpp->paths, i--);
 			orphan_path(pp, "path removed externally");
 		}
@@ -524,8 +517,6 @@ void sync_paths(struct multipath *mpp, vector pathvec)
 	update_mpp_paths(mpp, pathvec);
 	vector_foreach_slot (mpp->paths, pp, i)
 		pp->mpp = mpp;
-	if (mpp->hwe == NULL)
-		extract_hwe_from_path(mpp);
 }
 
 int
@@ -701,9 +692,15 @@ struct multipath *add_map_with_path(struct vectors *vecs, struct path *pp,
 
 	conf = get_multipath_config();
 	mpp->mpe = find_mpe(conf->mptable, pp->wwid);
-	mpp->hwe = pp->hwe;
 	put_multipath_config(conf);
 
+	/*
+	 * We need to call this before select_alias(),
+	 * because that accesses hwe properties.
+	 */
+	if (pp->hwe && !set_mpp_hwe(mpp, pp))
+		goto out;
+
 	strcpy(mpp->wwid, pp->wwid);
 	find_existing_alias(mpp, vecs);
 	if (select_alias(conf, mpp))
@@ -754,12 +751,6 @@ int verify_paths(struct multipath *mpp)
 			vector_del_slot(mpp->paths, i);
 			i--;
 
-			/* Make sure mpp->hwe doesn't point to freed memory.
-			 * We call extract_hwe_from_path() below to restore
-			 * mpp->hwe
-			 */
-			if (mpp->hwe == pp->hwe)
-				mpp->hwe = NULL;
 			/*
 			 * Don't delete path from pathvec yet. We'll do this
 			 * after the path has been removed from the map, in
@@ -771,7 +762,6 @@ int verify_paths(struct multipath *mpp)
 				mpp->alias, pp->dev, pp->dev_t);
 		}
 	}
-	extract_hwe_from_path(mpp);
 	return count;
 }
 
diff --git a/multipathd/main.c b/multipathd/main.c
index a4abbb2..eedc6c1 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1153,13 +1153,6 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
 		if (i != -1)
 			vector_del_slot(mpp->paths, i);
 
-		/*
-		 * Make sure mpp->hwe doesn't point to freed memory
-		 * We call extract_hwe_from_path() below to restore mpp->hwe
-		 */
-		if (mpp->hwe == pp->hwe)
-			mpp->hwe = NULL;
-
 		/*
 		 * remove the map IF removing the last path
 		 */
@@ -1191,9 +1184,6 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
 			 */
 		}
 
-		if (mpp->hwe == NULL)
-			extract_hwe_from_path(mpp);
-
 		if (setup_map(mpp, params, PARAMS_SIZE, vecs)) {
 			condlog(0, "%s: failed to setup map for"
 				" removal of path %s", mpp->alias, pp->dev);
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 03/12] libmultipath: dm_map_present_by_uuid(): fix dm_task_create() call

[mwilck]

From: Martin Wilck <mwilck@suse.com>

libmultipath shouldn't call dm_task_create() directly any more.

Fixes: 90535a3 ("multipath: centralize validation code")
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/devmapper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index b4d77cb..7394796 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -834,7 +834,7 @@ dm_map_present_by_uuid(const char *uuid)
 	if (safe_sprintf(prefixed_uuid, UUID_PREFIX "%s", uuid))
 		goto out;
 
-	if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
+	if (!(dmt = libmp_dm_task_create(DM_DEVICE_INFO)))
 		goto out;
 
 	dm_task_no_open_count(dmt);
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 04/12] libdmmp tests: fix compilation

[mwilck]

From: Martin Wilck <mwilck@suse.com>

These tests don't compile with -Werror=unused-parameter. Fix it.
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libdmmp/test/libdmmp_speed_test.c | 2 +-
 libdmmp/test/libdmmp_test.c       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libdmmp/test/libdmmp_speed_test.c b/libdmmp/test/libdmmp_speed_test.c
index 372cd39..d91ba50 100644
--- a/libdmmp/test/libdmmp_speed_test.c
+++ b/libdmmp/test/libdmmp_speed_test.c
@@ -27,7 +27,7 @@
 
 #include <libdmmp/libdmmp.h>
 
-int main(int argc, char *argv[])
+int main(void)
 {
 	struct dmmp_context *ctx = NULL;
 	struct dmmp_mpath **dmmp_mps = NULL;
diff --git a/libdmmp/test/libdmmp_test.c b/libdmmp/test/libdmmp_test.c
index d944e1e..a940b57 100644
--- a/libdmmp/test/libdmmp_test.c
+++ b/libdmmp/test/libdmmp_test.c
@@ -102,7 +102,7 @@ out:
 	return rc;
 }
 
-int main(int argc, char *argv[])
+int main(void)
 {
 	struct dmmp_context *ctx = NULL;
 	struct dmmp_mpath **dmmp_mps = NULL;
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 05/12] libmultipath: prio: constify some function parameters

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/prio.c | 4 ++--
 libmultipath/prio.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index 194563c..3a718ba 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -18,7 +18,7 @@ unsigned int get_prio_timeout(unsigned int timeout_ms,
 	return default_timeout;
 }
 
-int init_prio (char *multipath_dir)
+int init_prio (const char *multipath_dir)
 {
 	if (!add_prio(multipath_dir, DEFAULT_PRIO))
 		return 1;
@@ -87,7 +87,7 @@ int prio_set_args (struct prio * p, const char * args)
 	return snprintf(p->args, PRIO_ARGS_LEN, "%s", args);
 }
 
-struct prio * add_prio (char *multipath_dir, char * name)
+struct prio * add_prio (const char *multipath_dir, const char * name)
 {
 	char libname[LIB_PRIO_NAMELEN];
 	struct stat stbuf;
diff --git a/libmultipath/prio.h b/libmultipath/prio.h
index 599d1d8..26754f7 100644
--- a/libmultipath/prio.h
+++ b/libmultipath/prio.h
@@ -55,9 +55,9 @@ struct prio {
 
 unsigned int get_prio_timeout(unsigned int checker_timeout,
 			      unsigned int default_timeout);
-int init_prio (char *);
+int init_prio (const char *);
 void cleanup_prio (void);
-struct prio * add_prio (char *, char *);
+struct prio * add_prio (const char *, const char *);
 int prio_getprio (struct prio *, struct path *, unsigned int);
 void prio_get (char *, struct prio *, char *, char *);
 void prio_put (struct prio *);
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 06/12] libmultipath: checkers/prio: allow non-lazy .so loading

[mwilck]

From: Martin Wilck <mwilck@suse.com>

If compiled with -DLOAD_ALL_SHARED_LIBS, all known prioritizers
and checkers will be loaded immediately on startup. This is useful
for determining symbol usage (start executables with LD_BIND_NOW=1).

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers.c | 17 +++++++++++++++++
 libmultipath/prio.c     | 22 ++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c
index f7ddd53..18b1f5e 100644
--- a/libmultipath/checkers.c
+++ b/libmultipath/checkers.c
@@ -7,6 +7,7 @@
 #include "debug.h"
 #include "checkers.h"
 #include "vector.h"
+#include "util.h"
 
 struct checker_class {
 	struct list_head node;
@@ -375,7 +376,23 @@ void checker_get(const char *multipath_dir, struct checker *dst,
 
 int init_checkers(const char *multipath_dir)
 {
+#ifdef LOAD_ALL_SHARED_LIBS
+	static const char *const all_checkers[] = {
+		DIRECTIO,
+		TUR,
+		HP_SW,
+		RDAC,
+		EMC_CLARIION,
+		READSECTOR0,
+		CCISS_TUR,
+	};
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(all_checkers); i++)
+		add_checker_class(multipath_dir, all_checkers[i]);
+#else
 	if (!add_checker_class(multipath_dir, DEFAULT_CHECKER))
 		return 1;
+#endif
 	return 0;
 }
diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index 3a718ba..c92bde7 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -20,8 +20,30 @@ unsigned int get_prio_timeout(unsigned int timeout_ms,
 
 int init_prio (const char *multipath_dir)
 {
+#ifdef LOAD_ALL_SHARED_LIBS
+	static const char *const all_prios[] = {
+		PRIO_ALUA,
+		PRIO_CONST,
+		PRIO_DATACORE,
+		PRIO_EMC,
+		PRIO_HDS,
+		PRIO_HP_SW,
+		PRIO_ONTAP,
+		PRIO_RANDOM,
+		PRIO_RDAC,
+		PRIO_WEIGHTED_PATH,
+		PRIO_SYSFS,
+		PRIO_PATH_LATENCY,
+		PRIO_ANA,
+	};
+	unsigned int i;
+
+	for  (i = 0; i < ARRAY_SIZE(all_prios); i++)
+		add_prio(multipath_dir, all_prios[i]);
+#else
 	if (!add_prio(multipath_dir, DEFAULT_PRIO))
 		return 1;
+#endif
 	return 0;
 }
 
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 07/12] multipath-tools Makefiles: separate rules for .so and man pages

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Rely more on "make" functionality than on sequential command execution.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathcmd/Makefile     |  8 +++++---
 libmpathpersist/Makefile | 10 +++++++---
 libmultipath/Makefile    |  8 +++++---
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/libmpathcmd/Makefile b/libmpathcmd/Makefile
index 0f6b816..08ccb81 100644
--- a/libmpathcmd/Makefile
+++ b/libmpathcmd/Makefile
@@ -8,13 +8,15 @@ CFLAGS += $(LIB_CFLAGS)
 
 OBJS = mpath_cmd.o
 
-all: $(LIBS)
+all:	$(DEVLIB)
 
 $(LIBS): $(OBJS)
 	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ -o $@ $(OBJS) $(LIBDEPS)
-	$(LN) $@ $(DEVLIB)
 
-install: $(LIBS)
+$(DEVLIB): $(LIBS)
+	$(LN) $(LIBS) $@
+
+install: all
 	$(INSTALL_PROGRAM) -d $(DESTDIR)$(syslibdir)
 	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
 	$(LN) $(LIBS) $(DESTDIR)$(syslibdir)/$(DEVLIB)
diff --git a/libmpathpersist/Makefile b/libmpathpersist/Makefile
index 21fdad8..9e869fd 100644
--- a/libmpathpersist/Makefile
+++ b/libmpathpersist/Makefile
@@ -11,15 +11,19 @@ LIBDEPS += -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath \
 
 OBJS = mpath_persist.o mpath_updatepr.o mpath_pr_ioctl.o
 
-all: $(LIBS)
+all: $(DEVLIB) man
 
 $(LIBS): $(OBJS)
 	$(CC) $(LDFLAGS) $(SHARED_FLAGS) $(LIBDEPS) -Wl,-soname=$@ -o $@ $(OBJS)
-	$(LN) $(LIBS) $(DEVLIB)
+
+$(DEVLIB): $(LIBS)
+	$(LN) $(LIBS) $@
+
+man:
 	$(GZIP) mpath_persistent_reserve_in.3 > mpath_persistent_reserve_in.3.gz
 	$(GZIP) mpath_persistent_reserve_out.3 > mpath_persistent_reserve_out.3.gz
 
-install: $(LIBS)
+install: all
 	$(INSTALL_PROGRAM) -d $(DESTDIR)$(syslibdir)
 	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
 	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)$(syslibdir)
diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index e5dac5e..af5bb77 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -50,7 +50,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \
 	io_err_stat.o dm-generic.o generic.o foreign.o nvme-lib.o \
 	libsg.o valid.o
 
-all: $(LIBS)
+all:	$(DEVLIB)
 
 nvme-lib.o: nvme-lib.c nvme-ioctl.c nvme-ioctl.h
 	$(CC) $(CFLAGS) -Wno-unused-function -c -o $@ $<
@@ -70,9 +70,11 @@ nvme-ioctl.h: nvme/nvme-ioctl.h
 
 $(LIBS): $(OBJS)
 	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ -o $@ $(OBJS) $(LIBDEPS)
-	$(LN) $@ $(DEVLIB)
 
-install:
+$(DEVLIB): $(LIBS)
+	$(LN) $(LIBS) $@
+
+install: all
 	$(INSTALL_PROGRAM) -d $(DESTDIR)$(syslibdir)
 	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
 	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)$(libdir)
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 08/12] libmultipath: create separate .so for unit tests

[mwilck]

From: Martin Wilck <mwilck@suse.com>

The unit tests use a lot of internal symbols that we don't want
to add to the ABI if we don't have to. As long as we don't
have to make incompatible changes to functions, we can work around
that by simply using a non-versioned library for the unit tests.
Therefore we add a seperate rule here. Do this before actually
adding a version script, to avoid breakage during bisection.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/Makefile |  7 +++++++
 tests/Makefile        | 10 ++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index af5bb77..cf13561 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -74,6 +74,13 @@ $(LIBS): $(OBJS)
 $(DEVLIB): $(LIBS)
 	$(LN) $(LIBS) $@
 
+../tests/$(LIBS): $(OBJS) $(VERSION_SCRIPT)
+	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=`basename $@` \
+		-o $@ $(OBJS) $(LIBDEPS)
+	$(LN) $@ ${@:.so.0=.so}
+
+test-lib:	../tests/$(LIBS)
+
 install: all
 	$(INSTALL_PROGRAM) -d $(DESTDIR)$(syslibdir)
 	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
diff --git a/tests/Makefile b/tests/Makefile
index 502377f..f6ef16a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -10,7 +10,7 @@ W_MISSING_INITIALIZERS := $(call TEST_MISSING_INITIALIZERS)
 
 CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir) \
 	-Wno-unused-parameter $(W_MISSING_INITIALIZERS)
-LIBDEPS += -L$(multipathdir) -L$(mpathcmddir) -lmultipath -lmpathcmd -lcmocka
+LIBDEPS += -L. -L$(mpathcmddir) -lmultipath -lmpathcmd -lcmocka
 
 TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \
 	 alias directio valid devt
@@ -67,7 +67,7 @@ lib/libchecktur.so:
 
 %.out:	%-test lib/libchecktur.so
 	@echo == running $< ==
-	@LD_LIBRARY_PATH=$(multipathdir):$(mpathcmddir) ./$< >$@
+	@LD_LIBRARY_PATH=.:$(mpathcmddir) ./$< >$@
 
 %.vgr:  %-test lib/libchecktur.so
 	@echo == running valgrind for $< ==
@@ -77,7 +77,7 @@ lib/libchecktur.so:
 OBJS = $(TESTS:%=%.o) test-lib.o
 
 test_clean:
-	$(RM) $(TESTS:%=%.out) $(TESTS:%=%.vgr)
+	$(RM) $(TESTS:%=%.out) $(TESTS:%=%.vgr) *.so*
 
 valgrind_clean:
 	$(RM) $(TESTS:%=%.vgr)
@@ -97,12 +97,14 @@ dep_clean:
 	@sed -n 's/^.*__wrap_\([a-zA-Z0-9_]*\).*$$/-Wl,--wrap=\1/p' $< | \
 		sort -u | tr '\n' ' ' >$@
 
+libmultipath.so.0:
+	$(MAKE) -C $(multipathdir) test-lib
 
 # 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
+		libmultipath.so.0 Makefile
 	$(CC) $(CFLAGS) -o $@ $(LDFLAGS) $< $($@_TESTDEPS) $($@_OBJDEPS) \
 		$(LIBDEPS) $($@_LIBDEPS) \
 		$(shell cat $<.wrap) $(foreach dep,$($@_TESTDEPS),$(shell cat $(dep).wrap))
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 09/12] libmultipath: add linker version script

[mwilck]

From: Martin Wilck <mwilck@suse.com>

This version script documents the current ABI of libmultipath,
as used by multipath, multipathd, (lib)mpathpersist, and the
dynamically loaded libraries (prioritizers, checkers, and foreign).
The initial version string is set to "LIBMULTIPATH_1.0.0".

This reduces the amount of exported symbols of libmultipath.so.0
by more than a half (199 vs. 434).

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/Makefile             |   7 +-
 libmultipath/libmultipath.version | 248 ++++++++++++++++++++++++++++++
 2 files changed, 253 insertions(+), 2 deletions(-)
 create mode 100644 libmultipath/libmultipath.version

diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index cf13561..bfc7263 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -6,6 +6,7 @@ include ../Makefile.inc
 SONAME = 0
 DEVLIB = libmultipath.so
 LIBS = $(DEVLIB).$(SONAME)
+VERSION_SCRIPT := libmultipath.version
 
 CFLAGS += $(LIB_CFLAGS) -I$(mpathcmddir) -I$(mpathpersistdir) -I$(nvmedir)
 
@@ -68,8 +69,10 @@ nvme-ioctl.c: nvme/nvme-ioctl.c
 nvme-ioctl.h: nvme/nvme-ioctl.h
 	$(call make_static,$<,$@)
 
-$(LIBS): $(OBJS)
-	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ -o $@ $(OBJS) $(LIBDEPS)
+
+$(LIBS): $(OBJS) $(VERSION_SCRIPT)
+	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ \
+		-Wl,--version-script=$(VERSION_SCRIPT) -o $@ $(OBJS) $(LIBDEPS)
 
 $(DEVLIB): $(LIBS)
 	$(LN) $(LIBS) $@
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
new file mode 100644
index 0000000..a6bf821
--- /dev/null
+++ b/libmultipath/libmultipath.version
@@ -0,0 +1,248 @@
+/*
+ * Copyright (c) 2020 SUSE LLC
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * libmultipath ABI
+ *
+ * libmultipath doesn't have a stable ABI in the usual sense. In particular,
+ * the library does not attempt to ship different versions of the same symbol
+ * for backward compatibility.
+ *
+ * The ABI versioning only serves to avoid linking with a non-matching ABI, to
+ * cut down the set of exported symbols, and to describe it.
+ * The version string is LIBMULTIPATH_$MAJOR.$MINOR.$REL.
+ *
+ * Policy:
+ *
+ * * Bump $MAJOR for incompatible changes, like:
+ *   - symbols removed
+ *   - parameter list or return values changed for existing functions
+ *   - externally visible data structures changed in incompatible ways
+ *     (like offsets of previously existing struct members)
+ *   In this case, the new version doesn't inherit the previous versions,
+ *   because the new library doesn't provide the full previous ABI any more.
+ *   All predecessors are merged into the new version.
+ *
+ * * Bump $MINOR for compatible changes, like adding symbols.
+ *   The new version inherits the previous ones.
+ *
+ * * Bump $REL to describe deviations from upstream, e.g. in
+ *   multipath-tools packages shipped by distributions.
+ *   The new version inherits the previous ones.
+ */
+
+LIBMULTIPATH_1.0.0 {
+global:
+	/* symbols referenced by multipath and multipathd */
+	add_foreign;
+	add_map_with_path;
+	adopt_paths;
+	alloc_multipath;
+	alloc_path;
+	alloc_path_with_pathinfo;
+	alloc_strvec;
+	change_foreign;
+	check_alias_settings;
+	checker_clear_message;
+	checker_disable;
+	checker_enable;
+	checker_is_sync;
+	checker_message;
+	checker_name;
+	checker_state_name;
+	check_foreign;
+	cleanup_checkers;
+	cleanup_foreign;
+	cleanup_lock;
+	cleanup_prio;
+	close_fd;
+	coalesce_paths;
+	convert_dev;
+	count_active_paths;
+	delete_all_foreign;
+	delete_foreign;
+	disassemble_map;
+	disassemble_status;
+	dlog;
+	dm_cancel_deferred_remove;
+	dm_drv_version;
+	dm_enablegroup;
+	dm_fail_path;
+	_dm_flush_map;
+	dm_flush_map_nopaths;
+	dm_flush_maps;
+	dm_geteventnr;
+	dm_get_info;
+	dm_get_major_minor;
+	dm_get_map;
+	dm_get_maps;
+	dm_get_multipath;
+	dm_get_status;
+	dm_get_uuid;
+	dm_is_mpath;
+	dm_mapname;
+	dm_map_present;
+	dm_queue_if_no_path;
+	dm_reassign;
+	dm_reinstate_path;
+	dm_simplecmd_noflush;
+	dm_switchgroup;
+	dm_tgt_version;
+	domap;
+	ensure_directories_exist;
+	extract_hwe_from_path;
+	filter_devnode;
+	filter_path;
+	filter_wwid;
+	find_mp_by_alias;
+	find_mp_by_minor;
+	find_mp_by_str;
+	find_mp_by_wwid;
+	find_mpe;
+	find_path_by_dev;
+	find_path_by_devt;
+	find_slot;
+	foreign_multipath_layout;
+	foreign_path_layout;
+	free_config;
+	free_multipath;
+	free_multipathvec;
+	free_path;
+	free_pathvec;
+	free_strvec;
+	get_monotonic_time;
+	get_multipath_layout;
+	get_path_layout;
+	get_pgpolicy_id;
+	get_refwwid;
+	get_state;
+	get_udev_device;
+	get_uid;
+	get_used_hwes;
+	group_by_prio;
+	init_checkers;
+	init_foreign;
+	init_prio;
+	io_err_stat_attr;
+	io_err_stat_handle_pathfail;
+	is_path_valid;
+	is_quote;
+	libmp_dm_task_create;
+	libmp_udev_set_sync_support;
+	load_config;
+	log_thread_reset;
+	log_thread_start;
+	log_thread_stop;
+	need_io_err_check;
+	normalize_timespec;
+	orphan_path;
+	orphan_paths;
+	parse_prkey_flags;
+	pathcount;
+	path_discovery;
+	path_get_tpgs;
+	pathinfo;
+	path_offline;
+	print_all_paths;
+	print_foreign_topology;
+	_print_multipath_topology;
+	pthread_cond_init_mono;
+	recv_packet;
+	recv_packet_from_client;
+	reinstate_paths;
+	remember_wwid;
+	remove_map;
+	remove_map_by_alias;
+	remove_maps;
+	remove_wwid;
+	replace_wwids;
+	reset_checker_classes;
+	select_all_tg_pt;
+	select_action;
+	select_find_multipaths_timeout;
+	select_no_path_retry;
+	select_path_group;
+	select_reservation_key;
+	send_packet;
+	set_max_fds;
+	__set_no_path_retry;
+	set_path_removed;
+	set_prkey;
+	setup_map;
+	setup_thread_attr;
+	should_multipath;
+	snprint_blacklist_report;
+	snprint_config;
+	snprint_devices;
+	snprint_foreign_multipaths;
+	snprint_foreign_paths;
+	snprint_foreign_topology;
+	_snprint_multipath;
+	snprint_multipath_header;
+	snprint_multipath_map_json;
+	_snprint_multipath_topology;
+	snprint_multipath_topology_json;
+	_snprint_path;
+	snprint_path_header;
+	snprint_status;
+	snprint_wildcards;
+	stop_io_err_stat_thread;
+	store_path;
+	store_pathinfo;
+	strchop;
+	strlcpy;
+	sync_map_state;
+	sysfs_attr_set_value;
+	sysfs_get_size;
+	sysfs_is_multipathed;
+	timespecsub;
+	trigger_paths_udev_change;
+	uevent_dispatch;
+	uevent_get_dm_str;
+	uevent_get_env_positive_int;
+	uevent_is_mpath;
+	uevent_listen;
+	update_mpp_paths;
+	update_multipath_status;
+	update_multipath_strings;
+	update_multipath_table;
+	update_pathvec_from_dm;
+	update_queue_mode_add_path;
+	update_queue_mode_del_path;
+	ux_socket_listen;
+	valid_alias;
+	vector_alloc;
+	vector_alloc_slot;
+	vector_del_slot;
+	vector_free;
+	vector_reset;
+	vector_set_slot;
+	verify_paths;
+
+	/* checkers */
+	sg_read;
+
+	/* prioritizers */
+	get_asymmetric_access_state;
+	get_prio_timeout;
+	get_target_port_group;
+	get_target_port_group_support;
+	libmp_nvme_ana_log;
+	libmp_nvme_get_nsid;
+	libmp_nvme_identify_ns;
+	log_nvme_errcode;
+	nvme_id_ctrl_ana;
+	snprint_host_wwnn;
+	snprint_host_wwpn;
+	snprint_path_serial;
+	snprint_tgt_wwnn;
+	snprint_tgt_wwpn;
+	sysfs_get_asymmetric_access_state;
+
+	/* foreign */
+	free_scandir_result;
+	sysfs_attr_get_value;
+
+local:
+	*;
+};
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 10/12] libmpathpersist: add linker version script

[mwilck]

From: Martin Wilck <mwilck@suse.com>

This defines the ABI of libmpathpersist in the current state.
The initial version is set to "LIBMPATHPERSIST_1.0.0".

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/Makefile                |  6 +++--
 libmpathpersist/libmpathpersist.version | 32 +++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 libmpathpersist/libmpathpersist.version

diff --git a/libmpathpersist/Makefile b/libmpathpersist/Makefile
index 9e869fd..456ce4c 100644
--- a/libmpathpersist/Makefile
+++ b/libmpathpersist/Makefile
@@ -3,6 +3,7 @@ include ../Makefile.inc
 SONAME = 0
 DEVLIB = libmpathpersist.so
 LIBS = $(DEVLIB).$(SONAME)
+VERSION_SCRIPT := libmpathpersist.version
 
 CFLAGS += $(LIB_CFLAGS) -I$(multipathdir) -I$(mpathpersistdir) -I$(mpathcmddir)
 
@@ -13,8 +14,9 @@ OBJS = mpath_persist.o mpath_updatepr.o mpath_pr_ioctl.o
 
 all: $(DEVLIB) man
 
-$(LIBS): $(OBJS)
-	$(CC) $(LDFLAGS) $(SHARED_FLAGS) $(LIBDEPS) -Wl,-soname=$@ -o $@ $(OBJS)
+$(LIBS): $(OBJS) $(VERSION_SCRIPT)
+	$(CC) $(LDFLAGS) $(SHARED_FLAGS) $(LIBDEPS) -Wl,-soname=$@ \
+		-Wl,--version-script=$(VERSION_SCRIPT) -o $@ $(OBJS)
 
 $(DEVLIB): $(LIBS)
 	$(LN) $(LIBS) $@
diff --git a/libmpathpersist/libmpathpersist.version b/libmpathpersist/libmpathpersist.version
new file mode 100644
index 0000000..dc648ce
--- /dev/null
+++ b/libmpathpersist/libmpathpersist.version
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2020 SUSE LLC
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * libmpathpersist ABI
+ *
+ * The ABI of libmpathpersist is supposed to remain stable. Removing symbols
+ * or altering existing symbols' semantics is not allowed. When changing a
+ * a symbol, either use a new name, or explicit symver directives.
+ *
+ * See libmultipath.version for general policy about version numbers.
+ */
+LIBMPATHPERSIST_1.0.0 {
+global:
+
+	__mpath_persistent_reserve_in;
+	__mpath_persistent_reserve_out;
+	dumpHex;
+	mpath_alloc_prin_response;
+	mpath_lib_exit;
+	mpath_lib_init;
+	mpath_mx_alloc_len;
+	mpath_persistent_reserve_in;
+	mpath_persistent_reserve_init_vecs;
+	mpath_persistent_reserve_out;
+	mpath_persistent_reserve_free_vecs;
+	prin_do_scsi_ioctl;
+	prout_do_scsi_ioctl;
+	update_map_pr;
+
+local: *;
+};
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 11/12] libmpathcmd: add linker version script

[mwilck]

From: Martin Wilck <mwilck@suse.com>

For completeness, this isn't really necessary.
The version string is set to "LIBMPATHCMD_1.0.0".

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathcmd/Makefile            |  6 ++++--
 libmpathcmd/libmpathcmd.version | 25 +++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 libmpathcmd/libmpathcmd.version

diff --git a/libmpathcmd/Makefile b/libmpathcmd/Makefile
index 08ccb81..2591019 100644
--- a/libmpathcmd/Makefile
+++ b/libmpathcmd/Makefile
@@ -3,6 +3,7 @@ include ../Makefile.inc
 SONAME = 0
 DEVLIB = libmpathcmd.so
 LIBS = $(DEVLIB).$(SONAME)
+VERSION_SCRIPT := libmpathcmd.version
 
 CFLAGS += $(LIB_CFLAGS)
 
@@ -10,8 +11,9 @@ OBJS = mpath_cmd.o
 
 all:	$(DEVLIB)
 
-$(LIBS): $(OBJS)
-	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ -o $@ $(OBJS) $(LIBDEPS)
+$(LIBS): $(OBJS) $(VERSION_SCRIPT)
+	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ \
+		-Wl,--version-script=$(VERSION_SCRIPT) -o $@ $(OBJS) $(LIBDEPS)
 
 $(DEVLIB): $(LIBS)
 	$(LN) $(LIBS) $@
diff --git a/libmpathcmd/libmpathcmd.version b/libmpathcmd/libmpathcmd.version
new file mode 100644
index 0000000..f100628
--- /dev/null
+++ b/libmpathcmd/libmpathcmd.version
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2020 SUSE LLC
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * libmpathcmd ABI
+ *
+ * The ABI of libmpathcmd is supposed to remain stable. Removing symbols
+ * or altering existing symbols' semantics is not allowed. When changing a
+ * a symbol, either use a new name, or explicit symver directives.
+ *
+ * See libmultipath.version for general policy about version numbers.
+ */
+LIBMPATHCMD_1.0.0 {
+global:
+	__mpath_connect;
+	mpath_connect;
+	mpath_disconnect;
+	mpath_process_cmd;
+	mpath_recv_reply;
+	mpath_recv_reply_len;
+	mpath_recv_reply_data;
+	mpath_send_cmd;
+local:
+	*;
+};
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 12/12] libmpathpersist: initialize mpp->hwe in get_mpvec()

[mwilck]

From: Martin Wilck <mwilck@suse.com>

In __mpath_persistent_reserve_out, we call select_all_tg_pt(),
which requires mpp->hwe to be set. Initialize it in get_mpvec().

Fixes: 5b54e77 ("mpathpersist: add all_tg_pt option")
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_persist.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 1f9817e..4b3f3e0 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -341,11 +341,13 @@ get_mpvec (vector curmp, vector pathvec, char * refwwid)
 			continue;
 
 		if (update_multipath_table(mpp, pathvec, DI_CHECKER) != DMP_OK ||
-		    update_multipath_status(mpp) != DMP_OK) {
+		    update_multipath_status(mpp) != DMP_OK ||
+		    update_mpp_paths(mpp, pathvec)) {
 			condlog(1, "error parsing map %s", mpp->wwid);
 			remove_map(mpp, pathvec, curmp, PURGE_VEC);
 			i--;
-		}
+		} else
+			extract_hwe_from_path(mpp);
 	}
 	return MPATH_PR_SUCCESS ;
 }
-- 
2.28.0


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v2 08/12] libmultipath: create separate .so for unit tests

[Benjamin Marzinski]

On Fri, Oct 16, 2020 at 12:42:35PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> The unit tests use a lot of internal symbols that we don't want
> to add to the ABI if we don't have to. As long as we don't
> have to make incompatible changes to functions, we can work around
> that by simply using a non-versioned library for the unit tests.
> Therefore we add a seperate rule here. Do this before actually
> adding a version script, to avoid breakage during bisection.
> 
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/Makefile |  7 +++++++
>  tests/Makefile        | 10 ++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/libmultipath/Makefile b/libmultipath/Makefile
> index af5bb77..cf13561 100644
> --- a/libmultipath/Makefile
> +++ b/libmultipath/Makefile
> @@ -74,6 +74,13 @@ $(LIBS): $(OBJS)
>  $(DEVLIB): $(LIBS)
>  	$(LN) $(LIBS) $@
>  
> +../tests/$(LIBS): $(OBJS) $(VERSION_SCRIPT)
> +	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=`basename $@` \
> +		-o $@ $(OBJS) $(LIBDEPS)
> +	$(LN) $@ ${@:.so.0=.so}
> +
> +test-lib:	../tests/$(LIBS)
> +
>  install: all
>  	$(INSTALL_PROGRAM) -d $(DESTDIR)$(syslibdir)
>  	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
> diff --git a/tests/Makefile b/tests/Makefile
> index 502377f..f6ef16a 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -10,7 +10,7 @@ W_MISSING_INITIALIZERS := $(call TEST_MISSING_INITIALIZERS)
>  
>  CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir) \
>  	-Wno-unused-parameter $(W_MISSING_INITIALIZERS)
> -LIBDEPS += -L$(multipathdir) -L$(mpathcmddir) -lmultipath -lmpathcmd -lcmocka
> +LIBDEPS += -L. -L$(mpathcmddir) -lmultipath -lmpathcmd -lcmocka
>  
>  TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \
>  	 alias directio valid devt
> @@ -67,7 +67,7 @@ lib/libchecktur.so:
>  
>  %.out:	%-test lib/libchecktur.so
>  	@echo == running $< ==
> -	@LD_LIBRARY_PATH=$(multipathdir):$(mpathcmddir) ./$< >$@
> +	@LD_LIBRARY_PATH=.:$(mpathcmddir) ./$< >$@
>  
>  %.vgr:  %-test lib/libchecktur.so
>  	@echo == running valgrind for $< ==
> @@ -77,7 +77,7 @@ lib/libchecktur.so:
>  OBJS = $(TESTS:%=%.o) test-lib.o
>  
>  test_clean:
> -	$(RM) $(TESTS:%=%.out) $(TESTS:%=%.vgr)
> +	$(RM) $(TESTS:%=%.out) $(TESTS:%=%.vgr) *.so*
>  
>  valgrind_clean:
>  	$(RM) $(TESTS:%=%.vgr)
> @@ -97,12 +97,14 @@ dep_clean:
>  	@sed -n 's/^.*__wrap_\([a-zA-Z0-9_]*\).*$$/-Wl,--wrap=\1/p' $< | \
>  		sort -u | tr '\n' ' ' >$@
>  
> +libmultipath.so.0:
> +	$(MAKE) -C $(multipathdir) test-lib
>  
>  # 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
> +		libmultipath.so.0 Makefile
>  	$(CC) $(CFLAGS) -o $@ $(LDFLAGS) $< $($@_TESTDEPS) $($@_OBJDEPS) \
>  		$(LIBDEPS) $($@_LIBDEPS) \
>  		$(shell cat $<.wrap) $(foreach dep,$($@_TESTDEPS),$(shell cat $(dep).wrap))
> -- 
> 2.28.0

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v2 12/12] libmpathpersist: initialize mpp->hwe in get_mpvec()

[Benjamin Marzinski]

On Fri, Oct 16, 2020 at 12:42:39PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> In __mpath_persistent_reserve_out, we call select_all_tg_pt(),
> which requires mpp->hwe to be set. Initialize it in get_mpvec().
> 
> Fixes: 5b54e77 ("mpathpersist: add all_tg_pt option")
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmpathpersist/mpath_persist.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
> index 1f9817e..4b3f3e0 100644
> --- a/libmpathpersist/mpath_persist.c
> +++ b/libmpathpersist/mpath_persist.c
> @@ -341,11 +341,13 @@ get_mpvec (vector curmp, vector pathvec, char * refwwid)
>  			continue;
>  
>  		if (update_multipath_table(mpp, pathvec, DI_CHECKER) != DMP_OK ||
> -		    update_multipath_status(mpp) != DMP_OK) {
> +		    update_multipath_status(mpp) != DMP_OK ||
> +		    update_mpp_paths(mpp, pathvec)) {
>  			condlog(1, "error parsing map %s", mpp->wwid);
>  			remove_map(mpp, pathvec, curmp, PURGE_VEC);
>  			i--;
> -		}
> +		} else
> +			extract_hwe_from_path(mpp);
>  	}
>  	return MPATH_PR_SUCCESS ;
>  }
> -- 
> 2.28.0

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel