From: Martin Wilck <mwilck@suse.com>
To: Hannes Reinecke <hare@suse.de>,
Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: dm-devel@redhat.com, Xose Vazquez Perez <xose.vazquez@gmail.com>
Subject: Re: [PATCH v3 11/11] libmultipath: don't [un]set queue_if_no_path after domap
Date: Thu, 22 Jun 2017 11:34:14 +0200 [thread overview]
Message-ID: <1498124054.6002.3.camel@suse.com> (raw)
In-Reply-To: <fb72a1ed-58c6-fa6d-9411-8c03ce9541b7@suse.de>
On Thu, 2017-06-22 at 08:23 +0200, Hannes Reinecke wrote:
> On 06/21/2017 05:06 PM, Martin Wilck wrote:
> > We set the queue_if_no_path feature in assemble_map() already,
> > no need to set it here again.
> >
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> > libmultipath/configure.c | 15 ---------------
> > 1 file changed, 15 deletions(-)
> >
> > [...]
> Watch out.
> 'queue_if_no_path' is set _temporarily_ while 'no_path_retry' is
> active,
> and removed afterwards.
> So there might be valid reasons why it's set here.
> Have you checked?
Yes, I'm pretty certain that this is correct. We're in coalesce_paths()
here, while we are setting up or reconfiguring maps. The call sequence
is
setup_map()
assemble_map()
domap()
... and then comes the code I'm removing.
We set the feature string in assemble_map() correctly. Thus the removed
code just repeated the same setting that had already been applied in
domap(). This code has been in that place for a very long time, AFAICS
it originates from times where the features string was not correctly
set up before creating or reloading the map.
The logic for disabling queue_if_no_path if the retry count is reached
is implemented elsewhere, mainly in set_no_path_retry() (called e.g.
from ev_remove_path()) and retry_count_tick() (called from checker
loop).
Do you see a case that I have overlooked?
Best,
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
next prev parent reply other threads:[~2017-06-22 9:34 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-21 15:06 [PATCH v3 00/11] multipath-tools: no_path_retry/queue_if_no_path/hwhandler logic Martin Wilck
2017-06-21 15:06 ` [PATCH v3 01/11] libmultipath: load_config: skip setting unnecessary defaults Martin Wilck
2017-06-22 6:01 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 02/11] libmultipath: add/remove_feature: use const char* for feature Martin Wilck
2017-06-21 15:06 ` [PATCH v3 03/11] libmultipath: clarify option conflicts for "features" Martin Wilck
2017-06-22 6:02 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 04/11] libmultipath: merge_hwe: fix queue_if_no_path logic Martin Wilck
2017-06-22 6:04 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 05/11] libmultipath: assemble_map: " Martin Wilck
2017-06-22 6:05 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 06/11] multipath.conf.5: document no_path_retry vs. queue_if_no_path Martin Wilck
2017-06-22 6:05 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 07/11] multipath.conf.5: Remove ??? and other minor fixes Martin Wilck
2017-06-22 6:06 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 08/11] libmultipath: add deprecated warning for some features settings Martin Wilck
2017-06-22 6:06 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 09/11] libmultipath: retain_attached_hw_handler obsolete with 4.3+ Martin Wilck
2017-06-22 6:07 ` Hannes Reinecke
2017-06-21 15:06 ` [PATCH v3 10/11] libmultipath: don't try to set hwhandler if it is retained Martin Wilck
2017-06-22 6:21 ` Hannes Reinecke
2017-06-22 9:58 ` Martin Wilck
2017-06-21 15:06 ` [PATCH v3 11/11] libmultipath: don't [un]set queue_if_no_path after domap Martin Wilck
2017-06-22 6:23 ` Hannes Reinecke
2017-06-22 9:34 ` Martin Wilck [this message]
2017-06-22 19:21 ` Benjamin Marzinski
2017-06-22 20:44 ` Martin Wilck
2017-06-21 15:15 ` [PATCH v3 00/11] multipath-tools: no_path_retry/queue_if_no_path/hwhandler logic Martin Wilck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1498124054.6002.3.camel@suse.com \
--to=mwilck@suse.com \
--cc=christophe.varoqui@opensvc.com \
--cc=dm-devel@redhat.com \
--cc=hare@suse.de \
--cc=xose.vazquez@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox