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@xxxxxxxxxx>
>
> 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
