Re: [PATCH v2 10/20] libmultipath: indicate wwid failure in dm_addmap_create()

From: Martin Wilck <mwilck@suse.com>
To: Benjamin Marzinski <bmarzins@redhat.com>
Cc: dm-devel@redhat.com
Subject: Re: [PATCH v2 10/20] libmultipath: indicate wwid failure in dm_addmap_create()
Date: Mon, 26 Mar 2018 22:07:46 +0200	[thread overview]
Message-ID: <1522094866.19335.105.camel@suse.com> (raw)
In-Reply-To: <20180326175228.GO3103@octiron.msp.redhat.com>

On Mon, 2018-03-26 at 12:52 -0500, Benjamin Marzinski wrote:
> On Mon, Mar 19, 2018 at 04:01:45PM +0100, Martin Wilck wrote:
> > dm_addmap_create() is where we actually try to set up a new
> > multipath map. Depending on the result, mark the wwid as
> > failed (or not), and re-trigger an uevent if necessary.
> > If a path changes from multipath to non-multipath, use an "add"
> > event to make sure LVM2 rules pick it up. Increase log level
> > of this event to 3.
> 
> I'm not sure of a specific problem with this patch, but I am a little
> leery of sending out our own "add" events. Unless I am mistaken there
> is
> usually there is only one add event per device, and there is extra
> work
> done on add events that we might not want to do twice. I wonder if it
> would be better to make other udev rules be able to respond to change
> events from ex-multipath paths.  Have you looked into the
> implications
> of sending out these add events?

Yes. I reviewed the udev rules on a few current systems. There are only
very few rules that act on "add" only. For those devices that matter
here (block devices, non-dm) I see only the following two:

60-block.rules: a rule that enables parameters/events_dfl_poll_msecs.
  => not critical, we just disable/enable the polling once
     more.
69-dm-lvm-metad.rules:ACTION!="add", GOTO="lvm_end"
  =>  this is the rule that forced me to synthesize an "add" event.

So indeed, I think that at least for the current state of affairs
sending an "add" event is safe.

Wrt "make other udev rules be able to respond to change events" - 
LVM2 commit 756bcab explains in detail why LVM2 switched to scanning on
"add" events only. This code is >5 years old. It might be possible to
distinguish different kinds of "change" events for lvmetad (e.g.
remembering "DM_MULTIPATH_DEVICE_PATH" and re-scanning when it has
changed from "1" to "0") but to be honest, I'd rather not want to
depend on such a change being merged in lvm2. I've submitted a minor,
IMO rather un-controversial fix for 69-dm-lvmetad.rules 3 monts ago
("LVM2: fix lvmetad udev rules for CHANGE events") and sent out
multiple reminders, without receiving a reaction.

Moreover, we're sending out these events for devices which have
DM_MULTIPATH_DEVICE_PATH=1 and SYSTEMD_READY=0 set. For everything
above the multipath layer, it's actually as if the device had just been
"add"ed.

I think this will either work with synthesized "add" events, or not at
all. Please correct me if I've overlooked something.

Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel