On Mon, 2026-07-20 at 17:52 -0400, Benjamin Marzinski wrote: > From: Cykang <[email protected]> > > According to the SCSI-3 Persistent Reservations specification, when a > self-preemption is performed with a key that does not currently hold > an active reservation, the operation should be treated as an > unregister > and should not fail. > > The current code in do_mpath_persistent_reserve_out() does not check > whether the preempting key has a reservation before invoking the > self-preemption path. It unconditionally calls preempt_self() and > re-registers the key on the local paths, even when the key has no > reservation. This violates the SCSI-3 PR spec and can lead to > incorrect reservation states. > > This patch moves the self-preemption check inside the conditional > block so that it only executes when the key matches, but does not > bypass the general preemption path for keys without a reservation. > With this change, the code will fall through to the common PR_OUT > handling (e.g., MPATH_PROUT_RES_SA or CLEAR_SA) when the key is not > reserved, allowing the storage target to properly process the > operation as an unregister. > > Signed-off-by: Cykang <[email protected]> > Signed-off-by: Benjamin Marzinski <[email protected]> > --- > libmpathpersist/mpath_persist_int.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/libmpathpersist/mpath_persist_int.c > b/libmpathpersist/mpath_persist_int.c > index 3091c5c2..6ca3b97d 100644 > --- a/libmpathpersist/mpath_persist_int.c > +++ b/libmpathpersist/mpath_persist_int.c > @@ -841,6 +841,7 @@ int do_mpath_persistent_reserve_out(vector curmp, > vector pathvec, int fd, > bool unregistering, preempting_reservation = false; > bool updated_prkey = false; > bool failed_paths = false; > + bool self_preempt_unreg = false; > > ret = mpath_get_map(curmp, fd, &mpp); > if (ret != MPATH_PR_SUCCESS) > @@ -961,13 +962,20 @@ int do_mpath_persistent_reserve_out(vector > curmp, vector pathvec, int fd, > 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, noisy, > PREE_WORK_NONE); > + break; > + } > + } else if (memcmp(paramp->sa_key, paramp->key, 8) == > 0) { > + /* > + * We are self-preempting, but we don't hold > the > + * reservation. This will unregister the > device
key and sa_key are just parameters entered by the caller. How do you know, without calling check_holding_reservation(), whether or not we currently hold a reservation? Regards, Martin -- Dr. Martin Wilck <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg, Germany Geschäftsführer: Jochen Jaser, Andrew McDonald (HRB 36809,AG Nürnberg)
