| Message ID | 20241115232256.627933-4-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | multipath-tools: Handle tableless DM devices | expand |
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. Martin
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 >
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); - dm_flush_map_nosync(mpp->alias); + dm_device_remove(mpp->alias, 0); } if (err != EROFS) { condlog(3, "%s: failed to load map, error %d", diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index 23926e3f..0bd2b907 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -174,7 +174,6 @@ enum { int dm_flush_map__ (const char *mapname, int flags, int retries); #define dm_flush_map(mapname) dm_flush_map__(mapname, DMFL_NEED_SYNC, 0) -#define dm_flush_map_nosync(mapname) dm_flush_map__(mapname, DMFL_NONE, 0) #define dm_suspend_and_flush_map(mapname, retries) \ dm_flush_map__(mapname, DMFL_NEED_SYNC|DMFL_SUSPEND, retries) int dm_flush_map_nopaths(const char * mapname, int deferred_remove);
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(-)