On Wed, 2024-07-10 at 20:21 -0400, Benjamin Marzinski wrote:
> On Tue, Jul 09, 2024 at 11:39:19PM +0200, Martin Wilck wrote:
> > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> > ---
> > libmultipath/devmapper.c | 53 ++++++------------------------------
> > ----
> > 1 file changed, 8 insertions(+), 45 deletions(-)
> >
> > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> > index 859a861..6276041 100644
> > --- a/libmultipath/devmapper.c
> > +++ b/libmultipath/devmapper.c
> > @@ -965,52 +965,15 @@ static int dm_type_match(const char *name,
> > char *type)
> >
> > int dm_is_mpath(const char *name)
> > {
> > - int r = DM_IS_MPATH_ERR;
> > - struct dm_task __attribute__((cleanup(cleanup_dm_task)))
> > *dmt = NULL;
> > - struct dm_info info;
> > - uint64_t start, length;
> > - char *target_type = NULL;
> > - char *params;
> > - const char *uuid;
> > + char uuid[DM_UUID_LEN];
> > + int rc = libmp_mapinfo(DM_MAP_BY_NAME,
> > + (mapid_t) { .str = name },
> > + (mapinfo_t) { .uuid = uuid,
> > .tgt_type = TGT_MPATH });
> >
> > - if (!(dmt = libmp_dm_task_create(DM_DEVICE_TABLE)))
> > - goto out;
> > -
> > - if (!dm_task_set_name(dmt, name))
> > - goto out;
> > -
> > - if (!libmp_dm_task_run(dmt)) {
> > - dm_log_error(3, DM_DEVICE_TABLE, dmt);
> > - goto out;
> > - }
> > -
> > - if (!dm_task_get_info(dmt, &info))
> > - goto out;
> > -
> > - r = DM_IS_MPATH_NO;
> > -
> > - if (!info.exists)
> > - goto out;
> > -
> > - uuid = dm_task_get_uuid(dmt);
> > -
> > - if (!uuid || strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN)
> > != 0)
> > - goto out;
> > -
> > - /* Fetch 1st target */
> > - if (dm_get_next_target(dmt, NULL, &start, &length,
> > &target_type,
> > - ¶ms) != NULL)
> > - /* multiple targets */
> > - goto out;
> > -
> > - if (!target_type || strcmp(target_type, TGT_MPATH) != 0)
> > - goto out;
> > -
> > - r = DM_IS_MPATH_YES;
> > -out:
> > - if (r < 0)
> > - condlog(3, "%s: dm command failed in %s: %s",
> > name, __func__, strerror(errno));
> > - return r;
>
> Since dm_get_events() and watch_dmevents() need to differentiate
> between
> DM_IS_MPATH_NO and DM_IS_MPATH_ERR, this function needs to return
> DM_IS_MPATH_ERR if libmp_mapinfo() returns DM_ERR.
>
> Looking over this function, I did notice one thing. It returns
> DM_IS_MPATH_ERR if dm_task_get_info() fails, while libmp_mapinfo__()
> returns DMP_NOT_FOUND if dm_task_get_info() fails.
> Looking at the
> current lvm code, I can't see a way for dm_task_get_info() to fail
> if we just called libmp_dm_task_run() and it succeeded. On the other
> hand dm_task_get_info() failing does seem to be an error, not a
> a sign that the device doesn't exist, so perhaps ibmp_mapinfo__()
> should treat it like one. Although most of our code doesn't, and like
> I
> said, I don't see how it could fail unless dm_task_run() already
> failed
> (or wasn't called).
Right. I also looked at the lvm code, and dm_task_get_XYZ functions
hardly ever fail if running the task succeeded, and they probably never
will. But we shouldn't make any assumptions about libdevmapper in
libmultipath. I will change libmp_mapinfo() to return DMP_ERR in this
case.
Thanks,
Martin
>
> Any thoughts?
>
> -Ben
>
>
> > + if (rc != DMP_OK || strncmp(uuid, UUID_PREFIX,
> > UUID_PREFIX_LEN))
> > + return DM_IS_MPATH_NO;
> > + else
> > + return DM_IS_MPATH_YES;
> > }
> >
> > int dm_map_present_by_wwid(const char *wwid)
> > --
> > 2.45.2
>
