diff mbox series

[3/3] multipathd: Don't call select_action in reload_map

Message ID 20241205035638.1218953-4-bmarzins@redhat.com (mailing list archive)
State Not Applicable, archived
Delegated to: Benjamin Marzinski
Headers show
Series handle maps that need reloading | expand

Commit Message

Benjamin Marzinski Dec. 5, 2024, 3:56 a.m. UTC
reload_map() was calling select_action() with force_reload set. Since
mpp will always be a member of vecs->mpvec when reload_map() is called,
when cmpp and cmpp_by_name are looked up in select_action(), they will
just point to mpp. This means that none of the early checks related to
cmpp in select_action() will do anything, and the action will always
end up being ACT_RELOAD. So we can just set ACT_RELOAD in reload_map()
and skip the work.

Additionally, calling select_action() with force_reload set will set
force_udev_reload. This isn't necessary for any of the reload_map()
callers, and will just cause udev to do extra work. I did set
force_udev_reload in cli_reload(), so that it works like calling
"multipath -r", which does force udev to rescan the device.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/cli_handlers.c | 1 +
 multipathd/main.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Martin Wilck Dec. 5, 2024, 4:32 p.m. UTC | #1
On Wed, 2024-12-04 at 22:56 -0500, Benjamin Marzinski wrote:
> reload_map() was calling select_action() with force_reload set. Since
> mpp will always be a member of vecs->mpvec when reload_map() is
> called,
> when cmpp and cmpp_by_name are looked up in select_action(), they
> will
> just point to mpp. This means that none of the early checks related
> to
> cmpp in select_action() will do anything, and the action will always
> end up being ACT_RELOAD. So we can just set ACT_RELOAD in
> reload_map()
> and skip the work.
> 
> Additionally, calling select_action() with force_reload set will set
> force_udev_reload. This isn't necessary for any of the reload_map()
> callers, and will just cause udev to do extra work. I did set
> force_udev_reload in cli_reload(), so that it works like calling
> "multipath -r", which does force udev to rescan the device.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>
diff mbox series

Patch

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index ec330d81..0849fe51 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -818,6 +818,7 @@  cli_reload(void *v, struct strbuf *reply, void *data)
 		return 1;
 	}
 
+	mpp->force_udev_reload = 1;
 	return reload_and_sync_map(mpp, vecs);
 }
 
diff --git a/multipathd/main.c b/multipathd/main.c
index c1ae44f2..c0cf0d06 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2273,7 +2273,7 @@  static int reload_map(struct vectors *vecs, struct multipath *mpp,
 		condlog(0, "%s: failed to setup map", mpp->alias);
 		return 1;
 	}
-	select_action(mpp, vecs->mpvec, 1);
+	mpp->action = ACT_RELOAD;
 
 	r = domap(mpp, params, is_daemon);
 	if (r == DOMAP_FAIL || r == DOMAP_RETRY) {