Re: [PATCH 3/6] libmultipath: fix removing device after failed creation

[Benjamin Marzinski <bmarzins@redhat.com>]

On Mon, Nov 18, 2024 at 12:21:01PM +0100, Martin Wilck wrote:
> On Fri, 2024-11-15 at 18:22 -0500, Benjamin Marzinski wrote:
> > dm_flush_nap_nosync() only removes a device if it has a multipath
> > table.
> > On failed removes, there is no table, so this function does nothing.
> > Instead, if libmp_mapinfo() returns DMP_EMPTY, then there is an empty
> > map,
> > and it is removed using dm_device_remove().
> > 
> > Also, since there are no longer any callers of dm_flush_map_nosync(),
> > remove it.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  libmultipath/devmapper.c | 7 +++++--
> >  libmultipath/devmapper.h | 1 -
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> > index b718dccf..842e7c80 100644
> > --- a/libmultipath/devmapper.c
> > +++ b/libmultipath/devmapper.c
> > @@ -557,9 +557,12 @@ int dm_addmap_create (struct multipath *mpp,
> > char * params)
> >  		 * Failing the second part leaves an empty map.
> > Clean it up.
> >  		 */
> >  		err = errno;
> > -		if (dm_map_present(mpp->alias)) {
> > +		if (libmp_mapinfo(DM_MAP_BY_NAME |
> > MAPINFO_MPATH_ONLY |
> > +				  MAPINFO_CHECK_UUID,
> > +				(mapid_t) { .str = mpp->alias },
> > +				(mapinfo_t) { .uuid = NULL }) ==
> > DMP_EMPTY) {
> >  			condlog(3, "%s: failed to load map (a path
> > might be in use)", mpp->alias);
> 
> This error message seems wrong for emtpy tables.

This code exists because if you fail to fully create a DM device, you
are (or were, I'm not sure if this issue still exists) sometimes left
with an empty device that needs cleaning up. Previously multipath would
just delete any device that existed with the name of the device you were
trying to create. The new code only deletes empty devices, to avoid
accidentally deleting a valid device that got set up at the same time as
you were doing the create. But these empty devices should only exist if
you failed to load the table for the new device, perhaps because a path
was in use.

So the message still makes sense here, as much as it ever did.

-Ben

> 
> Martin
>