On Wed, 2024-01-17 at 14:51 -0800, Brian Bunker wrote:
> When update_prio is called, there is a check to see if the local path
> has a priority change. Then all the remaining paths are simliarly
> checked.
>
> Only the result of paths not matching the local path's priority are
> considered in the calculation of 'changed'. In the case of failed
> paths
> becoming again available this can lead to multipath not reloading.
>
> The result will look like this:
> 3624a93703c9f34490f6140a100011010 dm-2 PURE,FlashArray
> size=3.0T features='1 retain_attached_hw_handler' hwhandler='1 alua'
> wp=rw
> > -+- policy='service-time 0' prio=50 status=active
> > > - 1:0:0:1 sdb 8:16 active ready running
> > > - 9:0:0:1 sdc 8:32 active ready running
> > > - 11:0:0:1 sdd 8:48 active ready running
> > `- 13:0:0:1 sdh 8:112 active ready running
> `-+- policy='service-time 0' prio=50 status=enabled
> |- 8:0:0:1 sde 8:64 active ready running
> |- 10:0:0:1 sdf 8:80 active ready running
> |- 12:0:0:1 sdg 8:96 active ready running
> `- 14:0:0:1 sdi 8:128 active ready running
>
> Where the path's priorities are updated, but the path group is not
> activated.
>
> Signed-off-by: Brian Bunker <brian@xxxxxxxxxxxxxxx>
> Signed-off-by: Seamus Connor <sconnor@xxxxxxxxxxxxxxx>
> Signed-off-by: Krisha Kant <krishna.kant@xxxxxxxxxxxxxxx>
> ---
> multipathd/main.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
Thanks for the report and fix!
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 230c9d10..62b87bf8 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2127,7 +2127,7 @@ int update_prio(struct path *pp, int
> refresh_all)
> int oldpriority;
> struct path *pp1;
> struct pathgroup * pgp;
> - int i, j, changed = 0;
> + int i, j, local_changed = 0, other_changed = 0;
Why use two variables here? AFAICS, you can implement the same logic
with just the "changed" variable.
Other than that, this looks good.
When you submit a v2, please add bmarzins@xxxxxxxxxx and myself to cc,
and add
Fixes: 6ccd7b8 ("multipathd: only refresh priorities in update_prio()")
Regards,
Martin
> struct config *conf;
>
> oldpriority = pp->priority;
> @@ -2138,8 +2138,11 @@ int update_prio(struct path *pp, int
> refresh_all)
> pthread_cleanup_pop(1);
> }
>
> - if (pp->priority == oldpriority && !refresh_all)
> - return 0;
> + if (pp->priority != oldpriority)
> + local_changed = 1;
> + else
> + if (!refresh_all)
> + return 0;
>
> vector_foreach_slot (pp->mpp->pg, pgp, i) {
> vector_foreach_slot (pgp->paths, pp1, j) {
> @@ -2151,10 +2154,10 @@ int update_prio(struct path *pp, int
> refresh_all)
> pathinfo(pp1, conf, DI_PRIO);
> pthread_cleanup_pop(1);
> if (pp1->priority != oldpriority)
> - changed = 1;
> + other_changed = 1;
> }
> }
> - return changed;
> + return (local_changed || other_changed);
> }
>
> static int reload_map(struct vectors *vecs, struct multipath *mpp,
