Re: [PATCH v3 11/11] libmultipath: don't [un]set queue_if_no_path after domap

[Martin Wilck <mwilck@suse.com>]

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