diff mbox series

[v2,18/49] libmultipath: Use symbolic return values for dm_is_mpath()

Message ID 20240712171458.77611-19-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show
Series multipath-tools: devmapper API refactored | expand

Commit Message

Martin Wilck July 12, 2024, 5:14 p.m. UTC 
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_persist_int.c |  2 +-
 libmultipath/devmapper.c            | 22 ++++++++--------------
 libmultipath/devmapper.h            |  6 ++++++
 multipath/main.c                    |  4 ++--
 multipathd/dmevents.c               | 19 +++++++++++++------
 multipathd/main.c                   |  2 +-
 6 files changed, 31 insertions(+), 24 deletions(-)

Comments

Benjamin Marzinski July 15, 2024, 10:14 p.m. UTC | #1 
On Fri, Jul 12, 2024 at 07:14:26PM +0200, Martin Wilck wrote:
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmpathpersist/mpath_persist_int.c |  2 +-
>  libmultipath/devmapper.c            | 22 ++++++++--------------
>  libmultipath/devmapper.h            |  6 ++++++
>  multipath/main.c                    |  4 ++--
>  multipathd/dmevents.c               | 19 +++++++++++++------
>  multipathd/main.c                   |  2 +-
>  6 files changed, 31 insertions(+), 24 deletions(-)
> 
> diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c
> index 178c2f5..6da0401 100644
> --- a/libmpathpersist/mpath_persist_int.c
> +++ b/libmpathpersist/mpath_persist_int.c
> @@ -185,7 +185,7 @@ static int mpath_get_map(vector curmp, vector pathvec, int fd, char **palias,
>  
>  	condlog(3, "alias = %s", alias);
>  
> -	if (dm_map_present(alias) && dm_is_mpath(alias) != 1){
> +	if (dm_map_present(alias) && dm_is_mpath(alias) != DM_IS_MPATH_YES) {
>  		condlog(3, "%s: not a multipath device.", alias);
>  		goto out;
>  	}
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index a63154f..3abdc26 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -846,15 +846,9 @@ static int dm_type_match(const char *name, char *type)
>  		return DM_TYPE_NOMATCH;
>  }
>  
> -/*
> - * returns:
> - * 1  : is multipath device
> - * 0  : is not multipath device
> - * -1 : error
> - */
>  int dm_is_mpath(const char *name)
>  {
> -	int r = -1;
> +	int r = DM_IS_MPATH_ERR;
>  	struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL;
>  	struct dm_info info;
>  	uint64_t start, length;
> @@ -876,7 +870,7 @@ int dm_is_mpath(const char *name)
>  	if (!dm_task_get_info(dmt, &info))
>  		goto out;
>  
> -	r = 0;
> +	r = DM_IS_MPATH_NO;
>  
>  	if (!info.exists)
>  		goto out;
> @@ -895,10 +889,10 @@ int dm_is_mpath(const char *name)
>  	if (!target_type || strcmp(target_type, TGT_MPATH) != 0)
>  		goto out;
>  
> -	r = 1;
> +	r = DM_IS_MPATH_YES;
>  out:
> -	if (r < 0)
> -		condlog(3, "%s: dm command failed in %s: %s", name, __FUNCTION__, strerror(errno));
> +	if (r == DM_IS_MPATH_ERR)
> +		condlog(3, "%s: dm command failed in %s: %s", name, __func__, strerror(errno));
>  	return r;
>  }
>  
> @@ -1039,7 +1033,7 @@ int _dm_flush_map (const char *mapname, int flags, int retries)
>  	unsigned long long mapsize;
>  	char *params = NULL;
>  
> -	if (dm_is_mpath(mapname) != 1)
> +	if (dm_is_mpath(mapname) != DM_IS_MPATH_YES)
>  		return DM_FLUSH_OK; /* nothing to do */
>  
>  	/* if the device currently has no partitions, do not
> @@ -1086,7 +1080,7 @@ int _dm_flush_map (const char *mapname, int flags, int retries)
>  			}
>  			condlog(4, "multipath map %s removed", mapname);
>  			return DM_FLUSH_OK;
> -		} else if (dm_is_mpath(mapname) != 1) {
> +		} else if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
>  			condlog(4, "multipath map %s removed externally",
>  				mapname);
>  			return DM_FLUSH_OK; /* raced. someone else removed it */
> @@ -1316,7 +1310,7 @@ int dm_get_maps(vector mp)
>  	}
>  
>  	do {
> -		if (dm_is_mpath(names->name) != 1)
> +		if (dm_is_mpath(names->name) != DM_IS_MPATH_YES)
>  			goto next;
>  
>  		mpp = dm_get_multipath(names->name);
> diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
> index ff28575..9438c2d 100644
> --- a/libmultipath/devmapper.h
> +++ b/libmultipath/devmapper.h
> @@ -46,6 +46,12 @@ int dm_map_present (const char *name);
>  int dm_map_present_by_uuid(const char *uuid);
>  int dm_get_map(const char *name, unsigned long long *size, char **outparams);
>  int dm_get_status(const char *name, char **outstatus);
> +
> +enum {
> +	DM_IS_MPATH_NO,
> +	DM_IS_MPATH_YES,
> +	DM_IS_MPATH_ERR,
> +};
>  int dm_is_mpath(const char *name);
>  
>  enum {
> diff --git a/multipath/main.c b/multipath/main.c
> index ce702e7..c82bc86 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -247,7 +247,7 @@ static int check_usable_paths(struct config *conf,
>  		goto out;
>  	}
>  
> -	if (dm_is_mpath(mapname) != 1) {
> +	if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
>  		condlog(1, "%s is not a multipath map", devpath);
>  		goto free;
>  	}
> @@ -1080,7 +1080,7 @@ main (int argc, char *argv[])
>  		goto out;
>  	}
>  	if (cmd == CMD_FLUSH_ONE) {
> -		if (dm_is_mpath(dev) != 1) {
> +		if (dm_is_mpath(dev) != DM_IS_MPATH_YES) {
>  			condlog(0, "%s is not a multipath device", dev);
>  			r = RTVL_FAIL;
>  			goto out;
> diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c
> index 3fbdc55..af1e12e 100644
> --- a/multipathd/dmevents.c
> +++ b/multipathd/dmevents.c
> @@ -168,9 +168,13 @@ static int dm_get_events(void)
>  	while (names->dev) {
>  		uint32_t event_nr;
>  
> -		/* Don't delete device if dm_is_mpath() fails without
> -		 * checking the device type */
> -		if (dm_is_mpath(names->name) == 0)
> +		/*
> +		 * Don't delete device if dm_is_mpath() fails without
> +		 * checking the device type.
> +		 * IOW, only delete devices from the event list for which
> +		 *  we positively know that they aren't multipath devices.
> +		 */
> +		if (dm_is_mpath(names->name) == DM_IS_MPATH_NO)
>  			goto next;
>  
>  		event_nr = dm_event_nr(names);
> @@ -206,9 +210,12 @@ int watch_dmevents(char *name)
>  	struct dev_event *dev_evt, *old_dev_evt;
>  	int i;
>  
> -	/* We know that this is a multipath device, so only fail if
> -	 * device-mapper tells us that we're wrong */
> -	if (dm_is_mpath(name) == 0) {
> +	/*
> +	 * We know that this is a multipath device, so only fail if
> +	 * device-mapper tells us that we're wrong
> +	 * IOW, don't fail for DM generic errors.
> +	 */
> +	if (dm_is_mpath(name) == DM_IS_MPATH_NO) {
>  		condlog(0, "%s: not a multipath device. can't watch events",
>  			name);
>  		return -1;
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 58afe14..132bb2e 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -865,7 +865,7 @@ ev_add_map (char * dev, const char * alias, struct vectors * vecs)
>  	int reassign_maps;
>  	struct config *conf;
>  
> -	if (dm_is_mpath(alias) != 1) {
> +	if (dm_is_mpath(alias) != DM_IS_MPATH_YES) {
>  		condlog(4, "%s: not a multipath map", alias);
>  		return 0;
>  	}
> -- 
> 2.45.2
diff mbox series

Patch

diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c
index 178c2f5..6da0401 100644
--- a/libmpathpersist/mpath_persist_int.c
+++ b/libmpathpersist/mpath_persist_int.c
@@ -185,7 +185,7 @@  static int mpath_get_map(vector curmp, vector pathvec, int fd, char **palias,
 
 	condlog(3, "alias = %s", alias);
 
-	if (dm_map_present(alias) && dm_is_mpath(alias) != 1){
+	if (dm_map_present(alias) && dm_is_mpath(alias) != DM_IS_MPATH_YES) {
 		condlog(3, "%s: not a multipath device.", alias);
 		goto out;
 	}
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index a63154f..3abdc26 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -846,15 +846,9 @@  static int dm_type_match(const char *name, char *type)
 		return DM_TYPE_NOMATCH;
 }
 
-/*
- * returns:
- * 1  : is multipath device
- * 0  : is not multipath device
- * -1 : error
- */
 int dm_is_mpath(const char *name)
 {
-	int r = -1;
+	int r = DM_IS_MPATH_ERR;
 	struct dm_task __attribute__((cleanup(cleanup_dm_task))) *dmt = NULL;
 	struct dm_info info;
 	uint64_t start, length;
@@ -876,7 +870,7 @@  int dm_is_mpath(const char *name)
 	if (!dm_task_get_info(dmt, &info))
 		goto out;
 
-	r = 0;
+	r = DM_IS_MPATH_NO;
 
 	if (!info.exists)
 		goto out;
@@ -895,10 +889,10 @@  int dm_is_mpath(const char *name)
 	if (!target_type || strcmp(target_type, TGT_MPATH) != 0)
 		goto out;
 
-	r = 1;
+	r = DM_IS_MPATH_YES;
 out:
-	if (r < 0)
-		condlog(3, "%s: dm command failed in %s: %s", name, __FUNCTION__, strerror(errno));
+	if (r == DM_IS_MPATH_ERR)
+		condlog(3, "%s: dm command failed in %s: %s", name, __func__, strerror(errno));
 	return r;
 }
 
@@ -1039,7 +1033,7 @@  int _dm_flush_map (const char *mapname, int flags, int retries)
 	unsigned long long mapsize;
 	char *params = NULL;
 
-	if (dm_is_mpath(mapname) != 1)
+	if (dm_is_mpath(mapname) != DM_IS_MPATH_YES)
 		return DM_FLUSH_OK; /* nothing to do */
 
 	/* if the device currently has no partitions, do not
@@ -1086,7 +1080,7 @@  int _dm_flush_map (const char *mapname, int flags, int retries)
 			}
 			condlog(4, "multipath map %s removed", mapname);
 			return DM_FLUSH_OK;
-		} else if (dm_is_mpath(mapname) != 1) {
+		} else if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
 			condlog(4, "multipath map %s removed externally",
 				mapname);
 			return DM_FLUSH_OK; /* raced. someone else removed it */
@@ -1316,7 +1310,7 @@  int dm_get_maps(vector mp)
 	}
 
 	do {
-		if (dm_is_mpath(names->name) != 1)
+		if (dm_is_mpath(names->name) != DM_IS_MPATH_YES)
 			goto next;
 
 		mpp = dm_get_multipath(names->name);
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index ff28575..9438c2d 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -46,6 +46,12 @@  int dm_map_present (const char *name);
 int dm_map_present_by_uuid(const char *uuid);
 int dm_get_map(const char *name, unsigned long long *size, char **outparams);
 int dm_get_status(const char *name, char **outstatus);
+
+enum {
+	DM_IS_MPATH_NO,
+	DM_IS_MPATH_YES,
+	DM_IS_MPATH_ERR,
+};
 int dm_is_mpath(const char *name);
 
 enum {
diff --git a/multipath/main.c b/multipath/main.c
index ce702e7..c82bc86 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -247,7 +247,7 @@  static int check_usable_paths(struct config *conf,
 		goto out;
 	}
 
-	if (dm_is_mpath(mapname) != 1) {
+	if (dm_is_mpath(mapname) != DM_IS_MPATH_YES) {
 		condlog(1, "%s is not a multipath map", devpath);
 		goto free;
 	}
@@ -1080,7 +1080,7 @@  main (int argc, char *argv[])
 		goto out;
 	}
 	if (cmd == CMD_FLUSH_ONE) {
-		if (dm_is_mpath(dev) != 1) {
+		if (dm_is_mpath(dev) != DM_IS_MPATH_YES) {
 			condlog(0, "%s is not a multipath device", dev);
 			r = RTVL_FAIL;
 			goto out;
diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c
index 3fbdc55..af1e12e 100644
--- a/multipathd/dmevents.c
+++ b/multipathd/dmevents.c
@@ -168,9 +168,13 @@  static int dm_get_events(void)
 	while (names->dev) {
 		uint32_t event_nr;
 
-		/* Don't delete device if dm_is_mpath() fails without
-		 * checking the device type */
-		if (dm_is_mpath(names->name) == 0)
+		/*
+		 * Don't delete device if dm_is_mpath() fails without
+		 * checking the device type.
+		 * IOW, only delete devices from the event list for which
+		 *  we positively know that they aren't multipath devices.
+		 */
+		if (dm_is_mpath(names->name) == DM_IS_MPATH_NO)
 			goto next;
 
 		event_nr = dm_event_nr(names);
@@ -206,9 +210,12 @@  int watch_dmevents(char *name)
 	struct dev_event *dev_evt, *old_dev_evt;
 	int i;
 
-	/* We know that this is a multipath device, so only fail if
-	 * device-mapper tells us that we're wrong */
-	if (dm_is_mpath(name) == 0) {
+	/*
+	 * We know that this is a multipath device, so only fail if
+	 * device-mapper tells us that we're wrong
+	 * IOW, don't fail for DM generic errors.
+	 */
+	if (dm_is_mpath(name) == DM_IS_MPATH_NO) {
 		condlog(0, "%s: not a multipath device. can't watch events",
 			name);
 		return -1;
diff --git a/multipathd/main.c b/multipathd/main.c
index 58afe14..132bb2e 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -865,7 +865,7 @@  ev_add_map (char * dev, const char * alias, struct vectors * vecs)
 	int reassign_maps;
 	struct config *conf;
 
-	if (dm_is_mpath(alias) != 1) {
+	if (dm_is_mpath(alias) != DM_IS_MPATH_YES) {
 		condlog(4, "%s: not a multipath map", alias);
 		return 0;
 	}