On Wed, 2023-05-24 at 18:21 -0500, Benjamin Marzinski wrote:
> need_switch_pathgroup() only checks if the currently used pathgroup
> is
> not the highest priority pathgroup. If it isn't, all multipathd does
> is
> instruct the kernel to switch to the correct pathgroup. However, the
> kernel treats the pathgroups as if they were ordered by priority.
> When
> the kernel runs out of paths to use in the currently selected
> pathgroup,
> it will start checking the pathgroups in order until it finds one
> with
> usable paths.
>
> need_switch_pathgroup() should also check if the pathgroups are out
> of
> order, and if so, multipathd should reload the map to reorder them
> correctly.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> ---
> libmultipath/libmultipath.version | 5 ++++
> libmultipath/switchgroup.c | 27 ++++++++++++++++++++++
> libmultipath/switchgroup.h | 1 +
> multipathd/main.c | 38 +++++++++++++++++++++--------
> --
> 4 files changed, 59 insertions(+), 12 deletions(-)
>
> diff --git a/libmultipath/libmultipath.version
> b/libmultipath/libmultipath.version
> index 8f72c452..38074699 100644
> --- a/libmultipath/libmultipath.version
> +++ b/libmultipath/libmultipath.version
> @@ -237,3 +237,8 @@ global:
> local:
> *;
> };
> +
> +LIBMULTIPATH_19.1.0 {
> +global:
> + path_groups_in_order;
> +} LIBMULTIPATH_19.0.0;
> diff --git a/libmultipath/switchgroup.c b/libmultipath/switchgroup.c
> index b1e1f39b..b1180839 100644
> --- a/libmultipath/switchgroup.c
> +++ b/libmultipath/switchgroup.c
> @@ -7,6 +7,33 @@
> #include "structs.h"
> #include "switchgroup.h"
>
> +bool path_groups_in_order(struct multipath *mpp)
> +{
> + int i;
> + struct pathgroup *pgp;
> + bool seen_marginal_pg = false;
> + int last_prio = INT_MAX;
> +
> + if (VECTOR_SIZE(mpp->pg) < 2)
> + return true;
> +
> + vector_foreach_slot(mpp->pg, pgp, i) {
> + /* skip pgs with PRIO_UNDEF, since this is likely
> temporary */
> + if (!pgp->paths || pgp->priority == PRIO_UNDEF)
> + continue;
> + if (pgp->marginal && !seen_marginal_pg) {
> + last_prio = INT_MAX;
> + continue;
> + }
> + if (seen_marginal_pg && !pgp->marginal)
> + return false;
> + if (pgp->priority > last_prio)
> + return false;
> + last_prio = pgp->priority;
> + }
> + return true;
> +}
> +
I stared at this code again while we were discussing, and I figured I
don't quite understand it. First of all, you never update
seen_marginal_pg. I suppose you want to set it in the if (pgp->marginal
&& !seen_marginal_pg) code block. But then, why set last_prio =
INT_MAX? If any non-marginal GP would follow, you would return false
anyway.
Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
