scsi: handle reservation changes across migration
Other nodes in the cluster can preempt or clear SCSI Persistent Reservations at any time. When this happens across live migration, the reservation state transferred with the guest might be outdated. Attempt to handle such cases gracefully by checking the current reservation or registered keys to detect stale state before restoring. If the actual state of the disk has changed, do not modify it and accept that as the most up-to-date state. Do this using READ RESERVATION when the guest holds a reservation or READ KEYS when the guest has registered a key but does not hold a reservation. There is still a race condition between checking and restoring state, but it seems unavoidable and is no worse than before. Buglink: https://redhat.atlassian.net/browse/RHEL-153123 Fixes: ab57b51f1375b6a6f098a74c6f79207a9630948d ("scsi: save/load SCSI reservation state") Reported-by: Qing Wang Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Link: https://lore.kernel.org/r/20260415232906.212349-3-stefanha@redhat.com Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Stefan Hajnoczi committed
Apr 15, 2026 at 19:29 UTC
87c7b07fe4aa43dc7257b1e711faa835fac1cccb
1 file changed
+149
-24
hw/scsi/scsi-generic.c
+149
-24
@@ -479,13 +479,84 @@ static bool scsi_generic_pr_preempt(SCSIDevice *s, uint64_t key,
479
return true;
480
}
481
482
+/*
483
+ * Returns true if the given key is registered or false otherwise (including
484
+ * errors).
485
+ */
486
+static bool scsi_generic_pr_key_registered(SCSIDevice *s, uint64_t key,
487
+ Error **errp)
488
+{
489
+ const size_t key_list_offset = 8; /* in READ KEYS parameter data */
490
+ uint64_t key_be = cpu_to_be64(key);
491
+ uint8_t cmd[10] = {};
492
+ size_t buf_len;
493
+ g_autofree uint8_t *buf = NULL;
494
+ uint32_t additional_length = 16 * 8; /* initial key list size */
495
+
496
+ /*
497
+ * Loop to resize parameter data buffer when there are many keys. It would
498
+ * be simpler to hardcode the maximum buffer size (it's only 64 KB), but
499
+ * SG_IO can fail with EINVAL if the host kernel blkdev queue limits are
500
+ * too low.
501
+ */
502
+ do {
503
+ uint16_t allocation_length_be;
504
+ int ret;
505
+
506
+ buf_len = key_list_offset + additional_length;
507
+ buf = g_realloc(buf, buf_len);
508
+ memset(buf, 0, buf_len);
509
+
510
+ cmd[0] = PERSISTENT_RESERVE_IN;
511
+ cmd[1] = PRI_READ_KEYS;
512
+ allocation_length_be = cpu_to_be16(buf_len);
513
+ memcpy(&cmd[7], &allocation_length_be, sizeof(allocation_length_be));
514
+
515
+ ret = scsi_SG_IO(s->conf.blk, SG_DXFER_FROM_DEV, cmd, sizeof(cmd),
516
+ buf, buf_len, s->io_timeout, errp);
517
+ if (ret < 0) {
518
+ error_prepend(errp, "PERSISTENT RESERVE IN with READ KEYS: ");
519
+ return false;
520
+ }
521
+
522
+ memcpy(&additional_length, &buf[4], sizeof(additional_length));
523
+ be32_to_cpus(&additional_length);
524
+
525
+ /*
526
+ * The parameter data's ADDITIONAL LENGTH must not overflow the CDB's
527
+ * 16-bit ALLOCATION LENGTH field since the next loop iteration will
528
+ * compute ALLOCATION LENGTH based on ADDITIONAL LENGTH.
529
+ */
530
+ if (additional_length > UINT16_MAX - key_list_offset) {
531
+ error_setg(errp, "got invalid ADDITIONAL LENGTH %" PRIu32
532
+ " from READ KEYS", additional_length);
533
+ return false;
534
+ }
535
+
536
+ for (size_t i = key_list_offset; i < buf_len; i += sizeof(key_be)) {
537
+ if (i - key_list_offset >= additional_length) {
538
+ break; /* end of parameter list */
539
+ }
540
+
541
+ if (memcmp(&key_be, &buf[i], sizeof(key_be)) == 0) {
542
+ return true; /* key found */
543
+ }
544
+ }
545
+ } while (additional_length > buf_len - key_list_offset);
546
+
547
+ return false; /* key not found */
548
+}
549
+
550
/* Register keys and preempt reservations after live migration */
551
bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
552
{
553
SCSIPRState *pr_state = &s->pr_state;
554
+ Error *local_err = NULL;
555
+ bool check_stale_key = true;
556
uint64_t key;
557
uint8_t resv_type;
558
559
+ /* Get the migrated PR state */
560
WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
561
key = pr_state->key;
562
resv_type = pr_state->resv_type;
@@ -493,36 +564,90 @@ bool scsi_generic_pr_state_preempt(SCSIDevice *s, Error **errp)
564
565
trace_scsi_generic_pr_state_preempt(key, resv_type);
566
496
- if (key) {
497
- if (!scsi_generic_pr_register(s, key, errp)) {
567
+ /* Handle stale PR state (e.g. another node preempted) */
568
+ if (resv_type) {
569
+ uint64_t dev_key;
570
+ uint8_t dev_resv_type;
571
+
572
+ if (scsi_generic_read_reservation(s, &dev_key, &dev_resv_type,
573
+ errp) < 0) {
574
return false;
575
}
576
501
- /*
502
- * Two cases:
503
- *
504
- * 1. There is no reservation (resv_type is 0) and the other I_T nexus
505
- * will be unregistered. This is important so the source host does
506
- * not leak registered keys across live migration.
507
- *
508
- * 2. There is a reservation (resv_type is not 0) and the other I_T
509
- * nexus will be unregistered and its reservation is atomically
510
- * taken over by us. This is the scenario where a reservation is
511
- * migrated along with the guest.
512
- */
513
- if (!scsi_generic_pr_preempt(s, key, resv_type, errp)) {
514
- return false;
577
+ if (dev_resv_type != resv_type) {
578
+ /* vmstate had a stale reservation type */
579
+ g_autofree char *name = qdev_get_human_name(&s->qdev);
580
+ warn_report("Expected SCSI reservation type 0x%x on device '%s', "
581
+ "got 0x%x, using new type",
582
+ resv_type, name, dev_resv_type);
583
+ resv_type = dev_resv_type;
584
}
585
517
- /*
518
- * Some SCSI targets, like the Linux LIO target, remove our
519
- * registration when preempting without a reservation (resv_type is 0).
520
- * Try to register again but ignore the error since a RESERVATION
521
- * CONFLICT is expected if our registration remained in place.
522
- */
523
- if (resv_type == 0) {
524
- scsi_generic_pr_register(s, key, NULL);
586
+ if (dev_key == key) {
587
+ /* The reservation exists, no need to check for a stale key */
588
+ check_stale_key = false;
589
+ } else {
590
+ g_autofree char *name = qdev_get_human_name(&s->qdev);
591
+ warn_report("Expected SCSI reservation with key 0x%" PRIx64
592
+ " on device '%s', got 0x%" PRIx64 ", ignoring "
593
+ "reservation",
594
+ key, name, dev_key);
595
+ resv_type = 0; /* vmstate had a stale reservation */
596
+ }
597
+ }
598
+
599
+ if (key != 0 && check_stale_key &&
600
+ !scsi_generic_pr_key_registered(s, key, &local_err)) {
601
+ if (local_err) {
602
+ error_propagate(errp, local_err);
603
+ return false;
604
}
605
+
606
+ g_autofree char *name = qdev_get_human_name(&s->qdev);
607
+ warn_report("SCSI reservation key 0x%" PRIx64 " on device '%s' not "
608
+ "registered after migration, ignoring",
609
+ key, name);
610
+ key = 0; /* vmstate had a stale key */
611
+ }
612
+
613
+ /* Stale PR state may have been updated */
614
+ WITH_QEMU_LOCK_GUARD(&pr_state->mutex) {
615
+ pr_state->key = key;
616
+ pr_state->resv_type = resv_type;
617
+ }
618
+
619
+ if (key == 0) {
620
+ return true; /* no PR state, do nothing */
621
+ }
622
+
623
+ if (!scsi_generic_pr_register(s, key, errp)) {
624
+ return false;
625
+ }
626
+
627
+ /*
628
+ * Two cases:
629
+ *
630
+ * 1. There is no reservation (resv_type is 0) and the other I_T nexus
631
+ * will be unregistered. This is important so the source host does
632
+ * not leak registered keys across live migration.
633
+ *
634
+ * 2. There is a reservation (resv_type is not 0) and the other I_T
635
+ * nexus will be unregistered and its reservation is atomically
636
+ * taken over by us. This is the scenario where a reservation is
637
+ * migrated along with the guest.
638
+ */
639
+ if (!scsi_generic_pr_preempt(s, key, resv_type, errp)) {
640
+ return false;
641
+ }
642
+
643
+ /*
644
+ * Some SCSI targets, like the Linux LIO target, remove our
645
+ * registration when preempting without a reservation (resv_type is 0).
646
+ * Try to register again but ignore the error since a RESERVATION
647
+ * CONFLICT is expected if our registration remained in place.
648
+ */
649
+ if (resv_type == 0) {
650
+ scsi_generic_pr_register(s, key, NULL);
651
}
652
return true;
653
}