Re: [PATCH 0/7] no_path_retry/queue_if_no_path logic & logging/man page fixes

["Benjamin Marzinski" <bmarzins@redhat.com>]

On Wed, Jun 14, 2017 at 12:55:47AM +0200, Martin Wilck wrote:
> This patch set attempts to sanitize the logic used for consistently handling
> options that can be set both via the "features" string and explicit multipath.conf
> options. This is most prominently "no_path_retry" vs. "queue_if_no_path", but also
> "retain_attached_hw_handler" vs. the feature of the same name.
> 
> The logic this patch set follows is:
>  - If "no_path_retry" is set to any value, "queue_if_no_path" is ignored
>    (this is the case nowadays for almost all "real" storage arrays, thanks to hwtable).
>  - Otherwise, if "queue_if_no_path" is set, treat it like "no_path_retry queue".
> 
> ... likewise for "retain_attached_hw_handler".

I wholly approve of this change. I do have some minor issue with some of the
individual patches.
 
> The logic is implemented in select_features() (which needs to be called after
> select_no_path_retry()) for multipath groups, and similarly in merge_hwe() for
> hwtable entries. The actual feature setting is made in assemble_map(), I also
> fix a problem there.
> 
> The patch set documents the behavior in the man page, and adds some more
> man page fixes.
> 
> Finally, by skipping superfluous default initializations in load_config(), the
> log messages for the respective config settings become more appropriate.
> 
> Review and comments are highly welcome.
> 
> Regards,
> Martin
> 
> Martin Wilck (7):
>   libmultipath: load_config: skip setting unnecessary defaults
>   libmultipath: add/remove_feature: use const char* for feature
>   libmultipath: clarify option conflicts for "features"
>   libmultipath: merge_hwe: fix queue_if_no_path logic
>   libmultipath: assemble_map: fix queue_if_no_path logic
>   multipath.conf.5: document no_path_retry vs. queue_if_no_path
>   multipath.conf.5: Remove ??? and other minor fixes
> 
>  libmultipath/config.c      | 36 ++++++++++++++------------------
>  libmultipath/configure.c   |  4 ++--
>  libmultipath/dmparser.c    |  5 ++---
>  libmultipath/propsel.c     | 39 +++++++++++++++++++++-------------
>  libmultipath/structs.c     | 30 +++++++++++++-------------
>  libmultipath/structs.h     |  4 ++--
>  multipath/multipath.conf.5 | 52 ++++++++++++++++++++++++++--------------------
>  7 files changed, 92 insertions(+), 78 deletions(-)
> 
> -- 
> 2.13.0