[RFC 0/4] scsi: persistent reservation live migration

[RFC 0/4] scsi: persistent reservation live migration

[Stefan Hajnoczi]

Live migration does not work for SCSI Persistent Reservations acquired on
scsi-block devices. This patch series migrates the reservation key and
reservation type so that the destination QEMU can take over the persistent
reservation with the PREEMPT service action upon live migration.

The approach involves snooping PERSISTENT RESERVE OUT replies and tracking the
scsi-block device's current reservation key and reservation type. In most cases
this involves no additional SCSI commands. This approach isn't perfect: if
another machine modifies the reservation on the physical LUN, then QEMU's state
becomes stale. Persistent reservations are inherently cooperative, so this is
acceptable as long as real applications don't run into problems.

I am also working on a test suite called pr-tests that runs sg_persist(8)
commands across multiple machines in order to exercise various scenarios:
https://gitlab.com/stefanha/pr-tests

Stefan Hajnoczi (4):
  scsi: track SCSI reservation state for live migration
  scsi: generalize scsi_SG_IO_FROM_DEV() to scsi_SG_IO()
  scsi: add error reporting to scsi_SG_IO()
  scsi: save/load SCSI reservation state

 include/hw/scsi/scsi.h   |  15 +-
 include/scsi/constants.h |  21 +++
 hw/core/machine.c        |   4 +-
 hw/scsi/scsi-bus.c       |   3 +
 hw/scsi/scsi-disk.c      |  53 +++++++-
 hw/scsi/scsi-generic.c   | 286 +++++++++++++++++++++++++++++++++++++--
 hw/scsi/trace-events     |   2 +
 7 files changed, 369 insertions(+), 15 deletions(-)

-- 
2.52.0

[RFC 1/4] scsi: track SCSI reservation state for live migration

[Stefan Hajnoczi]

SCSI Persistent Reservations are stateful and external to the guest. In
order to transparently move reservations to the destination host during
live migration, it is necessary to track the state built up on the
source host before migration. Only then can the destination host ensure
an equivalent state is restored upon migration.

Snoop on successful PERSISTENT RESERVE OUT commands and save the
reservation key and reservation type. This will allow registered keys
and reservations to be migrated.

Also patch PERSISTENT RESERVE IN replies with the REPORT CAPABILITIES
service action since features that involve the physical SCSI bus target
ports must not be exposed to the guest (it sees a virtual SCSI bus).

Usually this plays out as follows:
1. The guest invokes the REGISTER service action to register a
   reservation key on its I_T nexus.
2. The guest invokes the RESERVE service action to create a reservation
   using the previously-registered key.

This commit implements the snooping and stores the reservation key and
type (if any) for each LUN. The snooped PR state and the migrate_pr flag
to enable PR migration will be used in later commits.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/scsi/scsi.h   |  10 +++
 include/scsi/constants.h |  21 +++++
 hw/scsi/scsi-bus.c       |   3 +
 hw/scsi/scsi-generic.c   | 160 +++++++++++++++++++++++++++++++++++++++
 hw/scsi/trace-events     |   1 +
 5 files changed, 195 insertions(+)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index d26f1127bb..23bd5b4b5e 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -57,6 +57,13 @@ struct SCSIRequest {
     QTAILQ_ENTRY(SCSIRequest) next;
 };
 
+/* Per-SCSIDevice Persistent Reservation state */
+typedef struct {
+    QemuMutex mutex;   /* protects all fields (e.g. from multiple IOThreads) */
+    uint64_t key;      /* 0 if no registered key */
+    uint8_t resv_type; /* 0 if no reservation */
+} SCSIPRState;
+
 #define TYPE_SCSI_DEVICE "scsi-device"
 OBJECT_DECLARE_TYPE(SCSIDevice, SCSIDeviceClass, SCSI_DEVICE)
 
@@ -97,6 +104,9 @@ struct SCSIDevice
     uint32_t io_timeout;
     bool needs_vpd_bl_emulation;
     bool hba_supports_iothread;
+
+    bool migrate_pr;
+    SCSIPRState pr_state;
 };
 
 extern const VMStateDescription vmstate_scsi_device;
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 9b98451912..cb97bdb636 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -319,4 +319,25 @@
 #define IDENT_DESCR_TGT_DESCR_SIZE 32
 #define XCOPY_BLK2BLK_SEG_DESC_SIZE 28
 
+/*
+ * PERSISTENT RESERVATION IN service action codes
+ */
+#define PRI_READ_KEYS           0x00
+#define PRI_READ_RESERVATION    0x01
+#define PRI_REPORT_CAPABILITIES 0x02
+#define PRI_READ_FULL_STATUS    0x03
+
+/*
+ * PERSISTENT RESERVATION OUT service action codes
+ */
+#define PRO_REGISTER                            0x00
+#define PRO_RESERVE                             0x01
+#define PRO_RELEASE                             0x02
+#define PRO_CLEAR                               0x03
+#define PRO_PREEMPT                             0x04
+#define PRO_PREEMPT_AND_ABORT                   0x05
+#define PRO_REGISTER_AND_IGNORE_EXISTING_KEY    0x06
+#define PRO_REGISTER_AND_MOVE                   0x07
+#define PRO_REPLACE_LOST_RESERVATION            0x08
+
 #endif
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f310ddafb9..9b8656dd83 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -393,6 +393,7 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
     }
 
     qemu_mutex_init(&dev->requests_lock);
+    qemu_mutex_init(&dev->pr_state.mutex);
     QTAILQ_INIT(&dev->requests);
     scsi_device_realize(dev, &local_err);
     if (local_err) {
@@ -417,6 +418,8 @@ static void scsi_qdev_unrealize(DeviceState *qdev)
 
     scsi_device_unrealize(dev);
 
+    qemu_mutex_destroy(&dev->pr_state.mutex);
+
     blockdev_mark_auto_del(dev->conf.blk);
 }
 
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 0a676a16fa..2ddd435856 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -264,6 +264,160 @@ static int scsi_generic_emulate_block_limits(SCSIGenericReq *r, SCSIDevice *s)
     return r->buflen;
 }
 
+/*
+ * Patch persistent reservation capabilities that are not emulated.
+ */
+static void scsi_handle_persistent_reserve_in_reply(SCSIGenericReq *r,
+                                                    SCSIDevice *s)
+{
+    uint8_t service_action = r->req.cmd.buf[1] & 0x1f;
+
+    if (!s->migrate_pr) {
+        return; /* when migration is disabled there is no need for patching */
+    }
+
+    if (service_action == PRI_REPORT_CAPABILITIES) {
+        assert(r->buflen >= 3);
+
+        /*
+         * Clear specify initiator ports capable (SIP_C) and all target ports
+         * capable (ATC_C).
+         *
+         * SPEC_I_PT is not supported because the guest sees an emulated SCSI
+         * bus and does not have the underlying transport IDs needed to use
+         * SPEC_I_PT.
+         *
+         * ALL_TG_PT is not supported because we only track the state of this
+         * emulated I_T nexus, not the underlying device's target ports.
+         */
+        r->buf[2] &= ~0xc;
+    }
+}
+
+static int scsi_generic_read_reservation(SCSIDevice *s, uint64_t *key,
+        uint8_t *resv_type, Error **errp)
+{
+    uint8_t cmd[10] = {};
+    uint8_t buf[24] = {};
+    uint32_t additional_length;
+    int ret;
+
+    *key = 0;
+    *resv_type = 0;
+
+    cmd[0] = PERSISTENT_RESERVE_IN;
+    cmd[1] = PRI_READ_RESERVATION;
+    cmd[8] = sizeof(buf);
+
+    ret = scsi_SG_IO(s->conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
+                     buf, sizeof(buf), s->io_timeout, errp);
+    if (ret < 0) {
+        return ret;
+    }
+
+    memcpy(&additional_length, &buf[4], sizeof(additional_length));
+    be32_to_cpus(&additional_length);
+
+    if (additional_length >= 0x10) {
+        memcpy(key, &buf[8], sizeof(*key));
+        be64_to_cpus(key);
+
+        *resv_type = buf[21] & 0xf;
+    }
+    return 0;
+}
+
+/*
+ * Snoop changes to registered keys and reservations so that this information
+ * can be transferred during live migration.
+ */
+static void scsi_handle_persistent_reserve_out_reply(
+        SCSIGenericReq *r,
+        SCSIDevice *s)
+{
+    SCSIPRState *pr_state = &s->pr_state;
+    uint8_t service_action = r->req.cmd.buf[1] & 0x1f;
+    uint8_t resv_type = r->req.cmd.buf[2] & 0xf;
+    uint64_t old_key;
+    uint64_t new_key;
+
+    assert(r->buflen >= 16);
+    memcpy(&old_key, &r->buf[0], sizeof(old_key));
+    memcpy(&new_key, &r->buf[8], sizeof(new_key));
+    be64_to_cpus(&old_key);
+    be64_to_cpus(&new_key);
+
+    trace_scsi_generic_persistent_reserve_out_reply(service_action, resv_type,
+                                                    old_key, new_key);
+
+    switch (service_action) {
+    case PRO_REGISTER: /* fallthrough */
+    case PRO_REGISTER_AND_IGNORE_EXISTING_KEY:
+        if (service_action == PRO_REGISTER && old_key == 0 && new_key == 0) {
+            /* Do nothing */
+        } else {
+            WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+                pr_state->key = new_key;
+                if (new_key == 0) {
+                    pr_state->resv_type = 0; /* release reservation */
+                }
+            }
+        }
+        break;
+
+    case PRO_RESERVE:
+        WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+            pr_state->resv_type = resv_type;
+        }
+        break;
+
+    case PRO_RELEASE:
+        WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+            pr_state->resv_type = 0;
+        }
+        break;
+
+    case PRO_CLEAR:
+        WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+            pr_state->key = 0;
+            pr_state->resv_type = 0;
+        }
+        break;
+
+    case PRO_REPLACE_LOST_RESERVATION:
+        WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+            pr_state->key = new_key;
+            pr_state->resv_type = resv_type;
+        }
+        break;
+
+    case PRO_PREEMPT: /* fallthrough */
+    case PRO_PREEMPT_AND_ABORT: {
+        uint64_t dev_key;
+        uint8_t dev_resv_type;
+
+        /* Not enough information to know actual state, ask the device */
+        if (!scsi_generic_read_reservation(s, &dev_key, &dev_resv_type, NULL)) {
+            WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+                if (pr_state->key == dev_key) {
+                    pr_state->resv_type = dev_resv_type;
+                } else {
+                    pr_state->resv_type = 0;
+                }
+            }
+        }
+        break;
+    }
+
+    /*
+     * PRO_REGISTER_AND_MOVE cannot be implemented since it involves the
+     * physical SCSI bus target ports.
+     */
+    default:
+        break; /* do nothing */
+    }
+}
+
 static void scsi_read_complete(void * opaque, int ret)
 {
     SCSIGenericReq *r = (SCSIGenericReq *)opaque;
@@ -346,6 +500,9 @@ static void scsi_read_complete(void * opaque, int ret)
     if (r->req.cmd.buf[0] == INQUIRY) {
         len = scsi_handle_inquiry_reply(r, s, len);
     }
+    if (r->req.cmd.buf[0] == PERSISTENT_RESERVE_IN) {
+        scsi_handle_persistent_reserve_in_reply(r, s);
+    }
 
 req_complete:
     scsi_req_data(&r->req, len);
@@ -395,6 +552,9 @@ static void scsi_write_complete(void * opaque, int ret)
         s->blocksize = (r->buf[9] << 16) | (r->buf[10] << 8) | r->buf[11];
         trace_scsi_generic_write_complete_blocksize(s->blocksize);
     }
+    if (r->req.cmd.buf[0] == PERSISTENT_RESERVE_OUT) {
+        scsi_handle_persistent_reserve_out_reply(r, s);
+    }
 
     scsi_command_complete_noio(r, ret);
 }
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 3e81f44dad..ff92fff7c5 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -390,3 +390,4 @@ scsi_generic_realize_blocksize(int blocksize) "block size %d"
 scsi_generic_aio_sgio_command(uint32_t tag, uint8_t cmd, uint32_t timeout) "generic aio sgio: tag=0x%x cmd=0x%x timeout=%u"
 scsi_generic_ioctl_sgio_command(uint8_t cmd, uint32_t timeout) "generic ioctl sgio: cmd=0x%x timeout=%u"
 scsi_generic_ioctl_sgio_done(uint8_t cmd, int ret, uint8_t status, uint8_t host_status) "generic ioctl sgio: cmd=0x%x ret=%d status=0x%x host_status=0x%x"
+scsi_generic_persistent_reserve_out_reply(uint8_t service_action, uint8_t resv_type, uint64_t old_key, uint64_t new_key) "persistent reserve out reply service_action=%u resv_type=%u old_key=0x%" PRIx64 " new_key=0x%" PRIx64
-- 
2.52.0

[RFC 2/4] scsi: generalize scsi_SG_IO_FROM_DEV() to scsi_SG_IO()

[Stefan Hajnoczi]

Add a direction argument so that scsi_SG_IO() can be used for
SG_DXFER_FROM_DEV and SG_DXFER_TO_DEV transfers.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/scsi/scsi.h |  4 ++--
 hw/scsi/scsi-disk.c    |  4 ++--
 hw/scsi/scsi-generic.c | 18 ++++++++++--------
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 23bd5b4b5e..1a01842c28 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -246,8 +246,8 @@ void scsi_device_report_change(SCSIDevice *dev, SCSISense sense);
 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_FROM_DEV(BlockBackend *blk, uint8_t *cmd, uint8_t cmd_size,
-                        uint8_t *buf, uint8_t buf_size, uint32_t timeout);
+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);
 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-disk.c b/hw/scsi/scsi-disk.c
index 0f896c27f4..97ae535a27 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2748,8 +2748,8 @@ static int get_device_type(SCSIDiskState *s)
     cmd[0] = INQUIRY;
     cmd[4] = sizeof(buf);
 
-    ret = scsi_SG_IO_FROM_DEV(s->qdev.conf.blk, cmd, sizeof(cmd),
-                              buf, sizeof(buf), s->qdev.io_timeout);
+    ret = scsi_SG_IO(s->qdev.conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
+                     buf, sizeof(buf), s->qdev.io_timeout);
     if (ret < 0) {
         return -1;
     }
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 2ddd435856..6ef673aa4b 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -684,8 +684,9 @@ static int read_naa_id(const uint8_t *p, uint64_t *p_wwn)
     return -EINVAL;
 }
 
-int scsi_SG_IO_FROM_DEV(BlockBackend *blk, uint8_t *cmd, uint8_t cmd_size,
-                        uint8_t *buf, uint8_t buf_size, uint32_t timeout)
+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)
 {
     sg_io_hdr_t io_header;
     uint8_t sensebuf[8];
@@ -693,7 +694,7 @@ int scsi_SG_IO_FROM_DEV(BlockBackend *blk, uint8_t *cmd, uint8_t cmd_size,
 
     memset(&io_header, 0, sizeof(io_header));
     io_header.interface_id = 'S';
-    io_header.dxfer_direction = SG_DXFER_FROM_DEV;
+    io_header.dxfer_direction = direction;
     io_header.dxfer_len = buf_size;
     io_header.dxferp = buf;
     io_header.cmdp = cmd;
@@ -733,8 +734,8 @@ static void scsi_generic_set_vpd_bl_emulation(SCSIDevice *s)
     cmd[2] = 0x00;
     cmd[4] = sizeof(buf);
 
-    ret = scsi_SG_IO_FROM_DEV(s->conf.blk, cmd, sizeof(cmd),
-                              buf, sizeof(buf), s->io_timeout);
+    ret = scsi_SG_IO(s->conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
+                     buf, sizeof(buf), s->io_timeout);
     if (ret < 0) {
         /*
          * Do not assume anything if we can't retrieve the
@@ -769,8 +770,8 @@ static void scsi_generic_read_device_identification(SCSIDevice *s)
     cmd[2] = 0x83;
     cmd[4] = sizeof(buf);
 
-    ret = scsi_SG_IO_FROM_DEV(s->conf.blk, cmd, sizeof(cmd),
-                              buf, sizeof(buf), s->io_timeout);
+    ret = scsi_SG_IO(s->conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
+                     buf, sizeof(buf), s->io_timeout);
     if (ret < 0) {
         return;
     }
@@ -821,7 +822,8 @@ static int get_stream_blocksize(BlockBackend *blk)
     cmd[0] = MODE_SENSE;
     cmd[4] = sizeof(buf);
 
-    ret = scsi_SG_IO_FROM_DEV(blk, cmd, sizeof(cmd), buf, sizeof(buf), 6);
+    ret = scsi_SG_IO(blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
+                     buf, sizeof(buf), 6);
     if (ret < 0) {
         return -1;
     }
-- 
2.52.0

[RFC 3/4] scsi: add error reporting to scsi_SG_IO()

[Stefan Hajnoczi]

Report the details of the SG_IO ioctl failure if an Error pointer is
provided. This information aids troubleshooting and will be used by the
SCSI Persistent Reservations migration code.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/scsi/scsi.h |  2 +-
 hw/scsi/scsi-disk.c    |  2 +-
 hw/scsi/scsi-generic.c | 33 ++++++++++++++++++++++++++++-----
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1a01842c28..c5ec58089b 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -247,7 +247,7 @@ 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);
+               uint8_t *buf, uint8_t 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-disk.c b/hw/scsi/scsi-disk.c
index 97ae535a27..76fe5f085b 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2749,7 +2749,7 @@ static int get_device_type(SCSIDiskState *s)
     cmd[4] = sizeof(buf);
 
     ret = scsi_SG_IO(s->qdev.conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
-                     buf, sizeof(buf), s->qdev.io_timeout);
+                     buf, sizeof(buf), s->qdev.io_timeout, NULL);
     if (ret < 0) {
         return -1;
     }
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 6ef673aa4b..f22a38f725 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -686,10 +686,10 @@ 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,
-               uint32_t timeout)
+               uint32_t timeout, Error **errp)
 {
     sg_io_hdr_t io_header;
-    uint8_t sensebuf[8];
+    uint8_t sensebuf[8] = {};
     int ret;
 
     memset(&io_header, 0, sizeof(io_header));
@@ -709,6 +709,29 @@ int scsi_SG_IO(BlockBackend *blk, int direction, uint8_t *cmd,
         io_header.driver_status || io_header.host_status) {
         trace_scsi_generic_ioctl_sgio_done(cmd[0], ret, io_header.status,
                                            io_header.host_status);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "SG_IO ioctl failed");
+        } else {
+            g_autofree char *sensebuf_hex =
+                g_strdup_printf("%02x%02x%02x%02x%02x%02x%02x%02x",
+                                sensebuf[0],
+                                sensebuf[1],
+                                sensebuf[2],
+                                sensebuf[3],
+                                sensebuf[4],
+                                sensebuf[5],
+                                sensebuf[6],
+                                sensebuf[7]);
+
+            error_setg(errp, "SG_IO SCSI command failed with status=0x%x "
+                    "driver_status=0x%x host_status=0x%x sensebuf=%s "
+                    "sb_len_wr=%u",
+                    io_header.status,
+                    io_header.driver_status,
+                    io_header.host_status,
+                    sensebuf_hex,
+                    io_header.sb_len_wr);
+        }
         return -1;
     }
     return 0;
@@ -735,7 +758,7 @@ static void scsi_generic_set_vpd_bl_emulation(SCSIDevice *s)
     cmd[4] = sizeof(buf);
 
     ret = scsi_SG_IO(s->conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
-                     buf, sizeof(buf), s->io_timeout);
+                     buf, sizeof(buf), s->io_timeout, NULL);
     if (ret < 0) {
         /*
          * Do not assume anything if we can't retrieve the
@@ -771,7 +794,7 @@ static void scsi_generic_read_device_identification(SCSIDevice *s)
     cmd[4] = sizeof(buf);
 
     ret = scsi_SG_IO(s->conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
-                     buf, sizeof(buf), s->io_timeout);
+                     buf, sizeof(buf), s->io_timeout, NULL);
     if (ret < 0) {
         return;
     }
@@ -823,7 +846,7 @@ static int get_stream_blocksize(BlockBackend *blk)
     cmd[4] = sizeof(buf);
 
     ret = scsi_SG_IO(blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
-                     buf, sizeof(buf), 6);
+                     buf, sizeof(buf), 6, NULL);
     if (ret < 0) {
         return -1;
     }
-- 
2.52.0

[RFC 4/4] scsi: save/load SCSI reservation state

[Stefan Hajnoczi]

Add a vmstate subsection to SCSIDiskState so that scsi-block devices can
transfer their reservation state during live migration. Upon loading the
subsection, the destination QEMU invokes the PERSISTENT RESERVE OUT
command's PREEMPT service action to atomically move the reservation from
the source I_T nexus to the destination I_T nexus. This results in
transparent live migration of SCSI reservations.

This approach is incomplete since SCSI reservations are cooperative and
other hosts could interfere. Neither the source QEMU nor the destination
QEMU are aware of changes made by other hosts. The assumption is that
reservation is not taken over by a third host without cooperation from
the source host.

I considered adding the vmstate subsection to SCSIDevice instead of
SCSIDiskState, since reservations are part of the SCSI Primary Commands
that other devices apart from disks could support. However, due to
fragility of migrating reservations, we will probably limit support to
scsi-block and maybe scsi-disk in the future. In the end, I think it
makes sense to place this within scsi-disk.c.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/scsi/scsi.h |  1 +
 hw/core/machine.c      |  4 +-
 hw/scsi/scsi-disk.c    | 49 ++++++++++++++++++++++++-
 hw/scsi/scsi-generic.c | 83 ++++++++++++++++++++++++++++++++++++++++++
 hw/scsi/trace-events   |  1 +
 5 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index c5ec58089b..d104557bac 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -253,6 +253,7 @@ SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int target, int lun);
 
 /* scsi-generic.c. */
 extern const SCSIReqOps scsi_generic_req_ops;
+bool scsi_generic_pr_state_post_load_errp(SCSIDevice *s, Error **errp);
 
 /* scsi-disk.c */
 #define SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR             0
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6411e68856..16134f8ce5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -38,7 +38,9 @@
 #include "hw/acpi/generic_event_device.h"
 #include "qemu/audio.h"
 
-GlobalProperty hw_compat_10_2[] = {};
+GlobalProperty hw_compat_10_2[] = {
+    { "scsi-block", "migrate-pr", "off" },
+};
 const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
 
 GlobalProperty hw_compat_10_1[] = {
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 76fe5f085b..82e5b59534 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -3209,6 +3209,46 @@ static const Property scsi_hd_properties[] = {
     DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
 };
 
+#ifdef __linux__
+static bool scsi_disk_pr_state_post_load_errp(void *opaque, int version_id, Error **errp)
+{
+    SCSIDiskState *s = opaque;
+    SCSIDevice *dev = &s->qdev;
+
+    return scsi_generic_pr_state_post_load_errp(dev, errp);
+}
+
+static bool scsi_disk_pr_state_needed(void *opaque)
+{
+    SCSIDiskState *s = opaque;
+    SCSIPRState *pr_state = &s->qdev.pr_state;
+    bool ret;
+
+    if (!s->qdev.migrate_pr) {
+        return false;
+    }
+
+    /* A reservation requires a key, so checking this field is enough */
+    WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+        ret = pr_state->key;
+    }
+    return ret;
+}
+
+static const VMStateDescription vmstate_scsi_disk_pr_state = {
+    .name = "scsi-disk/pr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load_errp = scsi_disk_pr_state_post_load_errp,
+    .needed = scsi_disk_pr_state_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT64(qdev.pr_state.key, SCSIDiskState),
+        VMSTATE_UINT8(qdev.pr_state.resv_type, SCSIDiskState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+#endif /* __linux__ */
+
 static const VMStateDescription vmstate_scsi_disk_state = {
     .name = "scsi-disk",
     .version_id = 1,
@@ -3221,7 +3261,13 @@ static const VMStateDescription vmstate_scsi_disk_state = {
         VMSTATE_BOOL(tray_open, SCSIDiskState),
         VMSTATE_BOOL(tray_locked, SCSIDiskState),
         VMSTATE_END_OF_LIST()
-    }
+    },
+    .subsections = (const VMStateDescription * const []) {
+#ifdef __linux__
+        &vmstate_scsi_disk_pr_state,
+#endif
+        NULL
+    },
 };
 
 static void scsi_hd_class_initfn(ObjectClass *klass, const void *data)
@@ -3301,6 +3347,7 @@ static const Property scsi_block_properties[] = {
                       -1),
     DEFINE_PROP_UINT32("io_timeout", SCSIDiskState, qdev.io_timeout,
                        DEFAULT_IO_TIMEOUT),
+    DEFINE_PROP_BOOL("migrate-pr", SCSIDiskState, qdev.migrate_pr, true),
 };
 
 static void scsi_block_class_initfn(ObjectClass *klass, const void *data)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index f22a38f725..2acfd21232 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -418,6 +418,89 @@ static void scsi_handle_persistent_reserve_out_reply(
     }
 }
 
+static bool scsi_generic_pr_register(SCSIDevice *s, uint64_t key, Error **errp)
+{
+    uint8_t cmd[10] = {};
+    uint8_t buf[24] = {};
+    uint64_t key_be = cpu_to_be64(key);
+    int ret;
+
+    cmd[0] = PERSISTENT_RESERVE_OUT;
+    cmd[1] = PRO_REGISTER;
+    cmd[8] = sizeof(buf);
+    memcpy(&buf[8], &key_be, sizeof(key_be));
+
+    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");
+        return false;
+    }
+    return true;
+}
+
+static bool scsi_generic_pr_preempt(SCSIDevice *s, uint64_t key, uint8_t resv_type, Error **errp)
+{
+    uint8_t cmd[10] = {};
+    uint8_t buf[24] = {};
+    uint64_t key_be = cpu_to_be64(key);
+    int ret;
+
+    cmd[0] = PERSISTENT_RESERVE_OUT;
+    cmd[1] = PRO_PREEMPT;
+    cmd[2] = resv_type & 0xf;
+    cmd[8] = sizeof(buf);
+    memcpy(&buf[0], &key_be, sizeof(key_be));
+    memcpy(&buf[8], &key_be, sizeof(key_be));
+
+    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");
+        return false;
+    }
+    return true;
+}
+
+/* Register keys and preempt reservations after live migration */
+bool scsi_generic_pr_state_post_load_errp(SCSIDevice *s, Error **errp)
+{
+    SCSIPRState *pr_state = &s->pr_state;
+    uint64_t key;
+    uint8_t resv_type;
+
+    WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+        key = pr_state->key;
+        resv_type = pr_state->resv_type;
+    }
+
+    trace_scsi_generic_pr_state_post_load_errp(key, resv_type);
+
+    if (key) {
+        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;
+        }
+    }
+    /* TODO is rollback needed on the source host if migration fails after this point? */
+    return true;
+}
+
 static void scsi_read_complete(void * opaque, int ret)
 {
     SCSIGenericReq *r = (SCSIGenericReq *)opaque;
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index ff92fff7c5..cff8235e9a 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -391,3 +391,4 @@ scsi_generic_aio_sgio_command(uint32_t tag, uint8_t cmd, uint32_t timeout) "gene
 scsi_generic_ioctl_sgio_command(uint8_t cmd, uint32_t timeout) "generic ioctl sgio: cmd=0x%x timeout=%u"
 scsi_generic_ioctl_sgio_done(uint8_t cmd, int ret, uint8_t status, uint8_t host_status) "generic ioctl sgio: cmd=0x%x ret=%d status=0x%x host_status=0x%x"
 scsi_generic_persistent_reserve_out_reply(uint8_t service_action, uint8_t resv_type, uint64_t old_key, uint64_t new_key) "persistent reserve out reply service_action=%u resv_type=%u old_key=0x%" PRIx64 " new_key=0x%" PRIx64
+scsi_generic_pr_state_post_load_errp(uint64_t key, uint8_t resv_type) "key=0x%" PRIx64 " resv_type=%u"
-- 
2.52.0

Re: [RFC 0/4] scsi: persistent reservation live migration

[Peter Krempa]

On Tue, Jan 13, 2026 at 16:53:15 -0500, Stefan Hajnoczi wrote:
> Live migration does not work for SCSI Persistent Reservations acquired on
> scsi-block devices. This patch series migrates the reservation key and
> reservation type so that the destination QEMU can take over the persistent
> reservation with the PREEMPT service action upon live migration.
> 
> The approach involves snooping PERSISTENT RESERVE OUT replies and tracking the
> scsi-block device's current reservation key and reservation type. In most cases
> this involves no additional SCSI commands. This approach isn't perfect: if
> another machine modifies the reservation on the physical LUN, then QEMU's state
> becomes stale. Persistent reservations are inherently cooperative, so this is
> acceptable as long as real applications don't run into problems.
> 
> I am also working on a test suite called pr-tests that runs sg_persist(8)
> commands across multiple machines in order to exercise various scenarios:
> https://gitlab.com/stefanha/pr-tests

I've also prepared libvirt RFC patches adding support for the feature:

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/KBZDAIQWFILAC4USJY3C3TDPYHI6K5WK/

Re: [RFC 0/4] scsi: persistent reservation live migration

[Paolo Bonzini]

On 1/13/26 22:53, Stefan Hajnoczi wrote:
> Live migration does not work for SCSI Persistent Reservations acquired on
> scsi-block devices. This patch series migrates the reservation key and
> reservation type so that the destination QEMU can take over the persistent
> reservation with the PREEMPT service action upon live migration.
> 
> The approach involves snooping PERSISTENT RESERVE OUT replies and tracking the
> scsi-block device's current reservation key and reservation type. In most cases
> this involves no additional SCSI commands. This approach isn't perfect: if
> another machine modifies the reservation on the physical LUN, then QEMU's state
> becomes stale. Persistent reservations are inherently cooperative, so this is
> acceptable as long as real applications don't run into problems.

One issue is that this would not transfer reservations done from a 
previous invocation of the VM.  Are you assuming that the restarted VM 
won't assume to still have the reservation?  I think this is fine, but 
it has to be documented, or maybe QEMU could issue a PR IN command at 
startup?

> I am also working on a test suite called pr-tests that runs sg_persist(8)
> commands across multiple machines in order to exercise various scenarios:
> https://gitlab.com/stefanha/pr-tests

Thank you so much for that!

Paolo

Re: [RFC 0/4] scsi: persistent reservation live migration

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3181 bytes --]

On Thu, Jan 15, 2026 at 06:11:37PM +0100, Paolo Bonzini wrote:
> On 1/13/26 22:53, Stefan Hajnoczi wrote:
> > Live migration does not work for SCSI Persistent Reservations acquired on
> > scsi-block devices. This patch series migrates the reservation key and
> > reservation type so that the destination QEMU can take over the persistent
> > reservation with the PREEMPT service action upon live migration.
> > 
> > The approach involves snooping PERSISTENT RESERVE OUT replies and tracking the
> > scsi-block device's current reservation key and reservation type. In most cases
> > this involves no additional SCSI commands. This approach isn't perfect: if
> > another machine modifies the reservation on the physical LUN, then QEMU's state
> > becomes stale. Persistent reservations are inherently cooperative, so this is
> > acceptable as long as real applications don't run into problems.
> 
> One issue is that this would not transfer reservations done from a previous
> invocation of the VM.  Are you assuming that the restarted VM won't assume
> to still have the reservation?  I think this is fine, but it has to be
> documented, or maybe QEMU could issue a PR IN command at startup?

Good point. The reason for this limitation is that I don't see a
reliable way to detect reservation keys and reservations that belong to
a guest. This is why the patch series uses the snooping approach.

The basic READ KEYS and READ RESERVATION service actions for PERSISTENT
RESERVATION IN only report the list of reservation keys that have been
registered and the key of the current reservation holder. There is no
way of tying that information back to the guest or even the host. It
would be necessary to know the guest's unique reservation key, but that
can be chosen by the guest at runtime. In addition, keys are not
guaranteed to be unique so there is no way to tell whether this host is
actually the reservation holder without potentially destructive probing
(e.g. attempting a command and seeing if it results in a RESERVATION
CONFLICT).

The optional READ FULL STATUS service action improves the situation by
identifying the initiator together with the reservation holder's key,
but it is not universally available. The hardware I'm testing on doesn't
implement this service action.

Therefore the assumption is that the guest configures persistent
reservations and if it is shut down, it reclaims the reservation while
starting up again. It also means that an administrator cannot configure
persistent reservations outside the guest and expect live migration to
move those persistent reservations with the guest.

The clean way to handle persistent reservation migration is using Fibre
Channel N_Port ID Virtualization (NPIV) so that each guest is a distinct
initiator that can migrate to another host without messing with the
persistent reservation state. NPIV requires the storage administrator to
set up the initiators and their zoning - something that few users go
through the trouble of doing.

Having said all this, if someone knows about a better way or I'm wrong
about how this works, please let me know!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]