* [PULL 1/5] scsi: change buf_size to unsigned int in scsi_SG_IO()
2026-05-09 5:22 [PULL 0/5] SCSI changes for 2026-05-08 Paolo Bonzini
@ 2026-05-09 5:23 ` Paolo Bonzini
2026-05-09 5:23 ` [PULL 2/5] scsi: adjust error_prepend() formatting Paolo Bonzini
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2026-05-09 5:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
SG_IO supports an unsigned int dxfer_len value. Existing callers use
less than 256 bytes, so scsi_SG_IO()'s uint8_t buf_size type was
sufficient. The next patch will use a larger value, so update the type.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Link: https://lore.kernel.org/r/20260415232906.212349-2-stefanha@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/scsi/scsi.h | 3 ++-
hw/scsi/scsi-generic.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index a3e246dbd92..5f83e58d1d0 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -247,7 +247,8 @@ void scsi_device_unit_attention_reported(SCSIDevice *dev);
void scsi_generic_read_device_inquiry(SCSIDevice *dev);
int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
int scsi_SG_IO(BlockBackend *blk, int direction, uint8_t *cmd, uint8_t cmd_size,
- uint8_t *buf, uint8_t buf_size, uint32_t timeout, Error **errp);
+ uint8_t *buf, unsigned int buf_size, uint32_t timeout,
+ Error **errp);
SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun);
SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int target, int lun);
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index a2316a5266e..e629b1756b0 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -774,7 +774,7 @@ static int read_naa_id(const uint8_t *p, uint64_t *p_wwn)
}
int scsi_SG_IO(BlockBackend *blk, int direction, uint8_t *cmd,
- uint8_t cmd_size, uint8_t *buf, uint8_t buf_size,
+ uint8_t cmd_size, uint8_t *buf, unsigned int buf_size,
uint32_t timeout, Error **errp)
{
sg_io_hdr_t io_header;
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 2/5] scsi: adjust error_prepend() formatting
2026-05-09 5:22 [PULL 0/5] SCSI changes for 2026-05-08 Paolo Bonzini
2026-05-09 5:23 ` [PULL 1/5] scsi: change buf_size to unsigned int in scsi_SG_IO() Paolo Bonzini
@ 2026-05-09 5:23 ` Paolo Bonzini
2026-05-09 5:23 ` [PULL 3/5] scsi: always send valid PREEMPT TYPE field Paolo Bonzini
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2026-05-09 5:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
The error strings will be concatenated so add a separator to make the
combined error message easy to read.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Link: https://lore.kernel.org/r/20260401171927.396672-2-stefanha@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-generic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index e629b1756b0..d0086d86d94 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -439,7 +439,7 @@ static bool scsi_generic_pr_register(SCSIDevice *s, uint64_t key, Error **errp)
ret = scsi_SG_IO(s->conf.blk, SG_DXFER_TO_DEV, cmd, sizeof(cmd),
buf, sizeof(buf), s->io_timeout, errp);
if (ret < 0) {
- error_prepend(errp, "PERSISTENT RESERVE OUT with REGISTER");
+ error_prepend(errp, "PERSISTENT RESERVE OUT with REGISTER: ");
return false;
}
return true;
@@ -463,7 +463,7 @@ static bool scsi_generic_pr_preempt(SCSIDevice *s, uint64_t key,
ret = scsi_SG_IO(s->conf.blk, SG_DXFER_TO_DEV, cmd, sizeof(cmd),
buf, sizeof(buf), s->io_timeout, errp);
if (ret < 0) {
- error_prepend(errp, "PERSISTENT RESERVE OUT with PREEMPT");
+ error_prepend(errp, "PERSISTENT RESERVE OUT with PREEMPT: ");
return false;
}
return true;
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 3/5] scsi: always send valid PREEMPT TYPE field
2026-05-09 5:22 [PULL 0/5] SCSI changes for 2026-05-08 Paolo Bonzini
2026-05-09 5:23 ` [PULL 1/5] scsi: change buf_size to unsigned int in scsi_SG_IO() Paolo Bonzini
2026-05-09 5:23 ` [PULL 2/5] scsi: adjust error_prepend() formatting Paolo Bonzini
@ 2026-05-09 5:23 ` Paolo Bonzini
2026-05-09 5:23 ` [PULL 4/5] scsi: register again after PREEMPT without reservation Paolo Bonzini
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2026-05-09 5:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Qing Wang
From: Stefan Hajnoczi <stefanha@redhat.com>
The SPC-6 specification says that the PREEMPT service action ignores the
TYPE field when there is no reservation. However, the LIO Linux iSCSI
target rejects commands with a zero TYPE field. The field never ends up
being used in this case, so replace it with a "valid" value to work
around the issue.
Reported-by: Qing Wang <qinwang@redhat.com>
Buglink: https://redhat.atlassian.net/browse/RHEL-155807
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Link: https://lore.kernel.org/r/20260401171927.396672-3-stefanha@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/scsi/constants.h | 10 ++++++++++
hw/scsi/scsi-generic.c | 10 ++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index cb97bdb6362..717e470a5d4 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -340,4 +340,14 @@
#define PRO_REGISTER_AND_MOVE 0x07
#define PRO_REPLACE_LOST_RESERVATION 0x08
+/*
+ * Persistent reservation types
+ */
+#define PR_TYPE_WRITE_EXCLUSIVE 0x1
+#define PR_TYPE_EXCLUSIVE_ACCESS 0x3
+#define PR_TYPE_WRITE_EXCLUSIVE_REG_ONLY 0x5
+#define PR_TYPE_EXCLUSIVE_ACCESS_REG_ONLY 0x6
+#define PR_TYPE_WRITE_EXCLUSIVE_ALL_REGS 0x7
+#define PR_TYPE_EXCLUSIVE_ACCESS_ALL_REGS 0x8
+
#endif
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index d0086d86d94..f1c328e6da6 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -453,6 +453,16 @@ static bool scsi_generic_pr_preempt(SCSIDevice *s, uint64_t key,
uint64_t key_be = cpu_to_be64(key);
int ret;
+ /*
+ * The LIO iSCSI target in Linux up to at least version 7.0 rejects PREEMPT
+ * commands with a zero TYPE field although the SPC-6 specification says
+ * the field should be ignored when there is no persistent reservation.
+ * Work around this by choosing an arbitrary valid PR type value.
+ */
+ if (resv_type == 0) {
+ resv_type = PR_TYPE_WRITE_EXCLUSIVE;
+ }
+
cmd[0] = PERSISTENT_RESERVE_OUT;
cmd[1] = PRO_PREEMPT;
cmd[2] = resv_type & 0xf;
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 4/5] scsi: register again after PREEMPT without reservation
2026-05-09 5:22 [PULL 0/5] SCSI changes for 2026-05-08 Paolo Bonzini
` (2 preceding siblings ...)
2026-05-09 5:23 ` [PULL 3/5] scsi: always send valid PREEMPT TYPE field Paolo Bonzini
@ 2026-05-09 5:23 ` Paolo Bonzini
2026-05-09 5:23 ` [PULL 5/5] scsi: handle reservation changes across migration Paolo Bonzini
2026-05-11 14:22 ` [PULL 0/5] SCSI changes for 2026-05-08 Stefan Hajnoczi
5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2026-05-09 5:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
The SCSI specification says PREEMPT without a reservation removes all
registrations with the given key. Try to register again after PREEMPT
since our key will have been removed.
In practice some SCSI targets keep the calling I_T nexus' registration
instead of removing it. Therefore we need to handle both the
spec-compliant and the non-compliant behavior.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Link: https://lore.kernel.org/r/20260401171927.396672-4-stefanha@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-generic.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index f1c328e6da6..29bc952af5d 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -513,6 +513,16 @@ bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
if (!scsi_generic_pr_preempt(s, key, resv_type, errp)) {
return false;
}
+
+ /*
+ * Some SCSI targets, like the Linux LIO target, remove our
+ * registration when preempting without a reservation (resv_type is 0).
+ * Try to register again but ignore the error since a RESERVATION
+ * CONFLICT is expected if our registration remained in place.
+ */
+ if (resv_type == 0) {
+ scsi_generic_pr_register(s, key, NULL);
+ }
}
return true;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL 5/5] scsi: handle reservation changes across migration
2026-05-09 5:22 [PULL 0/5] SCSI changes for 2026-05-08 Paolo Bonzini
` (3 preceding siblings ...)
2026-05-09 5:23 ` [PULL 4/5] scsi: register again after PREEMPT without reservation Paolo Bonzini
@ 2026-05-09 5:23 ` Paolo Bonzini
2026-05-11 14:22 ` [PULL 0/5] SCSI changes for 2026-05-08 Stefan Hajnoczi
5 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2026-05-09 5:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi
From: Stefan Hajnoczi <stefanha@redhat.com>
Other nodes in the cluster can preempt or clear SCSI Persistent
Reservations at any time. When this happens across live migration, the
reservation state transferred with the guest might be outdated.
Attempt to handle such cases gracefully by checking the current
reservation or registered keys to detect stale state before restoring.
If the actual state of the disk has changed, do not modify it and accept
that as the most up-to-date state.
Do this using READ RESERVATION when the guest holds a reservation or
READ KEYS when the guest has registered a key but does not hold a
reservation.
There is still a race condition between checking and restoring state,
but it seems unavoidable and is no worse than before.
Buglink: https://redhat.atlassian.net/browse/RHEL-153123
Fixes: ab57b51f1375b6a6f098a74c6f79207a9630948d ("scsi: save/load SCSI reservation state")
Reported-by: Qing Wang
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Link: https://lore.kernel.org/r/20260415232906.212349-3-stefanha@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-generic.c | 173 +++++++++++++++++++++++++++++++++++------
1 file changed, 149 insertions(+), 24 deletions(-)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 29bc952af5d..8999f3b7200 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -479,13 +479,84 @@ static bool scsi_generic_pr_preempt(SCSIDevice *s, uint64_t key,
return true;
}
+/*
+ * Returns true if the given key is registered or false otherwise (including
+ * errors).
+ */
+static bool scsi_generic_pr_key_registered(SCSIDevice *s, uint64_t key,
+ Error **errp)
+{
+ const size_t key_list_offset = 8; /* in READ KEYS parameter data */
+ uint64_t key_be = cpu_to_be64(key);
+ uint8_t cmd[10] = {};
+ size_t buf_len;
+ g_autofree uint8_t *buf = NULL;
+ uint32_t additional_length = 16 * 8; /* initial key list size */
+
+ /*
+ * Loop to resize parameter data buffer when there are many keys. It would
+ * be simpler to hardcode the maximum buffer size (it's only 64 KB), but
+ * SG_IO can fail with EINVAL if the host kernel blkdev queue limits are
+ * too low.
+ */
+ do {
+ uint16_t allocation_length_be;
+ int ret;
+
+ buf_len = key_list_offset + additional_length;
+ buf = g_realloc(buf, buf_len);
+ memset(buf, 0, buf_len);
+
+ cmd[0] = PERSISTENT_RESERVE_IN;
+ cmd[1] = PRI_READ_KEYS;
+ allocation_length_be = cpu_to_be16(buf_len);
+ memcpy(&cmd[7], &allocation_length_be, sizeof(allocation_length_be));
+
+ ret = scsi_SG_IO(s->conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
+ buf, buf_len, s->io_timeout, errp);
+ if (ret < 0) {
+ error_prepend(errp, "PERSISTENT RESERVE IN with READ KEYS: ");
+ return false;
+ }
+
+ memcpy(&additional_length, &buf[4], sizeof(additional_length));
+ be32_to_cpus(&additional_length);
+
+ /*
+ * The parameter data's ADDITIONAL LENGTH must not overflow the CDB's
+ * 16-bit ALLOCATION LENGTH field since the next loop iteration will
+ * compute ALLOCATION LENGTH based on ADDITIONAL LENGTH.
+ */
+ if (additional_length > UINT16_MAX - key_list_offset) {
+ error_setg(errp, "got invalid ADDITIONAL LENGTH %" PRIu32
+ " from READ KEYS", additional_length);
+ return false;
+ }
+
+ for (size_t i = key_list_offset; i < buf_len; i += sizeof(key_be)) {
+ if (i - key_list_offset >= additional_length) {
+ break; /* end of parameter list */
+ }
+
+ if (memcmp(&key_be, &buf[i], sizeof(key_be)) == 0) {
+ return true; /* key found */
+ }
+ }
+ } while (additional_length > buf_len - key_list_offset);
+
+ return false; /* key not found */
+}
+
/* Register keys and preempt reservations after live migration */
bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
{
SCSIPRState *pr_state = &s->pr_state;
+ Error *local_err = NULL;
+ bool check_stale_key = true;
uint64_t key;
uint8_t resv_type;
+ /* Get the migrated PR state */
WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
key = pr_state->key;
resv_type = pr_state->resv_type;
@@ -493,36 +564,90 @@ bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
trace_scsi_generic_pr_state_preempt(key, resv_type);
- if (key) {
- if (!scsi_generic_pr_register(s, key, errp)) {
+ /* Handle stale PR state (e.g. another node preempted) */
+ if (resv_type) {
+ uint64_t dev_key;
+ uint8_t dev_resv_type;
+
+ if (scsi_generic_read_reservation(s, &dev_key, &dev_resv_type,
+ errp) < 0) {
return false;
}
- /*
- * Two cases:
- *
- * 1. There is no reservation (resv_type is 0) and the other I_T nexus
- * will be unregistered. This is important so the source host does
- * not leak registered keys across live migration.
- *
- * 2. There is a reservation (resv_type is not 0) and the other I_T
- * nexus will be unregistered and its reservation is atomically
- * taken over by us. This is the scenario where a reservation is
- * migrated along with the guest.
- */
- if (!scsi_generic_pr_preempt(s, key, resv_type, errp)) {
+ if (dev_resv_type != resv_type) {
+ /* vmstate had a stale reservation type */
+ g_autofree char *name = qdev_get_human_name(&s->qdev);
+ warn_report("Expected SCSI reservation type 0x%x on device '%s', "
+ "got 0x%x, using new type",
+ resv_type, name, dev_resv_type);
+ resv_type = dev_resv_type;
+ }
+
+ if (dev_key == key) {
+ /* The reservation exists, no need to check for a stale key */
+ check_stale_key = false;
+ } else {
+ g_autofree char *name = qdev_get_human_name(&s->qdev);
+ warn_report("Expected SCSI reservation with key 0x%" PRIx64
+ " on device '%s', got 0x%" PRIx64 ", ignoring "
+ "reservation",
+ key, name, dev_key);
+ resv_type = 0; /* vmstate had a stale reservation */
+ }
+ }
+
+ if (key != 0 && check_stale_key &&
+ !scsi_generic_pr_key_registered(s, key, &local_err)) {
+ if (local_err) {
+ error_propagate(errp, local_err);
return false;
}
- /*
- * Some SCSI targets, like the Linux LIO target, remove our
- * registration when preempting without a reservation (resv_type is 0).
- * Try to register again but ignore the error since a RESERVATION
- * CONFLICT is expected if our registration remained in place.
- */
- if (resv_type == 0) {
- scsi_generic_pr_register(s, key, NULL);
- }
+ g_autofree char *name = qdev_get_human_name(&s->qdev);
+ warn_report("SCSI reservation key 0x%" PRIx64 " on device '%s' not "
+ "registered after migration, ignoring",
+ key, name);
+ key = 0; /* vmstate had a stale key */
+ }
+
+ /* Stale PR state may have been updated */
+ WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+ pr_state->key = key;
+ pr_state->resv_type = resv_type;
+ }
+
+ if (key == 0) {
+ return true; /* no PR state, do nothing */
+ }
+
+ if (!scsi_generic_pr_register(s, key, errp)) {
+ return false;
+ }
+
+ /*
+ * Two cases:
+ *
+ * 1. There is no reservation (resv_type is 0) and the other I_T nexus
+ * will be unregistered. This is important so the source host does
+ * not leak registered keys across live migration.
+ *
+ * 2. There is a reservation (resv_type is not 0) and the other I_T
+ * nexus will be unregistered and its reservation is atomically
+ * taken over by us. This is the scenario where a reservation is
+ * migrated along with the guest.
+ */
+ if (!scsi_generic_pr_preempt(s, key, resv_type, errp)) {
+ return false;
+ }
+
+ /*
+ * Some SCSI targets, like the Linux LIO target, remove our
+ * registration when preempting without a reservation (resv_type is 0).
+ * Try to register again but ignore the error since a RESERVATION
+ * CONFLICT is expected if our registration remained in place.
+ */
+ if (resv_type == 0) {
+ scsi_generic_pr_register(s, key, NULL);
}
return true;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PULL 0/5] SCSI changes for 2026-05-08
2026-05-09 5:22 [PULL 0/5] SCSI changes for 2026-05-08 Paolo Bonzini
` (4 preceding siblings ...)
2026-05-09 5:23 ` [PULL 5/5] scsi: handle reservation changes across migration Paolo Bonzini
@ 2026-05-11 14:22 ` Stefan Hajnoczi
5 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2026-05-11 14:22 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/11.1 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread