Re: [PATCH 0/8] multipath fixes to tableless device handling

[Benjamin Marzinski <bmarzins@redhat.com>]

On Thu, Nov 07, 2024 at 01:20:09PM +0100, Martin Wilck wrote:
> On Wed, 2024-11-06 at 17:52 +0100, Martin Wilck wrote:
> > On Thu, 2024-10-31 at 14:32 -0400, Benjamin Marzinski wrote:
> > > This patchset include a couple of miscellaneous cleanups, but is
> > > mostly
> > > in response to issues brought up in
> > > https://github.com/opensvc/multipath-tools/issues/100
> > > It adds auto restarting to the multipathd.service unit file. I'm
> > > fairly
> > > conflicted about the correct limits to be placed on these restarts,
> > > so
> > > if anyone has strong opinions and a good argument, I'm more than
> > > willing
> > > to change them.
> > > 
> > > The bulk of the changes deal with how multipath handles devices
> > > without
> > > any table. Multipath was supposed to delete these if they were left
> > > behind after a failed device creation, but that code was broken.
> > > However devices aren't typically left behind after failed creates,
> > > so
> > > it
> > > didn't matter.  
> > > 
> > > A bigger issue is that multipathd and multipath could fail if
> > > tableless
> > > devices were present. With this patchset, they will simply ignore
> > > tableless devices that don't have a multipath prefixed DM UUID. The
> > > last
> > > patch is a RFC patch that changes the behavior for tableless
> > > devices
> > > that do have a multipath prefixed DM UUID. It makes multipath
> > > remove
> > > these devices on the grounds that they are likely failed creates.
> > > However, looking at the libdevmapper code, I'm not sure that we
> > > actually
> > > want to do this.  When a DM_DEVICE_CREATE task is run, it will
> > > first
> > > create a tableless device, and then immediately load the table and
> > > resume the device. So it's possible any that tableless devices
> > > which
> > > multipath sees are actually in the process of getting created. I
> > > left
> > > the patch as part of the set, but I'm not sure that removing the
> > > devices
> > > is the right answer here. I haven't ever seen any tableless
> > > multipath
> > > devices lying around (and I couldn't force any to get created when
> > > I
> > > tried). Unless other people have seen these, then I don't think the
> > > final patch of this set should go in.
> > > 
> > 
> > Does it do any harm if we just keep these devices around? If there
> > are
> > path devices around with matching UUIDs, the right thing to do for
> > multipathd would be to reload the map's table with an appropriate
> > multipath target. Otherwise, the map will just float around empty and
> > do no harm. This is how multipathd 0.9.9 and earlier behave, and
> > I see no issue with that.
> 
> Thinking about it, I believe we should actually accept devices that
> have an mpath UUID and an empty table, and add them to our mpvec. We
> should treat them like devices that have a multipath target but no path
> devices. If any matching paths become available, the map will be
> reloaded and the issue will be fixed. IMO, this is less prone to race
> conditions than trying to delete and re-add the devices.

I'm not sure off the top of my head how easy it will be to handle
devices with no dm table at all, but I'll take a look into this.

-Ben
 
> Martin