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    | 81 ++++++++++++++++++++++++++++++++++++++++-
 hw/scsi/scsi-generic.c | 82 ++++++++++++++++++++++++++++++++++++++++++
 hw/scsi/trace-events   |  1 +
 5 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index c5ec58089b..a3e246dbd9 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_preempt(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..8845ab1192 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -28,6 +28,7 @@
 #include "qemu/hw-version.h"
 #include "qemu/memalign.h"
 #include "hw/scsi/scsi.h"
+#include "migration/misc.h"
 #include "migration/qemu-file-types.h"
 #include "migration/vmstate.h"
 #include "hw/scsi/emulation.h"
@@ -122,6 +123,7 @@ struct SCSIDiskState {
      */
     uint16_t rotation_rate;
     bool migrate_emulated_scsi_request;
+    NotifierWithReturn migration_notifier;
 };
 
 static void scsi_free_request(SCSIRequest *req)
@@ -2737,6 +2739,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
 }
 
 #ifdef __linux__
+/*
+ * Preempt on the SCSI Persistent Reservation on the source when migration
+ * fails because the destination may have already preempted and we need to get
+ * the reservation back.
+ */
+static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
+                                         MigrationEvent *e, Error **errp)
+{
+    if (e->type == MIG_EVENT_PRECOPY_FAILED) {
+        SCSIDiskState *s =
+            container_of(notifier, SCSIDiskState, migration_notifier);
+        SCSIDevice *d = &s->qdev;
+
+        /* MIG_EVENT_PRECOPY_FAILED cannot fail, so ignore errors */
+        scsi_generic_pr_state_preempt(d, NULL);
+    }
+    return 0;
+}
+
 static int get_device_type(SCSIDiskState *s)
 {
     uint8_t cmd[16];
@@ -2815,6 +2836,16 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
 
     scsi_realize(&s->qdev, errp);
     scsi_generic_read_device_inquiry(&s->qdev);
+
+    migration_add_notifier(&s->migration_notifier,
+                           scsi_block_migration_notifier);
+}
+
+static void scsi_block_unrealize(SCSIDevice *dev)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+
+    migration_remove_notifier(&s->migration_notifier);
 }
 
 typedef struct SCSIBlockReq {
@@ -3209,6 +3240,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_preempt(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 +3292,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 +3378,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)
@@ -3310,6 +3388,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, const void *data)
     SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
 
     sc->realize      = scsi_block_realize;
+    sc->unrealize    = scsi_block_unrealize;
     sc->alloc_req    = scsi_block_new_request;
     sc->parse_cdb    = scsi_block_parse_cdb;
     sdc->dma_readv   = scsi_block_dma_readv;
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 392647e2b2..e44979faeb 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -419,6 +419,88 @@ 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_preempt(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_preempt(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;
+        }
+    }
+    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..a8ac1e7f1d 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_preempt(uint64_t key, uint8_t resv_type) "key=0x%" PRIx64 " resv_type=%u"
-- 
2.52.0

On Mon, Jan 26, 2026 at 02:47:34PM -0500, Stefan Hajnoczi wrote:
> 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    | 81 ++++++++++++++++++++++++++++++++++++++++-
>  hw/scsi/scsi-generic.c | 82 ++++++++++++++++++++++++++++++++++++++++++
>  hw/scsi/trace-events   |  1 +
>  5 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index c5ec58089b..a3e246dbd9 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_preempt(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..8845ab1192 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -28,6 +28,7 @@
>  #include "qemu/hw-version.h"
>  #include "qemu/memalign.h"
>  #include "hw/scsi/scsi.h"
> +#include "migration/misc.h"
>  #include "migration/qemu-file-types.h"
>  #include "migration/vmstate.h"
>  #include "hw/scsi/emulation.h"
> @@ -122,6 +123,7 @@ struct SCSIDiskState {
>       */
>      uint16_t rotation_rate;
>      bool migrate_emulated_scsi_request;
> +    NotifierWithReturn migration_notifier;
>  };
>  
>  static void scsi_free_request(SCSIRequest *req)
> @@ -2737,6 +2739,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
>  }
>  
>  #ifdef __linux__
> +/*
> + * Preempt on the SCSI Persistent Reservation on the source when migration
> + * fails because the destination may have already preempted and we need to get
> + * the reservation back.
> + */
> +static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
> +                                         MigrationEvent *e, Error **errp)
> +{
> +    if (e->type == MIG_EVENT_PRECOPY_FAILED) {
> +        SCSIDiskState *s =
> +            container_of(notifier, SCSIDiskState, migration_notifier);
> +        SCSIDevice *d = &s->qdev;
> +
> +        /* MIG_EVENT_PRECOPY_FAILED cannot fail, so ignore errors */
> +        scsi_generic_pr_state_preempt(d, NULL);

I feel like we ought to 'warn_report'  any errors related to failing
to acquire persistent reservations.

In the unlikely event an error occurs, whomever has to deal with
the resulting support ticket will want to know something went wrong
from the QEMU logs.


> @@ -3209,6 +3240,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_preempt(dev, errp);
> +}

What if there are multiple SCSI disks, and on the target host
we successful acquire the reservation for the 1st, but fail
the second ? Are we ignoring the failure of the second, or
are we discarding the reservation of the 1st and failing
migration ?   Should we warn_report here too ?

> +
> +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 +3292,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 +3378,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)
> @@ -3310,6 +3388,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, const void *data)
>      SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
>  
>      sc->realize      = scsi_block_realize;
> +    sc->unrealize    = scsi_block_unrealize;
>      sc->alloc_req    = scsi_block_new_request;
>      sc->parse_cdb    = scsi_block_parse_cdb;
>      sdc->dma_readv   = scsi_block_dma_readv;
> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index 392647e2b2..e44979faeb 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -419,6 +419,88 @@ 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_preempt(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_preempt(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;
> +        }
> +    }
> +    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..a8ac1e7f1d 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_preempt(uint64_t key, uint8_t resv_type) "key=0x%" PRIx64 " resv_type=%u"
> -- 
> 2.52.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Mon, Jan 26, 2026 at 07:59:55PM +0000, Daniel P. Berrangé wrote:
> On Mon, Jan 26, 2026 at 02:47:34PM -0500, Stefan Hajnoczi wrote:
> > 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    | 81 ++++++++++++++++++++++++++++++++++++++++-
> >  hw/scsi/scsi-generic.c | 82 ++++++++++++++++++++++++++++++++++++++++++
> >  hw/scsi/trace-events   |  1 +
> >  5 files changed, 167 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> > index c5ec58089b..a3e246dbd9 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_preempt(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..8845ab1192 100644
> > --- a/hw/scsi/scsi-disk.c
> > +++ b/hw/scsi/scsi-disk.c
> > @@ -28,6 +28,7 @@
> >  #include "qemu/hw-version.h"
> >  #include "qemu/memalign.h"
> >  #include "hw/scsi/scsi.h"
> > +#include "migration/misc.h"
> >  #include "migration/qemu-file-types.h"
> >  #include "migration/vmstate.h"
> >  #include "hw/scsi/emulation.h"
> > @@ -122,6 +123,7 @@ struct SCSIDiskState {
> >       */
> >      uint16_t rotation_rate;
> >      bool migrate_emulated_scsi_request;
> > +    NotifierWithReturn migration_notifier;
> >  };
> >  
> >  static void scsi_free_request(SCSIRequest *req)
> > @@ -2737,6 +2739,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> >  }
> >  
> >  #ifdef __linux__
> > +/*
> > + * Preempt on the SCSI Persistent Reservation on the source when migration
> > + * fails because the destination may have already preempted and we need to get
> > + * the reservation back.
> > + */
> > +static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
> > +                                         MigrationEvent *e, Error **errp)
> > +{
> > +    if (e->type == MIG_EVENT_PRECOPY_FAILED) {
> > +        SCSIDiskState *s =
> > +            container_of(notifier, SCSIDiskState, migration_notifier);
> > +        SCSIDevice *d = &s->qdev;
> > +
> > +        /* MIG_EVENT_PRECOPY_FAILED cannot fail, so ignore errors */
> > +        scsi_generic_pr_state_preempt(d, NULL);
> 
> I feel like we ought to 'warn_report'  any errors related to failing
> to acquire persistent reservations.
> 
> In the unlikely event an error occurs, whomever has to deal with
> the resulting support ticket will want to know something went wrong
> from the QEMU logs.

Good idea.

I'm also not sure how to best approach logging in general. Usually QEMU
does little logging when the VM is running, but it has become
increasingly difficult to get information out of QEMU via tracing or
monitor commands since nowadays QEMU might be running in a locked down
container. Debugging PR migration issues would be easiest if the trace
events introduced in this series were actually printfs to stderr, but
that's not traditionally how QEMU did things.

> 
> 
> > @@ -3209,6 +3240,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_preempt(dev, errp);
> > +}
> 
> What if there are multiple SCSI disks, and on the target host
> we successful acquire the reservation for the 1st, but fail
> the second ? Are we ignoring the failure of the second, or
> are we discarding the reservation of the 1st and failing
> migration ?   Should we warn_report here too ?

Each disk is its own scsi-block device.
scsi_disk_pr_state_post_load_errp() will be invoked for each such
device. If one of these calls fails, the migration will abort and the
errp here will reported.

The rollback on the source host doesn't care which devices
succeeded/failed, it performs idempotent PREEMPT operations for all
devices so we're sure all reservations are back on the source host after
a failed migration.

> 
> > +
> > +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 +3292,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 +3378,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)
> > @@ -3310,6 +3388,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, const void *data)
> >      SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
> >  
> >      sc->realize      = scsi_block_realize;
> > +    sc->unrealize    = scsi_block_unrealize;
> >      sc->alloc_req    = scsi_block_new_request;
> >      sc->parse_cdb    = scsi_block_parse_cdb;
> >      sdc->dma_readv   = scsi_block_dma_readv;
> > diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> > index 392647e2b2..e44979faeb 100644
> > --- a/hw/scsi/scsi-generic.c
> > +++ b/hw/scsi/scsi-generic.c
> > @@ -419,6 +419,88 @@ 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_preempt(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_preempt(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;
> > +        }
> > +    }
> > +    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..a8ac1e7f1d 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_preempt(uint64_t key, uint8_t resv_type) "key=0x%" PRIx64 " resv_type=%u"
> > -- 
> > 2.52.0
> > 
> > 
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> 

On Mon, Jan 26, 2026 at 05:13:43PM -0500, Stefan Hajnoczi wrote:
> On Mon, Jan 26, 2026 at 07:59:55PM +0000, Daniel P. Berrangé wrote:
> > On Mon, Jan 26, 2026 at 02:47:34PM -0500, Stefan Hajnoczi wrote:
> > > 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    | 81 ++++++++++++++++++++++++++++++++++++++++-
> > >  hw/scsi/scsi-generic.c | 82 ++++++++++++++++++++++++++++++++++++++++++
> > >  hw/scsi/trace-events   |  1 +
> > >  5 files changed, 167 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> > > index c5ec58089b..a3e246dbd9 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_preempt(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..8845ab1192 100644
> > > --- a/hw/scsi/scsi-disk.c
> > > +++ b/hw/scsi/scsi-disk.c
> > > @@ -28,6 +28,7 @@
> > >  #include "qemu/hw-version.h"
> > >  #include "qemu/memalign.h"
> > >  #include "hw/scsi/scsi.h"
> > > +#include "migration/misc.h"
> > >  #include "migration/qemu-file-types.h"
> > >  #include "migration/vmstate.h"
> > >  #include "hw/scsi/emulation.h"
> > > @@ -122,6 +123,7 @@ struct SCSIDiskState {
> > >       */
> > >      uint16_t rotation_rate;
> > >      bool migrate_emulated_scsi_request;
> > > +    NotifierWithReturn migration_notifier;
> > >  };
> > >  
> > >  static void scsi_free_request(SCSIRequest *req)
> > > @@ -2737,6 +2739,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> > >  }
> > >  
> > >  #ifdef __linux__
> > > +/*
> > > + * Preempt on the SCSI Persistent Reservation on the source when migration
> > > + * fails because the destination may have already preempted and we need to get
> > > + * the reservation back.
> > > + */
> > > +static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
> > > +                                         MigrationEvent *e, Error **errp)
> > > +{
> > > +    if (e->type == MIG_EVENT_PRECOPY_FAILED) {
> > > +        SCSIDiskState *s =
> > > +            container_of(notifier, SCSIDiskState, migration_notifier);
> > > +        SCSIDevice *d = &s->qdev;
> > > +
> > > +        /* MIG_EVENT_PRECOPY_FAILED cannot fail, so ignore errors */
> > > +        scsi_generic_pr_state_preempt(d, NULL);
> > 
> > I feel like we ought to 'warn_report'  any errors related to failing
> > to acquire persistent reservations.
> > 
> > In the unlikely event an error occurs, whomever has to deal with
> > the resulting support ticket will want to know something went wrong
> > from the QEMU logs.
> 
> Good idea.
> 
> I'm also not sure how to best approach logging in general. Usually QEMU
> does little logging when the VM is running, but it has become
> increasingly difficult to get information out of QEMU via tracing or
> monitor commands since nowadays QEMU might be running in a locked down
> container. Debugging PR migration issues would be easiest if the trace
> events introduced in this series were actually printfs to stderr, but
> that's not traditionally how QEMU did things.

I wouldn't overthink this - just pass a locall "Error *err" object
instead of NULL, and then warn_report_err on the result. Just needs
to be a marker in the that something has gone wrong, which we could
not propagate to the mgmt app in the normal manner since we have no
error path here.

Debugging /anything/ in QEMU is easiest if the trace events were
actually prints, but that thinking leads to us enabling tracing
all the time, everywhere which is impractical

At the same time it is common for apps to have some level of
"always on" debugging collected and so we perhaps do have a
general conceptual gap in QEMU.

If we thinking of trace events as being equiv to "DEBUG" level
log events, would it help if we could annotate a subset of
trace events as "INFO" level and do something with them by
default. eg perhaps we have an ring buffer tracing target that
collects "INFO" events continuously and can be asked to dump
out its state in error scenarios ?

> > > @@ -3209,6 +3240,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_preempt(dev, errp);
> > > +}
> > 
> > What if there are multiple SCSI disks, and on the target host
> > we successful acquire the reservation for the 1st, but fail
> > the second ? Are we ignoring the failure of the second, or
> > are we discarding the reservation of the 1st and failing
> > migration ?   Should we warn_report here too ?
> 
> Each disk is its own scsi-block device.
> scsi_disk_pr_state_post_load_errp() will be invoked for each such
> device. If one of these calls fails, the migration will abort and the
> errp here will reported.

Oh right, the errp is fine as that will go up to libvirt and thus
be recorded.

> The rollback on the source host doesn't care which devices
> succeeded/failed, it performs idempotent PREEMPT operations for all
> devices so we're sure all reservations are back on the source host after
> a failed migration.

Ok

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Tue, Jan 27, 2026 at 08:54:24AM +0000, Daniel P. Berrangé wrote:
> On Mon, Jan 26, 2026 at 05:13:43PM -0500, Stefan Hajnoczi wrote:
> > On Mon, Jan 26, 2026 at 07:59:55PM +0000, Daniel P. Berrangé wrote:
> > > On Mon, Jan 26, 2026 at 02:47:34PM -0500, Stefan Hajnoczi wrote:
> > > > 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    | 81 ++++++++++++++++++++++++++++++++++++++++-
> > > >  hw/scsi/scsi-generic.c | 82 ++++++++++++++++++++++++++++++++++++++++++
> > > >  hw/scsi/trace-events   |  1 +
> > > >  5 files changed, 167 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> > > > index c5ec58089b..a3e246dbd9 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_preempt(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..8845ab1192 100644
> > > > --- a/hw/scsi/scsi-disk.c
> > > > +++ b/hw/scsi/scsi-disk.c
> > > > @@ -28,6 +28,7 @@
> > > >  #include "qemu/hw-version.h"
> > > >  #include "qemu/memalign.h"
> > > >  #include "hw/scsi/scsi.h"
> > > > +#include "migration/misc.h"
> > > >  #include "migration/qemu-file-types.h"
> > > >  #include "migration/vmstate.h"
> > > >  #include "hw/scsi/emulation.h"
> > > > @@ -122,6 +123,7 @@ struct SCSIDiskState {
> > > >       */
> > > >      uint16_t rotation_rate;
> > > >      bool migrate_emulated_scsi_request;
> > > > +    NotifierWithReturn migration_notifier;
> > > >  };
> > > >  
> > > >  static void scsi_free_request(SCSIRequest *req)
> > > > @@ -2737,6 +2739,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> > > >  }
> > > >  
> > > >  #ifdef __linux__
> > > > +/*
> > > > + * Preempt on the SCSI Persistent Reservation on the source when migration
> > > > + * fails because the destination may have already preempted and we need to get
> > > > + * the reservation back.
> > > > + */
> > > > +static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
> > > > +                                         MigrationEvent *e, Error **errp)
> > > > +{
> > > > +    if (e->type == MIG_EVENT_PRECOPY_FAILED) {
> > > > +        SCSIDiskState *s =
> > > > +            container_of(notifier, SCSIDiskState, migration_notifier);
> > > > +        SCSIDevice *d = &s->qdev;
> > > > +
> > > > +        /* MIG_EVENT_PRECOPY_FAILED cannot fail, so ignore errors */
> > > > +        scsi_generic_pr_state_preempt(d, NULL);
> > > 
> > > I feel like we ought to 'warn_report'  any errors related to failing
> > > to acquire persistent reservations.
> > > 
> > > In the unlikely event an error occurs, whomever has to deal with
> > > the resulting support ticket will want to know something went wrong
> > > from the QEMU logs.
> > 
> > Good idea.
> > 
> > I'm also not sure how to best approach logging in general. Usually QEMU
> > does little logging when the VM is running, but it has become
> > increasingly difficult to get information out of QEMU via tracing or
> > monitor commands since nowadays QEMU might be running in a locked down
> > container. Debugging PR migration issues would be easiest if the trace
> > events introduced in this series were actually printfs to stderr, but
> > that's not traditionally how QEMU did things.
> 
> I wouldn't overthink this - just pass a locall "Error *err" object
> instead of NULL, and then warn_report_err on the result. Just needs
> to be a marker in the that something has gone wrong, which we could
> not propagate to the mgmt app in the normal manner since we have no
> error path here.

Yes, sounds good.

> 
> Debugging /anything/ in QEMU is easiest if the trace events were
> actually prints, but that thinking leads to us enabling tracing
> all the time, everywhere which is impractical
> 
> At the same time it is common for apps to have some level of
> "always on" debugging collected and so we perhaps do have a
> general conceptual gap in QEMU.
> 
> If we thinking of trace events as being equiv to "DEBUG" level
> log events, would it help if we could annotate a subset of
> trace events as "INFO" level and do something with them by
> default. eg perhaps we have an ring buffer tracing target that
> collects "INFO" events continuously and can be asked to dump
> out its state in error scenarios ?

I need to research SystemTap inside container scenarios more first.
Ideally we could rely on the DTrace probes, but it needs to be easy for
users to collect information from their containers. I suspect there will
be permission problems as well as multiple steps required to just find
the container where QEMU is running, which makes this hard for users.

Stefan

On Tue, Jan 27, 2026 at 09:41:59AM -0500, Stefan Hajnoczi wrote:
> On Tue, Jan 27, 2026 at 08:54:24AM +0000, Daniel P. Berrangé wrote:
> > On Mon, Jan 26, 2026 at 05:13:43PM -0500, Stefan Hajnoczi wrote:
> > > On Mon, Jan 26, 2026 at 07:59:55PM +0000, Daniel P. Berrangé wrote:
> > > > On Mon, Jan 26, 2026 at 02:47:34PM -0500, Stefan Hajnoczi wrote:
> > > > > 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    | 81 ++++++++++++++++++++++++++++++++++++++++-
> > > > >  hw/scsi/scsi-generic.c | 82 ++++++++++++++++++++++++++++++++++++++++++
> > > > >  hw/scsi/trace-events   |  1 +
> > > > >  5 files changed, 167 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> > > > > index c5ec58089b..a3e246dbd9 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_preempt(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..8845ab1192 100644
> > > > > --- a/hw/scsi/scsi-disk.c
> > > > > +++ b/hw/scsi/scsi-disk.c
> > > > > @@ -28,6 +28,7 @@
> > > > >  #include "qemu/hw-version.h"
> > > > >  #include "qemu/memalign.h"
> > > > >  #include "hw/scsi/scsi.h"
> > > > > +#include "migration/misc.h"
> > > > >  #include "migration/qemu-file-types.h"
> > > > >  #include "migration/vmstate.h"
> > > > >  #include "hw/scsi/emulation.h"
> > > > > @@ -122,6 +123,7 @@ struct SCSIDiskState {
> > > > >       */
> > > > >      uint16_t rotation_rate;
> > > > >      bool migrate_emulated_scsi_request;
> > > > > +    NotifierWithReturn migration_notifier;
> > > > >  };
> > > > >  
> > > > >  static void scsi_free_request(SCSIRequest *req)
> > > > > @@ -2737,6 +2739,25 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> > > > >  }
> > > > >  
> > > > >  #ifdef __linux__
> > > > > +/*
> > > > > + * Preempt on the SCSI Persistent Reservation on the source when migration
> > > > > + * fails because the destination may have already preempted and we need to get
> > > > > + * the reservation back.
> > > > > + */
> > > > > +static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
> > > > > +                                         MigrationEvent *e, Error **errp)
> > > > > +{
> > > > > +    if (e->type == MIG_EVENT_PRECOPY_FAILED) {
> > > > > +        SCSIDiskState *s =
> > > > > +            container_of(notifier, SCSIDiskState, migration_notifier);
> > > > > +        SCSIDevice *d = &s->qdev;
> > > > > +
> > > > > +        /* MIG_EVENT_PRECOPY_FAILED cannot fail, so ignore errors */
> > > > > +        scsi_generic_pr_state_preempt(d, NULL);
> > > > 
> > > > I feel like we ought to 'warn_report'  any errors related to failing
> > > > to acquire persistent reservations.
> > > > 
> > > > In the unlikely event an error occurs, whomever has to deal with
> > > > the resulting support ticket will want to know something went wrong
> > > > from the QEMU logs.
> > > 
> > > Good idea.
> > > 
> > > I'm also not sure how to best approach logging in general. Usually QEMU
> > > does little logging when the VM is running, but it has become
> > > increasingly difficult to get information out of QEMU via tracing or
> > > monitor commands since nowadays QEMU might be running in a locked down
> > > container. Debugging PR migration issues would be easiest if the trace
> > > events introduced in this series were actually printfs to stderr, but
> > > that's not traditionally how QEMU did things.
> > 
> > I wouldn't overthink this - just pass a locall "Error *err" object
> > instead of NULL, and then warn_report_err on the result. Just needs
> > to be a marker in the that something has gone wrong, which we could
> > not propagate to the mgmt app in the normal manner since we have no
> > error path here.
> 
> Yes, sounds good.
> 
> > 
> > Debugging /anything/ in QEMU is easiest if the trace events were
> > actually prints, but that thinking leads to us enabling tracing
> > all the time, everywhere which is impractical
> > 
> > At the same time it is common for apps to have some level of
> > "always on" debugging collected and so we perhaps do have a
> > general conceptual gap in QEMU.
> > 
> > If we thinking of trace events as being equiv to "DEBUG" level
> > log events, would it help if we could annotate a subset of
> > trace events as "INFO" level and do something with them by
> > default. eg perhaps we have an ring buffer tracing target that
> > collects "INFO" events continuously and can be asked to dump
> > out its state in error scenarios ?
> 
> I need to research SystemTap inside container scenarios more first.
> Ideally we could rely on the DTrace probes, but it needs to be easy for
> users to collect information from their containers. I suspect there will
> be permission problems as well as multiple steps required to just find
> the container where QEMU is running, which makes this hard for users.

Agreed, I'd expect to use systemtap with a containerized QEMU would
generally involve an account outside the container on the host OS.
While it might be possible to open up the kernel / seccomp perms
in just the right way to get things working inside the container,
that's unlikely to be permitted in any public cloud solution, and
probably also blocked in many other places. I think this pushes
towards the need for a more traditional "logging" like facility.

We do have the "log" backend already of course but not neccessarily
enabled by distros, but that's something they need to consider.

An always-on ring buffer would be more like a "flight recorder"
scenario to reduce the overheads compared to actually logging to
disk / external service until a failure scenario arrives.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|