[dm-devel] [PATCH v4 00/11] multipath-tools: no_path_retry/queue_if_no_path/hwhandler logic

[Christophe Varoqui]

Patch 1 to 9 are now merged.


On Thu, Jun 22, 2017 at 4:59 PM, Martin Wilck <mwilck at suse.com> 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".
>
> In the long run, we should get rid of the "features" settings duplicating
> configuration options altogether; the patch set prepares this by printing
> warning messages.
>
> The logic is implemented in the new function reconcile_features_with_
> options,
> which is called from both select_features() and merge_hwe(). In
> setup_map(),
> we need to call select_features() after select_no_path_retry() to make
> this work.
> The actual feature setting for device-mapper is made in assemble_map(), the
> patch set also fixes the logic there.
>
> The patch set documents the behavior in the man page, and adds some more
> man page fixes.
>
> By skipping superfluous default initializations in load_config(), the
> log messages for the respective config settings become more appropriate.
>
> The logic for setting hardware handler is also improved. Since kernel 4.3,
> "retain_attached_hw_handler yes" is implicitly set by the kernel, and
> setting
> the hardware handler from user space is only possible in special
> situations.
> Acknowledge that in multipathd, and don't try to set or unset either this
> feature or the hwhandler attribute itself if it's doomed to fail.
>
> Review and comments are highly welcome.
>
> Changes wrt v1:
>   - Suggestions from Ben Marzinski:
>     * Made sure "multipathd show config" still works (1/8).
>     * Fixed logging for default setting of "max_sectors" (1/8)
>     * Consistent internal treatment of mp->features (3/8, 4/8)
>     * Fixed whitespace error (6/8)
>     * Added deprecated warnings (8/8)
>   - Made sure the same logic is used in propsel.c and config.c by
>     calling the same function (3/8, 4/8)
>   - Added deprecated warnings (8/8)
>
> Changes wrt v2:
>  - Added Acked-by:/Reviewed-by: tags
>  - 1/11: clarify comment in select_max_sectors_kb (Hannes Reinecke)
>  - 3/11: call select_retain_hwhandler before select_features
>  - 8/11: don't suggest using retain_attached_hw_handler in log msg
>  - 9/11, 10/11, 11/11: new
>
> Changes wrt v3:
>  - Added Reviewed-by: tags (kept Ben's Ack in 4/11 although patch slightly
> changed)
>  - 4/11: use a buffer on stack rather than malloc (Hannes Reinecke)
>  - 10/11: Simplify by checking dh_state only in select_handler (Hannes
> Reinecke)
>
> Martin Wilck (11):
>   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: add deprecated warning for some features settings
>   libmultipath: retain_attached_hw_handler obsolete with 4.3+
>   libmultipath: don't try to set hwhandler if it is retained
>   libmultipath: don't [un]set queue_if_no_path after domap
>
>  libmultipath/config.c      |  31 +++---------
>  libmultipath/configure.c   |  28 ++++-------
>  libmultipath/dict.c        |  11 +++--
>  libmultipath/dmparser.c    |   8 +--
>  libmultipath/propsel.c     | 121 ++++++++++++++++++++++++++++++
> +++++++++------
>  libmultipath/propsel.h     |   3 ++
>  libmultipath/structs.c     |  30 +++++------
>  libmultipath/structs.h     |   4 +-
>  libmultipath/util.c        |  36 ++++++++++++++
>  libmultipath/util.h        |   2 +
>  multipath/multipath.conf.5 |  67 +++++++++++++++----------
>  11 files changed, 231 insertions(+), 110 deletions(-)
>
> --
> 2.13.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/dm-devel/attachments/20170803/4127d8a0/attachment.htm>