diff mbox series

[v2,06/13] libmultipath: handle a create corner case for empty devices

Message ID 20241122211133.718861-7-bmarzins@redhat.com (mailing list archive)
State New
Headers show
Series multipath-tools: Handle tableless DM devices | expand

Commit Message

Benjamin Marzinski Nov. 22, 2024, 9:11 p.m. UTC
Multipath already notices if there is any empty device with the same
alias and wwid as a device it is trying to create, and switches to a
reload. However, if the empty device has a different alias but the same
wwid, instead of reloading and renaming the device like it would for
a device with a table. multipath will attempt to create a new device
and fail.

Fix this by checking for these devices, and doing a reload and rename.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/configure.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Martin Wilck Nov. 22, 2024, 9:54 p.m. UTC | #1
On Fri, 2024-11-22 at 16:11 -0500, Benjamin Marzinski wrote:
> Multipath already notices if there is any empty device with the same
> alias and wwid as a device it is trying to create, and switches to a
> reload. However, if the empty device has a different alias but the
> same
> wwid, instead of reloading and renaming the device like it would for
> a device with a table. multipath will attempt to create a new device
> and fail.
> 
> Fix this by checking for these devices, and doing a reload and
> rename.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/configure.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index a7257981..d0e9c958 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -835,7 +835,6 @@ int domap(struct multipath *mpp, char *params,
> int is_daemon)
>  {
>  	int r = DOMAP_FAIL;
>  	struct config *conf;
> -	char wwid[WWID_SIZE];
>  
>  	/*
>  	 * last chance to quit before touching the devmaps
> @@ -846,6 +845,7 @@ int domap(struct multipath *mpp, char *params,
> int is_daemon)
>  	}
>  
>  	if (mpp->action == ACT_CREATE) {
> +		char wwid[WWID_SIZE];
>  		int rc = dm_get_wwid(mpp->alias, wwid,
> sizeof(wwid));
>  
>  		if (rc == DMP_OK && !strncmp(mpp->wwid, wwid,
> sizeof(wwid))) {
> @@ -863,7 +863,26 @@ int domap(struct multipath *mpp, char *params,
> int is_daemon)
>  			mpp->action = ACT_REJECT;
>  		}
>  	}
> +	if (mpp->action == ACT_CREATE) {
> +		char alias[WWID_SIZE];
> +		int rc = dm_find_map_by_wwid(mpp->wwid, alias,
> NULL);
>  
> +		if (rc == DMP_NO_MATCH) {
> +			condlog(1, "%s: wwid \"%s\" already in use
> by non-multipath map \"%s\"",
> +				mpp->alias, mpp->wwid, alias);

Nitpick: This is almost the same message that dm_find_map_by_wwid() has
printed immediately before. Similar messages are also printed in all
other callers of dm_find_map_by_wwid(). I suggest that we either remove
the log message in the callers or in dm_find_map_by_wwid() itself.
Probably the latter, then we can decide on a case by case basis in the
caller whether the situation needs to be logged.

Martin
diff mbox series

Patch

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index a7257981..d0e9c958 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -835,7 +835,6 @@  int domap(struct multipath *mpp, char *params, int is_daemon)
 {
 	int r = DOMAP_FAIL;
 	struct config *conf;
-	char wwid[WWID_SIZE];
 
 	/*
 	 * last chance to quit before touching the devmaps
@@ -846,6 +845,7 @@  int domap(struct multipath *mpp, char *params, int is_daemon)
 	}
 
 	if (mpp->action == ACT_CREATE) {
+		char wwid[WWID_SIZE];
 		int rc = dm_get_wwid(mpp->alias, wwid, sizeof(wwid));
 
 		if (rc == DMP_OK && !strncmp(mpp->wwid, wwid, sizeof(wwid))) {
@@ -863,7 +863,26 @@  int domap(struct multipath *mpp, char *params, int is_daemon)
 			mpp->action = ACT_REJECT;
 		}
 	}
+	if (mpp->action == ACT_CREATE) {
+		char alias[WWID_SIZE];
+		int rc = dm_find_map_by_wwid(mpp->wwid, alias, NULL);
 
+		if (rc == DMP_NO_MATCH) {
+			condlog(1, "%s: wwid \"%s\" already in use by non-multipath map \"%s\"",
+				mpp->alias, mpp->wwid, alias);
+			mpp->action = ACT_REJECT;
+		} else if (rc == DMP_OK || rc == DMP_EMPTY) {
+			/*
+			 * we already handled the case were rc == DMO_OK and
+			 * the alias == mpp->alias above. So the alias must be
+			 * different here.
+			 */
+			condlog(3, "%s: map already present with a different name \"%s\". reloading",
+				mpp->alias, alias);
+			strlcpy(mpp->alias_old, alias, WWID_SIZE);
+			mpp->action = ACT_RELOAD_RENAME;
+		}
+	}
 	if (mpp->action == ACT_RENAME || mpp->action == ACT_SWITCHPG_RENAME ||
 	    mpp->action == ACT_RELOAD_RENAME ||
 	    mpp->action == ACT_RESIZE_RENAME) {