[PATCH v2 1/3] libmultipath: merge update_multipath_table() and update_multipath_status()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Martin Wilck <mwilck@xxxxxxxx>

Since 378cb66 ("multipath: use update_pathvec_from_dm()"),
we remove paths and even pathgroups from multipathd's data structures
in update_multipath_table() if these paths are found to be non-existent.
But update_multipath_status() is called afterwards, and it
uses the kernel's mapping of pathgroups and paths, which won't match
any more if any members had been removed. disassemble_status() returns
an error if the number of path groups doesn't match, causing the
entire structure setup to fail. And because disassemble_status()
doesn't check the dev_t against the corresponding values in multipathd's
data structures, it may assign wrong DM state to paths.

Fix this by calling disassemble_status() before making any changes to
the data structure in update_pathvec_from_dm(). This can be easily
done, because every call to update_multipath_status() is preceded
by a call to update_multipath_table() anyway, and vice versa. So
we simply merge the two functions into one. This actually simplifies
the code for all callers.

As we remove a symbol, the major library version must be bumped.

Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
---
Changes v1 -> v2:
 - log errors at -v2 in update_multipath_table() (Ben)
 - return error in update_multipath_strings() if
   update_multipath_table() fails (Ben)

---
 libmpathpersist/mpath_persist.c   |  1 -
 libmultipath/libmultipath.version | 30 ++++++++----------------
 libmultipath/structs_vec.c        | 38 ++++++++-----------------------
 multipath/main.c                  |  6 ++---
 multipathd/main.c                 |  5 +---
 5 files changed, 21 insertions(+), 59 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 5c95af2..190e970 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -408,7 +408,6 @@ 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_mpp_paths(mpp, pathvec)) {
 			condlog(1, "error parsing map %s", mpp->wwid);
 			remove_map(mpp, pathvec, curmp, PURGE_VEC);
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index e9b4608..0cff311 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -31,7 +31,7 @@
  *   The new version inherits the previous ones.
  */
 
-LIBMULTIPATH_4.0.0 {
+LIBMULTIPATH_5.0.0 {
 global:
 	/* symbols referenced by multipath and multipathd */
 	add_foreign;
@@ -198,7 +198,6 @@ global:
 	uevent_is_mpath;
 	uevent_listen;
 	update_mpp_paths;
-	update_multipath_status;
 	update_multipath_strings;
 	update_multipath_table;
 	update_pathvec_from_dm;
@@ -256,33 +255,22 @@ global:
 	libmultipath_init;
 	libmultipath_exit;
 
-local:
-	*;
-};
-
-LIBMULTIPATH_4.1.0 {
-global:
+	/* added in 4.1.0 */
 	libmp_verbosity;
-} LIBMULTIPATH_4.0.0;
 
-LIBMULTIPATH_4.2.0 {
-global:
+	/* added in 4.2.0 */
 	dm_prereq;
 	skip_libmp_dm_init;
-} LIBMULTIPATH_4.1.0;
 
-LIBMULTIPATH_4.3.0 {
-global:
+	/* added in 4.3.0 */
 	start_checker_thread;
-} LIBMULTIPATH_4.2.0;
 
-LIBMULTIPATH_4.4.0 {
-global:
+	/* added in 4.4.0 */
 	get_next_string;
-} LIBMULTIPATH_4.3.0;
 
-LIBMULITIPATH_4.5.0 {
-global:
+	/* added in 4.5.0 */
 	get_vpd_sgio;
 	trigger_partitions_udev_change;
-} LIBMULTIPATH_4.4.0;
+local:
+	*;
+};
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 57cd88a..0b069f4 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -423,44 +423,27 @@ update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
 
 	r = dm_get_map(mpp->alias, &mpp->size, params);
 	if (r != DMP_OK) {
-		condlog(3, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting table" : "map not present");
+		condlog(2, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting table" : "map not present");
 		return r;
 	}
 
 	if (disassemble_map(pathvec, params, mpp)) {
-		condlog(3, "%s: cannot disassemble map", mpp->alias);
+		condlog(2, "%s: cannot disassemble map", mpp->alias);
 		return DMP_ERR;
 	}
 
+	*params = '\0';
+	if (dm_get_status(mpp->alias, params) != DMP_OK)
+		condlog(2, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting status" : "map not present");
+	else if (disassemble_status(params, mpp))
+		condlog(2, "%s: cannot disassemble status", mpp->alias);
+
 	/* FIXME: we should deal with the return value here */
 	update_pathvec_from_dm(pathvec, mpp, flags);
 
 	return DMP_OK;
 }
 
-int
-update_multipath_status (struct multipath *mpp)
-{
-	int r = DMP_ERR;
-	char status[PARAMS_SIZE] = {0};
-
-	if (!mpp)
-		return r;
-
-	r = dm_get_status(mpp->alias, status);
-	if (r != DMP_OK) {
-		condlog(3, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting status" : "map not present");
-		return r;
-	}
-
-	if (disassemble_status(status, mpp)) {
-		condlog(3, "%s: cannot disassemble status", mpp->alias);
-		return DMP_ERR;
-	}
-
-	return DMP_OK;
-}
-
 static struct path *find_devt_in_pathgroups(const struct multipath *mpp,
 					    const char *dev_t)
 {
@@ -538,11 +521,8 @@ update_multipath_strings(struct multipath *mpp, vector pathvec)
 	r = update_multipath_table(mpp, pathvec, 0);
 	if (r != DMP_OK)
 		return r;
-	sync_paths(mpp, pathvec);
 
-	r = update_multipath_status(mpp);
-	if (r != DMP_OK)
-		return r;
+	sync_paths(mpp, pathvec);
 
 	vector_foreach_slot(mpp->pg, pgp, i)
 		if (pgp->paths)
diff --git a/multipath/main.c b/multipath/main.c
index 3f97582..ef89c7c 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -196,8 +196,7 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector pathvec, char * refwwid)
 			continue;
 		}
 
-		if (update_multipath_table(mpp, pathvec, flags) != DMP_OK ||
-		    update_multipath_status(mpp) != DMP_OK) {
+		if (update_multipath_table(mpp, pathvec, flags) != DMP_OK) {
 			condlog(1, "error parsing map %s", mpp->wwid);
 			remove_map(mpp, pathvec, curmp, PURGE_VEC);
 			i--;
@@ -263,8 +262,7 @@ static int check_usable_paths(struct config *conf,
 	if (mpp == NULL)
 		goto free;
 
-	if (update_multipath_table(mpp, pathvec, 0) != DMP_OK ||
-		    update_multipath_status(mpp) != DMP_OK)
+	if (update_multipath_table(mpp, pathvec, 0) != DMP_OK)
 		    goto free;
 
 	vector_foreach_slot (mpp->pg, pg, i) {
diff --git a/multipathd/main.c b/multipathd/main.c
index e0797cc..154a4ee 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -559,8 +559,6 @@ add_map_without_path (struct vectors *vecs, const char *alias)
 
 	if (update_multipath_table(mpp, vecs->pathvec, 0) != DMP_OK)
 		goto out;
-	if (update_multipath_status(mpp) != DMP_OK)
-		goto out;
 
 	if (!vector_alloc_slot(vecs->mpvec))
 		goto out;
@@ -1469,8 +1467,7 @@ map_discovery (struct vectors * vecs)
 		return 1;
 
 	vector_foreach_slot (vecs->mpvec, mpp, i)
-		if (update_multipath_table(mpp, vecs->pathvec, 0) != DMP_OK ||
-		    update_multipath_status(mpp) != DMP_OK) {
+		if (update_multipath_table(mpp, vecs->pathvec, 0) != DMP_OK) {
 			remove_map(mpp, vecs->pathvec, vecs->mpvec, PURGE_VEC);
 			i--;
 		}
-- 
2.30.1


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




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux