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

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

[Stefan Hajnoczi]

v2:
- Explicitly reject PR_HELPER_CLEAN commands in scsi_req_new() because
  this is an internal command that guests must not be able to invoke
- Move cdb[0] check after copy from io_hdr in pr-manager-helper.c
- Add missing endianness conversion for protocol flags in pr-manager-helper.c
- Ignore PR_HELPER_CLEANUP commands when there is no pr-manager to avoid
  passing through this QEMU-internal vendor-specific command to the
  underlying SCSI device.
- Add missing "forget" in comment (typo)

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.

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       |  18 +++--
 hw/scsi/scsi-bus.c       |  10 +++
 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 +
 11 files changed, 273 insertions(+), 43 deletions(-)

-- 
2.56.0

[PATCH v2 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>
Reviewed-by: Eric Blake <eblake@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 v2 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>
Reviewed-by: Eric Blake <eblake@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 v2 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 v2 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>
---
v2:
- Explicitly reject PR_HELPER_CLEAN commands in scsi_req_new() because
  this is an internal command that guests cannot send
- Add missing "forget" in comment (typo)
---
 meson.build              |   2 +
 include/scsi/constants.h |   1 +
 scsi/pr-helper.h         |   3 +
 hw/scsi/scsi-bus.c       |  10 +++
 scsi/qemu-pr-helper.c    | 146 ++++++++++++++++++++++++++++++++++++---
 5 files changed, 153 insertions(+), 9 deletions(-)

diff --git a/meson.build b/meson.build
index 48da6cf3e6b..6176935082b 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/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index aa02ff631b7..b4c38c083e1 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -873,6 +873,16 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
           */
          !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
         ops = &reqops_unit_attention;
+    } else if (buf[0] == PR_HELPER_CLEANUP) {
+        /*
+         * This is a QEMU-internal vendor-specific SCSI command that must only
+         * be submitted directly via blk_aio_ioctl() and not through SCSI
+         * emulation. Reject the command explicitly to guarantee a guest cannot
+         * execute it, although vendor-specific commands are not supported by
+         * SCSI emulation (including passthrough) anyway.
+         */
+        trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
+        goto invalid_opcode;
     } else if (lun != d->lun ||
                buf[0] == REPORT_LUNS ||
                (buf[0] == REQUEST_SENSE && d->sense_len)) {
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 9dcd3ec7111..54d196a503f 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 forget 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 v2 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.

pr_manager_helper_run() uses scsi_cdb_xfer() to determine the number of
bytes to transfer. Extend scsi_cdb_xfer() to know that this
vendor-specific command does not transfer any bytes.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v2:
- Move cdb[0] check after copy from io_hdr
- Add missing endianness conversion for protocol flags
---
 scsi/pr-manager-helper.c | 20 +++++++++++++++++---
 scsi/utils.c             |  3 +++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c
index 26ef3876f0e..e7162e21ff0 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,7 +131,9 @@ static int pr_manager_helper_initialize(PRManagerHelper *pr_mgr,
         goto out_close;
     }
 
-    flags = 0;
+    pr_mgr->proto_flags = be32_to_cpu(flags) & PR_HELPER_FEATURE_CLEANUP;
+
+    flags = cpu_to_be32(pr_mgr->proto_flags);
     r = pr_manager_helper_write(pr_mgr, -1, &flags, sizeof(flags), errp);
     if (r < 0) {
         goto out_close;
@@ -161,9 +165,19 @@ static int pr_manager_helper_run(PRManager *p,
     }
 
     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);
+
+    /* 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;
+    }
+
     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 v2 6/8] file-posix: forward PR_HELPER_CLEANUP commands to pr-manager

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v2:
- Ignore PR_HELPER_CLEANUP commands when there is no pr-manager to avoid
  passing through this QEMU-internal vendor-specific command to the
  underlying SCSI device.
---
 block/file-posix.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 328adb57233..df117f35d48 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -4362,12 +4362,20 @@ hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
         return ret;
     }
 
-    if (req == SG_IO && s->pr_mgr) {
+    if (req == SG_IO) {
         struct sg_io_hdr *io_hdr = buf;
-        if (io_hdr->cmdp[0] == PERSISTENT_RESERVE_OUT ||
-            io_hdr->cmdp[0] == PERSISTENT_RESERVE_IN) {
-            return pr_manager_execute(s->pr_mgr, qemu_get_current_aio_context(),
-                                      s->fd, io_hdr);
+
+        if (s->pr_mgr) {
+            if (io_hdr->cmdp[0] == PERSISTENT_RESERVE_OUT ||
+                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);
+            }
+        } else if (io_hdr->cmdp[0] == PR_HELPER_CLEANUP) {
+            /* This is a nop without pr-manager */
+            io_hdr->status = GOOD;
+            return 0;
         }
     }
 
-- 
2.56.0

[PATCH v2 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.

PR_HELPER_CLEANUP is a QEMU-internal command that is rejected by
scsi_req_new(), so note that this command is not exposed to guests
although direct blk_aio_ioctl(SG_IO) calls can make use of it.

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 v2 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 v2 3/8] hw/scsi-disk: move MIG_EVENT_FAILED into a new function

[Eric Blake]

On Tue, Oct 13, 2026 at 02:17:47PM -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(-)

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

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

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

[Eric Blake]

On Tue, Oct 13, 2026 at 02:17:48PM -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>
> ---
> v2:
> - Explicitly reject PR_HELPER_CLEAN commands in scsi_req_new() because
>   this is an internal command that guests cannot send
> - Add missing "forget" in comment (typo)
> ---
>  meson.build              |   2 +
>  include/scsi/constants.h |   1 +
>  scsi/pr-helper.h         |   3 +
>  hw/scsi/scsi-bus.c       |  10 +++
>  scsi/qemu-pr-helper.c    | 146 ++++++++++++++++++++++++++++++++++++---
>  5 files changed, 153 insertions(+), 9 deletions(-)
> 

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

> @@ -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.");

Worth removing the trailing . while you are in the neighborhood?

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

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

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

[Paolo Bonzini]

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

Il mar 13 ott 2026, 20:18 Stefan Hajnoczi <stefanha@redhat.com> ha scritto:

> 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>
>

Thanks, can you also document the changes in  docs/interop/pr-helper.rst
(no need for V3, can be a separate posting or a "patch 9 of 8")? Otherwise
it's all good, thanks!

Paolo

---
> v2:
> - Explicitly reject PR_HELPER_CLEAN commands in scsi_req_new() because
>   this is an internal command that guests cannot send
> - Add missing "forget" in comment (typo)
> ---
>  meson.build              |   2 +
>  include/scsi/constants.h |   1 +
>  scsi/pr-helper.h         |   3 +
>  hw/scsi/scsi-bus.c       |  10 +++
>  scsi/qemu-pr-helper.c    | 146 ++++++++++++++++++++++++++++++++++++---
>  5 files changed, 153 insertions(+), 9 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 48da6cf3e6b..6176935082b 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/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index aa02ff631b7..b4c38c083e1 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -873,6 +873,16 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t
> tag, uint32_t lun,
>            */
>           !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
>          ops = &reqops_unit_attention;
> +    } else if (buf[0] == PR_HELPER_CLEANUP) {
> +        /*
> +         * This is a QEMU-internal vendor-specific SCSI command that must
> only
> +         * be submitted directly via blk_aio_ioctl() and not through SCSI
> +         * emulation. Reject the command explicitly to guarantee a guest
> cannot
> +         * execute it, although vendor-specific commands are not
> supported by
> +         * SCSI emulation (including passthrough) anyway.
> +         */
> +        trace_scsi_req_parse_bad(d->id, lun, tag, buf[0]);
> +        goto invalid_opcode;
>      } else if (lun != d->lun ||
>                 buf[0] == REPORT_LUNS ||
>                 (buf[0] == REQUEST_SENSE && d->sense_len)) {
> diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
> index 9dcd3ec7111..54d196a503f 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 forget 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
>
>

[-- Attachment #2: Type: text/html, Size: 14755 bytes --]

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

[Eric Blake]

On Tue, Oct 13, 2026 at 02:17:49PM -0400, Stefan Hajnoczi wrote:
> qemu-pr-helper now supports the vendor-specific PR_HELPER_CLEANUP SCSI
> command. Add support to the client side.
> 
> pr_manager_helper_run() uses scsi_cdb_xfer() to determine the number of
> bytes to transfer. Extend scsi_cdb_xfer() to know that this
> vendor-specific command does not transfer any bytes.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v2:
> - Move cdb[0] check after copy from io_hdr
> - Add missing endianness conversion for protocol flags
> ---
>  scsi/pr-manager-helper.c | 20 +++++++++++++++++---
>  scsi/utils.c             |  3 +++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 

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

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

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

[Eric Blake]

On Tue, Oct 13, 2026 at 02:17:50PM -0400, Stefan Hajnoczi wrote:
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v2:
> - Ignore PR_HELPER_CLEANUP commands when there is no pr-manager to avoid
>   passing through this QEMU-internal vendor-specific command to the
>   underlying SCSI device.
> ---
>  block/file-posix.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>

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

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

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

[Eric Blake]

On Tue, Oct 13, 2026 at 02:17:51PM -0400, Stefan Hajnoczi wrote:
> 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.
> 
> PR_HELPER_CLEANUP is a QEMU-internal command that is rejected by
> scsi_req_new(), so note that this command is not exposed to guests
> although direct blk_aio_ioctl(SG_IO) calls can make use of it.
> 
> 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(-)
> 

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

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

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

[Eric Blake]

On Tue, Oct 13, 2026 at 02:17:52PM -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