Re: [PATCH 5/5] libmultipath: add "detect_pgpolicy" config option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2023-05-19 at 18:02 -0500, Benjamin Marzinski wrote:
> This allows configuations to use "group_by_tpg" if alua is
> autodetected
> and another policy if it isn't, so they can work with detect_prio.

This is a bit confusing. We might have introduced this kind of
autodetection without group_by_tpg; using group_by_prio for arrays with
ALUA support would have made quite a bit of sense.

What this patch really does is to make multipath-tools prefer
group_by_tpg over group_by_prio if it finds that ALUA is supported. 
Should this be a separate option, perhaps?

 - detect_pgpolicy: use an ALUA-based pgpolicy if available
 - detect_pgpolicy_prefer_tpg: prefer group_by_tpg over group_by_prio
   for arrays supporting ALUA.

This way users could benefit from ALUA autodetection without switching
to the TPG algorithm automatically.

Or do we have good arguments that group_by_tpg is always "better" than
group_by_prio if ALUA is supported? I guess it might be, but it still
needs to prove its usefulness it practice.

Also, if we add the auto-detection feature, I think it should default
to ON, at least upstream.

Regards,
Martin

> 
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> ---
>  libmultipath/config.c             |  2 ++
>  libmultipath/config.h             |  2 ++
>  libmultipath/configure.c          |  1 +
>  libmultipath/defaults.h           |  1 +
>  libmultipath/dict.c               | 11 +++++++++++
>  libmultipath/hwtable.c            |  1 +
>  libmultipath/libmultipath.version | 10 +++-------
>  libmultipath/propsel.c            | 23 ++++++++++++++++++++++-
>  libmultipath/propsel.h            |  1 +
>  libmultipath/structs.h            |  7 +++++++
>  multipath/multipath.conf.5        | 12 ++++++++++++
>  11 files changed, 63 insertions(+), 8 deletions(-)
> 
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index 5c5c0726..2e742373 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -452,6 +452,7 @@ merge_hwe (struct hwentry * dst, struct hwentry *
> src)
>         merge_num(retain_hwhandler);
>         merge_num(detect_prio);
>         merge_num(detect_checker);
> +       merge_num(detect_pgpolicy);
>         merge_num(deferred_remove);
>         merge_num(delay_watch_checks);
>         merge_num(delay_wait_checks);
> @@ -617,6 +618,7 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
>         hwe->retain_hwhandler = dhwe->retain_hwhandler;
>         hwe->detect_prio = dhwe->detect_prio;
>         hwe->detect_checker = dhwe->detect_checker;
> +       hwe->detect_pgpolicy = dhwe->detect_pgpolicy;
>         hwe->ghost_delay = dhwe->ghost_delay;
>         hwe->vpd_vendor_id = dhwe->vpd_vendor_id;
>  
> diff --git a/libmultipath/config.h b/libmultipath/config.h
> index 87947469..014c6849 100644
> --- a/libmultipath/config.h
> +++ b/libmultipath/config.h
> @@ -76,6 +76,7 @@ struct hwentry {
>         int retain_hwhandler;
>         int detect_prio;
>         int detect_checker;
> +       int detect_pgpolicy;
>         int deferred_remove;
>         int delay_watch_checks;
>         int delay_wait_checks;
> @@ -171,6 +172,7 @@ struct config {
>         int retain_hwhandler;
>         int detect_prio;
>         int detect_checker;
> +       int detect_pgpolicy;
>         int force_sync;
>         int deferred_remove;
>         int processed_main_config;
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 4a1c28bb..366b166f 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -304,6 +304,7 @@ int setup_map(struct multipath *mpp, char
> **params, struct vectors *vecs)
>         pthread_cleanup_push(put_multipath_config, conf);
>  
>         select_pgfailback(conf, mpp);
> +       select_detect_pgpolicy(conf, mpp);
>         select_pgpolicy(conf, mpp);
>  
>         /*
> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> index a5e9ea0c..090baa5c 100644
> --- a/libmultipath/defaults.h
> +++ b/libmultipath/defaults.h
> @@ -29,6 +29,7 @@
>  #define DEFAULT_RETAIN_HWHANDLER RETAIN_HWHANDLER_ON
>  #define DEFAULT_DETECT_PRIO    DETECT_PRIO_ON
>  #define DEFAULT_DETECT_CHECKER DETECT_CHECKER_ON
> +#define DEFAULT_DETECT_PGPOLICY        DETECT_PGPOLICY_OFF
>  #define DEFAULT_DEFERRED_REMOVE        DEFERRED_REMOVE_OFF
>  #define DEFAULT_DELAY_CHECKS   NU_NO
>  #define DEFAULT_ERR_CHECKS     NU_NO
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index dddd3cd6..edd4923d 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -868,6 +868,14 @@ declare_ovr_snprint(detect_checker,
> print_yes_no_undef)
>  declare_hw_handler(detect_checker, set_yes_no_undef)
>  declare_hw_snprint(detect_checker, print_yes_no_undef)
>  
> +declare_def_handler(detect_pgpolicy, set_yes_no_undef)
> +declare_def_snprint_defint(detect_pgpolicy, print_yes_no_undef,
> +                          DEFAULT_DETECT_PGPOLICY)
> +declare_ovr_handler(detect_pgpolicy, set_yes_no_undef)
> +declare_ovr_snprint(detect_pgpolicy, print_yes_no_undef)
> +declare_hw_handler(detect_pgpolicy, set_yes_no_undef)
> +declare_hw_snprint(detect_pgpolicy, print_yes_no_undef)
> +
>  declare_def_handler(force_sync, set_yes_no)
>  declare_def_snprint(force_sync, print_yes_no)
>  
> @@ -2112,6 +2120,7 @@ init_keywords(vector keywords)
>         install_keyword("retain_attached_hw_handler",
> &def_retain_hwhandler_handler, &snprint_def_retain_hwhandler);
>         install_keyword("detect_prio", &def_detect_prio_handler,
> &snprint_def_detect_prio);
>         install_keyword("detect_checker",
> &def_detect_checker_handler, &snprint_def_detect_checker);
> +       install_keyword("detect_pgpolicy",
> &def_detect_pgpolicy_handler, &snprint_def_detect_pgpolicy);
>         install_keyword("force_sync", &def_force_sync_handler,
> &snprint_def_force_sync);
>         install_keyword("strict_timing", &def_strict_timing_handler,
> &snprint_def_strict_timing);
>         install_keyword("deferred_remove",
> &def_deferred_remove_handler, &snprint_def_deferred_remove);
> @@ -2202,6 +2211,7 @@ init_keywords(vector keywords)
>         install_keyword("retain_attached_hw_handler",
> &hw_retain_hwhandler_handler, &snprint_hw_retain_hwhandler);
>         install_keyword("detect_prio", &hw_detect_prio_handler,
> &snprint_hw_detect_prio);
>         install_keyword("detect_checker", &hw_detect_checker_handler,
> &snprint_hw_detect_checker);
> +       install_keyword("detect_pgpolicy",
> &hw_detect_pgpolicy_handler, &snprint_hw_detect_pgpolicy);
>         install_keyword("deferred_remove",
> &hw_deferred_remove_handler, &snprint_hw_deferred_remove);
>         install_keyword("delay_watch_checks",
> &hw_delay_watch_checks_handler, &snprint_hw_delay_watch_checks);
>         install_keyword("delay_wait_checks",
> &hw_delay_wait_checks_handler, &snprint_hw_delay_wait_checks);
> @@ -2244,6 +2254,7 @@ init_keywords(vector keywords)
>         install_keyword("retain_attached_hw_handler",
> &ovr_retain_hwhandler_handler, &snprint_ovr_retain_hwhandler);
>         install_keyword("detect_prio", &ovr_detect_prio_handler,
> &snprint_ovr_detect_prio);
>         install_keyword("detect_checker",
> &ovr_detect_checker_handler, &snprint_ovr_detect_checker);
> +       install_keyword("detect_pgpolicy",
> &ovr_detect_pgpolicy_handler, &snprint_ovr_detect_pgpolicy);
>         install_keyword("deferred_remove",
> &ovr_deferred_remove_handler, &snprint_ovr_deferred_remove);
>         install_keyword("delay_watch_checks",
> &ovr_delay_watch_checks_handler, &snprint_ovr_delay_watch_checks);
>         install_keyword("delay_wait_checks",
> &ovr_delay_wait_checks_handler, &snprint_ovr_delay_wait_checks);
> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> index 65bca744..803230c1 100644
> --- a/libmultipath/hwtable.c
> +++ b/libmultipath/hwtable.c
> @@ -67,6 +67,7 @@
>                 .retain_hwhandler = RETAIN_HWHANDLER_ON,
>                 .detect_prio   = DETECT_PRIO_ON,
>                 .detect_checker = DETECT_CHECKER_ON,
> +               .detect_pgpolicy = DETECT_PGPOLICY_OFF,
>                 .deferred_remove = DEFERRED_REMOVE_OFF,
>                 .delay_watch_checks = DELAY_CHECKS_OFF,
>                 .delay_wait_checks = DELAY_CHECKS_OFF,
> diff --git a/libmultipath/libmultipath.version
> b/libmultipath/libmultipath.version
> index aba1a30e..8f72c452 100644
> --- a/libmultipath/libmultipath.version
> +++ b/libmultipath/libmultipath.version
> @@ -43,7 +43,7 @@ LIBMPATHCOMMON_1.0.0 {
>         put_multipath_config;
>  };
>  
> -LIBMULTIPATH_18.0.0 {
> +LIBMULTIPATH_19.0.0 {
>  global:
>         /* symbols referenced by multipath and multipathd */
>         add_foreign;
> @@ -116,6 +116,7 @@ global:
>         get_refwwid;
>         get_state;
>         get_udev_device;
> +       get_uid;
>         get_used_hwes;
>         get_vpd_sgio;
>         group_by_prio;
> @@ -141,6 +142,7 @@ global:
>         pathcount;
>         path_discovery;
>         path_get_tpgs;
> +       pathinfo;
>         path_offline;
>         print_all_paths;
>         print_foreign_topology;
> @@ -235,9 +237,3 @@ global:
>  local:
>         *;
>  };
> -
> -LIBMULTIPATH_18.1.0 {
> -global:
> -       get_uid;
> -       pathinfo;
> -} LIBMULTIPATH_18.0.0;
> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index d214281b..12f4825d 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -265,6 +265,21 @@ verify_alua_prio(struct multipath *mp)
>         return true;
>  }
>  
> +int select_detect_pgpolicy(struct config *conf, struct multipath
> *mp)
> +{
> +       const char *origin;
> +
> +       mp_set_ovr(detect_pgpolicy);
> +       mp_set_hwe(detect_pgpolicy);
> +       mp_set_conf(detect_pgpolicy);
> +       mp_set_default(detect_pgpolicy, DEFAULT_DETECT_PGPOLICY);
> +out:
> +       condlog(3, "%s: detect_pgpolicy = %s %s", mp->alias,
> +               (mp->detect_pgpolicy == DETECT_PGPOLICY_ON)? "yes" :
> "no",
> +                origin);
> +       return 0;
> +}
> +
>  int select_pgpolicy(struct config *conf, struct multipath * mp)
>  {
>         const char *origin;
> @@ -275,13 +290,19 @@ int select_pgpolicy(struct config *conf, struct
> multipath * mp)
>                 origin = cmdline_origin;
>                 goto out;
>         }
> +       if (mp->detect_pgpolicy == DETECT_PGPOLICY_ON &&
> verify_alua_prio(mp)) {
> +               mp->pgpolicy = GROUP_BY_TPG;
> +               origin = autodetect_origin;
> +               goto out;
> +       }
>         mp_set_mpe(pgpolicy);
>         mp_set_ovr(pgpolicy);
>         mp_set_hwe(pgpolicy);
>         mp_set_conf(pgpolicy);
>         mp_set_default(pgpolicy, DEFAULT_PGPOLICY);
>  out:
> -       if (mp->pgpolicy == GROUP_BY_TPG && !verify_alua_prio(mp)) {
> +       if (mp->pgpolicy == GROUP_BY_TPG && origin !=
> autodetect_origin &&
> +           !verify_alua_prio(mp)) {
>                 mp->pgpolicy = DEFAULT_PGPOLICY;
>                 origin = "(setting: emergency fallback - not all
> paths use alua prio)";
>                 log_prio = 1;
> diff --git a/libmultipath/propsel.h b/libmultipath/propsel.h
> index 152ca44c..513ee6ac 100644
> --- a/libmultipath/propsel.h
> +++ b/libmultipath/propsel.h
> @@ -1,5 +1,6 @@
>  int select_rr_weight (struct config *conf, struct multipath * mp);
>  int select_pgfailback (struct config *conf, struct multipath * mp);
> +int select_detect_pgpolicy (struct config *conf, struct multipath *
> mp);
>  int select_pgpolicy (struct config *conf, struct multipath * mp);
>  int select_selector (struct config *conf, struct multipath * mp);
>  int select_alias (struct config *conf, struct multipath * mp);
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 0eea19b4..682a7e17 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -143,6 +143,12 @@ enum detect_checker_states {
>         DETECT_CHECKER_ON = YNU_YES,
>  };
>  
> +enum detect_pgpolicy_states {
> +       DETECT_PGPOLICY_UNDEF = YNU_UNDEF,
> +       DETECT_PGPOLICY_OFF = YNU_NO,
> +       DETECT_PGPOLICY_ON = YNU_YES,
> +};
> +
>  enum deferred_remove_states {
>         DEFERRED_REMOVE_UNDEF = YNU_UNDEF,
>         DEFERRED_REMOVE_OFF = YNU_NO,
> @@ -389,6 +395,7 @@ enum prflag_value {
>  struct multipath {
>         char wwid[WWID_SIZE];
>         char alias_old[WWID_SIZE];
> +       int detect_pgpolicy;
>         int pgpolicy;
>         pgpolicyfn *pgpolicyfn;
>         int nextpg;
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index b65a543d..9f8be510 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -242,6 +242,18 @@ The default is: \fBfailover\fR
>  .
>  .
>  .TP
> +.B detect_pgpolicy
> +If set to \fIyes\fR and all path devcices are configured with either
> the
> +\fIalua\fR or \fIsysfs\fR prioritizer, the multipath device will
> automatically
> +use the \fIgroup_by_tpg\fR path_grouping_policy. If set to \fIno\fR,
> the
> +path_grouping_policy will be selected as usual.
> +.RS
> +.TP
> +The default is: \fIno\fR
> +.RE
> +.
> +.
> +.TP
>  .B pg_timeout
>  (Deprecated) This option is not supported any more, and the value is
> ignored.
>  .

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





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux