pp->pgindex is set in disassemble_map() when a map is parsed.
There are various possiblities for this index to become invalid.
pp->pgindex is only used in enable_group() and followover_should_fallback(),
and both callers take no action if it is 0, which is the right
thing to do if we don't know the path's pathgroup.

Make sure pp->pgindex is reset to 0 in various places:
- when it's orphaned,
- before (re)grouping paths,
- when we detect a bad mpp assignment in update_pathvec_from_dm().
- when a pathgroup is deleted in update_pathvec_from_dm(). In this
  case, pgindex needs to be invalidated for all paths in all pathgroups
  after the one that was deleted.

The hunk in group_paths is mostly redundant with the hunk in free_pgvec(), but
because we're looping over pg->paths in the former and over pg->pgp in
the latter, I think it's better too play safe.

Fixes: 99db1bd ("[multipathd] re-enable disabled PG when at least one path is 
up")
Fixes: https://github.com/opensvc/multipath-tools/issues/105
Signed-off-by: Martin Wilck <mwi...@suse.com>
---
 libmultipath/pgpolicies.c  |  6 ++++++
 libmultipath/structs.c     | 12 +++++++++++-
 libmultipath/structs_vec.c | 15 +++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index edc3c61..23ef2bd 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -127,6 +127,8 @@ fail:
 int group_paths(struct multipath *mp, int marginal_pathgroups)
 {
        vector normal, marginal;
+       struct path *pp;
+       int i;
 
        if (!mp->pg)
                mp->pg = vector_alloc();
@@ -138,6 +140,10 @@ int group_paths(struct multipath *mp, int 
marginal_pathgroups)
        if (!mp->pgpolicyfn)
                goto fail;
 
+       /* Reset pgindex, we're going to invalidate it */
+       vector_foreach_slot(mp->paths, pp, i)
+               pp->pgindex = 0;
+
        if (!marginal_pathgroups ||
            split_marginal_paths(mp->paths, &normal, &marginal) != 0) {
                if (mp->pgpolicyfn(mp, mp->paths) != 0)
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 61c8f32..4851725 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -239,8 +239,18 @@ free_pgvec (vector pgvec, enum free_path_mode free_paths)
        if (!pgvec)
                return;
 
-       vector_foreach_slot(pgvec, pgp, i)
+       vector_foreach_slot(pgvec, pgp, i) {
+
+               /* paths are going to be re-grouped, reset pgindex */
+               if (free_paths != FREE_PATHS) {
+                       struct path *pp;
+                       int j;
+
+                       vector_foreach_slot(pgp->paths, pp, j)
+                               pp->pgindex = 0;
+               }
                free_pathgroup(pgp, free_paths);
+       }
 
        vector_free(pgvec);
 }
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index d22056c..35883fc 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -108,6 +108,7 @@ static bool update_pathvec_from_dm(vector pathvec, struct 
multipath *mpp,
        struct config *conf;
        bool mpp_has_wwid;
        bool must_reload = false;
+       bool pg_deleted = false;
 
        if (!mpp->pg)
                return false;
@@ -125,6 +126,10 @@ static bool update_pathvec_from_dm(vector pathvec, struct 
multipath *mpp,
 
                vector_foreach_slot(pgp->paths, pp, j) {
 
+                       /* A pathgroup has been deleted before. Invalidate 
pgindex */
+                       if (pg_deleted)
+                               pp->pgindex = 0;
+
                        if (pp->mpp && pp->mpp != mpp) {
                                condlog(0, "BUG: %s: found path %s which is 
already in %s",
                                        mpp->alias, pp->dev, pp->mpp->alias);
@@ -139,6 +144,13 @@ static bool update_pathvec_from_dm(vector pathvec, struct 
multipath *mpp,
                                must_reload = true;
                                dm_fail_path(mpp->alias, pp->dev_t);
                                vector_del_slot(pgp->paths, j--);
+                               /*
+                                * pp->pgindex has been set in 
disassemble_map(),
+                                * which has probably been called just before 
for
+                                * mpp. So he pgindex relates to mpp and may be
+                                * wrong for pp->mpp. Invalidate it.
+                                */
+                               pp->pgindex = 0;
                                continue;
                        }
                        pp->mpp = mpp;
@@ -237,6 +249,8 @@ static bool update_pathvec_from_dm(vector pathvec, struct 
multipath *mpp,
                vector_del_slot(mpp->pg, i--);
                free_pathgroup(pgp, KEEP_PATHS);
                must_reload = true;
+               /* Invalidate pgindex for all other pathgroups */
+               pg_deleted = true;
        }
        mpp->need_reload = mpp->need_reload || must_reload;
        return must_reload;
@@ -354,6 +368,7 @@ void orphan_path(struct path *pp, const char *reason)
 {
        condlog(3, "%s: orphan path, %s", pp->dev, reason);
        pp->mpp = NULL;
+       pp->pgindex = 0;
        uninitialize_path(pp);
 }
 
-- 
2.47.0


Reply via email to