[dm-devel] [PATCH 0/3] multipath-tools: cleanup uevent generation on startup

[dm-devel] [PATCH 0/3] multipath-tools: cleanup uevent generation on startup

[mwilck]

From: Martin Wilck <mwilck@suse.com>

This series undoes several changes I did myself in an attempt to fix
issues with multipath maps incompletely initialized in udev. They are
now all superseded by 0d66e03 ("libmultipath: force map reload if 
udev incomplete"). Triggering artificial (spurious) uevents for
dm devices is useless in almost all cases. If some map is found in an
inconsistent or incomplete state, the only action that would fix it
is a full map reload.

While the code reverted here doesn't do actual harm, it unnecessarily
complicates matters, wastes resources, and may leave readers of
the code with wrong ideas how to handle udev issues in multipathd.

Martin Wilck (3):
  libmultipath: select_action(): skip is_mpp_known_to_udev() test
  libmultipath: coalesce_paths(): stop triggering spurious uevents
  Revert "multipathd: uev_trigger(): handle incomplete ADD events"

 libmultipath/configure.c | 39 ---------------------------------------
 multipathd/main.c        | 25 -------------------------
 2 files changed, 64 deletions(-)

-- 
2.29.2


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

[dm-devel] [PATCH 1/3] libmultipath: select_action(): skip is_mpp_known_to_udev() test

[mwilck]

From: Martin Wilck <mwilck@suse.com>

This test is now superseded by the check introduced in
0d66e03 ("libmultipath: force map reload if udev incomplete").

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index d9fd9cb..999f310 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -635,15 +635,6 @@ trigger_paths_udev_change(struct multipath *mpp, bool is_mpath)
 	mpp->needs_paths_uevent = 0;
 }
 
-static int
-is_mpp_known_to_udev(const struct multipath *mpp)
-{
-	struct udev_device *udd = get_udev_for_mpp(mpp);
-	int ret = (udd != NULL);
-	udev_device_unref(udd);
-	return ret;
-}
-
 static int
 sysfs_set_max_sectors_kb(struct multipath *mpp, int is_reload)
 {
@@ -865,12 +856,6 @@ void select_action (struct multipath *mpp, const struct _vector *curmp,
 			mpp->alias);
 		return;
 	}
-	if (!is_mpp_known_to_udev(cmpp)) {
-		mpp->action = ACT_RELOAD;
-		condlog(3, "%s: set ACT_RELOAD (udev device not initialized)",
-			mpp->alias);
-		return;
-	}
 	mpp->action = ACT_NOTHING;
 	condlog(3, "%s: set ACT_NOTHING (map unchanged)",
 		mpp->alias);
-- 
2.29.2


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

[dm-devel] [PATCH 2/3] libmultipath: coalesce_paths(): stop triggering spurious uevents

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Since 0d66e03 ("libmultipath: force map reload if udev incomplete"), we
force-reload maps that we find incompletely initialized by udev. If
select_action returns ACT_NOTHING nonetheless, the map must be initialized
in udev, and thus and "add" uevent must have been seen already. Triggering
this event once more is unlikely to fix anything for real.

Reverts: b516118 ("libmultipath: coalesce_paths: trigger uevent if nothing done")
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 999f310..3263bb0 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -527,18 +527,6 @@ get_udev_for_mpp(const struct multipath *mpp)
 	return udd;
 }
 
-static void
-trigger_udev_change(const struct multipath *mpp)
-{
-	static const char change[] = "change";
-	struct udev_device *udd = get_udev_for_mpp(mpp);
-	if (!udd)
-		return;
-	condlog(3, "triggering %s uevent for %s", change, mpp->alias);
-	sysfs_attr_set_value(udd, "uevent", change, sizeof(change)-1);
-	udev_device_unref(udd);
-}
-
 static void trigger_partitions_udev_change(struct udev_device *dev,
 					   const char *action, int len)
 {
@@ -1297,18 +1285,6 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 			continue;
 		}
 
-		if (r == DOMAP_EXIST && mpp->action == ACT_NOTHING &&
-		    force_reload == FORCE_RELOAD_WEAK)
-			/*
-			 * First time we're called, and no changes applied.
-			 * domap() was a noop. But we can't be sure that
-			 * udev has already finished setting up this device
-			 * (udev in initrd may have been shut down while
-			 * processing this device or its children).
-			 * Trigger a change event, just in case.
-			 */
-			trigger_udev_change(find_mp_by_wwid(curmp, mpp->wwid));
-
 		conf = get_multipath_config();
 		allow_queueing = conf->allow_queueing;
 		put_multipath_config(conf);
-- 
2.29.2


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

[dm-devel] [PATCH 3/3] Revert "multipathd: uev_trigger(): handle incomplete ADD events"

[mwilck]

From: Martin Wilck <mwilck@suse.com>

cb10d38 ("multipathd: uev_trigger(): handle incomplete ADD events") was an
attempt to fix issues with incompletely initialized multipath maps observed
in various scenarious. However, that patch was wrong. Spurious "change" events
as this patch would generate have no effect, because they are ignored by
the device-mapper udev rules. The correct fix for the problem we were
facing is 0d66e03 ("libmultipath: force map reload if udev incomplete"),
which forces a full map reload.

Reverts: cb10d38 ("multipathd: uev_trigger(): handle incomplete ADD events")
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 7612430..92c45d4 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1499,31 +1499,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 			uev_pathfail_check(uev, vecs);
 		} else if (!strncmp(uev->action, "remove", 6)) {
 			r = uev_remove_map(uev, vecs);
-		} else if (!strncmp(uev->action, "add", 3)) {
-			const char *ev_name;
-			char *dm_name;
-			int major = -1, minor = -1;
-
-			/*
-			 * If DM_NAME is not set for a valid map, trigger a
-			 * change event. This can happen during coldplug
-			 * if udev was killed between handling the 'add' and
-			 * 'change' events before.
-			 */
-			ev_name = uevent_get_dm_name(uev);
-			if (!ev_name) {
-				major = uevent_get_major(uev);
-				minor = uevent_get_minor(uev);
-				dm_name = dm_mapname(major, minor);
-				if (dm_name && *dm_name) {
-					condlog(2, "%s: received incomplete 'add' uevent, triggering change",
-						dm_name);
-					udev_device_set_sysattr_value(uev->udev,
-								      "uevent",
-								      "change");
-					free(dm_name);
-				}
-			}
 		}
 		goto out;
 	}
-- 
2.29.2


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

Re: [dm-devel] [PATCH 0/3] multipath-tools: cleanup uevent generation on startup

[Benjamin Marzinski]

On Fri, Jan 08, 2021 at 06:00:41PM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>

For the set

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> 
> This series undoes several changes I did myself in an attempt to fix
> issues with multipath maps incompletely initialized in udev. They are
> now all superseded by 0d66e03 ("libmultipath: force map reload if 
> udev incomplete"). Triggering artificial (spurious) uevents for
> dm devices is useless in almost all cases. If some map is found in an
> inconsistent or incomplete state, the only action that would fix it
> is a full map reload.
> 
> While the code reverted here doesn't do actual harm, it unnecessarily
> complicates matters, wastes resources, and may leave readers of
> the code with wrong ideas how to handle udev issues in multipathd.
> 
> Martin Wilck (3):
>   libmultipath: select_action(): skip is_mpp_known_to_udev() test
>   libmultipath: coalesce_paths(): stop triggering spurious uevents
>   Revert "multipathd: uev_trigger(): handle incomplete ADD events"
> 
>  libmultipath/configure.c | 39 ---------------------------------------
>  multipathd/main.c        | 25 -------------------------
>  2 files changed, 64 deletions(-)
> 
> -- 
> 2.29.2

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