On Tue, Jul 21, 2026 at 09:43:44AM +0200, Martin Wilck wrote: > 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?
In the "if" statement for this "else", we call reservation_key_matches(), so we know that nobody is holding a reservation with "sa_key". Well... no, actually we don't. reservation_key_matches() could return YNU_UNDEF. In that case who knows what's going on with the reservation. Perhaps in that case, we check if we need to do any self preemption (sa_key equals 0 or key). If so, we just fail, since a failure seems better than silently doing the wrong thing (either re-registering when we're not supposed to, or not re-registering when we are supposed to). The most likely reason why reservation_key_matches() returns YNU_UNDEF is that there are no usable paths, so we would fail sending the PREEMPT command anyways. If we don't need to do any self-preemption work, we might as well attempt to send the command, since it can't hurt. But I'm open to being convinced that the correct answer is just to always fail if reservation_key_matches() returns YNU_UNDEF, on the grounds that this code is complex enough without adding yet another special case workaround, that is itself likely to fail, since the paths are likely all down. -Ben > 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)
