diff mbox series

[1/7] libmultipath: change do_get_info returns

Message ID 1592439867-18427-2-git-send-email-bmarzins@redhat.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show
Series Fix muitpath/multipathd flush issue | expand

Commit Message

Benjamin Marzinski June 18, 2020, 12:24 a.m. UTC
Make do_get_info() differentiate between dm failures and missing
devices, and update callers to retain their current behavior. Also,
rename it and make it external. These changes will be used by future
commits.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c | 29 ++++++++++++++++-------------
 libmultipath/devmapper.h |  1 +
 2 files changed, 17 insertions(+), 13 deletions(-)

Comments

Martin Wilck June 18, 2020, 3:27 p.m. UTC | #1
On Wed, 2020-06-17 at 19:24 -0500, Benjamin Marzinski wrote:
> Make do_get_info() differentiate between dm failures and missing
> devices, and update callers to retain their current behavior. Also,
> rename it and make it external. These changes will be used by future
> commits.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/devmapper.c | 29 ++++++++++++++++-------------
>  libmultipath/devmapper.h |  1 +
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 27d52398..b44f7545 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -496,8 +496,14 @@ int dm_addmap_reload(struct multipath *mpp, char
> *params, int flush)
>  	return 0;
>  }
>  
> -static int
> -do_get_info(const char *name, struct dm_info *info)
> +/*
> + * Returns:
> + * -1: Error
> + *  0: device does not exist
> + *  1: device exists
> + */

Can we use symbolic values here please? In particular as you have
changed the "success" return value from 0 to 1...

One day we should come up with a proper return value scheme
for libmultipath, defining specific enums for every function
doesn't scale. But do it here for now nonetheless, please.

Apart from that, ok.

Regards
Martin
Benjamin Marzinski June 18, 2020, 11:17 p.m. UTC | #2
On Thu, Jun 18, 2020 at 03:27:22PM +0000, Martin Wilck wrote:
> On Wed, 2020-06-17 at 19:24 -0500, Benjamin Marzinski wrote:
> > Make do_get_info() differentiate between dm failures and missing
> > devices, and update callers to retain their current behavior. Also,
> > rename it and make it external. These changes will be used by future
> > commits.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  libmultipath/devmapper.c | 29 ++++++++++++++++-------------
> >  libmultipath/devmapper.h |  1 +
> >  2 files changed, 17 insertions(+), 13 deletions(-)
> > 
> > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> > index 27d52398..b44f7545 100644
> > --- a/libmultipath/devmapper.c
> > +++ b/libmultipath/devmapper.c
> > @@ -496,8 +496,14 @@ int dm_addmap_reload(struct multipath *mpp, char
> > *params, int flush)
> >  	return 0;
> >  }
> >  
> > -static int
> > -do_get_info(const char *name, struct dm_info *info)
> > +/*
> > + * Returns:
> > + * -1: Error
> > + *  0: device does not exist
> > + *  1: device exists
> > + */
> 
> Can we use symbolic values here please? In particular as you have
> changed the "success" return value from 0 to 1...
> 
> One day we should come up with a proper return value scheme
> for libmultipath, defining specific enums for every function
> doesn't scale. But do it here for now nonetheless, please.

Sure

-Ben

> 
> Apart from that, ok.
> 
> Regards
> Martin
> 
> -- 
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE  Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix
> Imendörffer
> 

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

Patch

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 27d52398..b44f7545 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -496,8 +496,14 @@  int dm_addmap_reload(struct multipath *mpp, char *params, int flush)
 	return 0;
 }
 
-static int
-do_get_info(const char *name, struct dm_info *info)
+/*
+ * Returns:
+ * -1: Error
+ *  0: device does not exist
+ *  1: device exists
+ */
+int
+do_dm_get_info(const char *name, struct dm_info *info)
 {
 	int r = -1;
 	struct dm_task *dmt;
@@ -516,10 +522,7 @@  do_get_info(const char *name, struct dm_info *info)
 	if (!dm_task_get_info(dmt, info))
 		goto out;
 
-	if (!info->exists)
-		goto out;
-
-	r = 0;
+	r = !!info->exists;
 out:
 	dm_task_destroy(dmt);
 	return r;
@@ -529,7 +532,7 @@  int dm_map_present(const char * str)
 {
 	struct dm_info info;
 
-	return (do_get_info(str, &info) == 0);
+	return (do_dm_get_info(str, &info) == 1);
 }
 
 int dm_get_map(const char *name, unsigned long long *size, char *outparams)
@@ -820,7 +823,7 @@  dm_dev_t (const char * mapname, char * dev_t, int len)
 {
 	struct dm_info info;
 
-	if (do_get_info(mapname, &info) != 0)
+	if (do_dm_get_info(mapname, &info) != 1)
 		return 1;
 
 	if (snprintf(dev_t, len, "%i:%i", info.major, info.minor) > len)
@@ -862,7 +865,7 @@  dm_get_major_minor(const char *name, int *major, int *minor)
 {
 	struct dm_info info;
 
-	if (do_get_info(name, &info) != 0)
+	if (do_dm_get_info(name, &info) != 1)
 		return -1;
 
 	*major = info.major;
@@ -1199,7 +1202,7 @@  dm_geteventnr (const char *name)
 {
 	struct dm_info info;
 
-	if (do_get_info(name, &info) != 0)
+	if (do_dm_get_info(name, &info) != 1)
 		return -1;
 
 	return info.event_nr;
@@ -1210,7 +1213,7 @@  dm_is_suspended(const char *name)
 {
 	struct dm_info info;
 
-	if (do_get_info(name, &info) != 0)
+	if (do_dm_get_info(name, &info) != 1)
 		return -1;
 
 	return info.suspended;
@@ -1383,7 +1386,7 @@  dm_get_deferred_remove (const char * mapname)
 {
 	struct dm_info info;
 
-	if (do_get_info(mapname, &info) != 0)
+	if (do_dm_get_info(mapname, &info) != 1)
 		return -1;
 
 	return info.deferred_remove;
@@ -1442,7 +1445,7 @@  dm_get_info (const char * mapname, struct dm_info ** dmi)
 	if (!*dmi)
 		return 1;
 
-	if (do_get_info(mapname, *dmi) != 0) {
+	if (do_dm_get_info(mapname, *dmi) != 1) {
 		memset(*dmi, 0, sizeof(struct dm_info));
 		FREE(*dmi);
 		*dmi = NULL;
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 5ed7edc5..79c9afb2 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -66,6 +66,7 @@  char * dm_mapname(int major, int minor);
 int dm_remove_partmaps (const char * mapname, int need_sync,
 			int deferred_remove);
 int dm_get_uuid(const char *name, char *uuid, int uuid_len);
+int do_dm_get_info(const char *, struct dm_info *);
 int dm_get_info (const char * mapname, struct dm_info ** dmi);
 int dm_rename (const char * old, char * new, char * delim, int skip_kpartx);
 int dm_reassign(const char * mapname);