On Wed, Apr 04, 2018 at 06:16:14PM +0200, Martin Wilck wrote:
> Change the "find_multipaths" option from yes/no to multi-value. This
> option now covers the effects of "find_multipaths" as it used to be,
> plus the -i option to multipath (ignore_wwids) and the -n option to
> multipathd (ignore_new_devs), excluding such combinations of the former
> options that are dangerous or inconsistent.
> 
> The possible values for "find_multipaths" are now:
> 
>  - strict: strictly stick to the WWIDs file; never add new maps automatically
>    (new default; upstream behaviour with with find_multipaths "yes" and
>    commit 64e27ec "multipathd: imply -n if find_multipaths is set")
>  - off|no: multipath like "strict", multipathd like "greedy" (previous
>    upstream default)
>  - on|yes: set up multipath if >1 paths are seen (current Red Hat/Ubuntu
>    behavior with find_multipaths "yes")
>  - greedy: set up multipath for all non-blacklisted devices (current SUSE
>    default)
>  - smart: Like "yes", but try to avoid inconsistencies between udev processing
>    and multipathd processing by waiting for path siblings to
>    appear (fully implemented in follow-up patches)
> 
> The default has been changed to "strict", because "no" may cause inconsistent
> behavior between "multipath -u" and multipathd. This is deliberately a
> conservative choice.
> 
> The patch also updates the related man pages.
> 

This patch adds trailing whitespace. Otherwise, it's still fine.

Reviewed-by: Benjamin Marzinski <bmarz...@redhat.com>

> Reviewed-by: Benjamin Marzinski <bmarz...@redhat.com>
> Signed-off-by: Martin Wilck <mwi...@suse.com>
> ---
>  libmultipath/config.h      |  2 --
>  libmultipath/defaults.h    |  2 +-
>  libmultipath/dict.c        | 47 
> ++++++++++++++++++++++++++++++++++++++++++++--
>  libmultipath/structs.h     | 26 +++++++++++++++++++++++++
>  libmultipath/wwids.c       |  8 ++++++--
>  multipath/main.c           |  9 +++++----
>  multipath/multipath.8      |  9 ++++++++-
>  multipath/multipath.conf.5 | 46 +++++++++++++++++++++++++--------------------
>  multipathd/main.c          |  6 +-----
>  multipathd/multipathd.8    |  8 +++++---
>  10 files changed, 123 insertions(+), 40 deletions(-)
> 
> diff --git a/libmultipath/config.h b/libmultipath/config.h
> index 329f3ed..c71d972 100644
> --- a/libmultipath/config.h
> +++ b/libmultipath/config.h
> @@ -139,7 +139,6 @@ struct config {
>       int max_fds;
>       int force_reload;
>       int queue_without_daemon;
> -     int ignore_wwids;
>       int checker_timeout;
>       int flush_on_last_del;
>       int attribute_flags;
> @@ -168,7 +167,6 @@ struct config {
>       int strict_timing;
>       int retrigger_tries;
>       int retrigger_delay;
> -     int ignore_new_devs;
>       int delayed_reconfig;
>       int uev_wait_timeout;
>       int skip_kpartx;
> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> index 2b270ca..690182c 100644
> --- a/libmultipath/defaults.h
> +++ b/libmultipath/defaults.h
> @@ -17,7 +17,7 @@
>  #define DEFAULT_NO_PATH_RETRY        NO_PATH_RETRY_UNDEF
>  #define DEFAULT_VERBOSITY    2
>  #define DEFAULT_REASSIGN_MAPS        0
> -#define DEFAULT_FIND_MULTIPATHS      0
> +#define DEFAULT_FIND_MULTIPATHS      FIND_MULTIPATHS_STRICT
>  #define DEFAULT_FAST_IO_FAIL 5
>  #define DEFAULT_DEV_LOSS_TMO 600
>  #define DEFAULT_RETAIN_HWHANDLER RETAIN_HWHANDLER_ON
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index ac9216c..e695fdc 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -234,8 +234,51 @@ declare_def_snprint(multipath_dir, print_str)
>  declare_def_handler(partition_delim, set_str)
>  declare_def_snprint(partition_delim, print_str)
>  
> -declare_def_handler(find_multipaths, set_yes_no)
> -declare_def_snprint(find_multipaths, print_yes_no)
> +static const char *find_multipaths_optvals[] = {
> +     [FIND_MULTIPATHS_OFF] = "off",
> +     [FIND_MULTIPATHS_ON] = "on",
> +     [FIND_MULTIPATHS_STRICT] = "strict",
> +     [FIND_MULTIPATHS_GREEDY] = "greedy",
> +};
> +
> +static int
> +def_find_multipaths_handler(struct config *conf, vector strvec)
> +{
> +     char *buff;
> +     int i;
> +
> +     if (set_yes_no_undef(strvec, &conf->find_multipaths) == 0 &&
> +         conf->find_multipaths != YNU_UNDEF)
> +             return 0;
> +
> +     buff = set_value(strvec);
> +     if (!buff)
> +             return 1;
> +
> +     for (i = FIND_MULTIPATHS_OFF; i < __FIND_MULTIPATHS_LAST; i++) {
> +             if (find_multipaths_optvals[i] != NULL &&
> +                 !strcmp(buff, find_multipaths_optvals[i])) {
> +                     conf->find_multipaths = i;
> +                     break;
> +             }
> +     }
> +
> +     if (conf->find_multipaths == YNU_UNDEF) {
> +             condlog(0, "illegal value for find_multipaths: %s", buff);
> +             conf->find_multipaths = DEFAULT_FIND_MULTIPATHS;
> +     }
> +
> +     FREE(buff);
> +     return 0;
> +}
> +
> +static int
> +snprint_def_find_multipaths(struct config *conf, char *buff, int len,
> +                         const void *data)
> +{
> +     return print_str(buff, len,
> +                      find_multipaths_optvals[conf->find_multipaths]);
> +}
>  
>  declare_def_handler(selector, set_str)
>  declare_def_snprint_defstr(selector, print_str, DEFAULT_SELECTOR)
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 88a4b78..c43f2c3 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -102,6 +102,32 @@ enum yes_no_undef_states {
>       YNU_YES,
>  };
>  
> +#define _FIND_MULTIPATHS_F (1 << 1)
> +#define _FIND_MULTIPATHS_I (1 << 2)
> +#define _FIND_MULTIPATHS_N (1 << 3)
> +/*
> + * _FIND_MULTIPATHS_F must have the same value as YNU_YES.
> + * Generate a compile time error if that isn't the case.
> + */
> +char ___error1___[-(_FIND_MULTIPATHS_F != YNU_YES)];
> +
> +#define find_multipaths_on(conf) \
> +     (!!((conf)->find_multipaths & _FIND_MULTIPATHS_F))
> +#define ignore_wwids_on(conf) \
> +     (!!((conf)->find_multipaths & _FIND_MULTIPATHS_I))
> +#define ignore_new_devs_on(conf) \
> +     (!!((conf)->find_multipaths & _FIND_MULTIPATHS_N))
> +
> +enum find_multipaths_states {
> +     FIND_MULTIPATHS_UNDEF = YNU_UNDEF,
> +     FIND_MULTIPATHS_OFF = YNU_NO,
> +     FIND_MULTIPATHS_ON = _FIND_MULTIPATHS_F,
> +     FIND_MULTIPATHS_STRICT = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_N,
> +     FIND_MULTIPATHS_GREEDY = _FIND_MULTIPATHS_I,
> +     FIND_MULTIPATHS_SMART = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_I,
> +     __FIND_MULTIPATHS_LAST,
> +};
> +
>  enum flush_states {
>       FLUSH_UNDEF = YNU_UNDEF,
>       FLUSH_DISABLED = YNU_NO,
> diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
> index 11afe09..da685be 100644
> --- a/libmultipath/wwids.c
> +++ b/libmultipath/wwids.c
> @@ -282,8 +282,12 @@ should_multipath(struct path *pp1, vector pathvec, 
> vector mpvec)
>       struct config *conf;
>  
>       conf = get_multipath_config();
> -     ignore_new_devs = conf->ignore_new_devs;
> -     find_multipaths = conf->find_multipaths;
> +     ignore_new_devs = ignore_new_devs_on(conf);
> +     find_multipaths = find_multipaths_on(conf);
> +     if (!find_multipaths && !ignore_new_devs) {
> +             put_multipath_config(conf);
> +             return 1;
> +     }
>       put_multipath_config(conf);
>       if (find_multipaths && !ignore_new_devs)
>               return 1;
> diff --git a/multipath/main.c b/multipath/main.c
> index 686b037..2be9b9c 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -441,11 +441,12 @@ configure (struct config *conf, enum mpath_cmds cmd,
>                * Paths listed in the wwids file are always considered valid.
>                */
>               if (cmd == CMD_VALID_PATH) {
> -                     if ((!conf->find_multipaths && conf->ignore_wwids) ||
> -                         check_wwids_file(refwwid, 0) == 0)
> +                     if ((!find_multipaths_on(conf) && ignore_wwids_on(conf))
> +                         || check_wwids_file(refwwid, 0) == 0)
>                               r = 0;
>                       if (r == 0 ||
> -                         !conf->find_multipaths || !conf->ignore_wwids) {
> +                         !find_multipaths_on(conf) ||
> +                         !ignore_wwids_on(conf)) {
>                               printf("%s %s a valid multipath device path\n",
>                                      devpath, r == 0 ? "is" : "is not");
>                               goto out;
> @@ -737,7 +738,7 @@ main (int argc, char *argv[])
>                       conf->force_reload = FORCE_RELOAD_YES;
>                       break;
>               case 'i':
> -                     conf->ignore_wwids = 1;
> +                     conf->find_multipaths |= _FIND_MULTIPATHS_I;
>                       break;
>               case 't':
>                       r = dump_config(conf);
> diff --git a/multipath/multipath.8 b/multipath/multipath.8
> index 56f8703..329658d 100644
> --- a/multipath/multipath.8
> +++ b/multipath/multipath.8
> @@ -94,7 +94,14 @@ Force devmap reload.
>  .
>  .TP
>  .B \-i
> -Ignore WWIDs file when processing devices.
> +Ignore WWIDs file when processing devices. If 
> +\fIfind_multipaths strict\fR or \fIfind_multipaths no\fR is set in
> +\fImultipath.conf\fR, multipath only considers devices that are
> +listed in the WWIDs file. This option overrides that behavior. For other 
> values
> +of \fIfind_multipaths\fR, this option has no effect. See the description of
> +\fIfind_multipaths\fR in 
> +.BR multipath.conf (5).
> +This option should only be used in rare circumstances.
>  .
>  .TP
>  .B \-B
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index c4d0789..6965dac 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -951,28 +951,34 @@ The default is: \fBno\fR
>  .
>  .TP
>  .B find_multipaths
> -If set to
> -.I yes
> -, instead of trying to create a multipath device for every non-blacklisted
> -path, multipath will only create a device if one of three condidions are
> -met.
> -.I 1
> -There are at least two non-blacklisted paths with the same WWID,
> -.I 2
> -the user manually forces the creation, by specifying a device with the 
> multipath
> -command, or
> -.I 3
> -a path has the same WWID as a multipath device that was previously created
> -while find_multipaths was set (even if that multipath device doesn't 
> currently
> -exist).
> -Whenever a multipath device is created with find_multipaths set, multipath 
> will
> -remember the WWID of the device, so that it will automatically create the
> -device again, as soon as it sees a path with that WWID. This should allow 
> most
> -users to have multipath automatically choose the correct paths to make into
> -multipath devices, without having to edit the blacklist.
> +This option controls whether multipath and multipathd try to create multipath
> +maps over non-blacklisted devices they encounter. This matters a) when a 
> device is
> +encountered by \fBmultipath -u\fR during udev rule processing (a device is
> +blocked from further processing by higher layers - such as LVM - if and only
> +if it\'s considered a valid multipath device path), and b) when multipathd
> +detects a new device. The following values are possible:
>  .RS
> +.TP 10
> +.I strict
> +Both multipath and multipathd treat only such devices as multipath devices
> +which have been part of a multipath map previously, and which are therefore
> +listed in the \fBwwids_file\fR. Users can manually set up multipath maps 
> using the
> +\fBmultipathd add map\fR command. Once set up manually, the map is
> +remembered in the wwids file and will be set up automatically in the future.
>  .TP
> -The default is: \fBno\fR
> +.I no
> +Multipath behaves like \fBstrict\fR. Multipathd behaves like \fBgreedy\fR.
> +.TP
> +.I yes
> +Both multipathd and multipath treat a device as multipath device if the
> +conditions for \fBstrict\fR are met, or if at least two non-blacklisted paths
> +with the same WWID have been detected.
> +.TP
> +.I greedy
> +Both multipathd and multipath treat every non-blacklisted device as multipath
> +device path.
> +.TP
> +The default is: \fBstrict\fR
>  .RE
>  .
>  .
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 644de72..e6f4e77 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2405,8 +2405,6 @@ reconfigure (struct vectors * vecs)
>               conf->verbosity = verbosity;
>       if (bindings_read_only)
>               conf->bindings_read_only = bindings_read_only;
> -     if (ignore_new_devs)
> -             conf->ignore_new_devs = ignore_new_devs;
>       uxsock_timeout = conf->uxsock_timeout;
>  
>       old = rcu_dereference(multipath_conf);
> @@ -2636,8 +2634,6 @@ child (void * param)
>               conf->verbosity = verbosity;
>       if (bindings_read_only)
>               conf->bindings_read_only = bindings_read_only;
> -     if (ignore_new_devs)
> -             conf->ignore_new_devs = ignore_new_devs;
>       uxsock_timeout = conf->uxsock_timeout;
>       rcu_assign_pointer(multipath_conf, conf);
>       if (init_checkers(conf->multipath_dir)) {
> @@ -2992,7 +2988,7 @@ main (int argc, char *argv[])
>                       bindings_read_only = 1;
>                       break;
>               case 'n':
> -                     ignore_new_devs = 1;
> +                     condlog(0, "WARNING: ignoring deprecated option -n, use 
> 'ignore_wwids = no' instead");
>                       break;
>               case 'w':
>                       poll_dmevents = 0;
> diff --git a/multipathd/multipathd.8 b/multipathd/multipathd.8
> index 5c96680..e78ac9e 100644
> --- a/multipathd/multipathd.8
> +++ b/multipathd/multipathd.8
> @@ -25,7 +25,6 @@ multipathd \- Multipath daemon.
>  .RB [\| \-v\ \c
>  .IR verbosity \|]
>  .RB [\| \-B \|]
> -.RB [\| \-n \|]
>  .
>  .
>  .\" 
> ----------------------------------------------------------------------------
> @@ -73,8 +72,11 @@ be viewed by entering '\fIhelp\fR'. When you are finished 
> entering commands, pre
>  .
>  .TP
>  .B \-n
> -Ignore new devices. multipathd will not create a multipath device unless the
> -WWID for the device is already listed in the WWIDs file.
> +\fBIGNORED\fR. Use the option
> +\fIfind_multipaths\fR to control the treatment of newly detected devices by
> +multipathd. See
> +.BR multipath.conf(5).
> +.
>  .
>  .
>  .\" 
> ----------------------------------------------------------------------------
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to