On Mon, 2025-09-22 at 14:09 -0400, Benjamin Marzinski wrote: > 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.
Ok, fine. Thanks for confirming. > > 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. Not necessary, thanks. Martin
