| Message ID | 20241031183301.391416-7-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Benjamin Marzinski |
| Headers | show |
| Series | multipath fixes to tableless device handling | expand |
On Thu, 2024-10-31 at 14:32 -0400, Benjamin Marzinski wrote: > if libmp_mapinfo() is run on a device that has a multipath prefixed > DM > UUID but no table (either active or inactive), it will now return > with a > new exit code, DMP_BAD_DEV, to signal that this is an invalid > multipath > device. Currently all code paths treat this identically to > DMP_NOT_FOUND. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > libmultipath/devmapper.c | 18 ++++++++++++++++++ > libmultipath/devmapper.h | 5 ++++- > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > index 13d8c1e5..6e11d5b5 100644 > --- a/libmultipath/devmapper.c > +++ b/libmultipath/devmapper.c > @@ -726,6 +726,22 @@ static int libmp_mapinfo__(int flags, mapid_t > id, mapinfo_t info, const char *ma > } > > if (info.target || info.status || info.size || flags & > MAPINFO_TGT_TYPE__) { > + if (!dmi.live_table) { > + /* > + * If this is device has a multipath uuid > but no > + * table, flag it, so multipath can clean it > up > + */ > + if (flags & MAPINFO_CHECK_UUID && > + !dmi.inactive_table) { > + condlog(2, "%s: multipath map %s has > no table", > + fname__, map_id); > + return DMP_BAD_DEV; What about the case when there's an inactive_table? AFAIU that means that a resume operation after a table (re)load failed or was interrupted, or that the program that (re)loaded the table (multipathd, probably) died before it could resume the table. In this case we'd fall through the the "map has no targets" case, but shouldn't we also treat it as DMP_BAD_DEV? Martin
On Wed, Nov 06, 2024 at 05:17:30PM +0100, Martin Wilck wrote: > On Thu, 2024-10-31 at 14:32 -0400, Benjamin Marzinski wrote: > > if libmp_mapinfo() is run on a device that has a multipath prefixed > > DM > > UUID but no table (either active or inactive), it will now return > > with a > > new exit code, DMP_BAD_DEV, to signal that this is an invalid > > multipath > > device. Currently all code paths treat this identically to > > DMP_NOT_FOUND. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > --- > > libmultipath/devmapper.c | 18 ++++++++++++++++++ > > libmultipath/devmapper.h | 5 ++++- > > 2 files changed, 22 insertions(+), 1 deletion(-) > > > > diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c > > index 13d8c1e5..6e11d5b5 100644 > > --- a/libmultipath/devmapper.c > > +++ b/libmultipath/devmapper.c > > @@ -726,6 +726,22 @@ static int libmp_mapinfo__(int flags, mapid_t > > id, mapinfo_t info, const char *ma > > } > > > > if (info.target || info.status || info.size || flags & > > MAPINFO_TGT_TYPE__) { > > + if (!dmi.live_table) { > > + /* > > + * If this is device has a multipath uuid > > but no > > + * table, flag it, so multipath can clean it > > up > > + */ > > + if (flags & MAPINFO_CHECK_UUID && > > + !dmi.inactive_table) { > > + condlog(2, "%s: multipath map %s has > > no table", > > + fname__, map_id); > > + return DMP_BAD_DEV; > > > What about the case when there's an inactive_table? AFAIU that means > that a resume operation after a table (re)load failed or was > interrupted, or that the program that (re)loaded the table (multipathd, > probably) died before it could resume the table. > > In this case we'd fall through the the "map has no targets" case, but > shouldn't we also treat it as DMP_BAD_DEV? This is because I was deleting these devices in patch 8, and I didn't want to delete devices that where in the process of getting modified by multipath. The code could still delete devices that were getting created if it ran during the window between when the dm device gets created, and a table gets loaded. In that case there would be no table. But there is another window that happens after this during creation and during a reload failure, where the device doesn't have an active table but still has an inactive one. I didn't want to delete devices like this, since it could effect existing devices. We could, for instance, end up deleting a device that multipath was currently working on and was about to clean up itself. But if we aren't deleting these devices, then yes, it would make more sense to characterize these as DMP_BAD_DEV. -Ben > > Martin
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c index 13d8c1e5..6e11d5b5 100644 --- a/libmultipath/devmapper.c +++ b/libmultipath/devmapper.c @@ -726,6 +726,22 @@ static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *ma } if (info.target || info.status || info.size || flags & MAPINFO_TGT_TYPE__) { + if (!dmi.live_table) { + /* + * If this is device has a multipath uuid but no + * table, flag it, so multipath can clean it up + */ + if (flags & MAPINFO_CHECK_UUID && + !dmi.inactive_table) { + condlog(2, "%s: multipath map %s has no table", + fname__, map_id); + return DMP_BAD_DEV; + } else { + condlog(2, "%s: map %s has no table", fname__, + map_id); + return DMP_NOT_FOUND; + } + } if (dm_get_next_target(dmt, NULL, &start, &length, &target_type, ¶ms) != NULL) { condlog(2, "%s: map %s has multiple targets", fname__, map_id); @@ -869,6 +885,7 @@ int dm_is_mpath(const char *name) return DM_IS_MPATH_YES; case DMP_NOT_FOUND: case DMP_NO_MATCH: + case DMP_BAD_DEV: return DM_IS_MPATH_NO; case DMP_ERR: default: @@ -1262,6 +1279,7 @@ int dm_get_maps(vector mp) } vector_set_slot(mp, mpp); break; + case DMP_BAD_DEV: case DMP_NO_MATCH: case DMP_NOT_FOUND: break; diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h index ba05e0a1..a2dcfb84 100644 --- a/libmultipath/devmapper.h +++ b/libmultipath/devmapper.h @@ -35,6 +35,7 @@ enum { DMP_OK, DMP_NOT_FOUND, DMP_NO_MATCH, + DMP_BAD_DEV, DMP_LAST__, }; @@ -99,7 +100,9 @@ typedef struct libmp_map_info { * @returns: * DMP_OK if successful. * DMP_NOT_FOUND if the map wasn't found, or has no or multiple targets. - * DMP_NO_MATCH if the map didn't match @tgt_type (see above). + * DMP_NO_MATCH if the map didn't match @tgt_type (see above) or didn't + * have a multipath uuid prefix. + * DMP_BAD_DEV if the map has a multipath uuid prefix but no table. * DMP_ERR if some other error occurred. * * This function obtains the requested information for the device-mapper map
if libmp_mapinfo() is run on a device that has a multipath prefixed DM UUID but no table (either active or inactive), it will now return with a new exit code, DMP_BAD_DEV, to signal that this is an invalid multipath device. Currently all code paths treat this identically to DMP_NOT_FOUND. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- libmultipath/devmapper.c | 18 ++++++++++++++++++ libmultipath/devmapper.h | 5 ++++- 2 files changed, 22 insertions(+), 1 deletion(-)