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
