On Tue, 2017-05-09 at 11:57 -0500, Benjamin Marzinski wrote: > Multipath has a number of functions that all do basically the same > thing to get members of the dm_info structure. This just refactors > them > to use common code to do it.
This is nice, but it might be even better to simply store all relevant information from DM_DEVICE_INFO in the mpp structure in the first place, avoiding the need to do this ioctl multiple times to get various pieces of information. That can be done in a separate patch, of course. Reviewed-by: Martin Wilck <[email protected]> Regards Martin > > Signed-off-by: Benjamin Marzinski <[email protected]> > --- > libmultipath/devmapper.c | 171 +++++++++--------------------------- > ---------- > libmultipath/devmapper.h | 3 +- > multipathd/cli_handlers.c | 24 ++----- > 3 files changed, 42 insertions(+), 156 deletions(-) > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > index 1c6b87e..2c4a13a 100644 > --- a/libmultipath/devmapper.c > +++ b/libmultipath/devmapper.c > @@ -399,16 +399,16 @@ int dm_addmap_reload(struct multipath *mpp, > char *params, int flush) > return r; > } > > -int dm_map_present(const char * str) > +static int > +do_get_info(const char *name, struct dm_info *info) > { > - int r = 0; > + int r = -1; > struct dm_task *dmt; > - struct dm_info info; > > if (!(dmt = dm_task_create(DM_DEVICE_INFO))) > - return 0; > + return r; > > - if (!dm_task_set_name(dmt, str)) > + if (!dm_task_set_name(dmt, name)) > goto out; > > dm_task_no_open_count(dmt); > @@ -416,16 +416,25 @@ int dm_map_present(const char * str) > if (!dm_task_run(dmt)) > goto out; > > - if (!dm_task_get_info(dmt, &info)) > + if (!dm_task_get_info(dmt, info)) > goto out; > > - if (info.exists) > - r = 1; > + if (!info->exists) > + goto out; > + > + r = 0; > out: > dm_task_destroy(dmt); > return r; > } > > +int dm_map_present(const char * str) > +{ > + struct dm_info info; > + > + return (do_get_info(str, &info) == 0); > +} > + > int dm_get_map(const char *name, unsigned long long *size, char > *outparams) > { > int r = 1; > @@ -646,29 +655,15 @@ out: > static int > dm_dev_t (const char * mapname, char * dev_t, int len) > { > - int r = 1; > - struct dm_task *dmt; > struct dm_info info; > > - if (!(dmt = dm_task_create(DM_DEVICE_INFO))) > - return 0; > - > - if (!dm_task_set_name(dmt, mapname)) > - goto out; > - > - if (!dm_task_run(dmt)) > - goto out; > - > - if (!dm_task_get_info(dmt, &info) || !info.exists) > - goto out; > + if (do_get_info(mapname, &info) != 0) > + return 1; > > if (snprintf(dev_t, len, "%i:%i", info.major, info.minor) > > len) > - goto out; > + return 1; > > - r = 0; > -out: > - dm_task_destroy(dmt); > - return r; > + return 0; > } > > int > @@ -700,59 +695,16 @@ out: > } > > int > -dm_get_major (char * mapname) > +dm_get_major_minor(const char *name, int *major, int *minor) > { > - int r = -1; > - struct dm_task *dmt; > struct dm_info info; > > - if (!(dmt = dm_task_create(DM_DEVICE_INFO))) > - return 0; > - > - if (!dm_task_set_name(dmt, mapname)) > - goto out; > - > - if (!dm_task_run(dmt)) > - goto out; > - > - if (!dm_task_get_info(dmt, &info)) > - goto out; > - > - if (!info.exists) > - goto out; > - > - r = info.major; > -out: > - dm_task_destroy(dmt); > - return r; > -} > - > -int > -dm_get_minor (char * mapname) > -{ > - int r = -1; > - struct dm_task *dmt; > - struct dm_info info; > - > - if (!(dmt = dm_task_create(DM_DEVICE_INFO))) > - return 0; > - > - if (!dm_task_set_name(dmt, mapname)) > - goto out; > - > - if (!dm_task_run(dmt)) > - goto out; > - > - if (!dm_task_get_info(dmt, &info)) > - goto out; > - > - if (!info.exists) > - goto out; > + if (do_get_info(name, &info) != 0) > + return -1; > > - r = info.minor; > -out: > - dm_task_destroy(dmt); > - return r; > + *major = info.major; > + *minor = info.minor; > + return 0; > } > > static int > @@ -1070,31 +1022,12 @@ out: > int > dm_geteventnr (char *name) > { > - struct dm_task *dmt; > struct dm_info info; > - int event = -1; > > - if (!(dmt = dm_task_create(DM_DEVICE_INFO))) > + if (do_get_info(name, &info) != 0) > return -1; > > - if (!dm_task_set_name(dmt, name)) > - goto out; > - > - dm_task_no_open_count(dmt); > - > - if (!dm_task_run(dmt)) > - goto out; > - > - if (!dm_task_get_info(dmt, &info)) > - goto out; > - > - if (info.exists) > - event = info.event_nr; > - > -out: > - dm_task_destroy(dmt); > - > - return event; > + return info.event_nr; > } > > char * > @@ -1262,26 +1195,12 @@ cancel_remove_partmap (const char *name, void > *unused) > static int > dm_get_deferred_remove (char * mapname) > { > - int r = -1; > - struct dm_task *dmt; > struct dm_info info; > > - if (!(dmt = dm_task_create(DM_DEVICE_INFO))) > + if (do_get_info(mapname, &info) != 0) > return -1; > > - if (!dm_task_set_name(dmt, mapname)) > - goto out; > - > - if (!dm_task_run(dmt)) > - goto out; > - > - if (!dm_task_get_info(dmt, &info)) > - goto out; > - > - r = info.deferred_remove; > -out: > - dm_task_destroy(dmt); > - return r; > + return info.deferred_remove; > } > > static int > @@ -1328,9 +1247,6 @@ alloc_dminfo (void) > int > dm_get_info (char * mapname, struct dm_info ** dmi) > { > - int r = 1; > - struct dm_task *dmt = NULL; > - > if (!mapname) > return 1; > > @@ -1340,32 +1256,13 @@ dm_get_info (char * mapname, struct dm_info > ** dmi) > if (!*dmi) > return 1; > > - if (!(dmt = dm_task_create(DM_DEVICE_INFO))) > - goto out; > - > - if (!dm_task_set_name(dmt, mapname)) > - goto out; > - > - dm_task_no_open_count(dmt); > - > - if (!dm_task_run(dmt)) > - goto out; > - > - if (!dm_task_get_info(dmt, *dmi)) > - goto out; > - > - r = 0; > -out: > - if (r) { > + if (do_get_info(mapname, *dmi) != 0) { > memset(*dmi, 0, sizeof(struct dm_info)); > FREE(*dmi); > *dmi = NULL; > + return 1; > } > - > - if (dmt) > - dm_task_destroy(dmt); > - > - return r; > + return 0; > } > > struct rename_data { > diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h > index aca4454..5b66865 100644 > --- a/libmultipath/devmapper.h > +++ b/libmultipath/devmapper.h > @@ -52,8 +52,7 @@ int dm_enablegroup(char * mapname, int index); > int dm_disablegroup(char * mapname, int index); > int dm_get_maps (vector mp); > int dm_geteventnr (char *name); > -int dm_get_major (char *name); > -int dm_get_minor (char *name); > +int dm_get_major_minor (const char *name, int *major, int *minor); > char * dm_mapname(int major, int minor); > int dm_remove_partmaps (const char * mapname, int need_sync, > int deferred_remove); > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index d598039..04c7386 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -746,7 +746,7 @@ cli_add_map (void * v, char ** reply, int * len, > void * data) > char * param = get_keyparam(v, MAP); > int major, minor; > char dev_path[PATH_SIZE]; > - char *alias, *refwwid; > + char *refwwid, *alias = NULL; > int rc, count = 0; > struct config *conf; > > @@ -763,14 +763,12 @@ cli_add_map (void * v, char ** reply, int * > len, void * data) > } > put_multipath_config(conf); > do { > - minor = dm_get_minor(param); > - if (minor < 0) > + if (dm_get_major_minor(param, &major, &minor) < 0) > condlog(2, "%s: not a device mapper table", > param); > - major = dm_get_major(param); > - if (major < 0) > - condlog(2, "%s: not a device mapper table", > param); > - sprintf(dev_path, "dm-%d", minor); > - alias = dm_mapname(major, minor); > + else { > + sprintf(dev_path, "dm-%d", minor); > + alias = dm_mapname(major, minor); > + } > /*if there is no mapname found, we first create the > device*/ > if (!alias && !count) { > condlog(2, "%s: mapname not found for > %d:%d", > @@ -804,23 +802,15 @@ cli_del_map (void * v, char ** reply, int * > len, void * data) > struct vectors * vecs = (struct vectors *)data; > char * param = get_keyparam(v, MAP); > int major, minor; > - char dev_path[PATH_SIZE]; > char *alias; > int rc; > > param = convert_dev(param, 0); > condlog(2, "%s: remove map (operator)", param); > - minor = dm_get_minor(param); > - if (minor < 0) { > - condlog(2, "%s: not a device mapper table", param); > - return 1; > - } > - major = dm_get_major(param); > - if (major < 0) { > + if (dm_get_major_minor(param, &major, &minor) < 0) { > condlog(2, "%s: not a device mapper table", param); > return 1; > } > - sprintf(dev_path,"dm-%d", minor); > alias = dm_mapname(major, minor); > if (!alias) { > condlog(2, "%s: mapname not found for %d:%d", -- Dr. Martin Wilck <[email protected]>, Tel. +49 (0)911 74053 2107 SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
