[dm-devel] [PATCH 5/7] multipathd: fully initialize paths added by update_pathvec_from_dm
Benjamin Marzinski
bmarzins at redhat.com
Wed Oct 20 19:15:20 UTC 2021
When paths are added by update_pathvec_from_dm(), udev may not have
initialized them. This means that it's possible that they are supposed
to be blacklisted by udev properties, but weren't. Also, in order to
avoid doing potentially stalling IO, update_pathvec_from_dm() doesn't
get all the path information in pathinfo(). These paths end up in the
unexpected state of INIT_MISSING_UDEV or INIT_NEW, but with their mpp
and wwid set.
If udev has already initialized the path, but multipathed wasn't
monitoring it, the blacklist checks and wwid determination in
update_pathvec_from_dm() will work correctly, so paths added by it are
safe, but not completely initialized. The most likely reason why this
would happen is if the path was manually removed from multipathd
monitoring with "multipathd del path". The most common time when
uninitialized paths would in be part of multipath devices is during
boot, after the pivot root, but before the udev coldplug happens. These
paths are not necessarily safe. It's possible that /etc/multipath.conf
in the initramfs and regular filesystem differ, and they should now be
either blacklisted by udev_property, or have a different wwid. However
an "add" event should appear for them shortly.
Multipath now has a new state to deal with these devices, INIT_PARTIAL.
Devices in this state are treated mostly like INIT_OK devices, but
when "multipathd add path" is called or an add/change uevent happens
on these devices, multipathd will finish initializing them, and remove
them if necessary.
Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
---
libmultipath/structs.h | 6 +++++
libmultipath/structs_vec.c | 5 ++--
multipathd/cli_handlers.c | 4 ++++
multipathd/main.c | 48 ++++++++++++++++++++++++++++++++++----
multipathd/main.h | 1 +
5 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index d0b266b7..69409fd4 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -200,6 +200,12 @@ enum initialized_states {
* mapped by some multipath map because of map reload failure.
*/
INIT_REMOVED,
+ /*
+ * INIT_PARTIAL: paths added by update_pathvec_from_dm() will not
+ * be fully initialized. This will be handled when an add or
+ * change uevent is received.
+ */
+ INIT_PARTIAL,
};
enum prkey_sources {
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index fb26437a..1de9175e 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -194,6 +194,7 @@ bool update_pathvec_from_dm(vector pathvec, struct multipath *mpp,
}
condlog(2, "%s: adding new path %s",
mpp->alias, pp->dev);
+ pp->initialized = INIT_PARTIAL;
store_path(pathvec, pp);
pp->tick = 1;
}
@@ -392,12 +393,12 @@ extract_hwe_from_path(struct multipath * mpp)
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 &&
+ if (pp->state == PATH_UP && pp->initialized != INIT_PARTIAL &&
pp->initialized != INIT_REMOVED && pp->hwe)
goto done;
}
vector_foreach_slot(mpp->paths, pp, i) {
- if (pp->state != PATH_UP &&
+ if ((pp->state != PATH_UP || pp->initialized == INIT_PARTIAL) &&
pp->initialized != INIT_REMOVED && pp->hwe)
goto done;
}
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 58b9916c..8d37431e 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -526,6 +526,10 @@ cli_add_path (void *v, struct strbuf *reply, void *data)
return 1;
}
+ if (pp->initialized == INIT_PARTIAL &&
+ finish_path_init(pp, vecs) < 0)
+ return 1;
+
if (pp->mpp)
return 0;
} else if (pp) {
diff --git a/multipathd/main.c b/multipathd/main.c
index cc4a4a5d..4b8e2cde 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -976,12 +976,19 @@ check_path_wwid_change(struct path *pp)
return false;
}
+/*
+ * uev_add_path can call uev_update_path, and uev_update_path can call
+ * uev_add_path
+ */
+static int uev_update_path (struct uevent *uev, struct vectors * vecs);
+
static int
uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
{
struct path *pp;
int ret = 0, i;
struct config *conf;
+ bool partial_init = false;
condlog(3, "%s: add path (uevent)", uev->kernel);
if (strstr(uev->kernel, "..") != NULL) {
@@ -1000,7 +1007,10 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
int r;
struct multipath *prev_mpp = NULL;
- if (pp->initialized == INIT_REMOVED) {
+ if (pp->initialized == INIT_PARTIAL) {
+ partial_init = true;
+ goto out;
+ } else if (pp->initialized == INIT_REMOVED) {
condlog(3, "%s: re-adding removed path", pp->dev);
pp->initialized = INIT_NEW;
prev_mpp = pp->mpp;
@@ -1110,6 +1120,8 @@ uev_add_path (struct uevent *uev, struct vectors * vecs, int need_do_map)
}
out:
lock_cleanup_pop(vecs->lock);
+ if (partial_init)
+ return uev_update_path(uev, vecs);
return ret;
}
@@ -1405,6 +1417,25 @@ fail:
return REMOVE_PATH_MAP_ERROR;
}
+int
+finish_path_init(struct path *pp, struct vectors * vecs)
+{
+ int r;
+ struct config *conf;
+
+ conf = get_multipath_config();
+ pthread_cleanup_push(put_multipath_config, conf);
+ r = pathinfo(pp, conf, DI_ALL|DI_BLACKLIST);
+ pthread_cleanup_pop(1);
+
+ if (r == PATHINFO_OK)
+ return 0;
+
+ condlog(0, "%s: error fully initializing path, removing", pp->dev);
+ ev_remove_path(pp, vecs, 1);
+ return -1;
+}
+
static int
uev_update_path (struct uevent *uev, struct vectors * vecs)
{
@@ -1443,7 +1474,7 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
}
/* Don't deal with other types of failed initialization
* now. check_path will handle it */
- if (!strlen(pp->wwid))
+ if (!strlen(pp->wwid) && pp->initialized != INIT_PARTIAL)
goto out;
strcpy(wwid, pp->wwid);
@@ -1451,12 +1482,20 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
if (rc != 0)
strcpy(pp->wwid, wwid);
- else if (strncmp(wwid, pp->wwid, WWID_SIZE) != 0) {
+ else if (strlen(wwid) &&
+ strncmp(wwid, pp->wwid, WWID_SIZE) != 0) {
condlog(0, "%s: path wwid changed from '%s' to '%s'",
uev->kernel, wwid, pp->wwid);
ev_remove_path(pp, vecs, 1);
needs_reinit = 1;
goto out;
+ } else if (pp->initialized == INIT_PARTIAL) {
+ udev_device_unref(pp->udev);
+ pp->udev = udev_device_ref(uev->udev);
+ if (finish_path_init(pp, vecs) < 0) {
+ retval = 1;
+ goto out;
+ }
} else {
udev_device_unref(pp->udev);
pp->udev = udev_device_ref(uev->udev);
@@ -1507,6 +1546,7 @@ out:
condlog(0, "%s: spurious uevent, path not found", uev->kernel);
}
+ /* pp->initalized must not be INIT_PARTIAL if needs_reinit is set */
if (needs_reinit)
retval = uev_add_path(uev, vecs, 1);
return retval;
@@ -2116,7 +2156,7 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
int marginal_pathgroups, marginal_changed = 0;
int ret;
- if (((pp->initialized == INIT_OK ||
+ if (((pp->initialized == INIT_OK || pp->initialized == INIT_PARTIAL ||
pp->initialized == INIT_REQUESTED_UDEV) && !pp->mpp) ||
pp->initialized == INIT_REMOVED)
return 0;
diff --git a/multipathd/main.h b/multipathd/main.h
index c8a1ce92..4acd1b8c 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -66,4 +66,5 @@ int reload_and_sync_map(struct multipath *mpp, struct vectors *vecs,
void handle_path_wwid_change(struct path *pp, struct vectors *vecs);
bool check_path_wwid_change(struct path *pp);
+int finish_path_init(struct path *pp, struct vectors * vecs);
#endif /* MAIN_H */
--
2.17.2
More information about the dm-devel
mailing list