From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Tue, 31 Jan 2023 12:00:31 -0600 Subject: [PATCH] libmultipath: keep renames from stopping other multipath actions If select_action() is called and a multipath device needs to be renamed, the code currently checks if force_reload is set, and if so, does the reload after the rename. But if force_reload isn't set, only the rename happens, regardless of what other actions are needed. This can happen if multipathd starts up and a device needs both a reload and a rename. Make multipath check for resize, reload, and switch pathgroup along with rename, and do both if necessary. Signed-off-by: Benjamin Marzinski --- libmultipath/configure.c | 92 +++++++++++++++++++++------------------- libmultipath/configure.h | 4 +- 2 files changed, 51 insertions(+), 45 deletions(-) diff --git a/libmultipath/configure.c b/libmultipath/configure.c index 303d2380..65a0b208 100644 --- a/libmultipath/configure.c +++ b/libmultipath/configure.c @@ -690,6 +690,7 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) struct multipath * cmpp_by_name; char * mpp_feat, * cmpp_feat; + mpp->action = ACT_NOTHING; cmpp = find_mp_by_wwid(curmp, mpp->wwid); cmpp_by_name = find_mp_by_alias(curmp, mpp->alias); @@ -712,14 +713,8 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) mpp->alias); strlcpy(mpp->alias_old, cmpp->alias, WWID_SIZE); mpp->action = ACT_RENAME; - if (force_reload) { - mpp->force_udev_reload = 1; - mpp->action = ACT_FORCERENAME; - } - return; - } - - if (cmpp != cmpp_by_name) { + /* don't return here. Check for other needed actions */ + } else if (cmpp != cmpp_by_name) { condlog(2, "%s: unable to rename %s to %s (%s is used by %s)", mpp->wwid, cmpp->alias, mpp->alias, mpp->alias, cmpp_by_name->wwid); @@ -727,12 +722,13 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) FREE(mpp->alias); mpp->alias = STRDUP(cmpp->alias); mpp->action = ACT_IMPOSSIBLE; - return; + /* don't return here. Check for other needed actions */ } if (cmpp->size != mpp->size) { mpp->force_udev_reload = 1; - mpp->action = ACT_RESIZE; + mpp->action = mpp->action == ACT_RENAME ? ACT_RESIZE_RENAME : + ACT_RESIZE; condlog(3, "%s: set ACT_RESIZE (size change)", mpp->alias); return; @@ -740,7 +736,8 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) if (force_reload) { mpp->force_udev_reload = 1; - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (forced by user)", mpp->alias); return; @@ -749,7 +746,8 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) if (mpp->no_path_retry != NO_PATH_RETRY_UNDEF && !!strstr(mpp->features, "queue_if_no_path") != !!strstr(cmpp->features, "queue_if_no_path")) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (no_path_retry change)", mpp->alias); return; @@ -759,7 +757,8 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) (strlen(cmpp->hwhandler) != strlen(mpp->hwhandler) || strncmp(cmpp->hwhandler, mpp->hwhandler, strlen(mpp->hwhandler)))) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (hwhandler change)", mpp->alias); return; @@ -769,7 +768,8 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) !!strstr(mpp->features, "retain_attached_hw_handler") != !!strstr(cmpp->features, "retain_attached_hw_handler") && get_linux_version_code() < KERNEL_VERSION(4, 3, 0)) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (retain_hwhandler change)", mpp->alias); return; @@ -783,9 +783,13 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) remove_feature(&cmpp_feat, "queue_if_no_path"); remove_feature(&cmpp_feat, "retain_attached_hw_handler"); if (strncmp(mpp_feat, cmpp_feat, PARAMS_SIZE)) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? + ACT_RELOAD_RENAME : ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (features change)", mpp->alias); + FREE(cmpp_feat); + FREE(mpp_feat); + return; } } FREE(cmpp_feat); @@ -793,44 +797,49 @@ select_action (struct multipath * mpp, vector curmp, int force_reload) if (!cmpp->selector || strncmp(cmpp->selector, mpp->selector, strlen(mpp->selector))) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (selector change)", mpp->alias); return; } if (cmpp->minio != mpp->minio) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (minio change, %u->%u)", mpp->alias, cmpp->minio, mpp->minio); return; } if (!cmpp->pg || VECTOR_SIZE(cmpp->pg) != VECTOR_SIZE(mpp->pg)) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (path group number change)", mpp->alias); return; } if (pgcmp(mpp, cmpp)) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_RELOAD_RENAME : + ACT_RELOAD; condlog(3, "%s: set ACT_RELOAD (path group topology change)", mpp->alias); return; } if (cmpp->nextpg != mpp->bestpg) { - mpp->action = ACT_SWITCHPG; + mpp->action = mpp->action == ACT_RENAME ? ACT_SWITCHPG_RENAME : + ACT_SWITCHPG; condlog(3, "%s: set ACT_SWITCHPG (next path group change)", mpp->alias); return; } if (!is_mpp_known_to_udev(cmpp)) { - mpp->action = ACT_RELOAD; + mpp->action = mpp->action == ACT_RENAME ? ACT_SWITCHPG_RENAME : + ACT_SWITCHPG; 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); + if (mpp->action == ACT_NOTHING) + condlog(3, "%s: set ACT_NOTHING (map unchanged)", mpp->alias); return; } @@ -924,6 +933,17 @@ int domap(struct multipath *mpp, char *params, int is_daemon) mpp->action = ACT_RELOAD; } + if (mpp->action == ACT_RENAME || mpp->action == ACT_SWITCHPG_RENAME || + mpp->action == ACT_RELOAD_RENAME || + mpp->action == ACT_RESIZE_RENAME) { + conf = get_multipath_config(); + pthread_cleanup_push(put_multipath_config, conf); + r = dm_rename(mpp->alias_old, mpp->alias, + conf->partition_delim, mpp->skip_kpartx); + pthread_cleanup_pop(1); + if (r == DOMAP_FAIL) + return r; + } switch (mpp->action) { case ACT_REJECT: case ACT_NOTHING: @@ -931,6 +951,7 @@ int domap(struct multipath *mpp, char *params, int is_daemon) return DOMAP_EXIST; case ACT_SWITCHPG: + case ACT_SWITCHPG_RENAME: dm_switchgroup(mpp->alias, mpp->bestpg); /* * we may have avoided reinstating paths because there where in @@ -957,6 +978,7 @@ int domap(struct multipath *mpp, char *params, int is_daemon) break; case ACT_RELOAD: + case ACT_RELOAD_RENAME: sysfs_set_max_sectors_kb(mpp, 1); if (mpp->ghost_delay_tick > 0 && pathcount(mpp, PATH_UP)) mpp->ghost_delay_tick = 0; @@ -964,6 +986,7 @@ int domap(struct multipath *mpp, char *params, int is_daemon) break; case ACT_RESIZE: + case ACT_RESIZE_RENAME: sysfs_set_max_sectors_kb(mpp, 1); if (mpp->ghost_delay_tick > 0 && pathcount(mpp, PATH_UP)) mpp->ghost_delay_tick = 0; @@ -971,29 +994,10 @@ int domap(struct multipath *mpp, char *params, int is_daemon) break; case ACT_RENAME: - conf = get_multipath_config(); - pthread_cleanup_push(put_multipath_config, conf); - r = dm_rename(mpp->alias_old, mpp->alias, - conf->partition_delim, mpp->skip_kpartx); - pthread_cleanup_pop(1); - break; - - case ACT_FORCERENAME: - conf = get_multipath_config(); - pthread_cleanup_push(put_multipath_config, conf); - r = dm_rename(mpp->alias_old, mpp->alias, - conf->partition_delim, mpp->skip_kpartx); - pthread_cleanup_pop(1); - if (r) { - sysfs_set_max_sectors_kb(mpp, 1); - if (mpp->ghost_delay_tick > 0 && - pathcount(mpp, PATH_UP)) - mpp->ghost_delay_tick = 0; - r = dm_addmap_reload(mpp, params, 0); - } break; default: + r = DOMAP_FAIL; break; } diff --git a/libmultipath/configure.h b/libmultipath/configure.h index 5cf08d45..1a93f49d 100644 --- a/libmultipath/configure.h +++ b/libmultipath/configure.h @@ -18,9 +18,11 @@ enum actions { ACT_RENAME, ACT_CREATE, ACT_RESIZE, - ACT_FORCERENAME, + ACT_RELOAD_RENAME, ACT_DRY_RUN, ACT_IMPOSSIBLE, + ACT_RESIZE_RENAME, + ACT_SWITCHPG_RENAME, }; /*