On Mon, Sep 22, 2025 at 06:53:05PM +0200, Martin Wilck wrote:
> On Wed, 2025-09-10 at 18:58 -0400, Benjamin Marzinski wrote:
> > All Registrants reservations (types 7 and 8) are held by key 0x0.
> > When
> > preempting one, all registrations are cleared, except for the one on
> > the
> > path that issued the PREEMPT. libmpathpersist needs to handle this
> > just
> > like the other cases of self-preemption, so that all the paths of the
> > preempting multipath device have their registrations restored while
> > the
> > device is suspended.
> >
> > Signed-off-by: Benjamin Marzinski <[email protected]>
> > ---
> > libmpathpersist/mpath_persist_int.c | 31 +++++++++++++++++++++++++--
> > --
> > 1 file changed, 27 insertions(+), 4 deletions(-)
> >
> > diff --git a/libmpathpersist/mpath_persist_int.c
> > b/libmpathpersist/mpath_persist_int.c
> > index 9f301859..ab3d70cd 100644
> > --- a/libmpathpersist/mpath_persist_int.c
> > +++ b/libmpathpersist/mpath_persist_int.c
> > @@ -569,8 +569,9 @@ static int mpath_prout_reg(struct multipath
> > *mpp,int rq_servact, int rq_scope,
> > * is suspended before issuing the preemption, and the keys are
> > reregistered
> > * before resuming it.
> > */
> > -static int preempt_self(struct multipath *mpp, int rq_servact, int
> > rq_scope,
> > - unsigned int rq_type, int noisy, bool
> > do_release)
> > +static int do_preempt_self(struct multipath *mpp, struct be64
> > sa_key,
> > + int rq_servact, int rq_scope, unsigned
> > int rq_type,
> > + int noisy, bool do_release)
> > {
> > int status, rel_status = MPATH_PR_SUCCESS;
> > struct path *pp = NULL;
> > @@ -583,7 +584,7 @@ static int preempt_self(struct multipath *mpp,
> > int rq_servact, int rq_scope,
> > }
> >
> > memcpy(paramp.key, &mpp->reservation_key, 8);
> > - memcpy(paramp.sa_key, &mpp->reservation_key, 8);
> > + memcpy(paramp.sa_key, &sa_key, 8);
> > status = mpath_prout_common(mpp, rq_servact, rq_scope,
> > rq_type,
> > ¶mp, noisy, &pp);
> > if (status != MPATH_PR_SUCCESS) {
> > @@ -619,6 +620,21 @@ fail_resume:
> > return (rel_status != MPATH_PR_SUCCESS) ? rel_status :
> > status;
> > }
> >
> > +static int preempt_self(struct multipath *mpp, int rq_servact, int
> > rq_scope,
> > + unsigned int rq_type, int noisy, bool
> > do_release)
> > +{
> > + return do_preempt_self(mpp, mpp->reservation_key,
> > rq_servact, rq_scope,
> > + rq_type, noisy, do_release);
> > +}
> > +
> > +static int preempt_all(struct multipath *mpp, int rq_servact, int
> > rq_scope,
> > + unsigned int rq_type, int noisy)
> > +{
> > + struct be64 zerokey = {0};
> > + return do_preempt_self(mpp, zerokey, rq_servact, rq_scope,
> > rq_type,
> > + noisy, false);
> > +}
> > +
> > static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int
> > rq_scope,
> > unsigned int rq_type,
> > struct prout_param_descriptor * paramp,
> > int noisy)
> > @@ -889,8 +905,15 @@ int do_mpath_persistent_reserve_out(vector
> > curmp, vector pathvec, int fd,
> > break;
> > case MPATH_PROUT_PREE_SA:
> > case MPATH_PROUT_PREE_AB_SA:
> > - if (reservation_key_matches(mpp, paramp->sa_key,
> > noisy) == YNU_YES)
> > + if (reservation_key_matches(mpp, paramp->sa_key,
> > noisy) == YNU_YES) {
> > preempting_reservation = true;
> > + if (memcmp(paramp->sa_key, &zerokey, 8) ==
> > 0) {
>
>
> Don't you need to check if pr_type is 7 or 8 here?
I don't think so. When we get here, we know that the reservation is
reported to be held by the key 0x0. That's obviously not a valid
registered key. I'm pretty sure that a SCSI device will only report this
reservation key in the "all registrants" case, and that "all
registrants" reservations will always report 0x0 as the key.
That being said, I needed to make reservation_key_matches() return the
reservation type to handle a different issue, so it's pretty trivial to
make this code double-check it for this case.
-Ben
>
> > + /* all registrants case */
> > + ret = preempt_all(mpp, rq_servact,
> > rq_scope,
> > + rq_type, noisy);
> > + break;
> > + }
> > + }
> > /* if we are preempting ourself */
> > if (memcmp(paramp->sa_key, paramp->key, 8) == 0) {
> > ret = preempt_self(mpp, rq_servact,
> > rq_scope, rq_type,
