[dm-devel] [RFC PATCH 05/20] libmultipath: don't update path groups when printing

[Martin Wilck]

On Wed, 2018-02-28 at 17:40 -0600, Benjamin Marzinski wrote:
> On Tue, Feb 20, 2018 at 02:26:43PM +0100, Martin Wilck wrote:
> > Updating the prio values for printing makes no sense. The user
> > wants to see
> > the prio values multipath is actually using for path group
> > selection, and
> > updating the values here means actually lying to the user if the
> > prio values
> > have changed, but multipathd hasn't updated them internally.
> > 
> > If we really don't update the pathgroup prios when we need to, this
> > should be
> > fixed elsewhere. The current wrong output would just hide that if
> > it occured.
> > 
> > Moreover, correctness forbids changing properties so deeply in a
> > code path
> > that's supposed to print them only. Finally, this piece of code
> > prevents the
> > print.c code to be converted to proper "const" usage.
> 
> Well, it is true that we've only been updating the path group
> priority
> when we've needed it, and we've only need it to be uptodate when we
> are
> picking a new pathgroup, or are printing it out. When failback is set
> to
> "manual", we rarely are picking a new pathgroup, so we rarely update
> the
> pathgroup prio. 
> 
> [...]
>
> I'd be fine with simply updating the path group priority whever we
> change a
> path's priority, if we aren't updating it when printing it. The
> bigger
> work of actually making sure that the path group order it the table
> is always uptodate needs to happen, but it doesn't need to happen in
> this patchset.

I just reviewed the code flow in check_path(), and here's what I see:

1. calls update_prio(pp, new_path_up)
   -> updates path's prio, or all paths' prios if the path was
      reinstated

Now it calls either

2a. update_path_groups() (for group_by_prio and failback immediate)
  (-> reload_map() -> setup_map() -> select_path_group()
  -> path_group_prio_update()), or
  
2b. need_switch_pathgroup() (otherwise)
  
So all we need to make sure is that need_switch_pathgroup() calls 
select_path_group(). It does that already, except for the "failback
manual" case.

So all we need is IMO the attached patch. Tell me what you think.

[All of the above is only called if (!mpp->wait_for_udev), but if
wait_for_udev is set, either when the event arrives or the wait times
out, we'll call reconfigure() which makes sure all priorities are
correct].

Best,
Martin

-- 
Dr. Martin Wilck <mwilck at suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-multipathd-update-path-group-prio-in-check_path.patch
Type: text/x-patch
Size: 1361 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20180302/d283ff67/attachment.bin>