Re: [PATCH 4/7] libmultipath: add name and minor outputs for dm_map_present_by_uuid()

[Benjamin Marzinski <bmarzins@redhat.com>]

On Thu, Jun 27, 2024 at 11:27:41AM +0200, Martin Wilck wrote:
> On Wed, 2024-06-05 at 19:22 -0400, Benjamin Marzinski wrote:
> > add arguments to dm_map_present_by_uuid() to allow optionally
> > fetching
> > the device name and minor number for the devices found by WWID. These
> > will be used by a later patch.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> I have to say I don't like this much. At least the function name should
> be changed. But actually, our handling of DM_DEVICE_INFO and
> DM_DEVICE_STATUS dm tasks needs refactoring. As far as the kernel is
> concerned, all dm ioctls that refer to an existing map will find the
> map either by uuid, by name, or by major/minor, and the map uuid and
> name will always be filled in.
> 
> I'll see if I can come up with something to clean this up.

Do you want be to resend this patch with a name change, or will that
just happen as part of your cleanup?

-Ben

> 
> Regards
> Martin 
> 
> 
> > ---
> >  libmultipath/devmapper.c          | 11 ++++++++++-
> >  libmultipath/devmapper.h          |  2 +-
> >  libmultipath/libmultipath.version |  2 +-
> >  libmultipath/valid.c              |  2 +-
> >  4 files changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> > index 08bb3c51..7fe68841 100644
> > --- a/libmultipath/devmapper.c
> > +++ b/libmultipath/devmapper.c
> > @@ -936,7 +936,7 @@ out:
> >   *  -1 : error
> >   */
> >  int
> > -dm_map_present_by_uuid(const char *uuid)
> > +dm_map_present_by_uuid(const char *uuid, char **name_p, int *minor)
> >  {
> >  	struct dm_task *dmt;
> >  	struct dm_info info;
> > @@ -966,6 +966,15 @@ dm_map_present_by_uuid(const char *uuid)
> >  		goto out_task;
> >  
> >  	r = !!info.exists;
> > +	if (!r)
> > +		goto out_task;
> > +
> > +	if (name_p) {
> > +		const char *name = dm_task_get_name(dmt);
> > +		*name_p = (name && strlen(name)) ? strdup(name) :
> > NULL;
> > +	}
> > +	if (minor)
> > +		*minor = info.minor;
> >  
> >  out_task:
> >  	dm_task_destroy(dmt);
> > diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
> > index 19b79c5b..d9c71e92 100644
> > --- a/libmultipath/devmapper.h
> > +++ b/libmultipath/devmapper.h
> > @@ -43,7 +43,7 @@ int dm_simplecmd_noflush (int task, const char
> > *name, uint16_t udev_flags);
> >  int dm_addmap_create (struct multipath *mpp, char *params);
> >  int dm_addmap_reload (struct multipath *mpp, char *params, int
> > flush);
> >  int dm_map_present (const char *name);
> > -int dm_map_present_by_uuid(const char *uuid);
> > +int dm_map_present_by_uuid(const char *uuid, char **name_p, int
> > *minor);
> >  int dm_get_map(const char *name, unsigned long long *size, char
> > **outparams);
> >  int dm_get_status(const char *name, char **outstatus);
> >  int dm_type(const char *name, char *type);
> > diff --git a/libmultipath/libmultipath.version
> > b/libmultipath/libmultipath.version
> > index eb511749..600de394 100644
> > --- a/libmultipath/libmultipath.version
> > +++ b/libmultipath/libmultipath.version
> > @@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 {
> >  	put_multipath_config;
> >  };
> >  
> > -LIBMULTIPATH_24.0.0 {
> > +LIBMULTIPATH_25.0.0 {
> >  global:
> >  	/* symbols referenced by multipath and multipathd */
> >  	add_foreign;
> > diff --git a/libmultipath/valid.c b/libmultipath/valid.c
> > index f2237787..3b060192 100644
> > --- a/libmultipath/valid.c
> > +++ b/libmultipath/valid.c
> > @@ -360,7 +360,7 @@ is_path_valid(const char *name, struct config
> > *conf, struct path *pp,
> >  	if (check_wwids_file(pp->wwid, 0) == 0)
> >  		return PATH_IS_VALID_NO_CHECK;
> >  
> > -	if (dm_map_present_by_uuid(pp->wwid) == 1)
> > +	if (dm_map_present_by_uuid(pp->wwid, NULL, NULL) == 1)
> >  		return PATH_IS_VALID;
> >  
> >  	/* all these act like FIND_MULTIPATHS_STRICT for finding if
> > a