Re: [PATCH 23/44] libmultipath: improve dm_get_wwid() return value logic

[Benjamin Marzinski <bmarzins@redhat.com>]

On Thu, Jul 11, 2024 at 02:25:54PM +0200, Martin Wilck wrote:
> On Wed, 2024-07-10 at 19:34 -0400, Benjamin Marzinski wrote:
> > On Tue, Jul 09, 2024 at 11:39:14PM +0200, Martin Wilck wrote:
> > > Make dm_get_wwid() return different status codes for non-existing
> > > maps,
> > > maps that exists but are not multipath maps, and generic error
> > > case,
> > > and handle these return codes appropriately in callers.
> > > 
> > > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > > ---
> > >  libmultipath/alias.c     |  5 +++--
> > >  libmultipath/configure.c | 23 +++++++++++------------
> > >  libmultipath/devmapper.c | 21 ++++++++++++++++-----
> > >  libmultipath/wwids.c     |  2 +-
> > >  tests/alias.c            | 10 +++++-----
> > >  5 files changed, 36 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/libmultipath/alias.c b/libmultipath/alias.c
> > > index 10e58a7..2ab9499 100644
> > > --- a/libmultipath/alias.c
> > > +++ b/libmultipath/alias.c
> > > @@ -408,13 +408,14 @@ static bool alias_already_taken(const char
> > > *alias, const char *map_wwid)
> > >  {
> > >  
> > >  	char wwid[WWID_SIZE];
> > > +	int rc = dm_get_wwid(alias, wwid, sizeof(wwid));
> > >  
> > >  	/* If the map doesn't exist, it's fine */
> > > -	if (dm_get_wwid(alias, wwid, sizeof(wwid)) != 0)
> > 
> > We used to assume the alias was not taken if dm_get_wwid() failed
> > with
> > an error. Now we assume that the alias is taken. I think we should
> > continue to assume we can use the alias if we get DM_ERR.
> 
> I am not sure about this. but I can change it. I guess it's a corner
> case anyway, where there's no obvious right or wrong behavior. 
> 
> A DM_DEVICE_INFO call with this map name failed with an error other
> than ENXIO. By going on with this map name, we clearly risk that future
> failures will happen when we try to create or reload the map in
> question.

Fair enough. The worst that could happen if we keep your method is that
we skip a usable name, which would get used by a future device anyway.
I'm dropping my objection.

-Ben