On Tue, 2026-07-21 at 11:31 -0400, Benjamin Marzinski wrote: > On Tue, Jul 21, 2026 at 09:43:44AM +0200, Martin Wilck wrote: > > > > 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.
My bad, I should have re-read the function reservation_key_matches(). > 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. It's quite likely to fail if the PRIN command failed beforehand. > 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. I agree. If this was just a single path and the path was down, the command would fail, and nothing would happen on the storage side. So this is what we should do here. There's a minimal chance that the PROUT would succeed even after the PRIN had failed, but in that case we wouldn't know what to do with our registered keys, as you described correctly. So we should just fail. Regards Martin
