[PATCH 0/8] scsi: clean up persistent reservation state after migration

[PATCH 0/8] scsi: clean up persistent reservation state after migration

[Stefan Hajnoczi]

When a guest using SCSI persistent reservations is migrated on a Linux host
using multipathd, some state is left behind and this can cause multipathd to
register the old key again in the future. One way this can be observed is when
migrating a guest back-and-forth between two hosts: the REGISTER command will
fail when migrating back to the first host if the LUN is detached and
re-attached between migrations (i.e. multipathd sees that a path has come up).

This patch series modifies qemu-pr-helper to send multipathd commands that
clear the state once migration has completed successfully. This is implemented
as a vendor-specific SCSI command called PR_HELPER_CLEANUP that QEMU sends to
qemu-pr-helper just like PERSISTENT RESERVE IN and PERSISTENT RESERVE OUT
commands.

I considered adding a separate interface or more complex qemu-pr-helper
protocol extension that does not reuse SCSI commands because that would be
cleaner. Since the command does not have security implications (it's a variant
of REGISTER AND IGNORE EXISTING KEYS), I went with this approach for
simplicity. Let me know if you prefer a different interface and what that it
should look like.

Buglink: https://redhat.atlassian.net/browse/RHEL-274966

Stefan Hajnoczi (8):
  pr-manager-helper: fix sense buffer overflow
  scsi: increase sense buffer size in scsi_SG_IO()
  hw/scsi-disk: move MIG_EVENT_FAILED into a new function
  qemu-pr-helper: add PR_HELPER_CLEANUP for live migration
  pr-manager-helper: add PR_HELPER_CLEANUP command support
  file-posix: forward PR_HELPER_CLEANUP commands to pr-manager
  scsi: clean up persistent reservation state after migration
  scsi: use REGISTER AND IGNORE EXISTING KEY for robustness

 meson.build              |   2 +
 include/hw/scsi/scsi.h   |   1 +
 include/scsi/constants.h |   1 +
 scsi/pr-helper.h         |   3 +
 block/file-posix.c       |   3 +-
 hw/scsi/scsi-disk.c      |  45 +++++++++---
 hw/scsi/scsi-generic.c   |  64 +++++++++++++----
 scsi/pr-manager-helper.c |  23 ++++--
 scsi/qemu-pr-helper.c    | 146 ++++++++++++++++++++++++++++++++++++---
 scsi/utils.c             |   3 +
 10 files changed, 251 insertions(+), 40 deletions(-)

-- 
2.56.0

[PATCH 1/8] pr-manager-helper: fix sense buffer overflow

[Stefan Hajnoczi]

scsi_build_sense() assumes the sense buffer is SCSI_SENSE_LEN (18) bytes
long, so the caller's buffer with an arbitrary size cannot be used. Use
our own sense buffer (resp.sense) as an intermediate buffer and honor
the caller's sense buffer size (io_hdr->mx_sb_len).

This is triggered when scsi-generic.c:scsi_SG_IO() calls blk_ioctl()
with an 8-byte sense buffer. There does not appear to be a way for the
guest to trigger this buffer overflow directly since
SCSIRequest->sense[] is 252 bytes long.

Fixes: 9bad2a6b9d0a ("scsi: add persistent reservation manager using qemu-pr-helper")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scsi/pr-manager-helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c
index 53432e6d606..26ef3876f0e 100644
--- a/scsi/pr-manager-helper.c
+++ b/scsi/pr-manager-helper.c
@@ -234,10 +234,11 @@ static int pr_manager_helper_run(PRManager *p,
 
 out:
     if (ret < 0) {
-        int sense_len = scsi_build_sense(io_hdr->sbp,
+        int sense_len = scsi_build_sense(resp.sense,
                                          SENSE_CODE(LUN_COMM_FAILURE));
         io_hdr->driver_status = SG_ERR_DRIVER_SENSE;
         io_hdr->sb_len_wr = MIN(io_hdr->mx_sb_len, sense_len);
+        memcpy(io_hdr->sbp, resp.sense, io_hdr->sb_len_wr);
         io_hdr->status = CHECK_CONDITION;
     }
     qemu_mutex_unlock(&pr_mgr->lock);
-- 
2.56.0

[PATCH 2/8] scsi: increase sense buffer size in scsi_SG_IO()

[Stefan Hajnoczi]

There are different SCSI sense formats. Increase the size so we can get
more detailed sense data.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/scsi-generic.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 8999f3b7200..153a5275567 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -923,7 +923,7 @@ int scsi_SG_IO(BlockBackend *blk, int direction, uint8_t *cmd,
                uint32_t timeout, Error **errp)
 {
     sg_io_hdr_t io_header;
-    uint8_t sensebuf[8] = {};
+    uint8_t sensebuf[SCSI_SENSE_LEN] = {};
     int ret;
 
     memset(&io_header, 0, sizeof(io_header));
@@ -947,15 +947,18 @@ int scsi_SG_IO(BlockBackend *blk, int direction, uint8_t *cmd,
             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]);
+                g_strdup_printf("%02x%02x%02x%02x%02x%02x%02x%02x"
+                                "%02x%02x%02x%02x%02x%02x%02x%02x"
+                                "%02x%02x",
+                                sensebuf[0], sensebuf[1],
+                                sensebuf[2], sensebuf[3],
+                                sensebuf[4], sensebuf[5],
+                                sensebuf[6], sensebuf[7],
+                                sensebuf[8], sensebuf[9],
+                                sensebuf[10], sensebuf[11],
+                                sensebuf[12], sensebuf[13],
+                                sensebuf[14], sensebuf[15],
+                                sensebuf[16], sensebuf[17]);
 
             error_setg(errp, "SG_IO SCSI command failed with status=0x%x "
                     "driver_status=0x%x host_status=0x%x sensebuf=%s "
-- 
2.56.0

[PATCH 3/8] hw/scsi-disk: move MIG_EVENT_FAILED into a new function

[Stefan Hajnoczi]

Move the MIG_EVENT_FAILED logic from scsi_block_migration_notifier()
into its own new scsi_block_migration_failed() function. The next commit
adds logic for handling another event, so doing that in dedicated
functions makes the code easier to read.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/scsi-disk.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index deb33ac1dda..3d88e57eded 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2761,20 +2761,26 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
  * fails because the destination may have already preempted and we need to get
  * the reservation back.
  */
+static void scsi_block_migration_failed(SCSIDiskState *s)
+{
+    SCSIDevice *d = &s->qdev;
+    Error *local_err = NULL;
+
+    if (!scsi_generic_pr_state_preempt(d, &local_err)) {
+        /* MIG_EVENT_FAILED cannot fail, so just warn */
+        error_prepend(&local_err, "scsi-block migration rollback: ");
+        warn_report_err(local_err);
+    }
+}
+
 static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
                                          MigrationEvent *e, Error **errp)
 {
-    if (e->type == MIG_EVENT_FAILED) {
-        SCSIDiskState *s =
-            container_of(notifier, SCSIDiskState, migration_notifier);
-        SCSIDevice *d = &s->qdev;
-        Error *local_err = NULL;
+    SCSIDiskState *s =
+        container_of(notifier, SCSIDiskState, migration_notifier);
 
-        if (!scsi_generic_pr_state_preempt(d, &local_err)) {
-            /* MIG_EVENT_FAILED cannot fail, so just warn */
-            error_prepend(&local_err, "scsi-block migration rollback: ");
-            warn_report_err(local_err);
-        }
+    if (e->type == MIG_EVENT_FAILED) {
+        scsi_block_migration_failed(s);
     }
     return 0;
 }
-- 
2.56.0

[PATCH 4/8] qemu-pr-helper: add PR_HELPER_CLEANUP for live migration

[Stefan Hajnoczi]

Introduce a new vendor-specific SCSI command that acts similar to
REGISTER AND IGNORE EXISTING KEY but also discards multipathd's state
(including /etc/multipath/prkeys). This command will be used on the
source after live migration completes successfully.

This cleanup step is necessary because multipathd will register the key
it has stored when the path comes up again. This is undesirable when the
guest has migrated to another host and can interfere with the migration
process or the application running inside the guest.

Add a qemu-pr-manager protocol feature bit indicating that the new
PR_HELPER_CLEANUP command is available. The command has no CDB fields
other than OPCODE and the response contains no data aside from the usual
result and sense data.

Note that the linker needs an explicit libmpathcmd dependency for
mpath_disconnect() even though qemu-pr-helper.c already #included the
mpath_cmd.h header before.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 meson.build              |   2 +
 include/scsi/constants.h |   1 +
 scsi/pr-helper.h         |   3 +
 scsi/qemu-pr-helper.c    | 146 ++++++++++++++++++++++++++++++++++++---
 4 files changed, 143 insertions(+), 9 deletions(-)

diff --git a/meson.build b/meson.build
index 828100de816..34038eb8e52 100644
--- a/meson.build
+++ b/meson.build
@@ -1486,6 +1486,8 @@ if host_os == 'linux' and have_tools and get_option('mpath').allowed()
     endif
     mpathlibs += cc.find_library('multipath',
                                  required: get_option('mpath'))
+    mpathlibs += cc.find_library('mpathcmd',
+                                 required: get_option('mpath'))
     foreach lib: mpathlibs
       if not lib.found()
         mpathlibs = []
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 717e470a5d4..bb3981a779b 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -149,6 +149,7 @@
 #define MECHANISM_STATUS      0xbd
 #define READ_CD               0xbe
 #define SEND_DVD_STRUCTURE    0xbf
+#define PR_HELPER_CLEANUP     0xff /* vendor-specific QEMU command */
 
 /*
  * SERVICE ACTION IN subcodes
diff --git a/scsi/pr-helper.h b/scsi/pr-helper.h
index e26e104ec76..d088d0dc193 100644
--- a/scsi/pr-helper.h
+++ b/scsi/pr-helper.h
@@ -31,6 +31,9 @@
 #define PR_HELPER_SENSE_SIZE   96
 #define PR_HELPER_DATA_SIZE    8192
 
+/* Protocol features */
+#define PR_HELPER_FEATURE_CLEANUP (1u << 0)
+
 typedef struct PRHelperResponse {
     int32_t result;
     int32_t sz;
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 9dcd3ec7111..fdbb4748dc0 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -537,6 +537,110 @@ static int coroutine_fn multipath_pr_out(int fd, const uint8_t *cdb, uint8_t *se
                                      paramp, noisy, verbose);
     return mpath_reconstruct_sense(fd, r, sense);
 }
+
+/* Get the device-mapper device name given its block device fd */
+static char *get_dm_name(int fd, Error **errp)
+{
+    struct stat st;
+    int ret;
+
+    assert(is_mpath(fd));
+
+    ret = fstat(fd, &st);
+    if (ret != 0) {
+        error_setg_errno(errp, errno, "fstat failed on fd %d", fd);
+        return NULL;
+    }
+
+    return g_strdup_printf("dm-%u", minor(st.st_rdev));
+}
+
+/* Perform a multipathd command */
+static bool multipathd_cmd(int fd, const char *cmd, Error **errp)
+{
+    const unsigned int timeout_ms = 60000;
+    char *reply = NULL;
+    int last_errno;
+    int ret;
+
+    ret = mpath_process_cmd(fd, cmd, &reply, timeout_ms);
+    last_errno = errno;
+    if (reply && strcmp(reply, "ok\n") != 0) {
+        error_setg(errp, "mpath_process_cmd(%d, \"%s\") replied \"%s\"",
+                   fd, cmd, reply);
+        free(reply);
+        return false;
+    }
+    free(reply);
+    if (ret == -1) {
+        error_setg_errno(errp, last_errno,
+                         "mpath_process_cmd(%d, \"%s\") failed",
+                         fd, cmd);
+        return false;
+    }
+    return true;
+}
+
+/* Unregister so this I_T nexus has no key registered anymore */
+static int coroutine_fn multipath_pr_cleanup_unregister(int fd, uint8_t *sense,
+                                                        Error **errp)
+{
+    uint8_t cdb[PR_HELPER_CDB_SIZE] = {};
+    uint8_t param[PR_OUT_FIXED_PARAM_SIZE] = {};
+
+    /* Fill in CDB */
+    cdb[0] = PERSISTENT_RESERVE_OUT;               /* OPERATION CODE */
+    cdb[1] = PRO_REGISTER_AND_IGNORE_EXISTING_KEY; /* SERVICE ACTION */
+    stl_be_p(&cdb[5], sizeof(param));              /* PARAMETER LIST LENGTH */
+
+    /* The parameter list is all zeroes, so there is nothing to fill in */
+
+    return multipath_pr_out(fd, cdb, sense, param, sizeof(param));
+}
+
+/*
+ * A special version of REGISTER AND IGNORE EXISTING KEY with sark=0 to
+ * unregister the key without triggering multipathd's preemption. This succeeds
+ * when no key is registered too. This is essential so the migration source
+ * cleans up its registered key without multipathd preempting the destination
+ * host.
+ */
+static int coroutine_fn multipath_pr_cleanup(int bdev_fd, uint8_t *sense,
+                                             Error **errp)
+{
+    g_autofree char *bdev_dm_name = get_dm_name(bdev_fd, errp);
+    g_autofree char *cmd = NULL;
+    int multipathd_fd;
+
+    if (bdev_dm_name == NULL) {
+        return -1;
+    }
+
+    /* First make multipathd the key and reservation state */
+    multipathd_fd = mpath_connect();
+    if (multipathd_fd == -1) {
+        error_setg_errno(errp, errno, "mpath_connect() failed");
+        return -1;
+    }
+
+    cmd = g_strdup_printf("unsetprkey map %s", bdev_dm_name);
+    if (!multipathd_cmd(multipathd_fd, cmd, errp)) {
+        mpath_disconnect(multipathd_fd);
+        return -1;
+    }
+
+    g_free(cmd);
+    cmd = g_strdup_printf("unsetprstatus map %s", bdev_dm_name);
+    if (!multipathd_cmd(multipathd_fd, cmd, errp)) {
+        mpath_disconnect(multipathd_fd);
+        return -1;
+    }
+
+    mpath_disconnect(multipathd_fd);
+
+    /* Now unregister the key at the SCSI level */
+    return multipath_pr_cleanup_unregister(bdev_fd, sense, errp);
+}
 #endif
 
 static int coroutine_fn do_pr_in(int fd, const uint8_t *cdb, uint8_t *sense,
@@ -578,6 +682,21 @@ static int coroutine_fn do_pr_out(int fd, const uint8_t *cdb, uint8_t *sense,
                    SG_DXFER_TO_DEV);
 }
 
+static int coroutine_fn do_pr_cleanup(int fd, uint8_t *sense, Error **errp)
+{
+    if ((fcntl(fd, F_GETFL) & O_ACCMODE) == O_RDONLY) {
+        scsi_build_sense(sense, SENSE_CODE(INVALID_OPCODE));
+        return CHECK_CONDITION;
+    }
+
+#ifdef CONFIG_MPATH
+    if (is_mpath(fd)) {
+        return multipath_pr_cleanup(fd, sense, errp);
+    }
+#endif
+    return GOOD;
+}
+
 /* Client */
 
 typedef struct PRHelperClient {
@@ -667,7 +786,8 @@ static int coroutine_fn prh_read_request(PRHelperClient *client,
     }
 
     if (req->cdb[0] != PERSISTENT_RESERVE_OUT &&
-        req->cdb[0] != PERSISTENT_RESERVE_IN) {
+        req->cdb[0] != PERSISTENT_RESERVE_IN &&
+        req->cdb[0] != PR_HELPER_CLEANUP) {
         error_setg(errp, "Invalid CDB, closing socket.");
         goto out_close;
     }
@@ -703,7 +823,8 @@ static int coroutine_fn prh_write_response(PRHelperClient *client,
     ssize_t r;
     size_t sz;
 
-    if (req->cdb[0] == PERSISTENT_RESERVE_IN && resp->result == GOOD) {
+    if (req->cdb[0] == PERSISTENT_RESERVE_IN &&
+        resp->result == GOOD) {
         assert(resp->sz <= req->sz && resp->sz <= sizeof(client->data));
     } else {
         assert(resp->sz == 0);
@@ -739,10 +860,8 @@ static void coroutine_fn prh_co_entry(void *opaque)
 
     qio_channel_set_follow_coroutine_ctx(QIO_CHANNEL(client->ioc), true);
 
-    /* A very simple negotiation for future extensibility.  No features
-     * are defined so write 0.
-     */
-    flags = cpu_to_be32(0);
+    /* A very simple negotiation for future extensibility */
+    flags = cpu_to_be32(PR_HELPER_FEATURE_CLEANUP);
     r = qio_channel_write_all(QIO_CHANNEL(client->ioc),
                              (char *) &flags, sizeof(flags), NULL);
     if (r < 0) {
@@ -751,7 +870,7 @@ static void coroutine_fn prh_co_entry(void *opaque)
 
     r = qio_channel_read_all(QIO_CHANNEL(client->ioc),
                              (char *) &flags, sizeof(flags), NULL);
-    if (be32_to_cpu(flags) != 0 || r < 0) {
+    if ((be32_to_cpu(flags) & ~PR_HELPER_FEATURE_CLEANUP) != 0 || r < 0) {
         goto out;
     }
 
@@ -766,15 +885,24 @@ static void coroutine_fn prh_co_entry(void *opaque)
         }
 
         num_active_sockets++;
-        if (req.cdb[0] == PERSISTENT_RESERVE_OUT) {
+        switch (req.cdb[0]) {
+        case PERSISTENT_RESERVE_OUT:
             r = do_pr_out(req.fd, req.cdb, resp.sense,
                           client->data, sz);
             resp.sz = 0;
-        } else {
+            break;
+        case PERSISTENT_RESERVE_IN:
             resp.sz = sizeof(client->data);
             r = do_pr_in(req.fd, req.cdb, resp.sense,
                          client->data, &resp.sz);
             resp.sz = MIN(resp.sz, sz);
+            break;
+        case PR_HELPER_CLEANUP:
+            r = do_pr_cleanup(req.fd, resp.sense, &local_err);
+            resp.sz = 0;
+            break;
+        default:
+            abort();
         }
         num_active_sockets--;
         close(req.fd);
-- 
2.56.0

[PATCH 5/8] pr-manager-helper: add PR_HELPER_CLEANUP command support

[Stefan Hajnoczi]

qemu-pr-helper now supports the vendor-specific PR_HELPER_CLEANUP SCSI
command. Add support to the client side.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scsi/pr-manager-helper.c | 20 ++++++++++++++++----
 scsi/utils.c             |  3 +++
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c
index 26ef3876f0e..fa6611ed676 100644
--- a/scsi/pr-manager-helper.c
+++ b/scsi/pr-manager-helper.c
@@ -38,6 +38,8 @@ struct PRManagerHelper {
 
     QemuMutex lock;
     QIOChannel *ioc;
+
+    uint32_t proto_flags; /* flags negotiated with pr-helper */
 };
 
 static void pr_manager_send_status_changed_event(PRManagerHelper *pr_mgr)
@@ -129,8 +131,9 @@ static int pr_manager_helper_initialize(PRManagerHelper *pr_mgr,
         goto out_close;
     }
 
-    flags = 0;
-    r = pr_manager_helper_write(pr_mgr, -1, &flags, sizeof(flags), errp);
+    pr_mgr->proto_flags = flags & PR_HELPER_FEATURE_CLEANUP;
+    r = pr_manager_helper_write(pr_mgr, -1, &pr_mgr->proto_flags,
+                                sizeof(pr_mgr->proto_flags), errp);
     if (r < 0) {
         goto out_close;
     }
@@ -160,10 +163,19 @@ static int pr_manager_helper_run(PRManager *p,
         return -EINVAL;
     }
 
+    /* The command is only available when pr-helper supports it */
+    if (cdb[0] == PR_HELPER_CLEANUP &&
+        !(pr_mgr->proto_flags & PR_HELPER_FEATURE_CLEANUP)) {
+        return -ENOTSUP;
+    }
+
     memcpy(cdb, io_hdr->cmdp, io_hdr->cmd_len);
-    assert(cdb[0] == PERSISTENT_RESERVE_OUT || cdb[0] == PERSISTENT_RESERVE_IN);
+    assert(cdb[0] == PERSISTENT_RESERVE_OUT ||
+           cdb[0] == PERSISTENT_RESERVE_IN ||
+           cdb[0] == PR_HELPER_CLEANUP);
     expected_dir =
-        (cdb[0] == PERSISTENT_RESERVE_OUT ? SG_DXFER_TO_DEV : SG_DXFER_FROM_DEV);
+        (cdb[0] == PERSISTENT_RESERVE_IN ?
+         SG_DXFER_FROM_DEV : SG_DXFER_TO_DEV);
     if (io_hdr->dxfer_direction != expected_dir) {
         return -EINVAL;
     }
diff --git a/scsi/utils.c b/scsi/utils.c
index daee90ecf0e..1ee68e68221 100644
--- a/scsi/utils.c
+++ b/scsi/utils.c
@@ -29,6 +29,9 @@ uint32_t scsi_data_cdb_xfer(uint8_t *buf)
 
 uint32_t scsi_cdb_xfer(uint8_t *buf)
 {
+    if (buf[0] == PR_HELPER_CLEANUP) {
+        return 0;
+    }
     switch (buf[0] >> 5) {
     case 0:
         return buf[4];
-- 
2.56.0

[PATCH 6/8] file-posix: forward PR_HELPER_CLEANUP commands to pr-manager

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/file-posix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 9aad156aad4..75c71e83106 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -4346,7 +4346,8 @@ hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
     if (req == SG_IO && s->pr_mgr) {
         struct sg_io_hdr *io_hdr = buf;
         if (io_hdr->cmdp[0] == PERSISTENT_RESERVE_OUT ||
-            io_hdr->cmdp[0] == PERSISTENT_RESERVE_IN) {
+            io_hdr->cmdp[0] == PERSISTENT_RESERVE_IN ||
+            io_hdr->cmdp[0] == PR_HELPER_CLEANUP) {
             return pr_manager_execute(s->pr_mgr, qemu_get_current_aio_context(),
                                       s->fd, io_hdr);
         }
-- 
2.56.0

[PATCH 7/8] scsi: clean up persistent reservation state after migration

[Stefan Hajnoczi]

Live migration of persistent reservations is designed to work without
coordination between the source and destination. The destination uses
the PREEMPT command to atomically take over the reservation or
unregister keys belonging to the source. This approach avoids race
conditions where other nodes in the cluster might acquire the
reservation and it minimizes communication during the downtime window.

While this works at the SCSI level, this approach is incomplete when
using multipathd. Multipathd tracks registered keys and automatically
registers them again when paths come up. It is unaware that the
destination has preempted the reservation and therefore it will still
attempt to register the key on the source host.

Use the new pr-helper CLEANUP command to communicate with multipathd on
the source after migration completes successfully. The effect is similar
to REGISTER AND IGNORE EXISTING KEY, except it also discards
multipathd's state so that the source will no longer attempt to register
the key.

Buglink: https://redhat.atlassian.net/browse/RHEL-274966
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/scsi/scsi.h |  1 +
 hw/scsi/scsi-disk.c    | 21 ++++++++++++++++++++-
 hw/scsi/scsi-generic.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 3eab339b0d2..a2f6f0be7fc 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -256,6 +256,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);
+bool scsi_generic_pr_state_cleanup(SCSIDevice *s, Error **errp);
 
 /* scsi-disk.c */
 #define SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR             0
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 3d88e57eded..6341089ddf2 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2773,13 +2773,32 @@ static void scsi_block_migration_failed(SCSIDiskState *s)
     }
 }
 
+/*
+ * Unregister keys on the source after migration since the destination has
+ * preempted and taken over. This cleanup is not necessary at the SCSI level
+ * but multipathd tracks registered keys and must be informed that this host no
+ * longer wishes to be registered.
+ */
+static void scsi_block_migration_done(SCSIDiskState *s)
+{
+    SCSIDevice *d = &s->qdev;
+    Error *local_err = NULL;
+
+    if (!scsi_generic_pr_state_cleanup(d, &local_err)) {
+        error_prepend(&local_err, "scsi-block migration done: ");
+        warn_report_err(local_err);
+    }
+}
+
 static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
                                          MigrationEvent *e, Error **errp)
 {
     SCSIDiskState *s =
         container_of(notifier, SCSIDiskState, migration_notifier);
 
-    if (e->type == MIG_EVENT_FAILED) {
+    if (e->type == MIG_EVENT_DONE) {
+        scsi_block_migration_done(s);
+    } else if (e->type == MIG_EVENT_FAILED) {
         scsi_block_migration_failed(s);
     }
     return 0;
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 153a5275567..e41e973aa92 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -652,6 +652,35 @@ bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
     return true;
 }
 
+/* Clean up keys on source after migration */
+bool scsi_generic_pr_state_cleanup(SCSIDevice *s, Error **errp)
+{
+    SCSIPRState *pr_state = &s->pr_state;
+    uint8_t cmd[10] = {
+        PR_HELPER_CLEANUP, /* OPCODE */
+    };
+    int ret;
+
+    WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+        if (pr_state->key == 0) {
+            return true; /* nothing to do */
+        }
+    }
+
+    ret = scsi_SG_IO(s->conf.blk, SG_DXFER_TO_DEV, cmd, sizeof(cmd), NULL, 0,
+                     s->io_timeout, errp);
+    if (ret < 0) {
+        error_prepend(errp, "pr-helper CLEANUP failed: ");
+        return false;
+    }
+
+    WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
+        pr_state->key = 0;
+        pr_state->resv_type = 0;
+    }
+    return true;
+}
+
 static void scsi_read_complete(void * opaque, int ret)
 {
     SCSIGenericReq *r = (SCSIGenericReq *)opaque;
-- 
2.56.0

[PATCH 8/8] scsi: use REGISTER AND IGNORE EXISTING KEY for robustness

[Stefan Hajnoczi]

The regular REGISTER service action fails with RESERVATION CONFLICT when
the I_T nexus already has a key registered. While we don't expect a key
to be registered, it is fragile to rely on this. If there is a key
registered for some reason, live migration will fail when the
destination hits the RESERVATION CONFLICT error.

Use REGISTER AND IGNORE EXISTING KEY so that the command is likely to
succeed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/scsi-generic.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index e41e973aa92..e48fcb926f1 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -424,7 +424,8 @@ static void scsi_handle_persistent_reserve_out_reply(
     }
 }
 
-static bool scsi_generic_pr_register(SCSIDevice *s, uint64_t key, Error **errp)
+static bool scsi_generic_pr_register_ignore(SCSIDevice *s, uint64_t key,
+                                            Error **errp)
 {
     uint8_t cmd[10] = {};
     uint8_t buf[24] = {};
@@ -432,14 +433,15 @@ static bool scsi_generic_pr_register(SCSIDevice *s, uint64_t key, Error **errp)
     int ret;
 
     cmd[0] = PERSISTENT_RESERVE_OUT;
-    cmd[1] = PRO_REGISTER;
+    cmd[1] = PRO_REGISTER_AND_IGNORE_EXISTING_KEY;
     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: ");
+        error_prepend(errp, "PERSISTENT RESERVE OUT with "
+                      "REGISTER AND IGNORE EXISTING KEY: ");
         return false;
     }
     return true;
@@ -620,7 +622,7 @@ bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
         return true; /* no PR state, do nothing */
     }
 
-    if (!scsi_generic_pr_register(s, key, errp)) {
+    if (!scsi_generic_pr_register_ignore(s, key, errp)) {
         return false;
     }
 
@@ -647,7 +649,7 @@ bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
      * CONFLICT is expected if our registration remained in place.
      */
     if (resv_type == 0) {
-        scsi_generic_pr_register(s, key, NULL);
+        scsi_generic_pr_register_ignore(s, key, NULL);
     }
     return true;
 }
-- 
2.56.0

Re: [PATCH 1/8] pr-manager-helper: fix sense buffer overflow

[Eric Blake]

On Fri, Oct 09, 2026 at 11:30:51AM -0400, Stefan Hajnoczi wrote:
> scsi_build_sense() assumes the sense buffer is SCSI_SENSE_LEN (18) bytes
> long, so the caller's buffer with an arbitrary size cannot be used. Use
> our own sense buffer (resp.sense) as an intermediate buffer and honor
> the caller's sense buffer size (io_hdr->mx_sb_len).
> 
> This is triggered when scsi-generic.c:scsi_SG_IO() calls blk_ioctl()
> with an 8-byte sense buffer. There does not appear to be a way for the
> guest to trigger this buffer overflow directly since
> SCSIRequest->sense[] is 252 bytes long.
> 
> Fixes: 9bad2a6b9d0a ("scsi: add persistent reservation manager using qemu-pr-helper")
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  scsi/pr-manager-helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 2/8] scsi: increase sense buffer size in scsi_SG_IO()

[Eric Blake]

On Fri, Oct 09, 2026 at 11:30:52AM -0400, Stefan Hajnoczi wrote:
> There are different SCSI sense formats. Increase the size so we can get
> more detailed sense data.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/scsi/scsi-generic.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 3/8] hw/scsi-disk: move MIG_EVENT_FAILED into a new function

[Eric Blake]

On Fri, Oct 09, 2026 at 11:30:53AM -0400, Stefan Hajnoczi wrote:
> Move the MIG_EVENT_FAILED logic from scsi_block_migration_notifier()
> into its own new scsi_block_migration_failed() function. The next commit
> adds logic for handling another event, so doing that in dedicated
> functions makes the code easier to read.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/scsi/scsi-disk.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)

> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index deb33ac1dda..3d88e57eded 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2761,20 +2761,26 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
>   * fails because the destination may have already preempted and we need to get
>   * the reservation back.
>   */

I think this comment...

> +static void scsi_block_migration_failed(SCSIDiskState *s)
> +{
> +    SCSIDevice *d = &s->qdev;
> +    Error *local_err = NULL;
> +
> +    if (!scsi_generic_pr_state_preempt(d, &local_err)) {
> +        /* MIG_EVENT_FAILED cannot fail, so just warn */
> +        error_prepend(&local_err, "scsi-block migration rollback: ");
> +        warn_report_err(local_err);
> +    }
> +}
> +
>  static int scsi_block_migration_notifier(NotifierWithReturn *notifier,

...belongs better with the function it was originally introducing.

Fix that, and you can add

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 5/8] pr-manager-helper: add PR_HELPER_CLEANUP command support

[Paolo Bonzini]

On 10/9/26 17:30, Stefan Hajnoczi wrote:
> qemu-pr-helper now supports the vendor-specific PR_HELPER_CLEANUP SCSI
> command. Add support to the client side.

Hi Stefan, up to this point it's all good; qemu-pr-helper works in SCSI 
commands so a vendor-specific command is the obvious way to add the 
functionality---and I'm glad the feature negotiation worked out.

For the next steps, however, I'm not sure about using blk_ioctl to send 
the migration done command from SCSIDevice to BlockBackend, which (if I 
understand correctly) implicitly exposes the command to the guest as well.

In particular, perhaps it makes sense to add a migration_done callback 
to BlockDriverState and BlockBackend?  They can be called either only 
from SCSIGeneric, or from a new block layer migration notifier that 
walks the BlockBackend list.

If instead the command is not exposed to the guest, please just add a 
comment explaining why.

Thanks,

Paolo

> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   scsi/pr-manager-helper.c | 20 ++++++++++++++++----
>   scsi/utils.c             |  3 +++
>   2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c
> index 26ef3876f0e..fa6611ed676 100644
> --- a/scsi/pr-manager-helper.c
> +++ b/scsi/pr-manager-helper.c
> @@ -38,6 +38,8 @@ struct PRManagerHelper {
>   
>       QemuMutex lock;
>       QIOChannel *ioc;
> +
> +    uint32_t proto_flags; /* flags negotiated with pr-helper */
>   };
>   
>   static void pr_manager_send_status_changed_event(PRManagerHelper *pr_mgr)
> @@ -129,8 +131,9 @@ static int pr_manager_helper_initialize(PRManagerHelper *pr_mgr,
>           goto out_close;
>       }
>   
> -    flags = 0;
> -    r = pr_manager_helper_write(pr_mgr, -1, &flags, sizeof(flags), errp);
> +    pr_mgr->proto_flags = flags & PR_HELPER_FEATURE_CLEANUP;
> +    r = pr_manager_helper_write(pr_mgr, -1, &pr_mgr->proto_flags,
> +                                sizeof(pr_mgr->proto_flags), errp);
>       if (r < 0) {
>           goto out_close;
>       }
> @@ -160,10 +163,19 @@ static int pr_manager_helper_run(PRManager *p,
>           return -EINVAL;
>       }
>   
> +    /* The command is only available when pr-helper supports it */
> +    if (cdb[0] == PR_HELPER_CLEANUP &&
> +        !(pr_mgr->proto_flags & PR_HELPER_FEATURE_CLEANUP)) {
> +        return -ENOTSUP;
> +    }
> +
>       memcpy(cdb, io_hdr->cmdp, io_hdr->cmd_len);
> -    assert(cdb[0] == PERSISTENT_RESERVE_OUT || cdb[0] == PERSISTENT_RESERVE_IN);
> +    assert(cdb[0] == PERSISTENT_RESERVE_OUT ||
> +           cdb[0] == PERSISTENT_RESERVE_IN ||
> +           cdb[0] == PR_HELPER_CLEANUP);
>       expected_dir =
> -        (cdb[0] == PERSISTENT_RESERVE_OUT ? SG_DXFER_TO_DEV : SG_DXFER_FROM_DEV);
> +        (cdb[0] == PERSISTENT_RESERVE_IN ?
> +         SG_DXFER_FROM_DEV : SG_DXFER_TO_DEV);
>       if (io_hdr->dxfer_direction != expected_dir) {
>           return -EINVAL;
>       }
> diff --git a/scsi/utils.c b/scsi/utils.c
> index daee90ecf0e..1ee68e68221 100644
> --- a/scsi/utils.c
> +++ b/scsi/utils.c
> @@ -29,6 +29,9 @@ uint32_t scsi_data_cdb_xfer(uint8_t *buf)
>   
>   uint32_t scsi_cdb_xfer(uint8_t *buf)
>   {
> +    if (buf[0] == PR_HELPER_CLEANUP) {
> +        return 0;
> +    }
>       switch (buf[0] >> 5) {
>       case 0:
>           return buf[4];

Re: [PATCH 5/8] pr-manager-helper: add PR_HELPER_CLEANUP command support

[Stefan Hajnoczi]

On Tue, Oct 13, 2026 at 06:25:28PM +0200, Paolo Bonzini wrote:
> On 10/9/26 17:30, Stefan Hajnoczi wrote:
> > qemu-pr-helper now supports the vendor-specific PR_HELPER_CLEANUP SCSI
> > command. Add support to the client side.
> 
> Hi Stefan, up to this point it's all good; qemu-pr-helper works in SCSI
> commands so a vendor-specific command is the obvious way to add the
> functionality---and I'm glad the feature negotiation worked out.
> 
> For the next steps, however, I'm not sure about using blk_ioctl to send the
> migration done command from SCSIDevice to BlockBackend, which (if I
> understand correctly) implicitly exposes the command to the guest as well.
> 
> In particular, perhaps it makes sense to add a migration_done callback to
> BlockDriverState and BlockBackend?  They can be called either only from
> SCSIGeneric, or from a new block layer migration notifier that walks the
> BlockBackend list.
> 
> If instead the command is not exposed to the guest, please just add a
> comment explaining why.

I looked into this: hw/scsi/scsi-bus.c:scsi_req_new() does not support
vendor-specific commands since scsi_cdb_length() returns -1. So
PR_HELPER_CLEANUP (0xff) from the guest will be rejected immediately
by any emulated SCSI HBA.

However, I will send a v2 patch series that goes further: let's
explicitly reject PR_HELPER_CLEANUP in scsi_req_new() so that even if
scsi_cdb_length() or related code changes in the future, it will never
be possible to submit the command from a guest.

> 
> Thanks,
> 
> Paolo
> 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >   scsi/pr-manager-helper.c | 20 ++++++++++++++++----
> >   scsi/utils.c             |  3 +++
> >   2 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c
> > index 26ef3876f0e..fa6611ed676 100644
> > --- a/scsi/pr-manager-helper.c
> > +++ b/scsi/pr-manager-helper.c
> > @@ -38,6 +38,8 @@ struct PRManagerHelper {
> >       QemuMutex lock;
> >       QIOChannel *ioc;
> > +
> > +    uint32_t proto_flags; /* flags negotiated with pr-helper */
> >   };
> >   static void pr_manager_send_status_changed_event(PRManagerHelper *pr_mgr)
> > @@ -129,8 +131,9 @@ static int pr_manager_helper_initialize(PRManagerHelper *pr_mgr,
> >           goto out_close;
> >       }
> > -    flags = 0;
> > -    r = pr_manager_helper_write(pr_mgr, -1, &flags, sizeof(flags), errp);
> > +    pr_mgr->proto_flags = flags & PR_HELPER_FEATURE_CLEANUP;
> > +    r = pr_manager_helper_write(pr_mgr, -1, &pr_mgr->proto_flags,
> > +                                sizeof(pr_mgr->proto_flags), errp);
> >       if (r < 0) {
> >           goto out_close;
> >       }
> > @@ -160,10 +163,19 @@ static int pr_manager_helper_run(PRManager *p,
> >           return -EINVAL;
> >       }
> > +    /* The command is only available when pr-helper supports it */
> > +    if (cdb[0] == PR_HELPER_CLEANUP &&
> > +        !(pr_mgr->proto_flags & PR_HELPER_FEATURE_CLEANUP)) {
> > +        return -ENOTSUP;
> > +    }
> > +
> >       memcpy(cdb, io_hdr->cmdp, io_hdr->cmd_len);
> > -    assert(cdb[0] == PERSISTENT_RESERVE_OUT || cdb[0] == PERSISTENT_RESERVE_IN);
> > +    assert(cdb[0] == PERSISTENT_RESERVE_OUT ||
> > +           cdb[0] == PERSISTENT_RESERVE_IN ||
> > +           cdb[0] == PR_HELPER_CLEANUP);
> >       expected_dir =
> > -        (cdb[0] == PERSISTENT_RESERVE_OUT ? SG_DXFER_TO_DEV : SG_DXFER_FROM_DEV);
> > +        (cdb[0] == PERSISTENT_RESERVE_IN ?
> > +         SG_DXFER_FROM_DEV : SG_DXFER_TO_DEV);
> >       if (io_hdr->dxfer_direction != expected_dir) {
> >           return -EINVAL;
> >       }
> > diff --git a/scsi/utils.c b/scsi/utils.c
> > index daee90ecf0e..1ee68e68221 100644
> > --- a/scsi/utils.c
> > +++ b/scsi/utils.c
> > @@ -29,6 +29,9 @@ uint32_t scsi_data_cdb_xfer(uint8_t *buf)
> >   uint32_t scsi_cdb_xfer(uint8_t *buf)
> >   {
> > +    if (buf[0] == PR_HELPER_CLEANUP) {
> > +        return 0;
> > +    }
> >       switch (buf[0] >> 5) {
> >       case 0:
> >           return buf[4];
> 

Re: [PATCH 3/8] hw/scsi-disk: move MIG_EVENT_FAILED into a new function

[Stefan Hajnoczi]

On Tue, Oct 13, 2026 at 10:00:15AM -0500, Eric Blake wrote:
> On Fri, Oct 09, 2026 at 11:30:53AM -0400, Stefan Hajnoczi wrote:
> > Move the MIG_EVENT_FAILED logic from scsi_block_migration_notifier()
> > into its own new scsi_block_migration_failed() function. The next commit
> > adds logic for handling another event, so doing that in dedicated
> > functions makes the code easier to read.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  hw/scsi/scsi-disk.c | 26 ++++++++++++++++----------
> >  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> > 
> > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> > index deb33ac1dda..3d88e57eded 100644
> > --- a/hw/scsi/scsi-disk.c
> > +++ b/hw/scsi/scsi-disk.c
> > @@ -2761,20 +2761,26 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> >   * fails because the destination may have already preempted and we need to get
> >   * the reservation back.
> >   */
> 
> I think this comment...
> 
> > +static void scsi_block_migration_failed(SCSIDiskState *s)
> > +{
> > +    SCSIDevice *d = &s->qdev;
> > +    Error *local_err = NULL;
> > +
> > +    if (!scsi_generic_pr_state_preempt(d, &local_err)) {
> > +        /* MIG_EVENT_FAILED cannot fail, so just warn */
> > +        error_prepend(&local_err, "scsi-block migration rollback: ");
> > +        warn_report_err(local_err);
> > +    }
> > +}
> > +
> >  static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
> 
> ...belongs better with the function it was originally introducing.

I think it's clearer to explain scsi_block_migration_failed()'s purpose
in its doc comment rather than in scsi_block_migration_notifier(), which
this patch series turns into just a dispatch function that doesn't do
anything itself.

scsi_block_migration_notifier() will dispatch MIG_EVENT_DONE in a later
commit, so it's more general and no longer does the MIG_EVENT_FAILED
work itself.

What do you think?

> 
> Fix that, and you can add
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.
> Virtualization:  qemu.org | libguestfs.org
> 

Re: [PATCH 4/8] qemu-pr-helper: add PR_HELPER_CLEANUP for live migration

[Eric Blake]

On Fri, Oct 09, 2026 at 11:30:54AM -0400, Stefan Hajnoczi wrote:
> Introduce a new vendor-specific SCSI command that acts similar to
> REGISTER AND IGNORE EXISTING KEY but also discards multipathd's state
> (including /etc/multipath/prkeys). This command will be used on the
> source after live migration completes successfully.
> 
> This cleanup step is necessary because multipathd will register the key
> it has stored when the path comes up again. This is undesirable when the
> guest has migrated to another host and can interfere with the migration
> process or the application running inside the guest.
> 
> Add a qemu-pr-manager protocol feature bit indicating that the new
> PR_HELPER_CLEANUP command is available. The command has no CDB fields
> other than OPCODE and the response contains no data aside from the usual
> result and sense data.
> 
> Note that the linker needs an explicit libmpathcmd dependency for
> mpath_disconnect() even though qemu-pr-helper.c already #included the
> mpath_cmd.h header before.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  meson.build              |   2 +
>  include/scsi/constants.h |   1 +
>  scsi/pr-helper.h         |   3 +
>  scsi/qemu-pr-helper.c    | 146 ++++++++++++++++++++++++++++++++++++---
>  4 files changed, 143 insertions(+), 9 deletions(-)
> 

> +++ b/scsi/qemu-pr-helper.c

> @@ -703,7 +823,8 @@ static int coroutine_fn prh_write_response(PRHelperClient *client,
>      ssize_t r;
>      size_t sz;
>  
> -    if (req->cdb[0] == PERSISTENT_RESERVE_IN && resp->result == GOOD) {
> +    if (req->cdb[0] == PERSISTENT_RESERVE_IN &&
> +        resp->result == GOOD) {
>          assert(resp->sz <= req->sz && resp->sz <= sizeof(client->data));

Spurious hunk?

Otherwise,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 8/8] scsi: use REGISTER AND IGNORE EXISTING KEY for robustness

[Eric Blake]

On Fri, Oct 09, 2026 at 11:30:58AM -0400, Stefan Hajnoczi wrote:
> The regular REGISTER service action fails with RESERVATION CONFLICT when
> the I_T nexus already has a key registered. While we don't expect a key
> to be registered, it is fragile to rely on this. If there is a key
> registered for some reason, live migration will fail when the
> destination hits the RESERVATION CONFLICT error.
> 
> Use REGISTER AND IGNORE EXISTING KEY so that the command is likely to
> succeed.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/scsi/scsi-generic.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org