[dm-devel] [PATCH v2] multipathd: fix missing persistent reseravtion for active path

[Benjamin Marzinski]

On Mon, Sep 13, 2021 at 09:01:11AM +0200, Martin Wilck wrote:
> Hello lixiaokeng,
> 
> On Mon, 2021-09-13 at 10:43 +0800, lixiaokeng wrote:
> > There are two paths(sucu as sda and adb) for one LUN. The two
> > paths log in, but before the two uevents have been processed
> > (for example there are many uevent), users use multipathd add
> > path /dev/sda to cause mpatha and use mpathpersist -o -I to
> > register prkey for mpatha. The add map uevent is after add path
> > uevent, the the uevent(add sdb) will delay and missing persistent
> > reseravtion check.
> > 
> > Here, we add persistent reseravtion check in update_map() which
> > is called ev_add_map().
> > 
> > Signed-off-by: Lixiaokeng <lixiaokeng at huawei.com>
> 
> Thank you, this looks ok to me. Have you tested it?
> 
> I'll wait for Ben's opinion nonetheless, because he's more exprerienced
> with this part of the code than myself.
> 
> This said, I would like to have multipathd record which paths have
> already registered the key, to avoid doing that repeatedly.
>
Other than adding this, the patch looks fine.
 
> Additional question to Ben in this context: what's the reason that we
> don't actively register keys (that we found in multipath.conf or
> prkeys) during multipathd startup / reconfigure?

Like I said in my reply to the first patch, the goal was to make
persitent reservations to multipath devices work just like with scsi
devices.

-Ben

> Regards
> Martin
> 
> 
> 
> > ---
> >  multipathd/main.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/multipathd/main.c b/multipathd/main.c
> > index 3aff241d..1defeaf1 100644
> > --- a/multipathd/main.c
> > +++ b/multipathd/main.c
> > @@ -490,6 +490,8 @@ update_map (struct multipath *mpp, struct vectors
> > *vecs, int new_map)
> >  {
> >         int retries = 3;
> >         char *params __attribute__((cleanup(cleanup_charp))) = NULL;
> > +       struct path *pp;
> > +       int i;
> > 
> >  retry:
> >         condlog(4, "%s: updating new map", mpp->alias);
> > @@ -502,6 +504,15 @@ retry:
> >         verify_paths(mpp);
> >         mpp->action = ACT_RELOAD;
> > 
> > +       if (mpp->prflag) {
> > +               vector_foreach_slot(mpp->paths, pp, i) {
> > +                       if ((pp->state == PATH_UP)  || (pp->state ==
> > PATH_GHOST)) {
> > +                               /* persistent reseravtion check*/
> > +                               mpath_pr_event_handle(pp);
> > +                       }
> > +               }
> > +       }
> > +
> >         if (setup_map(mpp, &params, vecs)) {
> >                 condlog(0, "%s: failed to setup new map in update",
> > mpp->alias);
> >                 retries = -1;
>