On Sun, 2020-07-19 at 22:44 -0500, Benjamin Marzinski wrote:
> On Thu, Jul 09, 2020 at 01:03:26PM +0200, mwilck@xxxxxxxx wrote:
> > From: Martin Wilck <mwilck@xxxxxxxx>
> >
> > If we are in the reconfigure() code path, and we encounter maps to
> > be reloaded, we usually set the DM_SUBSYSTEM_UDEV_FLAG0 flag to
> > tell
> > udev not to repeat device detection steps above the multipath
> > layer.
> > However, if the map was previously uninitialized, we have to force
> > udev to reload.
> >
> > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
> > ---
> > libmultipath/configure.c | 61 ++++++++++++++++++++++++----------
> > ------
> > 1 file changed, 37 insertions(+), 24 deletions(-)
> >
> > diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> > index 2509053..efb5808 100644
> > --- a/libmultipath/configure.c
> > +++ b/libmultipath/configure.c
> > @@ -660,6 +660,32 @@ sysfs_set_max_sectors_kb(struct multipath
> > *mpp, int is_reload)
> > return err;
> > }
> >
> > +static void
> > +select_reload_action(struct multipath *mpp, const char *reason)
> > +{
> > + struct udev_device *mpp_ud;
> > + const char *env;
> > +
> > + /*
> > + * MPATH_DEVICE_READY != 1 can mean two things:
> > + * (a) no usable paths
> > + * (b) device was never fully processed (e.g. udev killed)
> > + * If we are in this code path (startup or forced reconfigure),
> > + * (b) can mean that upper layers like kpartx have never been
> > + * run for this map. Thus force udev reload.
> > + */
> > +
>
> This looks like it wants multipath to check if there are valid
> devices
> before setting force reload. But looking at the udev rules, I'm
> pretty
> sure it's safe. If we have no valid paths, then we will have
>
> ENV{DM_NOSCAN}="1 and ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
>
> which means it doesn't matter that force_udev_reload will cause
>
> ENV{DM_ACTIVATION}="1" and ENV{MPATH_UNCHANGED}=""
>
> It does get sort of confusing with the number of udev properties that
> can
> effect things. So nothing really needs to get done for this to be
> correct, but perhaps this code should only set force reload is there
> are
> valid paths, just to be clear.
Will do. Full ack wrt the confusing udev rules (patches welcome :D)
Note btw that multipathd triggers a synthetic change event in
reconfigure if no changes were applied; but that code is hardly ever
executed because we normally set queue_if_no_path, and during startup
multipathd will never encounter a queueing map.
Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
