[dm-devel] [PATCH] libmultipath: fix multipath -q command logic

[Benjamin Marzinski]

On Tue, Oct 11, 2016 at 12:33:43PM +0200, Hannes Reinecke wrote:
> On 10/11/2016 11:17 AM, tang.junhui at zte.com.cn wrote:
> >Hello Hannes, Ben,
> >Could you have a review for this patch, any comment will be highly
> >appreciated.
> >
> >Thanks,
> >Tang
> >
> >
> >
> >
> >发件人:         Christophe Varoqui <christophe.varoqui at opensvc.com>
> >收件人:         tang.junhui at zte.com.cn,
> >抄送:        Bart Van Assche <bart.vanassche at sandisk.com>, device-mapper
> >development <dm-devel at redhat.com>, zhang.kai16 at zte.com.cn
> >日期:         2016/10/11 14:59
> >主题:        Re: [dm-devel] [PATCH] libmultipath: fix multipath -q
> >command logic
> >发件人:        dm-devel-bounces at redhat.com
> >------------------------------------------------------------------------
> >
> >
> >
> >Hannes, Ben,
> >
> >are you ok with the solution to these two issues.
> >Seems sane to me.
> >
> This actually is only part of the story.
> 
> The whole idea of issuing 'multipath' is to check if a given path _should_
> be multipathed (as this is typically called from an udev event).
> But as it's called from an udev event we cannot rely on the multipath daemon
> to be started; we might just handle an event which came in before multipathd
> got started from systemd.
> So checking for the PID file is not enough, we need to check if the daemon
> will be started eventually.
> And in fact checking the PID file or calling mpath_connect() is equivalent,
> with the added advantage the mpath_connect() will start the daemon _if
> enabled by systemd_.
> So this patch doesn't help much, as it doesn't solve the main problem of
> figuring out if multipathd _should_ be started.
> 
> I've done a patch for checking the '.wants' directories from systemd, but
> this obviously will only work if the OS is systemd-based.
> And it's not really perfect, as there are corner-cases where just checking
> for the .wants directory is not enough.

I think you are dealing with a different issue.  the "-q" option for
multipath just overrides the default behaviour of not enabling
queue_if_no_path when creating a multipath device is multipathd isn't
running.  For this, we don't care if multipathd will start running in
the future, because if/when multipathd does start up, it will set the
queue_if_no_path flag itself. We only care about now, when we know it's
not running.

Really, I doubt that anyone ever uses the -q option. So your change in
how multipath checks if multipathd is running is fine by me. However,
you also changed what the "-q" option does.  Previously, it just kept
multipath from disabling "queue_if_no_path" on devices that were
configured to have it, when multipathd wasn't running.  With your code,
doesn't it now make all devices set queue_if_no_path if multipathd isn't
running, including devices that weren't configured to set it even when
multipathd is running? Is there a reason for this? In general, setting
"queue_if_no_path" when multiapthd isn't running is a bad idea, since
the paths will never come back, and nothing will ever disable the
queueing.  That's why I said that I doubt anybody uses the option.
Forcing all devices to set "queue_if_no_path", even if they weren't
configured to, seems even less useful.  Or is there actually a use-case
that I'm missing here?

-Ben

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke		      zSeries & Storage
> hare at suse.de			      +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)