* [PATCH QEMU HCI-8.1 v3 01/28] VZ: migration/cpr-exec: fix Error leak in preserve_ram()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
@ 2026-10-16 15:25 ` Andrey Drobyshev
2026-10-16 15:28 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 02/28] migration/cpr-exec: restore RAM write access if exec fails Andrey Drobyshev
` (26 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:25 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
migrate_set_error() stores its own copy of the error, so free ours.
Fixes: a3745506daf9 ("VZ: migration/cpr-exec: mark saved RAM blocks with MADV_DOEXEC before execve")
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
migration/cpr-exec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index edace3f0bd8..9fd3515753a 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -148,6 +148,7 @@ static bool preserve_ram(cpr_ramblock_t ram, char *name)
error_report_err(error_copy(err));
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
migrate_set_error(s, err);
+ error_free(err);
return false;
}
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 02/28] migration/cpr-exec: restore RAM write access if exec fails
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
2026-10-16 15:25 ` [PATCH QEMU HCI-8.1 v3 01/28] VZ: migration/cpr-exec: fix Error leak in preserve_ram() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:28 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 03/28] vfio/region: create BAR dma-bufs with O_CLOEXEC Andrey Drobyshev
` (25 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
With -cpr-ram-rdonly the RAM blocks are made read-only right before
execve(). If execve() fails, the VM resumes in the old QEMU with
read-only RAM and crashes on the first write. Make it writable again.
Fixes: 91da25f20471 ("migration/cpr-exec: add '-cpr-ram-rdonly' option to mark RAM blocks as RO #VSTOR-101579")
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
migration/cpr-exec.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index 9fd3515753a..c4bdf2cb99f 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -224,6 +224,9 @@ static void cpr_exec_cb(void *opaque)
error_setg_errno(&err, errno, "execvp %s failed", argv[0]);
g_clear_pointer(&argv, g_strfreev);
cpr_exec_unpreserve_fds();
+ if (cpr_ram_rdonly) {
+ cpr_ram_restore_rw();
+ }
error_report_err(error_copy(err));
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 03/28] vfio/region: create BAR dma-bufs with O_CLOEXEC
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
2026-10-16 15:25 ` [PATCH QEMU HCI-8.1 v3 01/28] VZ: migration/cpr-exec: fix Error leak in preserve_ram() Andrey Drobyshev
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 02/28] migration/cpr-exec: restore RAM write access if exec fails Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:30 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 04/28] migration: Plug memory leaks after migrate_set_error() Andrey Drobyshev
` (24 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
The dma-buf fds are created without O_CLOEXEC, so they leak into any
exec'ed program. With cpr-exec, every update leaks one fd per mmapped
BAR, as new QEMU creates its own.
Fixes: 96163e0dbcb4 ("hw/vfio/region: Create dmabuf for PCI BAR per region")
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/vfio/region.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index 1deaef12fd0..b0332f27fcd 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -258,7 +258,7 @@ static bool vfio_region_create_dma_buf(VFIORegion *region, Error **errp)
dma_buf = (void *)feature->data;
*dma_buf = (struct vfio_device_feature_dma_buf) {
.region_index = region->nr,
- .open_flags = O_RDWR,
+ .open_flags = O_RDWR | O_CLOEXEC,
.nr_ranges = region->nr_mmaps,
};
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 04/28] migration: Plug memory leaks after migrate_set_error()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (2 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 03/28] vfio/region: create BAR dma-bufs with O_CLOEXEC Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:29 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 05/28] hw/vfio: Fix liveness check in vfio_connect_kvm_msi_virq() Andrey Drobyshev
` (23 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Markus Armbruster <armbru@redhat.com>
migrate_set_error(s, err) stores a copy of @err in @s. The original
@err is not freed. Most callers free it immediately. Some callers
free it later, or pass it on. And some leak it. Fix those.
Perhaps migrate_set_error(s, err) should take ownership of @err. The
callers that free it immediately would become simpler, and avoid a
copy and a deallocation. The others would have to pass
error_copy(err).
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit 93817ec39656.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251115083500.2753895-2-armbru@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/cpr-exec.c | 3 ++-
migration/multifd.c | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index c4bdf2cb99f..d37293baec5 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -231,11 +231,12 @@ static void cpr_exec_cb(void *opaque)
error_report_err(error_copy(err));
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
migrate_set_error(s, err);
+ error_free(err);
+ err = NULL;
/* Note, we can go from state COMPLETED to FAILED */
migration_call_notifiers(MIG_EVENT_PRECOPY_FAILED, NULL);
- err = NULL;
if (!migration_block_activate(&err)) {
/* error was already reported */
error_free(err);
diff --git a/migration/multifd.c b/migration/multifd.c
index 5a723ae25d2..e369421b3ff 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -966,6 +966,7 @@ bool multifd_send_setup(void)
if (!multifd_new_send_channel_create(p, &local_err)) {
migrate_set_error(s, local_err);
+ error_free(local_err);
ret = -1;
}
}
@@ -990,6 +991,7 @@ bool multifd_send_setup(void)
ret = multifd_send_state->ops->send_setup(p, &local_err);
if (ret) {
migrate_set_error(s, local_err);
+ error_free(local_err);
goto err;
}
assert(p->iov);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 05/28] hw/vfio: Fix liveness check in vfio_connect_kvm_msi_virq()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (3 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 04/28] migration: Plug memory leaks after migrate_set_error() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:30 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 06/28] vfio/pci: Reject invalid MSI-X Table and PBA BIR values Andrey Drobyshev
` (22 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: "Tycho Andersen (AMD)" <tycho@kernel.org>
While working on savevm/loadvm for a new vfio device, I encountered the
crash below. Since vfio_connect_kvm_msi_virq() didn't check the ->use flag
for the vector, it would pass an unused vector down to
vfio_cpr_load_vector_fd() which would crash.
Fix this by checking the ->use flag along with the virq number to detect
whether a vector is valid or not.
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x0000555555a891ef in vfio_cpr_load_vector_fd (vdev=vdev@entry=0x0,
name=name@entry=0x555555eeb27e "kvm_interrupt", nr=nr@entry=1) at ../hw/vfio/cpr.c:44
44 g_autofree char *fdname = STRDUP_VECTOR_FD_NAME(vdev, name);
(gdb) bt
(vdev=vdev@entry=0x0, name=name@entry=0x555555eeb27e "kvm_interrupt", nr=nr@entry=1)
at ../hw/vfio/cpr.c:44
(vdev=0x0, e=e@entry=0x5555586971b4, name=name@entry=0x555555eeb27e "kvm_interrupt", nr=nr@entry=1, errp=errp@entry=0x0) at ../hw/vfio/pci.c:79
at ../hw/vfio/pci.c:601
at ../hw/vfio/pci.c:597
at ../hw/vfio/pci.c:3088
at ../hw/vfio/migration.c:278
(f=f@entry=0x5555574a43d0, se=se@entry=0x5555591edd40, errp=errp@entry=0x7fffffffe130)
at ../migration/savevm.c:971
(f=f@entry=0x5555574a43d0, type=type@entry=4 '\004', errp=errp@entry=0x7fffffffe130)
at ../migration/savevm.c:2654
(f=f@entry=0x5555574a43d0, mis=mis@entry=0x5555571de5a0, errp=0x7fffffffe130,
errp@entry=0x555557157c10 <error_fatal>) at ../migration/savevm.c:2973
(f=f@entry=0x5555574a43d0, errp=errp@entry=0x555557157c10 <error_fatal>)
at ../migration/savevm.c:3058
(name=0x7fffffffecc9 "foo", vmstate=vmstate@entry=0x0, has_devices=has_devices@entry=false, devices=devices@entry=0x0, errp=errp@entry=0x555557157c10 <error_fatal>) at ../migration/savevm.c:3452
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit f498c21b3595.
Fixes: 30edcb4d4e7a ("vfio-pci: preserve MSI")
Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260727150038.2684512-1-tycho@kernel.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index fd73a0fdf87..5e281b2e1d9 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -589,7 +589,7 @@ static void vfio_connect_kvm_msi_virq(VFIOMSIVector *vector, int nr)
{
const char *name = "kvm_interrupt";
- if (vector->virq < 0) {
+ if (!vector->use || vector->virq < 0) {
return;
}
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 06/28] vfio/pci: Reject invalid MSI-X Table and PBA BIR values
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (4 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 05/28] hw/vfio: Fix liveness check in vfio_connect_kvm_msi_virq() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:30 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 07/28] vfio/pci: Fix information leak in vfio_rom_read() Andrey Drobyshev
` (21 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Cédric Le Goater <clg@redhat.com>
The 3-bit MSI-X BIR fields permit values 0-7, but VFIOPCIDevice::bars[]
only contains BAR0-BAR5 (6 entries). An invalid BIR value of 6 or 7
can cause an out-of-bounds array access (CWE-129) in vfio_msix_early_setup().
Add range checks immediately after extracting the BIR values.
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit 9daa7347ff25.
Reported-by: Feifan Qian <bea1e@proton.me>
Fixes: 65501a745dba ("vfio: vfio-pci device assignment driver")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3878
Reviewed-by: Alex Williamson <alex@shazbot.org>
Link: https://lore.kernel.org/qemu-devel/20260706161334.2165482-1-clg@redhat.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5e281b2e1d9..d930ef46309 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1791,6 +1791,14 @@ static bool vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
+ if (msix->table_bar >= ARRAY_SIZE(vdev->bars) ||
+ msix->pba_bar >= ARRAY_SIZE(vdev->bars)) {
+ error_setg(errp, "invalid MSI-X BIR, table_bar=%d pba_bar=%d",
+ msix->table_bar, msix->pba_bar);
+ g_free(msix);
+ return false;
+ }
+
ret = vfio_device_get_irq_info(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX,
&irq_info);
if (ret < 0) {
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 07/28] vfio/pci: Fix information leak in vfio_rom_read()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (5 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 06/28] vfio/pci: Reject invalid MSI-X Table and PBA BIR values Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:31 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 08/28] vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom() Andrey Drobyshev
` (20 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Cédric Le Goater <clg@redhat.com>
vfio_rom_read() can leak uninitialized stack data to the guest when a
read occurs at the end boundary of the ROM BAR.
Fix this by initializing the val union to 0xff, matching the ROM
initialization pattern used in vfio_pci_load_rom().
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit 402fd5a75659.
Reviewed-by: Philippe Mathieu-Daudé <philmd@mailo.com>
Reviewed-by: Mario Casquero <mcasquer@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260602074446.1206892-1-clg@redhat.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d930ef46309..fa2ca0d779d 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1153,7 +1153,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
uint16_t word;
uint32_t dword;
uint64_t qword;
- } val;
+ } val = { .qword = ~0ULL };
uint64_t data = 0;
/* Load the ROM lazily when the guest tries to read it */
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 08/28] vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (6 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 07/28] vfio/pci: Fix information leak in vfio_rom_read() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:32 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 09/28] vfio/pci: Grow buffer in vfio_pci_host_match() Andrey Drobyshev
` (19 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Mario Casquero <mcasquer@redhat.com>
When vfio_device_get_region_info() fails in vfio_pci_load_rom(),
the function returns without setting vdev->rom_read_failed to true,
and without allocating vdev->rom. This leaves vdev->rom as NULL.
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit c822ecd80661.
Signed-off-by: Mario Casquero <mcasquer@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260602070857.356526-1-mcasquer@redhat.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index fa2ca0d779d..bea478d521f 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1039,7 +1039,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
}
}
-static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
+static bool vfio_pci_load_rom(VFIOPCIDevice *vdev)
{
VFIODevice *vbasedev = &vdev->vbasedev;
struct vfio_region_info *reg_info = NULL;
@@ -1053,7 +1053,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
if (ret != 0) {
error_report("vfio: Error getting ROM info: %s", strerror(-ret));
- return;
+ return false;
}
trace_vfio_pci_load_rom(vbasedev->name, (unsigned long)reg_info->size,
@@ -1064,12 +1064,11 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
vdev->rom_offset = reg_info->offset;
if (!vdev->rom_size) {
- vdev->rom_read_failed = true;
error_report("vfio-pci: Cannot read device rom at %s", vbasedev->name);
error_printf("Device option ROM contents are probably invalid "
"(check dmesg).\nSkip option ROM probe with rombar=0, "
"or load from file with romfile=\n");
- return;
+ return false;
}
vdev->rom = g_malloc(size);
@@ -1125,6 +1124,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
data[6] = -csum;
}
}
+
+ return true;
}
/* "Raw" read of underlying config space. */
@@ -1158,7 +1159,7 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr addr, unsigned size)
/* Load the ROM lazily when the guest tries to read it */
if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
- vfio_pci_load_rom(vdev);
+ vdev->rom_read_failed = !vfio_pci_load_rom(vdev);
}
memcpy(&val, vdev->rom + addr,
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 09/28] vfio/pci: Grow buffer in vfio_pci_host_match()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (7 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 08/28] vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:31 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 10/28] q35: Fix migration of SMRAM state Andrey Drobyshev
` (18 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Each field of PCIHostDeviceAddress is an unsigned int, therefore
while a valid address is limited to 13 characters, an invalid
address could exceed the specified format, up to:
ffffffff:ffffffff:ffffffff.ffffffff<NUL>
This requires 36 characters with the terminator.
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit 1785bf9685ec.
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Reviewed-by: Alex Williamson <alex.williamson@nvidia.com>
Message-ID: <20260305-nvme-v4-2-b65b9de1839f@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/vfio/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index bea478d521f..50498148275 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2691,7 +2691,7 @@ void vfio_pci_post_reset(VFIOPCIDevice *vdev)
bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
{
- char tmp[13];
+ char tmp[36];
sprintf(tmp, "%04x:%02x:%02x.%1x", addr->domain,
addr->bus, addr->slot, addr->function);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 10/28] q35: Fix migration of SMRAM state
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (8 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 09/28] vfio/pci: Grow buffer in vfio_pci_host_match() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:33 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 11/28] migration: ensure APIC is loaded prior to VFIO PCI devices Andrey Drobyshev
` (17 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Igor Mammedov <imammedo@redhat.com>
When migrating, dst QEMU by default has SMRAM unlocked,
and since wmask is not migrated, the migrated value of
MCH_HOST_BRIDGE_F_SMBASE in config space fall to prey of
mch_update_smbase_smram()
...
if (pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] == 0xff) {
*reg = 0x00;
and is getting cleared and leads to unlocked smram
on dst even if on source it's been locked.
As Andrey has pointed out [1], we should derive wmask
from config and not other way around.
Drop offending chunk and resync wmask based on MCH_HOST_BRIDGE_F_SMBASE
register value. That would preserve the register during
migration and set smram regions into corresponding state.
What that changes is:
that it would let guest write junk values in register
(with no apparent effect) until it's stumbles upon
reserved 0x1 [|] 0x2 values, at which point it
would be only possible to lock register and trigger
switch to SMRAM blackhole in CPU AS.
While at it, fix up test by removing junk discard before negotiation hunk.
PS2:
Instead of adding a dedicated post_load handler for it,
reuse mch_update->mch_update_smbase_smram call chain
that is called on write/reset/post_load to be consistent
with how we handle mch registers.
PS3:
for prosterity here is erro message Andrey got due to this bug:
qemu: vfio_container_dma_map(0x..., 0x0, 0xa0000, 0x....) = -22 (Invalid argument)
qemu: hardware error: vfio: DMA mapping failed, unable to continue
1) https://patchew.org/QEMU/20251203180851.6390-1-arbn@yandex-team.com/
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit 66cf169e29b0.
Fixes: f404220e279c ("q35: implement 128K SMRAM at default SMBASE address")
Reported-by: Andrey Ryabinin <arbn@yandex-team.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Andrey Ryabinin <arbn@yandex-team.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20251211165454.288476-1-imammedo@redhat.com>
---
hw/pci-host/q35.c | 25 +++++++++++--------------
tests/qtest/q35-test.c | 6 ------
2 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index a708758d361..946342ba585 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -432,30 +432,27 @@ static void mch_update_smbase_smram(MCHPCIState *mch)
}
if (*reg == MCH_HOST_BRIDGE_F_SMBASE_QUERY) {
- pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] =
- MCH_HOST_BRIDGE_F_SMBASE_LCK;
+ pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] = MCH_HOST_BRIDGE_F_SMBASE_LCK;
*reg = MCH_HOST_BRIDGE_F_SMBASE_IN_RAM;
return;
}
/*
- * default/reset state, discard written value
- * which will disable SMRAM balackhole at SMBASE
+ * reg value can come from register write/reset/migration source,
+ * update wmask to be in sync with it regardless of source
*/
- if (pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] == 0xff) {
- *reg = 0x00;
+ if (*reg == MCH_HOST_BRIDGE_F_SMBASE_IN_RAM) {
+ pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] = MCH_HOST_BRIDGE_F_SMBASE_LCK;
+ return;
}
-
- memory_region_transaction_begin();
if (*reg & MCH_HOST_BRIDGE_F_SMBASE_LCK) {
- /* disable all writes */
- pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] &=
- ~MCH_HOST_BRIDGE_F_SMBASE_LCK;
+ /* lock register at 0x2 and disable all writes */
+ pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] = 0;
*reg = MCH_HOST_BRIDGE_F_SMBASE_LCK;
- lck = true;
- } else {
- lck = false;
}
+
+ lck = *reg & MCH_HOST_BRIDGE_F_SMBASE_LCK;
+ memory_region_transaction_begin();
memory_region_set_enabled(&mch->smbase_blackhole, lck);
memory_region_set_enabled(&mch->smbase_window, lck);
memory_region_transaction_commit();
diff --git a/tests/qtest/q35-test.c b/tests/qtest/q35-test.c
index 62fff49fc89..4e3a4457f62 100644
--- a/tests/qtest/q35-test.c
+++ b/tests/qtest/q35-test.c
@@ -206,12 +206,6 @@ static void test_smram_smbase_lock(void)
qtest_writeb(qts, SMBASE, SMRAM_TEST_PATTERN);
g_assert_cmpint(qtest_readb(qts, SMBASE), ==, SMRAM_TEST_PATTERN);
- /* check that writing junk to 0x9c before before negotiating is ignored */
- for (i = 0; i < 0xff; i++) {
- qpci_config_writeb(pcidev, MCH_HOST_BRIDGE_F_SMBASE, i);
- g_assert(qpci_config_readb(pcidev, MCH_HOST_BRIDGE_F_SMBASE) == 0);
- }
-
/* enable SMRAM at SMBASE */
qpci_config_writeb(pcidev, MCH_HOST_BRIDGE_F_SMBASE, 0xff);
g_assert(qpci_config_readb(pcidev, MCH_HOST_BRIDGE_F_SMBASE) == 0x01);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 10/28] q35: Fix migration of SMRAM state
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 10/28] q35: Fix migration of SMRAM state Andrey Drobyshev
@ 2026-10-16 15:33 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:33 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
> From: Igor Mammedov <imammedo@redhat.com>
>
> When migrating, dst QEMU by default has SMRAM unlocked,
> and since wmask is not migrated, the migrated value of
> MCH_HOST_BRIDGE_F_SMBASE in config space fall to prey of
>
> mch_update_smbase_smram()
> ...
> if (pd->wmask[MCH_HOST_BRIDGE_F_SMBASE] == 0xff) {
> *reg = 0x00;
>
> and is getting cleared and leads to unlocked smram
> on dst even if on source it's been locked.
>
> As Andrey has pointed out [1], we should derive wmask
> [ ... ]
> Reported-by: Andrey Ryabinin <arbn@yandex-team.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Andrey Ryabinin <arbn@yandex-team.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=10
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 11/28] migration: ensure APIC is loaded prior to VFIO PCI devices
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (9 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 10/28] q35: Fix migration of SMRAM state Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:32 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 12/28] migration: Fix regression of passing error_fatal into vmstate_load_state() Andrey Drobyshev
` (16 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Yanfei Xu <yanfei.xu@bytedance.com>
The load procedure of VFIO PCI devices involves setting up IRT
for each VFIO PCI devices. This requires determining whether an
interrupt is single-destination interrupt to decide between
Posted Interrupt(PI) or remapping mode for the IRTE. However,
determining this may require accessing the VM's APIC registers.
For example:
ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irqs)
...
kvm_arch_irq_bypass_add_producer
kvm_x86_call(pi_update_irte)
vmx_pi_update_irte
kvm_intr_is_single_vcpu
If the LAPIC has not been loaded yet, interrupts will use remapping
mode. To prevent the fallback of interrupt mode, keep APIC is always
loaded prior to VFIO PCI devices.
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit d943cef76090.
Signed-off-by: Yicong Shen <shenyicong.1023@bytedance.com>
Signed-off-by: Yanfei Xu <yanfei.xu@bytedance.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20250818131127.1021648-1-yanfei.xu@bytedance.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/intc/apic_common.c | 1 +
include/migration/vmstate.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 37a7a7019d3..394fe020134 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -379,6 +379,7 @@ static const VMStateDescription vmstate_apic_common = {
.pre_load = apic_pre_load,
.pre_save = apic_dispatch_pre_save,
.post_load = apic_dispatch_post_load,
+ .priority = MIG_PRI_APIC,
.fields = (const VMStateField[]) {
VMSTATE_UINT32(apicbase, APICCommonState),
VMSTATE_UINT8(id, APICCommonState),
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 6839117be87..c3d156c514f 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -163,6 +163,7 @@ typedef enum {
MIG_PRI_IOMMU, /* Must happen before PCI devices */
MIG_PRI_PCI_BUS, /* Must happen before IOMMU */
MIG_PRI_VIRTIO_MEM, /* Must happen before IOMMU */
+ MIG_PRI_APIC, /* Must happen before PCI devices */
MIG_PRI_GICV3_ITS, /* Must happen before PCI devices */
MIG_PRI_GICV3, /* Must happen before the ITS */
MIG_PRI_MAX,
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 12/28] migration: Fix regression of passing error_fatal into vmstate_load_state()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (10 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 11/28] migration: ensure APIC is loaded prior to VFIO PCI devices Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:35 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 13/28] migration: vmstate_save_state_v: fix double error_setg Andrey Drobyshev
` (15 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Arun Menon <armenon@redhat.com>
error_fatal is passed to vmstate_load_state() and vmstate_save_state()
functions. This was introduced in commit c632ffbd74. This would exit(1)
on error, and therefore does not allow to propagate the error back to
the caller.
To maintain consistency with prior error handling i.e. either propagating
the error to the caller or reporting it, we must set the error within a
local Error object instead of using error_fatal.
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Link: https://lore.kernel.org/r/20251028-solve_error_fatal_regression-v2-1-dab24c808a28@redhat.com
[peterx: always uninit var ret, per Akihiko]
[peterx: touchups on line ordering, spacings etc.]
andrey.drobyshev:
This commit was cherry-picked from the upstream commit 986c3292c6b8,
adapted for the hci-dev base, which predates the upstream commit
bcb536cabe10 ("error: Kill @error_warn"): virtio_gpu_load() has no
'err' local there yet, so it is declared here.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/display/virtio-gpu.c | 22 +++++++++++++++-------
hw/pci/pci.c | 15 +++++++++++++--
hw/s390x/virtio-ccw.c | 17 +++++++++++++++--
hw/scsi/spapr_vscsi.c | 10 ++++++++--
hw/virtio/virtio-mmio.c | 15 +++++++++++++--
hw/virtio/virtio-pci.c | 15 +++++++++++++--
hw/virtio/virtio.c | 10 +++++++---
7 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 477ec700a13..4bc8c778508 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1223,7 +1223,8 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
{
VirtIOGPU *g = opaque;
struct virtio_gpu_simple_resource *res;
- int i;
+ Error *err = NULL;
+ int i, ret;
/* in 2d mode we should never find unprocessed commands here */
assert(QTAILQ_EMPTY(&g->cmdq));
@@ -1246,8 +1247,12 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
}
qemu_put_be32(f, 0); /* end of list */
- return vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL,
- &error_fatal);
+ ret = vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL,
+ &err);
+ if (ret < 0) {
+ error_report_err(err);
+ }
+ return ret;
}
static bool virtio_gpu_load_restore_mapping(VirtIOGPU *g,
@@ -1283,9 +1288,10 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
const VMStateField *field)
{
VirtIOGPU *g = opaque;
+ Error *err = NULL;
struct virtio_gpu_simple_resource *res;
uint32_t resource_id, pformat;
- int i;
+ int i, ret;
g->hostmem = 0;
@@ -1344,9 +1350,11 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
}
/* load & apply scanout state */
- vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1, &error_fatal);
-
- return 0;
+ ret = vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1, &err);
+ if (ret < 0) {
+ error_report_err(err);
+ }
+ return ret;
}
static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 5d7d17f6529..50f062045a9 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -926,21 +926,32 @@ const VMStateDescription vmstate_pci_device = {
void pci_device_save(PCIDevice *s, QEMUFile *f)
{
+ Error *local_err = NULL;
+ int ret;
+
/* Clear interrupt status bit: it is implicit
* in irq_state which we are saving.
* This makes us compatible with old devices
* which never set or clear this bit. */
s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
- vmstate_save_state(f, &vmstate_pci_device, s, NULL, &error_fatal);
+ ret = vmstate_save_state(f, &vmstate_pci_device, s, NULL, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
/* Restore the interrupt status bit. */
pci_update_irq_status(s);
}
int pci_device_load(PCIDevice *s, QEMUFile *f)
{
+ Error *local_err = NULL;
int ret;
+
ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id,
- &error_fatal);
+ &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
/* Restore the interrupt status bit. */
pci_update_irq_status(s);
return ret;
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 4cb1ced001a..4a3ffb84f8f 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1130,13 +1130,26 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f)
static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f)
{
VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
- vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL, &error_fatal);
+ Error *local_err = NULL;
+ int ret;
+
+ ret = vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
}
static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
{
VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
- return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1, &error_fatal);
+ Error *local_err = NULL;
+ int ret;
+
+ ret = vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
+ return ret;
}
static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index f0a7dd2b882..a6591319db7 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -628,10 +628,16 @@ static const VMStateDescription vmstate_spapr_vscsi_req = {
static void vscsi_save_request(QEMUFile *f, SCSIRequest *sreq)
{
vscsi_req *req = sreq->hba_private;
- assert(req->active);
+ Error *local_err = NULL;
+ int rc;
- vmstate_save_state(f, &vmstate_spapr_vscsi_req, req, NULL, &error_fatal);
+ assert(req->active);
+ rc = vmstate_save_state(f, &vmstate_spapr_vscsi_req, req, NULL, &local_err);
+ if (rc < 0) {
+ error_report_err(local_err);
+ return;
+ }
trace_spapr_vscsi_save_request(req->qtag, req->cur_desc_num,
req->cur_desc_offset);
}
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index fb58c364527..c05c00bcd4a 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -612,15 +612,26 @@ static const VMStateDescription vmstate_virtio_mmio = {
static void virtio_mmio_save_extra_state(DeviceState *opaque, QEMUFile *f)
{
VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+ Error *local_err = NULL;
+ int ret;
- vmstate_save_state(f, &vmstate_virtio_mmio, proxy, NULL, &error_fatal);
+ ret = vmstate_save_state(f, &vmstate_virtio_mmio, proxy, NULL, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
}
static int virtio_mmio_load_extra_state(DeviceState *opaque, QEMUFile *f)
{
VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+ Error *local_err = NULL;
+ int ret;
- return vmstate_load_state(f, &vmstate_virtio_mmio, proxy, 1, &error_fatal);
+ ret = vmstate_load_state(f, &vmstate_virtio_mmio, proxy, 1, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
+ return ret;
}
static bool virtio_mmio_has_extra_state(DeviceState *opaque)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 7273da4c018..c276fbb017c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -189,15 +189,26 @@ static bool virtio_pci_has_extra_state(DeviceState *d)
static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+ Error *local_err = NULL;
+ int ret;
- vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL, &error_fatal);
+ ret = vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
}
static int virtio_pci_load_extra_state(DeviceState *d, QEMUFile *f)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+ Error *local_err = NULL;
+ int ret;
- return vmstate_load_state(f, &vmstate_virtio_pci, proxy, 1, &error_fatal);
+ ret = vmstate_load_state(f, &vmstate_virtio_pci, proxy, 1, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
+ return ret;
}
static void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index a7063d5555e..d4b2e67c274 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3016,7 +3016,7 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
uint32_t guest_features_lo = (vdev->guest_features & 0xffffffff);
- int i;
+ int i, ret;
Error *local_err = NULL;
if (k->save_config) {
@@ -3061,7 +3061,7 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
}
if (vdc->vmsd) {
- int ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL, &local_err);
+ ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL, &local_err);
if (ret) {
error_report_err(local_err);
return ret;
@@ -3069,7 +3069,11 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
}
/* Subsections */
- return vmstate_save_state(f, &vmstate_virtio, vdev, NULL, &error_fatal);
+ ret = vmstate_save_state(f, &vmstate_virtio, vdev, NULL, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
+ return ret;
}
/* A wrapper for use as a VMState .put function */
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 12/28] migration: Fix regression of passing error_fatal into vmstate_load_state()
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 12/28] migration: Fix regression of passing error_fatal into vmstate_load_state() Andrey Drobyshev
@ 2026-10-16 15:35 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:35 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] migration: Error leaked when vmstate hooks return positive ret (ret < 0 check)
- [Low] migration: void vmstate save wrappers only report errors and leave QEMUFile unmarked
--
commit e28996c9e3a0b9aff33ccbdf09f181f304ec549c
Author: Arun Menon <armenon@redhat.com>
migration: Fix regression of passing error_fatal into vmstate_load_state()
This commit replaces error_fatal with a local Error object in the callers of
vmstate_load_state() and vmstate_save_state(). Failures are now reported with
error_report_err() and, where the caller can return a value, propagated.
Link: https://lore.kernel.org/r/20251028-solve_error_fatal_regression-v2-1-dab24c808a28@redhat.com
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
[ ... ]
> void pci_device_save(PCIDevice *s, QEMUFile *f)
> {
> + Error *local_err = NULL;
> + int ret;
> +
> /* Clear interrupt status bit: it is implicit
> * in irq_state which we are saving.
> * This makes us compatible with old devices
> * which never set or clear this bit. */
> s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
> - vmstate_save_state(f, &vmstate_pci_device, s, NULL, &error_fatal);
> + ret = vmstate_save_state(f, &vmstate_pci_device, s, NULL, &local_err);
> + if (ret < 0) {
> + error_report_err(local_err);
> + }
> /* Restore the interrupt status bit. */
> pci_update_irq_status(s);
> }
[Severity: Low]
pci_device_save() is a void function. It only prints the error and carries
on to pci_update_irq_status().
vmstate_save_state_v() does not call qemu_file_set_error() on failure, so
the QEMUFile is not marked as failed here.
Can a failed nested save (a nonzero pre_save, put or post_save return) in
pci_device_save() leave a truncated device section in the stream while the
save still looks successful?
The same pattern is present in virtio_ccw_save_config(),
virtio_mmio_save_extra_state() and virtio_pci_save_extra_state().
[ ... ]
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
[ ... ]
> + rc = vmstate_save_state(f, &vmstate_spapr_vscsi_req, req, NULL, &local_err);
> + if (rc < 0) {
> + error_report_err(local_err);
> + return;
> + }
[Severity: Low]
In vscsi_save_request(), the error is printed and then dropped. The
QEMUFile is not marked failed, and the caller cannot tell that the request
was not saved.
Should the failure be recorded on the stream, for example with
qemu_file_set_error(), so that this and the other void save helpers do not
hide it?
The commit message says the change is meant to keep "either propagating the
error to the caller or reporting it". That holds for the load paths,
virtio_gpu_save() and virtio_save(), but not for these void helpers.
[ ... ]
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
[ ... ]
> /* Subsections */
> - return vmstate_save_state(f, &vmstate_virtio, vdev, NULL, &error_fatal);
> + ret = vmstate_save_state(f, &vmstate_virtio, vdev, NULL, &local_err);
> + if (ret < 0) {
> + error_report_err(local_err);
> + }
> + return ret;
> }
[Severity: Low]
The earlier vdc->vmsd call in virtio_save() tests "if (ret)". This new call
tests "ret < 0".
vmstate_save_state_v() calls error_setg(errp, ...) for any nonzero return
from pre_save, and vmstate_load_state() does the same for any nonzero
pre_load return. The load side also returns early on a nonzero
vmstate_subsection_load() result.
If one of those hooks returns a positive value, local_err is set but never
passed to error_report_err(). Can that leak the Error and leave the failure
unreported?
The same "ret < 0" check is used in pci_device_save(), pci_device_load(),
virtio_ccw_save_config(), virtio_ccw_load_config(),
virtio_mmio_save_extra_state(), virtio_mmio_load_extra_state(),
virtio_pci_save_extra_state(), virtio_pci_load_extra_state(),
virtio_gpu_save(), virtio_gpu_load() and vscsi_save_request().
I did not find an in-tree hook that returns a positive value. This looks
like a conditional problem, because hooks conventionally return negative
errno. Would testing "if (ret)" here, as the vdc->vmsd call does, be more
consistent?
The follow-up commit e68e95a3b426 ("migration: vmstate_save_state_v: fix
double error_setg") does not change this. The nonzero-return error_setg()
calls in vmstate.c are unchanged, and the call sites still use "ret < 0".
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=12
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 13/28] migration: vmstate_save_state_v: fix double error_setg
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (11 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 12/28] migration: Fix regression of passing error_fatal into vmstate_load_state() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:38 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 14/28] migration: yank: Move register instance earlier Andrey Drobyshev
` (14 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
We may call error_setg twice on same errp if inner
vmstate_save_state_v() or vmstate_save_state() call fails. Next we will
crash on assertion in error_setv().
andrey.drobyshev:
This commit is cherry-picked as-is, with no modifications, from the
upstream commit d41ce10d0f5a.
Fixes: 848a0503422d043 "migration: Update error description outside migration.c"
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20260304212303.667141-2-vsementsov@yandex-team.ru
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index aea41c09946..04903131257 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -523,6 +523,9 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
} else {
ret = inner_field->info->put(f, curr_elem, size,
inner_field, vmdesc_loop);
+ if (ret < 0) {
+ error_setg(errp, "put failed");
+ }
}
written_bytes = qemu_file_transferred(f) - old_offset;
@@ -535,8 +538,8 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
}
if (ret) {
- error_setg(errp, "Save of field %s/%s failed",
- vmsd->name, field->name);
+ error_prepend(errp, "Save of field %s/%s failed: ",
+ vmsd->name, field->name);
if (vmsd->post_save) {
vmsd->post_save(opaque);
}
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 13/28] migration: vmstate_save_state_v: fix double error_setg
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 13/28] migration: vmstate_save_state_v: fix double error_setg Andrey Drobyshev
@ 2026-10-16 15:38 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:38 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] migration: vmstate_save_state_v() sets Error only for ret < 0 but prepends for any ret != 0
--
commit e68e95a3b4262b7b5d21222eb858ca4b5d64cc07
Author: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
migration: vmstate_save_state_v: fix double error_setg
This patch avoids calling error_setg() twice on the same errp in
vmstate_save_state_v(). It adds error_setg() after a failing put()
callback, and it changes the later "Save of field" error_setg() to
error_prepend().
Link: https://lore.kernel.org/qemu-devel/20260304212303.667141-2-vsementsov@yandex-team.ru
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index aea41c099463..04903131257b 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
[ ... ]
> @@ -523,6 +523,9 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> } else {
> ret = inner_field->info->put(f, curr_elem, size,
> inner_field, vmdesc_loop);
> + if (ret < 0) {
> + error_setg(errp, "put failed");
> + }
> }
>
> written_bytes = qemu_file_transferred(f) - old_offset;
> @@ -535,8 +538,8 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> }
>
> if (ret) {
> - error_setg(errp, "Save of field %s/%s failed",
> - vmsd->name, field->name);
> + error_prepend(errp, "Save of field %s/%s failed: ",
> + vmsd->name, field->name);
> if (vmsd->post_save) {
> vmsd->post_save(opaque);
> }
[Severity: Medium]
Can error_prepend() dereference a NULL *errp here?
The new error_setg(errp, "put failed") after put() is guarded by
"ret < 0", but the failure branch below is guarded by "if (ret)". For a
positive non-zero ret, nothing sets *errp before:
if (ret) {
error_prepend(errp, "Save of field %s/%s failed: ", ...);
and error_vprepend() only checks !errp before using the message:
util/error.c:error_vprepend() {
...
if (!errp) {
return;
}
newmsg = g_string_new(NULL);
g_string_vprintf(newmsg, fmt, ap);
g_string_append(newmsg, (*errp)->msg);
...
}
Before this patch every non-zero ret went through error_setg(), so this
asymmetry is new.
A positive value could reach this point through the wrapper callbacks
that return the nested vmstate_save_state() result unchanged while
passing a local Error instead of the outer errp, for example:
migration/vmstate-types.c:put_tmp() {
...
ret = vmstate_save_state(f, vmsd, tmp, vmdesc, &local_err);
if (ret) {
error_report_err(local_err);
}
g_free(tmp);
return ret;
}
put_gtree() has the same shape and returns capsule.ret. The nested
vmstate_save_state_v() returns any non-zero pre_save or post_save result:
ret = vmsd->pre_save(opaque);
...
if (ret) {
error_setg(errp, "pre-save failed: %s", vmsd->name);
return ret;
}
so a VMStateDescription whose pre_save or post_save returns a positive
value would leave the outer *errp unset and crash in error_prepend().
The same crash would also occur for a negative ret if errp were
&error_warn, since error_handle() leaves *errp NULL in that case. I did
not find a caller that passes error_warn.
I did not find an in-tree put() callback that returns a positive value
today, so this looks speculative, but I did not audit every pre_save,
post_save or target/ put callback. The later patches in the series do
not change migration/vmstate.c or migration/vmstate-types.c.
Would it make sense to make the two guards consistent, for example by
calling error_setg() for any non-zero ret, or by checking
"if (ret && !*errp)" before error_setg()? Using error_propagate_prepend()
semantics is another option.
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=13
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 14/28] migration: yank: Move register instance earlier
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (12 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 13/28] migration: vmstate_save_state_v: fix double error_setg Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:41 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 15/28] migration: Free cpr-transfer MigrationAddress along with gsource Andrey Drobyshev
` (13 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Fabiano Rosas <farosas@suse.de>
Move the register_instance call to migrate_prepare() so it can be
paired with the unregister_instance at migration_cleanup(). Otherwise,
the cleanup cannot be run when cpr_state_save() fails because the
instance is registered only after it.
When resuming from a paused postcopy migration, migrate_prepare()
returns early, but migration_cleanup() doesn't run, so the yank will
remain paired.
andrey.drobyshev:
This commit was cherry-picked from the upstream commit 7e714392437c,
adapted for the hci-dev base, which predates the upstream
migration_connect_set_error() -> migration_connect_error_propagate()
rename (77c82edadbe0): the error propagation calls in qmp_migrate()
and qmp_migrate_finish() keep the older spelling.
Also fixed here: upstream left the yank instance registered when
qmp_migrate_finish() fails before connecting (nothing calls
migration_cleanup() then), so the next migrate command aborted on
yank_register_instance(&error_abort); and the unregister at the out:
label ran on a resume, which never registered the instance. Upstream
fixed both in 6b587af5ec9d ("migration: Fold migration_cleanup() into
migration_connect_error_propagate()"), which needs the above rework;
this is the equivalent for our base.
Also refuse migrate while an incoming postcopy migration is still in
progress: the guest already runs then, but the incoming side still
holds the yank instance, so registering it with &error_abort would
abort QEMU.
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260123141656.6765-11-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 2f8ef20c036..0b830956532 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2174,6 +2174,15 @@ static bool migrate_prepare(MigrationState *s, bool resume, Error **errp)
return false;
}
+ /*
+ * The guest already runs during incoming postcopy, but the incoming
+ * migration still holds the yank instance registered below.
+ */
+ if (migration_incoming_postcopy_advised()) {
+ error_setg(errp, "Incoming postcopy migration is still in progress");
+ return false;
+ }
+
if (runstate_check(RUN_STATE_POSTMIGRATE)) {
error_setg(errp, "Can't migrate the vm that was paused due to "
"previous migration");
@@ -2235,11 +2244,12 @@ static bool migrate_prepare(MigrationState *s, bool resume, Error **errp)
return false;
}
+ yank_register_instance(MIGRATION_YANK_INSTANCE, &error_abort);
+
return true;
}
-static void qmp_migrate_finish(MigrationAddress *addr, bool resume_requested,
- Error **errp);
+static void qmp_migrate_finish(MigrationAddress *addr, Error **errp);
static void migrate_hup_add(MigrationState *s, QIOChannel *ioc, GSourceFunc cb,
void *opaque)
@@ -2264,7 +2274,7 @@ static gboolean qmp_migrate_finish_cb(QIOChannel *channel,
{
MigrationAddress *addr = opaque;
- qmp_migrate_finish(addr, false, NULL);
+ qmp_migrate_finish(addr, NULL);
cpr_state_close();
migrate_hup_delete(migrate_get_current());
@@ -2276,7 +2286,6 @@ void qmp_migrate(const char *uri, bool has_channels,
MigrationChannelList *channels, bool has_detach, bool detach,
bool has_resume, bool resume, Error **errp)
{
- bool resume_requested;
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
g_autoptr(MigrationChannel) channel = NULL;
@@ -2329,8 +2338,7 @@ void qmp_migrate(const char *uri, bool has_channels,
return;
}
- resume_requested = has_resume && resume;
- if (!migrate_prepare(s, resume_requested, errp)) {
+ if (!migrate_prepare(s, has_resume && resume, errp)) {
/* Error detected, put into errp */
return;
}
@@ -2354,27 +2362,26 @@ void qmp_migrate(const char *uri, bool has_channels,
QAPI_CLONE(MigrationAddress, addr));
} else {
- qmp_migrate_finish(addr, resume_requested, errp);
+ qmp_migrate_finish(addr, errp);
}
out:
if (local_err) {
+ /* A resume didn't register it, see migrate_prepare() */
+ if (s->state != MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP) {
+ yank_unregister_instance(MIGRATION_YANK_INSTANCE);
+ }
migration_connect_set_error(s, local_err);
error_propagate(errp, local_err);
}
}
-static void qmp_migrate_finish(MigrationAddress *addr, bool resume_requested,
- Error **errp)
+static void qmp_migrate_finish(MigrationAddress *addr, Error **errp)
{
MigrationState *s = migrate_get_current();
Error *local_err = NULL;
-
- if (!resume_requested) {
- if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
- return;
- }
- }
+ /* Before the start functions below change the state */
+ bool resume = s->state == MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP;
if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
SocketAddress *saddr = &addr->u.socket;
@@ -2401,7 +2408,8 @@ static void qmp_migrate_finish(MigrationAddress *addr, bool resume_requested,
}
if (local_err) {
- if (!resume_requested) {
+ /* Nothing connected: migration_cleanup() won't unregister it */
+ if (!resume) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
}
migration_connect_set_error(s, local_err);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 14/28] migration: yank: Move register instance earlier
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 14/28] migration: yank: Move register instance earlier Andrey Drobyshev
@ 2026-10-16 15:41 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:41 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] migration: stale incoming_postcopy_state makes migrate_prepare() refuse all later outgoing migrations
- [Medium] migration: migrate_prepare() registers yank with &error_abort, aborting QEMU if cleanup is pending
--
Commit ce0aafa8f0f4f6ebc0a21db9c39794339fc1b277
Author: Fabiano Rosas <farosas@suse.de>
migration: yank: Move register instance earlier
This commit moves yank_register_instance() into migrate_prepare() so it
pairs with the unregister in migration_cleanup(). It also adds a guard
that refuses a non-resume migrate while an incoming postcopy migration is
in progress, and reworks the error-path unregister logic in
qmp_migrate() and qmp_migrate_finish().
Link: https://lore.kernel.org/qemu-devel/20260123141656.6765-11-farosas@suse.de
> diff --git a/migration/migration.c b/migration/migration.c
> index 2f8ef20c0369..0b8309565323 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
[ ... ]
> @@ -2174,6 +2174,15 @@ static bool migrate_prepare(MigrationState *s, bool resume, Error **errp)
> return false;
> }
>
> + /*
> + * The guest already runs during incoming postcopy, but the incoming
> + * migration still holds the yank instance registered below.
> + */
> + if (migration_incoming_postcopy_advised()) {
> + error_setg(errp, "Incoming postcopy migration is still in progress");
> + return false;
> + }
> +
[Severity: High]
Can this guard reject every later outgoing migrate on a VM whose incoming
migration has already finished?
migration_incoming_postcopy_advised() evaluates:
PostcopyState ps = postcopy_state_get();
return ps >= POSTCOPY_INCOMING_ADVISE && ps < POSTCOPY_INCOMING_END;
Only the postcopy listen thread ever sets POSTCOPY_INCOMING_END. Consider
this sequence on the destination with postcopy-ram enabled:
loadvm_postcopy_handle_advise()
postcopy_state_set(POSTCOPY_INCOMING_ADVISE);
(the len == 0 case with !migrate_postcopy_ram() also returns 0
without resetting the state)
Precopy then completes without postcopy starting, so LISTEN and the
listen thread never run:
process_incoming_migration_co()
if (ps == POSTCOPY_INCOMING_ADVISE) {
postcopy_ram_incoming_cleanup(mis);
}
...
process_incoming_migration_bh()
migration_incoming_state_destroy() <- yank instance released here
postcopy_ram_incoming_cleanup() does not reset the state, and none of the
other postcopy_state_set() callers run on this path or on the ret < 0
fail: path. The state therefore stays at ADVISE (or DISCARD, LISTENING or
RUNNING after some failures).
After that, migration_incoming_postcopy_advised() stays true, and every
non-resume QMP migrate returns "Incoming postcopy migration is still in
progress", even though the incoming side no longer holds the yank
instance. Chained migration (A->B->C) looks like a realistic trigger.
The refusal appears to last until a new incoming migration starts or QEMU
is restarted.
I did not find anything later in the series that resets the state or
narrows this check.
Should the guard test whether the incoming side still holds the yank
instance, or should the postcopy state be reset when incoming migration
finishes or fails?
[ ... ]
> @@ -2235,11 +2244,12 @@ static bool migrate_prepare(MigrationState *s, bool resume, Error **errp)
> return false;
> }
>
> + yank_register_instance(MIGRATION_YANK_INSTANCE, &error_abort);
> +
> return true;
> }
[Severity: Medium]
Can this abort QEMU instead of returning a QMP error when the previous
outgoing migration's cleanup has not run yet?
The unregister is the last step of migration_cleanup(), which runs only
from the bottom half scheduled in migration_iteration_finish():
migration_thread()
s->state set to FAILED (or COMPLETED)
migration_iteration_finish()
bql_lock();
...
migration_bh_schedule(migration_cleanup_bh, s);
bql_unlock();
Between the terminal state change and the bottom half running, the main
loop can process a QMP migrate command. In that window
migration_is_running() returns false, since FAILED, CANCELLED and
COMPLETED fall into its default case:
case MIGRATION_STATUS_CANCELLING:
case MIGRATION_STATUS_COLO:
return true;
default:
return false;
The RUN_STATE_POSTMIGRATE check does not catch it either, as that is set
later in migration_iteration_finish(). migrate_init() does not look at a
pending cleanup. So migrate_prepare() can reach:
yank_register_instance(MIGRATION_YANK_INSTANCE, &error_abort);
while the old instance is still registered. yank_register_instance() then
does error_setg(errp, "duplicate yank instance"), which with &error_abort
calls abort().
Before this patch the registration in qmp_migrate_finish() used the
caller's errp, so the same race produced a QMP error.
The commit message adds a guard for the incoming postcopy case for this
reason, but nothing similar covers the outgoing cleanup-pending window.
The later patches through the series end do not change this.
Could this use a non-fatal errp and return an error from migrate_prepare()
instead, or could migrate_prepare() reject the request while cleanup is
still pending?
[ ... ]
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=14
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 15/28] migration: Free cpr-transfer MigrationAddress along with gsource
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (13 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 14/28] migration: yank: Move register instance earlier Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:39 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 16/28] migration: Move CPR HUP watch to cpr-transfer.c Andrey Drobyshev
` (12 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Fabiano Rosas <farosas@suse.de>
When setting a callback on a Glib source and giving it a data pointer,
it's natural to also provide the destructor for the data in question.
Since migrate_hup_add() already needs to clone the MigrationAddress
when setting the qmp_migrate_finish_cb callback, also pass the
qapi_free_MigrationAddress as the GDestroyNotify callback.
With this the address doesn't need to be freed at the callback body,
making the management of that memory slightly simpler.
andrey.drobyshev:
This commit was cherry-picked from the upstream commit 6a887119ce18,
adapted for the hci-dev base: qmp_migrate_finish_cb() here still
closes the cpr state and deletes the HUP source itself (upstream had
already moved that into migration_cleanup()), so only the explicit
qapi_free_MigrationAddress() call goes away from it.
Cc: Mark Kanda <mark.kanda@oracle.com>
Cc: Ben Chaney <bchaney@akamai.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260123141656.6765-23-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 0b830956532..9ac11d57d5b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2254,9 +2254,11 @@ static void qmp_migrate_finish(MigrationAddress *addr, Error **errp);
static void migrate_hup_add(MigrationState *s, QIOChannel *ioc, GSourceFunc cb,
void *opaque)
{
- s->hup_source = qio_channel_create_watch(ioc, G_IO_HUP);
- g_source_set_callback(s->hup_source, cb, opaque, NULL);
- g_source_attach(s->hup_source, NULL);
+ s->hup_source = qio_channel_create_watch(ioc, G_IO_HUP);
+ g_source_set_callback(s->hup_source, cb,
+ QAPI_CLONE(MigrationAddress, opaque),
+ (GDestroyNotify)qapi_free_MigrationAddress);
+ g_source_attach(s->hup_source, NULL);
}
static void migrate_hup_delete(MigrationState *s)
@@ -2278,7 +2280,6 @@ static gboolean qmp_migrate_finish_cb(QIOChannel *channel,
cpr_state_close();
migrate_hup_delete(migrate_get_current());
- qapi_free_MigrationAddress(addr);
return G_SOURCE_REMOVE;
}
@@ -2359,7 +2360,7 @@ void qmp_migrate(const char *uri, bool has_channels,
*/
if (migrate_mode() == MIG_MODE_CPR_TRANSFER) {
migrate_hup_add(s, cpr_state_ioc(), (GSourceFunc)qmp_migrate_finish_cb,
- QAPI_CLONE(MigrationAddress, addr));
+ addr);
} else {
qmp_migrate_finish(addr, errp);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 15/28] migration: Free cpr-transfer MigrationAddress along with gsource
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 15/28] migration: Free cpr-transfer MigrationAddress along with gsource Andrey Drobyshev
@ 2026-10-16 15:39 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:39 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
> From: Fabiano Rosas <farosas@suse.de>
>
> When setting a callback on a Glib source and giving it a data pointer,
> it's natural to also provide the destructor for the data in question.
>
> Since migrate_hup_add() already needs to clone the MigrationAddress
> when setting the qmp_migrate_finish_cb callback, also pass the
> qapi_free_MigrationAddress as the GDestroyNotify callback.
>
> With this the address doesn't need to be freed at the callback body,
> making the management of that memory slightly simpler.
>
> andrey.drobyshev:
> This commit was cherry-picked from the upstream commit 6a887119ce18,
> adapted for the hci-dev base: qmp_migrate_finish_cb() here still
> closes the cpr state and deletes the HUP source itself (upstream had
> already moved that into migration_cleanup()), so only the explicit
> qapi_free_MigrationAddress() call goes away from it.
>
> Cc: Mark Kanda <mark.kanda@oracle.com>
> Cc: Ben Chaney <bchaney@akamai.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
> Link: https://lore.kernel.org/qemu-devel/20260123141656.6765-23-farosas@suse.de
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=15
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 16/28] migration: Move CPR HUP watch to cpr-transfer.c
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (14 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 15/28] migration: Free cpr-transfer MigrationAddress along with gsource Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:40 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 17/28] migration: Fix crash on second migration when cancel early Andrey Drobyshev
` (11 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Fabiano Rosas <farosas@suse.de>
Move this CPR-specific code into a cpr file. While here, give the
functions more significant names.
This makes the new idea (after cpr-transfer) of having two parts to
qmp_migrate slightly more obvious: either wait for the hangup or
continue directly.
andrey.drobyshev:
This commit was cherry-picked from the upstream commit f523e5aa5f56,
adapted for the hci-dev base, which predates the upstream qmp_migrate()
rework around MigrationChannel: the watch is still armed with the
plain MigrationAddress from qmp_migrate() rather than main_ch->addr,
and qmp_migrate_finish_cb() keeps closing the cpr state and destroying
the HUP source itself.
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260123141656.6765-24-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/migration/cpr.h | 5 +++++
migration/cpr-transfer.c | 23 +++++++++++++++++++++++
migration/migration.c | 29 ++++-------------------------
3 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/include/migration/cpr.h b/include/migration/cpr.h
index 8fb79eb0a28..ae441322ff5 100644
--- a/include/migration/cpr.h
+++ b/include/migration/cpr.h
@@ -9,6 +9,7 @@
#define MIGRATION_CPR_H
#include "qapi/qapi-types-migration.h"
+#include "io/channel.h"
#include "qemu/queue.h"
#define MIG_MODE_NONE -1
@@ -76,6 +77,10 @@ int cpr_get_fd_param(const char *name, const char *fdname, int index, bool cpr,
QEMUFile *cpr_transfer_output(MigrationChannel *channel, Error **errp);
QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp);
+void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func,
+ void *opaque);
+void cpr_transfer_source_destroy(MigrationState *s);
+
void cpr_exec_init(void);
QEMUFile *cpr_exec_output(Error **errp);
QEMUFile *cpr_exec_input(Error **errp);
diff --git a/migration/cpr-transfer.c b/migration/cpr-transfer.c
index 00371d17c3a..61d5c9dce2b 100644
--- a/migration/cpr-transfer.c
+++ b/migration/cpr-transfer.c
@@ -6,7 +6,10 @@
*/
#include "qemu/osdep.h"
+#include "qapi/clone-visitor.h"
#include "qapi/error.h"
+#include "qapi/qapi-visit-migration.h"
+#include "io/channel.h"
#include "io/channel-file.h"
#include "io/channel-socket.h"
#include "io/net-listener.h"
@@ -72,3 +75,23 @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp)
return NULL;
}
}
+
+void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func,
+ void *opaque)
+{
+ s->hup_source = qio_channel_create_watch(cpr_state_ioc(), G_IO_HUP);
+ g_source_set_callback(s->hup_source,
+ (GSourceFunc)func,
+ QAPI_CLONE(MigrationAddress, opaque),
+ (GDestroyNotify)qapi_free_MigrationAddress);
+ g_source_attach(s->hup_source, NULL);
+}
+
+void cpr_transfer_source_destroy(MigrationState *s)
+{
+ if (s->hup_source) {
+ g_source_destroy(s->hup_source);
+ g_source_unref(s->hup_source);
+ s->hup_source = NULL;
+ }
+}
diff --git a/migration/migration.c b/migration/migration.c
index 9ac11d57d5b..c138fa40e10 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -110,7 +110,6 @@ static bool migration_object_check(MigrationState *ms, Error **errp);
static bool migration_switchover_start(MigrationState *s, Error **errp);
static bool close_return_path_on_source(MigrationState *s);
static void migration_completion_end(MigrationState *s);
-static void migrate_hup_delete(MigrationState *s);
static void migration_downtime_start(MigrationState *s)
{
@@ -1500,7 +1499,7 @@ static void migration_cleanup(MigrationState *s)
qemu_savevm_state_cleanup();
cpr_state_close();
- migrate_hup_delete(s);
+ cpr_transfer_source_destroy(s);
close_return_path_on_source(s);
@@ -1661,7 +1660,7 @@ void migration_cancel(void)
migrate_set_state(&s->state, MIGRATION_STATUS_CANCELLING,
MIGRATION_STATUS_CANCELLED);
cpr_state_close();
- migrate_hup_delete(s);
+ cpr_transfer_source_destroy(s);
}
}
@@ -2251,25 +2250,6 @@ static bool migrate_prepare(MigrationState *s, bool resume, Error **errp)
static void qmp_migrate_finish(MigrationAddress *addr, Error **errp);
-static void migrate_hup_add(MigrationState *s, QIOChannel *ioc, GSourceFunc cb,
- void *opaque)
-{
- s->hup_source = qio_channel_create_watch(ioc, G_IO_HUP);
- g_source_set_callback(s->hup_source, cb,
- QAPI_CLONE(MigrationAddress, opaque),
- (GDestroyNotify)qapi_free_MigrationAddress);
- g_source_attach(s->hup_source, NULL);
-}
-
-static void migrate_hup_delete(MigrationState *s)
-{
- if (s->hup_source) {
- g_source_destroy(s->hup_source);
- g_source_unref(s->hup_source);
- s->hup_source = NULL;
- }
-}
-
static gboolean qmp_migrate_finish_cb(QIOChannel *channel,
GIOCondition cond,
void *opaque)
@@ -2279,7 +2259,7 @@ static gboolean qmp_migrate_finish_cb(QIOChannel *channel,
qmp_migrate_finish(addr, NULL);
cpr_state_close();
- migrate_hup_delete(migrate_get_current());
+ cpr_transfer_source_destroy(migrate_get_current());
return G_SOURCE_REMOVE;
}
@@ -2359,8 +2339,7 @@ void qmp_migrate(const char *uri, bool has_channels,
* connection, so qmp_migrate_finish will fail to connect, and then recover.
*/
if (migrate_mode() == MIG_MODE_CPR_TRANSFER) {
- migrate_hup_add(s, cpr_state_ioc(), (GSourceFunc)qmp_migrate_finish_cb,
- addr);
+ cpr_transfer_add_hup_watch(s, qmp_migrate_finish_cb, addr);
} else {
qmp_migrate_finish(addr, errp);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 17/28] migration: Fix crash on second migration when cancel early
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (15 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 16/28] migration: Move CPR HUP watch to cpr-transfer.c Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:44 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 18/28] kvm: tolerate ioctls after kvm_close() Andrey Drobyshev
` (10 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
From: Peter Xu <peterx@redhat.com>
Marc-André reported an issue on QEMU crash when retrying a cancelled
migration during early setup phase, see "Link:" for more information, and
also easy way to reproduce.
This patch is a replacement of the prior fix proposed by not only switching
to migration_cleanup(), but also fixing it from CPR side, so that we track
hup_source properly to know if src QEMU is waiting or the HUP signal.
To put it simple: this chunk of special casing in migration_cancel() should
not affect normal migration, but only cpr-transfer migration to cover the
small window when the src QEMU is waiting for a HUP signal on cpr
channel (so that src QEMU can continue the migration on the main channel).
To achieve that, we'll also need to remember to detach the hup_source
whenenver invoked: after that point, we should always be able to cleanup
the migration.
It's not a generic operation to explicitly detach a gsource from its
context while in its dispatch() function. But it should be safe, because
gsource disptch() will only happen with a boosted refcount for the
dispatcher so that the gsource will not be freed until the callback
completes. It's also safe to return G_SOURCE_REMOVE after the gsource is
detached, as glib will simply ignore the G_SOURCE_REMOVE.
One can refer to latest 2.86.5 glib code in g_main_dispatch() for that:
https://github.com/GNOME/glib/blob/2.86.5/glib/gmain.c#L3592
When at this, add a bunch of assertions to make sure nothing surprises us.
After this patch applied, the 2nd migration will not crash QEMU, instead
it'll be in CANCELLING until the socket connection times out (it will take
~2min on my Fedora default kernel). During this process no 2nd migration
will be allowed, and after it timed out migration can be restarted.
It's because so far we don't have control over socket_connect_outgoing(),
or anything yet managed by a task executed in qio_task_run_in_thread().
Speeding up the cancellation to be left for future.
I also tested cpr-transfer by only providing cpr channel not the main
channel (with -incoming defer), kickoff migration on source, then cancel it
on source directly without providing the main channel. It keeps working.
I wanted to add an unit test for that but it'll need to refactor current
cpr-transfer tests first; let's leave it for later.
andrey.drobyshev:
This commit was cherry-picked from the upstream commit 4f08b870f8f0,
adapted for the hci-dev base, which predates the upstream qmp_migrate()
rework: the HUP callback is still qmp_migrate_finish_cb() rather than
migration_connect_outgoing_cb(), so the early
cpr_transfer_source_destroy() call lands there, ahead of
qmp_migrate_finish() and the cpr_state_close() the callback keeps
doing itself.
Link: https://lore.kernel.org/r/20260417184742.293061-1-marcandre.lureau@redhat.com
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20260421175820.302795-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/cpr.h | 1 +
migration/cpr-transfer.c | 10 ++++++++++
migration/migration.c | 32 ++++++++++++++++++++++++--------
migration/migration.h | 5 +++++
4 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/include/migration/cpr.h b/include/migration/cpr.h
index ae441322ff5..4dd397a7c13 100644
--- a/include/migration/cpr.h
+++ b/include/migration/cpr.h
@@ -80,6 +80,7 @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp);
void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func,
void *opaque);
void cpr_transfer_source_destroy(MigrationState *s);
+bool cpr_transfer_source_active(MigrationState *s);
void cpr_exec_init(void);
QEMUFile *cpr_exec_output(Error **errp);
diff --git a/migration/cpr-transfer.c b/migration/cpr-transfer.c
index 61d5c9dce2b..9defe7bad78 100644
--- a/migration/cpr-transfer.c
+++ b/migration/cpr-transfer.c
@@ -6,6 +6,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
#include "qapi/clone-visitor.h"
#include "qapi/error.h"
#include "qapi/qapi-visit-migration.h"
@@ -79,6 +80,7 @@ QEMUFile *cpr_transfer_input(MigrationChannel *channel, Error **errp)
void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func,
void *opaque)
{
+ assert(bql_locked());
s->hup_source = qio_channel_create_watch(cpr_state_ioc(), G_IO_HUP);
g_source_set_callback(s->hup_source,
(GSourceFunc)func,
@@ -89,9 +91,17 @@ void cpr_transfer_add_hup_watch(MigrationState *s, QIOChannelFunc func,
void cpr_transfer_source_destroy(MigrationState *s)
{
+ assert(bql_locked());
if (s->hup_source) {
g_source_destroy(s->hup_source);
g_source_unref(s->hup_source);
s->hup_source = NULL;
}
}
+
+bool cpr_transfer_source_active(MigrationState *s)
+{
+ /* Whenever the HUP gsource is available, it's active. */
+ assert(bql_locked());
+ return s->hup_source;
+}
diff --git a/migration/migration.c b/migration/migration.c
index c138fa40e10..f48e75e8fad 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1653,14 +1653,19 @@ void migration_cancel(void)
}
/*
- * If qmp_migrate_finish has not been called, then there is no path that
- * will complete the cancellation. Do it now.
+ * This is cpr-transfer specific processing.
+ *
+ * If this is true, it means cpr-transfer migration is waiting for the
+ * destination to send HUP event on CPR channel to continue the next
+ * phase. If so, do the cleanup proactively to avoid get stuck in
+ * CANCELLING state.
*/
- if (setup && !s->to_dst_file) {
- migrate_set_state(&s->state, MIGRATION_STATUS_CANCELLING,
- MIGRATION_STATUS_CANCELLED);
- cpr_state_close();
- cpr_transfer_source_destroy(s);
+ if (cpr_transfer_source_active(s)) {
+ assert(migrate_mode() == MIG_MODE_CPR_TRANSFER);
+ assert(setup && !s->to_dst_file);
+ migration_cleanup(s);
+ /* Now all things should have been released */
+ assert(!cpr_transfer_source_active(s));
}
}
@@ -2255,11 +2260,22 @@ static gboolean qmp_migrate_finish_cb(QIOChannel *channel,
void *opaque)
{
MigrationAddress *addr = opaque;
+ MigrationState *s = migrate_get_current();
+
+ /*
+ * Detach and release the GSource right after use. We rely on this to
+ * detect this small cpr-transfer window of "waiting for HUP event".
+ */
+ cpr_transfer_source_destroy(s);
qmp_migrate_finish(addr, NULL);
cpr_state_close();
- cpr_transfer_source_destroy(migrate_get_current());
+
+ /*
+ * This is redundant as we do cpr_transfer_source_destroy() at the
+ * entry, but it's benign; glib will just skip the detach.
+ */
return G_SOURCE_REMOVE;
}
diff --git a/migration/migration.h b/migration/migration.h
index a7ea43e925d..ee15ca88461 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -509,6 +509,11 @@ struct MigrationState {
/* Is this a rdma migration */
bool rdma_migration;
+ /*
+ * When set, it means cpr-transfer is waiting for the HUP signal from
+ * destination to continue the 2nd step of migration via the main
+ * channel.
+ */
GSource *hup_source;
};
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 17/28] migration: Fix crash on second migration when cancel early
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 17/28] migration: Fix crash on second migration when cancel early Andrey Drobyshev
@ 2026-10-16 15:44 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:44 UTC (permalink / raw)
To: svt-core, Andrey Drobyshev; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] migration: migration_cancel() asserts on mode that migrate-set-parameters can change
--
commit 1a45829d1f41fe592f4ff72499c683f6dac53858
Author: Peter Xu <peterx@redhat.com>
migration: Fix crash on second migration when cancel early
This commit changes migration_cancel() to call migration_cleanup() when
the cpr-transfer HUP gsource is still pending. The goal is to avoid a
crash on a second migration after an early cancel. It also clears
hup_source as soon as the HUP callback runs, and adds assertions.
Link: https://lore.kernel.org/r/20260417184742.293061-1-marcandre.lureau@redhat.com
Link: https://lore.kernel.org/r/20260421175820.302795-1-peterx@redhat.com
> diff --git a/migration/migration.c b/migration/migration.c
> --- a/migration/migration.c
> +++ b/migration/migration.c
[ ... ]
> @@ -1653,14 +1653,19 @@ void migration_cancel(void)
> }
>
> /*
> - * If qmp_migrate_finish has not been called, then there is no path that
> - * will complete the cancellation. Do it now.
> + * This is cpr-transfer specific processing.
> + *
> + * If this is true, it means cpr-transfer migration is waiting for the
> + * destination to send HUP event on CPR channel to continue the next
> + * phase. If so, do the cleanup proactively to avoid get stuck in
> + * CANCELLING state.
> */
> - if (setup && !s->to_dst_file) {
> - migrate_set_state(&s->state, MIGRATION_STATUS_CANCELLING,
> - MIGRATION_STATUS_CANCELLED);
> - cpr_state_close();
> - cpr_transfer_source_destroy(s);
> + if (cpr_transfer_source_active(s)) {
> + assert(migrate_mode() == MIG_MODE_CPR_TRANSFER);
[Severity: Medium]
Can this assert abort QEMU? migrate_mode() on the source falls back to
s->parameters.mode, because cpr_get_incoming_mode() returns
MIG_MODE_NONE there. That value can be changed while hup_source is still
set.
The sequence would be:
qmp_migrate() with mode=cpr-transfer
-> cpr_transfer_add_hup_watch()
(state stays SETUP, hup_source set until the destination closes
the CPR channel)
qmp_migrate_set_parameters() with mode=normal
-> migrate_params_check()
-> migrate_params_apply()
migrate_cancel, or migration_shutdown() at QEMU exit
-> migration_cancel()
-> cpr_transfer_source_active(s) is true
-> assert(migrate_mode() == MIG_MODE_CPR_TRANSFER) fails
As far as I can tell, neither migrate_params_check() nor the apply step
rejects a mode change while a migration is running or while hup_source
is set. The migration_is_running() check in options.c is in the
capabilities path, not the parameters path.
The wait for the destination to close the CPR channel has no upper bound,
so this window can stay open for a long time. Before this patch, the
cancel path in this window did not assert on the mode.
Would it make sense to reject a mode change while a migration is
running, or to drop the mode assertion here?
> + assert(setup && !s->to_dst_file);
> + migration_cleanup(s);
> + /* Now all things should have been released */
> + assert(!cpr_transfer_source_active(s));
> }
> }
[ ... ]
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=17
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 18/28] kvm: tolerate ioctls after kvm_close()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (16 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 17/28] migration: Fix crash on second migration when cancel early Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:44 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 19/28] kvm: fix coalesced MMIO flush racing with kvm_close() Andrey Drobyshev
` (9 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
Since commit 7ed0919119b0 ("migration: close kvm after cpr") a source
QEMU that has handed its VFIO devices over via cpr-transfer (or
cpr-exec) calls kvm_close() once the migration completes, so that the
destination can register its irq bypass consumers. kvm_close() closes
the VM and vCPU descriptors and sets kvm_state to NULL, but the source
process keeps living with all its device models intact until the
management layer terminates it.
Nothing prevents those device models from raising interrupts in the
meantime, and the regular exit path even guarantees one: net_cleanup()
deletes the netdev behind an emulated virtio-net, the NIC sees the link
go down and calls virtio_notify_config(), MSI-X delivers through the
in-kernel APIC, and kvm_irqchip_send_msi() dereferences the NULL
kvm_state:
#0 kvm_vm_ioctl (s=0x0, type=KVM_SIGNAL_MSI) accel/kvm/kvm-all.c
#1 kvm_irqchip_send_msi (s=0x0)
#2 kvm_send_msi hw/i386/kvm/apic.c
#3 kvm_apic_mem_write
...
#9 pci_msi_trigger
#11 msix_notify
#12 virtio_pci_notify
#14 virtio_notify_config
#15 virtio_net_set_link_status
#16 qemu_del_net_client
#17 net_cleanup
#18 qemu_cleanup
Every retired cpr-transfer source with a VFIO device thus dies with
SIGSEGV on SIGTERM/quit and leaves a core dump behind. Without a VFIO
device kvm_close() never runs and the same exit is harmless.
Make the ioctl wrappers honour the closed state instead of assuming a
live KVMState: kvm_ioctl(), kvm_vm_ioctl() and kvm_vcpu_ioctl() return
-ENODEV when kvm_close() has already been called, so any late interrupt
or query on the retired source fails gracefully. Teach kvm_send_msi()
that -ENODEV in this situation is expected and not worth the "MSI lost"
warning on every such interrupt.
Fixes: 7ed0919119b0 ("migration: close kvm after cpr")
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
accel/kvm/kvm-all.c | 15 +++++++++++++++
hw/i386/kvm/apic.c | 6 +++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index a39c1ba260a..ea027c37829 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3371,6 +3371,11 @@ int kvm_ioctl(KVMState *s, unsigned long type, ...)
arg = va_arg(ap, void *);
va_end(ap);
+ /* kvm_close() has been called: there is no KVM to talk to any more */
+ if (!s || s->fd == -1) {
+ return -ENODEV;
+ }
+
trace_kvm_ioctl(type, arg);
ret = ioctl(s->fd, type, arg);
if (ret == -1) {
@@ -3389,6 +3394,11 @@ int kvm_vm_ioctl(KVMState *s, unsigned long type, ...)
arg = va_arg(ap, void *);
va_end(ap);
+ /* kvm_close() has been called: there is no VM to talk to any more */
+ if (!s || s->vmfd == -1) {
+ return -ENODEV;
+ }
+
trace_kvm_vm_ioctl(type, arg);
accel_ioctl_begin();
ret = ioctl(s->vmfd, type, arg);
@@ -3409,6 +3419,11 @@ int kvm_vcpu_ioctl(CPUState *cpu, unsigned long type, ...)
arg = va_arg(ap, void *);
va_end(ap);
+ /* kvm_close() has been called: the vCPU fds are gone */
+ if (cpu->kvm_fd == -1) {
+ return -ENODEV;
+ }
+
trace_kvm_vcpu_ioctl(cpu->cpu_index, type, arg);
accel_cpu_ioctl_begin(cpu);
ret = ioctl(cpu->kvm_fd, type, arg);
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 1be9bfe36e9..0c9f7759b6e 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -196,7 +196,11 @@ static void kvm_send_msi(MSIMessage *msg)
msg->address = kvm_swizzle_msi_ext_dest_id(msg->address);
ret = kvm_irqchip_send_msi(kvm_state, *msg);
- if (ret < 0) {
+ /*
+ * -ENODEV means kvm_close() has already run (the VM was handed over
+ * via CPR); the interrupt is meaningless here and not worth a warning.
+ */
+ if (ret < 0 && ret != -ENODEV) {
fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n",
strerror(-ret));
}
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 18/28] kvm: tolerate ioctls after kvm_close()
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 18/28] kvm: tolerate ioctls after kvm_close() Andrey Drobyshev
@ 2026-10-16 15:44 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:44 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] kvm: late INTx via kvm_set_irq() still crashes after kvm_close()
- [Medium] kvm: kvm_set_ioeventfd_mmio() returns stale -errno on new -ENODEV path
- [Low] kvm: unsynchronized closed-state check in ioctl wrappers races with kvm_close()
--
commit 6d5d5f6faa5b401fc25d81071e7ee69ce2c8465b
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
kvm: tolerate ioctls after kvm_close()
This patch makes kvm_ioctl(), kvm_vm_ioctl() and kvm_vcpu_ioctl() return
-ENODEV once kvm_close() has run. It also stops kvm_send_msi() from
printing the "MSI lost" warning for that case. The goal is to stop the
retired cpr-transfer source from crashing on late interrupts.
> kvm: tolerate ioctls after kvm_close()
> Make the ioctl wrappers honour the closed state instead of assuming a
> live KVMState: kvm_ioctl(), kvm_vm_ioctl() and kvm_vcpu_ioctl() return
> -ENODEV when kvm_close() has already been called, so any late interrupt
> or query on the retired source fails gracefully.
The commit message says any late interrupt fails gracefully. Is that
accurate? Only the MSI path through kvm_send_msi() is fixed. The
findings below describe other late-interrupt paths that still crash.
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
[ ... ]
> @@ -3389,6 +3394,11 @@ int kvm_vm_ioctl(KVMState *s, unsigned long type, ...)
> arg = va_arg(ap, void *);
> va_end(ap);
>
> + /* kvm_close() has been called: there is no VM to talk to any more */
> + if (!s || s->vmfd == -1) {
> + return -ENODEV;
> + }
> +
> trace_kvm_vm_ioctl(type, arg);
> accel_ioctl_begin();
> ret = ioctl(s->vmfd, type, arg);
[Severity: Medium]
Can a late INTx or GSI line interrupt still crash after kvm_close()?
kvm_close() sets kvm_state to NULL, but kvm_kernel_irqchip and
kvm_async_interrupts_allowed stay true. A late line interrupt then takes
this path:
kvm_ioapic_set_irq() / kvm_pic_set_irq()
kvm_set_irq(kvm_state = NULL, ...)
kvm_vm_ioctl(s, s->irq_set_ioctl, &event)
The argument s->irq_set_ioctl is evaluated in kvm_set_irq() before
kvm_vm_ioctl() runs. With s == NULL it faults there, so the new "!s"
check is never reached.
If s were non-NULL but closed, the new -ENODEV would make this code take
the error branch:
ret = kvm_vm_ioctl(s, s->irq_set_ioctl, &event);
if (ret < 0) {
perror("kvm_set_irq");
abort();
}
That would turn the late interrupt into an abort() instead.
kvm_irqchip_commit_routes() has the same problem:
ret = kvm_vm_ioctl(s, KVM_SET_GSI_ROUTING, s->irq_routes);
assert(ret == 0);
The new -ENODEV would trip the assert.
I compared the end of the series against this patch for accel/kvm and
hw/i386/kvm. The only change there is the coalesced MMIO ring handling in
kvm_close(). Nothing guards kvm_set_irq(), the IOAPIC and PIC callbacks,
or the irqchip route helpers. Should those callers be taught about the
closed state as well?
[Severity: Low]
Is the new closed-state check safe against a concurrent kvm_close()?
The check is a plain, unlocked load of s->vmfd:
if (!s || s->vmfd == -1) {
return -ENODEV;
}
...
accel_ioctl_begin();
ret = ioctl(s->vmfd, type, arg);
It is not atomic with the later use of s->vmfd. accel_ioctl_begin()
returns immediately when the BQL is held and only takes the lockcnt for
non-BQL callers. kvm_close() does not call kvm_ioctl_inhibit_begin().
kvm_close() also closes the fd before it stores -1:
close(kvm_state->vmfd);
kvm_state->vmfd = -1;
Can a non-BQL thread pass the -1 check and then call ioctl() on a closed
or recycled descriptor? The likely result is a spurious -EBADF. A
recycled fd number could refer to another file.
I did not find a specific non-BQL caller that runs during kvm_close().
The wrappers just don't exclude one. The BQL-held callers and the vCPU
threads (stopped by cpu_remove_sync()) are covered.
The same applies to kvm_vcpu_ioctl() and cpu->kvm_fd:
> @@ -3409,6 +3419,11 @@ int kvm_vcpu_ioctl(CPUState *cpu, unsigned long type, ...)
> arg = va_arg(ap, void *);
> va_end(ap);
>
> + /* kvm_close() has been called: the vCPU fds are gone */
> + if (cpu->kvm_fd == -1) {
> + return -ENODEV;
> + }
> +
> trace_kvm_vcpu_ioctl(cpu->cpu_index, type, arg);
> accel_cpu_ioctl_begin(cpu);
> ret = ioctl(cpu->kvm_fd, type, arg);
Similarly, kvm_send_msi() reads the global kvm_state without any
synchronization against kvm_close() setting it to NULL. The KVMState
object itself is never freed, so only the fd value is racy.
[Severity: Medium]
Can the new early return in kvm_vm_ioctl() give kvm_set_ioeventfd_mmio()
a stale errno?
kvm_vm_ioctl() already returns a negative errno value. The new -ENODEV
return never calls ioctl(), so errno is not set. kvm_set_ioeventfd_mmio()
ignores the returned value and re-reads errno:
ret = kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &iofd);
if (ret < 0) {
return -errno;
}
After kvm_close() this returns whatever errno an earlier failed syscall
left behind. If errno is 0, the failure is reported as success. If errno
is non-zero, the caller reports a misleading error.
The callers kvm_mem_ioeventfd_add() and kvm_mem_ioeventfd_del() abort()
on any negative return:
r = kvm_set_ioeventfd_mmio(fd, section->offset_within_address_space,
data, true, int128_get64(section->size),
match_data);
if (r < 0) {
fprintf(stderr, "%s: error adding ioeventfd: %s (%d)\n",
__func__, strerror(-r), -r);
abort();
}
Should kvm_set_ioeventfd_mmio() return ret instead of -errno? The
sibling kvm_set_ioeventfd_pio() already returns the kvm_vm_ioctl() value
directly.
I did not confirm that kvm_enabled() stays true after kvm_close(). I also
did not find a path that deletes an ioeventfd after kvm_close(), such as
device unrealize during qemu_cleanup. So I'm not sure this can be
triggered today.
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=18
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 19/28] kvm: fix coalesced MMIO flush racing with kvm_close()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (17 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 18/28] kvm: tolerate ioctls after kvm_close() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:46 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 20/28] migration/cpr: add cpr_is_in_progress() Andrey Drobyshev
` (8 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
kvm_close() destroys the vCPUs, which unmaps their kvm_run areas, and the
coalesced MMIO ring lives in the kvm_run area of the first vCPU. Nothing
stops the RCU thread from flushing that ring at the same time: finalizing
a memory region of a freed flatview calls memory_region_transaction_begin()
and thus kvm_flush_coalesced_mmio_buffer(). kvm_state is still set at
that point, so the flush reads the ring after it has been unmapped:
#0 kvm_flush_coalesced_mmio_buffer accel/kvm/kvm-all.c
#1 qemu_flush_coalesced_mmio_buffer
#2 memory_region_transaction_begin
#3 memory_region_finalize
...
#10 flatview_destroy
#11 call_rcu_thread
while the main thread is in:
kvm_close -> cpu_remove_sync
vfio_kvm_device_close
vfio_cpr_kvm_close_notifier (MIG_EVENT_PRECOPY_DONE)
Seen as a SIGSEGV of the old QEMU during repeated cpr-exec of a VM with
a VFIO device.
Clear the ring pointer first, so that new flushes skip it, and wait for
the pending RCU callbacks before destroying the vCPUs.
Fixes: 7ed0919119b0 ("migration: close kvm after cpr")
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
accel/kvm/kvm-all.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index ea027c37829..c29d467f9f5 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -630,6 +630,14 @@ void kvm_close(void)
return;
}
+ /*
+ * Destroying vCPUs unmaps the coalesced MMIO ring, while the RCU thread
+ * may be flushing it from a memory region finalizer. Stop new flushes
+ * and wait for the pending RCU callbacks first.
+ */
+ kvm_state->coalesced_mmio_ring = NULL;
+ drain_call_rcu();
+
CPU_FOREACH(cpu) {
cpu_remove_sync(cpu);
close(cpu->kvm_fd);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 19/28] kvm: fix coalesced MMIO flush racing with kvm_close()
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 19/28] kvm: fix coalesced MMIO flush racing with kvm_close() Andrey Drobyshev
@ 2026-10-16 15:46 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:46 UTC (permalink / raw)
To: svt-core, Andrey Drobyshev; +Cc: andrey.drobyshev
> kvm_close() destroys the vCPUs, which unmaps their kvm_run areas, and the
> coalesced MMIO ring lives in the kvm_run area of the first vCPU. Nothing
> stops the RCU thread from flushing that ring at the same time: finalizing
> a memory region of a freed flatview calls memory_region_transaction_begin()
> and thus kvm_flush_coalesced_mmio_buffer(). kvm_state is still set at
> that point, so the flush reads the ring after it has been unmapped:
>
> #0 kvm_flush_coalesced_mmio_buffer accel/kvm/kvm-all.c
> #1 qemu_flush_coalesced_mmio_buffer
> #2 memory_region_transaction_begin
> #3 memory_region_finalize
> ...
> #10 flatview_destroy
> #11 call_rcu_thread
>
> while the main thread is in:
>
> kvm_close -> cpu_remove_sync
> vfio_kvm_device_close
> vfio_cpr_kvm_close_notifier (MIG_EVENT_PRECOPY_DONE)
>
> Seen as a SIGSEGV of the old QEMU during repeated cpr-exec of a VM with
> a VFIO device.
>
> Clear the ring pointer first, so that new flushes skip it, and wait for
> the pending RCU callbacks before destroying the vCPUs.
>
> Fixes: 7ed0919119b0 ("migration: close kvm after cpr")
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=19
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 20/28] migration/cpr: add cpr_is_in_progress()
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (18 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 19/28] kvm: fix coalesced MMIO flush racing with kvm_close() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:46 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 21/28] vfio/migration: don't migrate device state during CPR Andrey Drobyshev
` (7 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
cpr_incoming_needed() only looks at the migration mode. That's fine for
vmstate .needed callbacks, which run during a save or load. But code that
runs at any time, e.g. VM state change handlers, can't tell from it
whether a CPR migration is actually going on: on the source the mode is
just a parameter, set by migrate-set-parameters whether or not a migrate
follows.
Add cpr_is_in_progress(), which is true on the target until the incoming
migration is done, and on the source only while the outgoing one runs.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
include/migration/cpr.h | 1 +
migration/cpr.c | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/include/migration/cpr.h b/include/migration/cpr.h
index 4dd397a7c13..e73c5da1d9c 100644
--- a/include/migration/cpr.h
+++ b/include/migration/cpr.h
@@ -64,6 +64,7 @@ MigMode cpr_get_incoming_mode(void);
void cpr_set_incoming_mode(MigMode mode);
bool cpr_is_incoming(void);
bool cpr_exec_is_incoming(void);
+bool cpr_is_in_progress(void);
bool cpr_state_save(MigrationChannel *channel, Error **errp);
int cpr_state_load(MigrationChannel *channel, Error **errp);
diff --git a/migration/cpr.c b/migration/cpr.c
index a149e790c10..74acdfc4123 100644
--- a/migration/cpr.c
+++ b/migration/cpr.c
@@ -549,6 +549,28 @@ bool cpr_exec_is_incoming(void)
return incoming_mode == MIG_MODE_CPR_EXEC;
}
+bool cpr_is_in_progress(void)
+{
+ /* The incoming mode on the target, the 'mode' parameter on the source */
+ MigMode mode = migrate_mode();
+
+ if (mode != MIG_MODE_CPR_TRANSFER && mode != MIG_MODE_CPR_EXEC) {
+ return false;
+ }
+
+ /* Target: the incoming mode is set until the incoming migration is done */
+ if (cpr_is_incoming()) {
+ return true;
+ }
+
+ /*
+ * Source: the parameter alone means nothing, it may just have been set.
+ * But a running outgoing migration uses it. A snapshot runs as a
+ * migration too, and it is never a CPR one.
+ */
+ return migration_is_running() && !runstate_check(RUN_STATE_SAVE_VM);
+}
+
bool cpr_state_save(MigrationChannel *channel, Error **errp)
{
int ret;
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 20/28] migration/cpr: add cpr_is_in_progress()
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 20/28] migration/cpr: add cpr_is_in_progress() Andrey Drobyshev
@ 2026-10-16 15:46 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:46 UTC (permalink / raw)
To: svt-core, Andrey Drobyshev; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] migration/cpr: cpr_is_in_progress() misreports snapshots of a paused VM
--
commit 3d6e15a074763ee7756f891bee028184ab64eb6c
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
migration/cpr: add cpr_is_in_progress()
This adds cpr_is_in_progress(), which reports whether a CPR migration is
actually in progress. It returns true on the target until the incoming
migration is done, and on the source only while the outgoing migration
runs. On the source it tries to exclude snapshots by checking for
RUN_STATE_SAVE_VM.
> diff --git a/migration/cpr.c b/migration/cpr.c
> index a149e790c103..74acdfc4123e 100644
> --- a/migration/cpr.c
> +++ b/migration/cpr.c
[ ... ]
> @@ -549,6 +549,28 @@ bool cpr_exec_is_incoming(void)
> return incoming_mode == MIG_MODE_CPR_EXEC;
> }
>
> +bool cpr_is_in_progress(void)
> +{
[ ... ]
> + /*
> + * Source: the parameter alone means nothing, it may just have been set.
> + * But a running outgoing migration uses it. A snapshot runs as a
> + * migration too, and it is never a CPR one.
> + */
> + return migration_is_running() && !runstate_check(RUN_STATE_SAVE_VM);
> +}
[Severity: Medium]
Can this return true during an ordinary snapshot of a VM that is already
paused?
The snapshot exclusion relies on the runstate being RUN_STATE_SAVE_VM.
But save_snapshot() calls vm_stop(RUN_STATE_SAVE_VM), which reaches
do_vm_stop(), and that only changes the runstate when the VM was live:
system/cpus.c:do_vm_stop() {
...
if (runstate_is_live(oldstate)) {
vm_was_suspended = (oldstate == RUN_STATE_SUSPENDED);
runstate_set(state);
...
}
If the VM is paused or in prelaunch, the runstate never becomes
RUN_STATE_SAVE_VM, so !runstate_check(RUN_STATE_SAVE_VM) stays true.
Meanwhile qemu_savevm_state() calls migrate_init(), which puts
MigrationState into SETUP:
migration/savevm.c:qemu_savevm_state() {
...
ret = migrate_init(ms, errp);
...
}
migration_is_running() counts MIGRATION_STATUS_SETUP as running.
cpr_is_incoming() is false on the source.
So if migrate-set-parameters has set mode to cpr-transfer or cpr-exec and
savevm is then run on a paused VM, cpr_is_in_progress() returns true. That
contradicts the comment "A snapshot runs as a migration too, and it is
never a CPR one" and the commit message.
Would callers such as the VFIO save path then skip device state during
such a snapshot?
Would an explicit outgoing-CPR flag be a more robust discriminator? It
could be set in cpr_state_save() and cleared on completion, failure or
cancel.
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=20
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 21/28] vfio/migration: don't migrate device state during CPR
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (19 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 20/28] migration/cpr: add cpr_is_in_progress() Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:55 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 22/28] vfio/listener: skip dirty page tracking " Andrey Drobyshev
` (6 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
cpr-transfer and cpr-exec hand the running VFIO device over to the new
QEMU as is: the device fd, its mappings and interrupts are preserved, and
the device itself is never stopped.
But if the device supports VFIO migration, its migration handlers still
run. The old QEMU stops the device and saves its state, then the new
QEMU puts the very same device into RESUMING and loads that state back.
On top of that, vfio_pci_load_config() re-programs MSI-X and disables
vector 0.
For an NVIDIA vGPU this breaks the device after qemu-update: restoring
the state into the live vGPU fails and the guest loses the device.
Devices without migration support aren't affected, since no migration
handlers are registered for them.
Skip VFIO migration while a CPR migration is in progress: don't save or
load the device state, and don't change the device state on VM
stop/start. All of it is gated on cpr_is_in_progress(), not on the mode
alone: the mode may merely be set, and then a plain stop/cont must still
change the device state, and a snapshot must still save it.
Also reset multifd_transfer in vfio_multifd_cleanup(): it's only set in
vfio_multifd_setup(), which CPR now skips, so a stale value from an
earlier migration would still let the multifd save thread run.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/vfio/migration-multifd.c | 2 ++
hw/vfio/migration.c | 38 +++++++++++++++++++++++++++++++++++++-
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index e4785031a73..452f772dd7a 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -567,6 +567,8 @@ void vfio_multifd_cleanup(VFIODevice *vbasedev)
VFIOMigration *migration = vbasedev->migration;
g_clear_pointer(&migration->multifd, vfio_multifd_free);
+ /* Decided anew by vfio_multifd_setup() of the next migration */
+ migration->multifd_transfer = false;
}
bool vfio_multifd_transfer_supported(void)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 4c06e3db936..b142c9f2289 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -19,6 +19,7 @@
#include "hw/vfio/vfio-device.h"
#include "hw/vfio/vfio-migration.h"
#include "migration/misc.h"
+#include "migration/cpr.h"
#include "migration/savevm.h"
#include "migration/vmstate.h"
#include "migration/qemu-file.h"
@@ -513,7 +514,6 @@ static void vfio_save_cleanup(void *opaque)
Error *local_err = NULL;
int ret;
- /* Currently a NOP, done for symmetry with load_cleanup() */
vfio_multifd_cleanup(vbasedev);
/*
@@ -674,6 +674,16 @@ static void vfio_save_state(QEMUFile *f, void *opaque)
Error *local_err = NULL;
int ret;
+ /*
+ * .is_active doesn't cover the non-iterable stage, so the section is
+ * still sent: leave it empty, or new QEMU would load the config into
+ * the preserved device and re-program its MSI-X.
+ */
+ if (cpr_is_in_progress()) {
+ qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+ return;
+ }
+
if (vfio_multifd_transfer_enabled(vbasedev)) {
if (vfio_load_config_after_iter(vbasedev)) {
qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_LOAD_READY);
@@ -810,6 +820,11 @@ static bool vfio_switchover_ack_needed(void *opaque)
{
VFIODevice *vbasedev = opaque;
+ /* No device state is sent during CPR, so no ACK to wait for */
+ if (cpr_is_in_progress()) {
+ return false;
+ }
+
return vfio_precopy_supported(vbasedev);
}
@@ -824,7 +839,19 @@ static int vfio_switchover_start(void *opaque)
return 0;
}
+/*
+ * cpr-transfer and cpr-exec hand the running device over to the new QEMU
+ * as is: its fds, mappings and interrupts are preserved and it never stops.
+ * Device state migration must stay out of it, else the device is stopped,
+ * its state saved and then resumed into the very same device.
+ */
+static bool vfio_is_active(void *opaque)
+{
+ return !cpr_is_in_progress();
+}
+
static const SaveVMHandlers savevm_vfio_handlers = {
+ .is_active = vfio_is_active,
.save_prepare = vfio_save_prepare,
.save_setup = vfio_save_setup,
.save_cleanup = vfio_save_cleanup,
@@ -857,6 +884,11 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
Error *local_err = NULL;
int ret;
+ /* The device is preserved, not a plain stop/cont: keep its state */
+ if (cpr_is_in_progress()) {
+ return;
+ }
+
new_state = migration->device_state == VFIO_DEVICE_STATE_PRE_COPY ?
VFIO_DEVICE_STATE_PRE_COPY_P2P :
VFIO_DEVICE_STATE_RUNNING_P2P;
@@ -882,6 +914,10 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
Error *local_err = NULL;
int ret;
+ if (cpr_is_in_progress()) {
+ return;
+ }
+
if (running) {
new_state = VFIO_DEVICE_STATE_RUNNING;
} else {
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 21/28] vfio/migration: don't migrate device state during CPR
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 21/28] vfio/migration: don't migrate device state during CPR Andrey Drobyshev
@ 2026-10-16 15:55 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:55 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] vfio/migration: stale cached device_state when CPR starts from a paused VM
- [Medium] vfio/migration: CPR skips VFIO state using local-only checks, breaking old-source to new-target streams
- [Medium] vfio/migration: cached device_state not synced after CPR when guest was paused
--
commit 5d90a1a10c891a1bfb1b58a35b36f468c5d28350
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
vfio/migration: don't migrate device state during CPR
This patch skips VFIO device state save and load while a CPR migration
(cpr-transfer or cpr-exec) is in progress. It gates the save, load and
VM stop/start handlers on cpr_is_in_progress(). It also resets
multifd_transfer in vfio_multifd_cleanup().
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
[ ... ]
> @@ -674,6 +674,16 @@ static void vfio_save_state(QEMUFile *f, void *opaque)
> Error *local_err = NULL;
> int ret;
>
> + /*
> + * .is_active doesn't cover the non-iterable stage, so the section is
> + * still sent: leave it empty, or new QEMU would load the config into
> + * the preserved device and re-program its MSI-X.
> + */
> + if (cpr_is_in_progress()) {
> + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
> + return;
> + }
> +
[Severity: High]
Can the new QEMU end up with a stale cached device_state when CPR starts
from an already paused VM?
If the user stops the VM before CPR, vfio_vmstate_change(running=false)
runs while cpr_is_in_progress() is false. It moves the kernel device to
STOP and caches STOP in migration->device_state.
Once CPR starts, vfio_save_state() emits only END_OF_STATE, so no
device_state reaches the target. The new QEMU then runs
vfio_migration_init(), which does:
migration->device_state = VFIO_DEVICE_STATE_RUNNING;
It never queries the kernel. The preserved device is still in STOP.
The target stays paused, because process_incoming_migration_bh() calls
runstate_set(RUN_STATE_PAUSED) and then migration_incoming_state_destroy().
That clears the incoming mode, so cpr_is_in_progress() becomes false.
A later 'cont' then reaches vfio_vmstate_change(running=true), which
requests RUNNING. In vfio_migration_set_state() this hits:
if (new_state == migration->device_state) {
return 0;
}
No ioctl is issued, and the kernel device stays in STOP while the guest
runs. Could the cached state be synchronized with the preserved device in
this case?
The commit message says the device "is never stopped". Is that accurate
for a source that was paused before CPR?
[ ... ]
> @@ -810,6 +820,11 @@ static bool vfio_switchover_ack_needed(void *opaque)
> {
> VFIODevice *vbasedev = opaque;
>
> + /* No device state is sent during CPR, so no ACK to wait for */
> + if (cpr_is_in_progress()) {
> + return false;
> + }
> +
> return vfio_precopy_supported(vbasedev);
> }
[ ... ]
> +static bool vfio_is_active(void *opaque)
> +{
> + return !cpr_is_in_progress();
> +}
[Severity: Medium]
Is it a problem that the decision to send or load VFIO state is made only
from local runtime state?
Nothing in the stream tells the peer whether VFIO sections are present.
Consider a CPR incoming migration from a source that does not have this
patch:
vfio_is_active() returns false on the target, so loadvm_state_setup() skips
vfio_load_setup() and migration->data_fd stays -1. The old source still
sends VFIO_MIG_FLAG_DEV_DATA_STATE. vfio_load_state() has no CPR guard:
case VFIO_MIG_FLAG_DEV_DATA_STATE:
{
uint64_t data_size = qemu_get_be64(f);
if (data_size) {
ret = vfio_load_buffer(f, vbasedev, data_size);
Would that call qemu_file_get_to_fd(f, -1, size) and fail? The config
section would also still be loaded into the live device.
The reverse pair (new source, old target) looks similar. The old target
runs vfio_load_setup() and puts the live device into RESUMING. Its
vfio_switchover_ack_needed() has no CPR check, so it waits for an ACK that
the new source never sends.
docs/devel/migration/CPR.rst describes the incoming side as a new QEMU
binary, so mixed-version CPR seems to be a documented use. Is a stream flag
or a compat property needed here, or is mixed-version CPR intentionally
unsupported? I found no later patch in the series that adds a CPR guard in
vfio_load_state().
[ ... ]
> @@ -882,6 +914,10 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
> Error *local_err = NULL;
> int ret;
>
> + if (cpr_is_in_progress()) {
> + return;
> + }
> +
> if (running) {
> new_state = VFIO_DEVICE_STATE_RUNNING;
> } else {
[Severity: Medium]
Does this early return leave the cached migration->device_state out of sync
with the preserved device on the target?
On the target, vm_start() runs while the incoming mode is still set, so
this handler is skipped. For a guest paused before CPR, the kernel device
is in STOP, but vfio_migration_init() has cached RUNNING.
After the incoming migration completes, the later 'cont' calls
vfio_vmstate_change(running=true):
vfio_vmstate_change()
vfio_migration_set_state()
if (new_state == migration->device_state)
return 0;
The request is silently skipped, so the kernel device is never resumed.
Should the cache be resynchronized, or the real state queried, when the
device is inherited through CPR?
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=21
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 22/28] vfio/listener: skip dirty page tracking during CPR
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (20 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 21/28] vfio/migration: don't migrate device state during CPR Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:50 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 23/28] VZ: vfio/cpr: don't close KVM on cpr-exec Andrey Drobyshev
` (5 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
During cpr-transfer and cpr-exec guest RAM is preserved in place, not
copied, so there's no need to track which pages the device writes to.
Yet starting the migration also starts VFIO dirty tracking, on the
container or on the device itself (DMA logging). With cpr-exec it isn't
stopped before exec, so e.g. the DMA page tracker of an NVIDIA vGPU is
left running after the update.
Don't start dirty tracking while a CPR migration is in progress. Gate
it on cpr_is_in_progress(), not on the mode alone, so that a snapshot
taken while the mode parameter is set to a CPR mode still tracks dirty
pages. Stopping stays unconditional: it's a no-op if tracking wasn't
started, and the mode may have changed in between.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/vfio/listener.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
index afcf518f567..1270b9d5f3a 100644
--- a/hw/vfio/listener.c
+++ b/hw/vfio/listener.c
@@ -41,6 +41,7 @@
#include "trace.h"
#include "qapi/error.h"
#include "migration/misc.h"
+#include "migration/cpr.h"
#include "migration/qemu-file.h"
#include "system/tcg.h"
#include "system/tpm.h"
@@ -1012,6 +1013,11 @@ static bool vfio_listener_log_global_start(MemoryListener *listener,
listener);
bool ret;
+ /* Preserved devices keep DMA-ing into preserved RAM, nothing to track */
+ if (cpr_is_in_progress()) {
+ return true;
+ }
+
if (vfio_container_devices_dirty_tracking_is_supported(bcontainer)) {
ret = vfio_devices_dma_logging_start(bcontainer, errp);
} else {
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 22/28] vfio/listener: skip dirty page tracking during CPR
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 22/28] vfio/listener: skip dirty page tracking " Andrey Drobyshev
@ 2026-10-16 15:50 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:50 UTC (permalink / raw)
To: Andrey Drobyshev, svt-core; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vfio: CPR skip in log_global_start() is never retried when another dirty-tracking flag is active
--
commit a7865175c300cb526642364e7a12bfbf13a29863
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
vfio/listener: skip dirty page tracking during CPR
This patch makes vfio_listener_log_global_start() return early without
starting VFIO dirty tracking while a CPR migration (cpr-transfer or
cpr-exec) is in progress. The gate is cpr_is_in_progress(), so snapshots
still track dirty pages. Stopping stays unconditional.
> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
> index afcf518f567f..1270b9d5f3aa 100644
> --- a/hw/vfio/listener.c
> +++ b/hw/vfio/listener.c
[ ... ]
> @@ -1012,6 +1013,11 @@ static bool vfio_listener_log_global_start(MemoryListener *listener,
> listener);
> bool ret;
>
> + /* Preserved devices keep DMA-ing into preserved RAM, nothing to track */
> + if (cpr_is_in_progress()) {
> + return true;
> + }
> +
[Severity: High]
Can this early return leave VFIO dirty tracking permanently off for a later
normal migration?
The listener hook runs only when the first dirty-tracking flag is set:
system/memory.c:memory_global_dirty_log_start() {
old_flags = global_dirty_tracking;
global_dirty_tracking |= flags;
...
if (!old_flags) {
if (!memory_global_dirty_log_do_start(errp)) {
...
}
Consider this sequence:
1. A CPR migration starts as the first consumer.
memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION) sees
old_flags == 0, so vfio_listener_log_global_start() runs. Because
cpr_is_in_progress() is true, it returns true without starting tracking.
2. Another consumer starts, for example calculate-dirty-rate
(GLOBAL_DIRTY_DIRTY_RATE) or the dirty limit (GLOBAL_DIRTY_LIMIT).
old_flags is now non-zero, so no listener is called.
3. The CPR migration fails or is cancelled. Only the migration flag is
cleared in memory_global_dirty_log_do_stop():
global_dirty_tracking &= ~flags;
...
if (!global_dirty_tracking) {
...
MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse);
global_dirty_tracking stays non-zero, so no listener hook runs.
4. The user sets mode=normal and migrates.
memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION) again sees
old_flags != 0 and skips the listeners, so VFIO tracking is never
started.
5. vfio_log_sync_needed() then returns false:
if (!vfio_container_dirty_tracking_is_started(bcontainer)) {
return false;
}
Both vbasedev->dirty_tracking and bcontainer->dirty_pages_started are
still false, so pages dirtied by device DMA are not synced.
The destination could then receive stale guest RAM during the later normal
migration. Before this patch, VFIO tracking always started with the first
consumer, so this state was not reachable.
Nothing later in the series touches hw/vfio/listener.c or system/memory.c,
so the series does not restart tracking in this case.
Could the skip be retried, or could VFIO tracking be started when a later
consumer or a later non-CPR migration needs it?
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=22
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 23/28] VZ: vfio/cpr: don't close KVM on cpr-exec
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (21 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 22/28] vfio/listener: skip dirty page tracking " Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:52 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 24/28] VZ: migration/cpr-exec: abort cpr-exec if memory can't be preserved Andrey Drobyshev
` (4 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
With VFIO devices, KVM is closed on the source when the migration
completes, so that the new QEMU can create its own KVM VM with the same
devices. That is done from a migration notifier, which for cpr-exec
runs before cpr_exec_cb(). But cpr_exec_cb() may still give up, when
the memory can't be preserved or execve() fails, and resume the VM in
the old QEMU. A VM without KVM can't resume: kvm_close() removes the
vCPU threads, and the first KVM ioctl of vm_start() aborts QEMU
(KVM_SET_CLOCK in kvmclock_vm_state_change()).
cpr-exec doesn't need it anyway: the KVM fds are close-on-exec, so the
kernel closes them during execve(), before the new QEMU runs, and
kvm_vm_release() tears down the irqfds then. Keep the notifier for
cpr-transfer only, where the source process lives on.
Tested on the vfio-net rig: cpr-exec 5/5; a cpr-exec aborted in
preserve_ram() and one with a bogus cpr-exec-command both left the VM
running with no lost pings, and a later cpr-exec completed.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/vfio/cpr.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
index db462aabcbf..7ca761d5bf5 100644
--- a/hw/vfio/cpr.c
+++ b/hw/vfio/cpr.c
@@ -192,13 +192,16 @@ static int vfio_cpr_kvm_close_notifier(NotifierWithReturn *notifier,
return 0;
}
+/*
+ * Not for cpr-exec: the exec closes the KVM fds anyway, and until then
+ * cpr_exec_cb() may still give up and resume the VM, which needs KVM.
+ */
void vfio_cpr_add_kvm_notifier(void)
{
if (!kvm_close_notifier.notify) {
- migration_add_notifier_modes(&kvm_close_notifier,
- vfio_cpr_kvm_close_notifier,
- MIG_MODE_CPR_TRANSFER, MIG_MODE_CPR_EXEC,
- -1);
+ migration_add_notifier_mode(&kvm_close_notifier,
+ vfio_cpr_kvm_close_notifier,
+ MIG_MODE_CPR_TRANSFER);
}
}
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 23/28] VZ: vfio/cpr: don't close KVM on cpr-exec
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 23/28] VZ: vfio/cpr: don't close KVM on cpr-exec Andrey Drobyshev
@ 2026-10-16 15:52 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:52 UTC (permalink / raw)
To: svt-core, Andrey Drobyshev; +Cc: andrey.drobyshev
> With VFIO devices, KVM is closed on the source when the migration
> completes, so that the new QEMU can create its own KVM VM with the same
> devices. That is done from a migration notifier, which for cpr-exec
> runs before cpr_exec_cb(). But cpr_exec_cb() may still give up, when
> the memory can't be preserved or execve() fails, and resume the VM in
> the old QEMU. A VM without KVM can't resume: kvm_close() removes the
> vCPU threads, and the first KVM ioctl of vm_start() aborts QEMU
> (KVM_SET_CLOCK in kvmclock_vm_state_change()).
>
> cpr-exec doesn't need it anyway: the KVM fds are close-on-exec, so the
> kernel closes them during execve(), before the new QEMU runs, and
> kvm_vm_release() tears down the irqfds then. Keep the notifier for
> cpr-transfer only, where the source process lives on.
>
> Tested on the vfio-net rig: cpr-exec 5/5; a cpr-exec aborted in
> preserve_ram() and one with a bogus cpr-exec-command both left the VM
> running with no lost pings, and a later cpr-exec completed.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=23
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 24/28] VZ: migration/cpr-exec: abort cpr-exec if memory can't be preserved
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (22 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 23/28] VZ: vfio/cpr: don't close KVM on cpr-exec Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:55 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 25/28] VZ: migration/cpr-exec: take the vhost-net devices back if exec fails Andrey Drobyshev
` (3 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
cpr_exec_cb() marks all saved ramblocks with MADV_DOEXEC before exec, but
ignores the result. If madvise() fails for some block, the walk stops
there and the remaining blocks are not marked, yet execvp() still runs.
The new QEMU then reuses addresses of guest RAM that wasn't preserved,
and the guest is lost.
Treat that as a failed cpr-exec: go to the same failure path as when
execvp() fails, so the VM keeps running in the old QEMU.
On that path also clear MADV_DOEXEC from all saved ramblocks again, just
like the preserved fds get close-on-exec back.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
migration/cpr-exec.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index d37293baec5..dd2ff9b26ff 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -155,6 +155,19 @@ static bool preserve_ram(cpr_ramblock_t ram, char *name)
return true;
}
+static bool unpreserve_ram(cpr_ramblock_t ram, char *name)
+{
+#ifdef MADV_DONTEXEC
+ qemu_madvise((void *) ram.addr, ram.length, MADV_DONTEXEC);
+#endif
+ return true;
+}
+
+static void cpr_exec_unpreserve_ram(void)
+{
+ cpr_walk_ramblock(unpreserve_ram);
+}
+
static bool mark_ram_rdonly(cpr_ramblock_t ram, char *name)
{
/*
@@ -203,7 +216,10 @@ static void cpr_exec_cb(void *opaque)
*/
cpr_exec_preserve_fds();
- cpr_walk_ramblock(preserve_ram);
+ if (!cpr_walk_ramblock(preserve_ram)) {
+ /* preserve_ram() has already reported the error and failed migration */
+ goto fail;
+ }
if (cpr_ram_rdonly) {
cpr_walk_ramblock(mark_ram_rdonly);
}
@@ -222,18 +238,20 @@ static void cpr_exec_cb(void *opaque)
* or the system is very short on resources.
*/
error_setg_errno(&err, errno, "execvp %s failed", argv[0]);
- g_clear_pointer(&argv, g_strfreev);
- cpr_exec_unpreserve_fds();
- if (cpr_ram_rdonly) {
- cpr_ram_restore_rw();
- }
-
error_report_err(error_copy(err));
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
migrate_set_error(s, err);
error_free(err);
err = NULL;
+fail:
+ g_clear_pointer(&argv, g_strfreev);
+ cpr_exec_unpreserve_fds();
+ cpr_exec_unpreserve_ram();
+ if (cpr_ram_rdonly) {
+ cpr_ram_restore_rw();
+ }
+
/* Note, we can go from state COMPLETED to FAILED */
migration_call_notifiers(MIG_EVENT_PRECOPY_FAILED, NULL);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 25/28] VZ: migration/cpr-exec: take the vhost-net devices back if exec fails
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (23 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 24/28] VZ: migration/cpr-exec: abort cpr-exec if memory can't be preserved Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:58 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 26/28] VZ: migration/cpr-exec: don't exec if a vhost-net device can't be released Andrey Drobyshev
` (2 subsequent siblings)
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
Right before execve(), cpr_exec_cb() releases the kernel vhost-net
devices with VHOST_RESET_OWNER, so that the new QEMU can take them. If
execve() fails, the VM goes on in the old QEMU, but nobody takes the
devices back: the kernel refuses the ioctls of the next
vhost_net_start(), and the guest is left without network.
Take them back with VHOST_SET_OWNER when the exec fails. The devices
are stopped at that point, and vhost_net_start() sets up the rest again
when the VM resumes. Only the devices that were actually released are
taken back: vhost_net_cpr_reset_owners() marks them. vhost-vsock does
the same in its CPR notifier.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/net/vhost_net.c | 28 ++++++++++++++++++++++++++++
include/hw/virtio/vhost.h | 2 ++
include/net/vhost_net.h | 1 +
migration/cpr-exec.c | 2 ++
4 files changed, 33 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 56c697f4bcd..ae98574fcfe 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -594,6 +594,34 @@ void vhost_net_cpr_reset_owners(void)
if (r < 0 && r != -ENOSYS) {
warn_report("cpr: failed to reset vhost-net owner for %s: %s",
nc->name, strerror(-r));
+ } else if (r == 0) {
+ net->cpr_owner_reset = true;
+ }
+ }
+ }
+}
+
+/*
+ * Take the ownership back when the exec fails and the VM goes on in the
+ * old QEMU. VHOST_RESET_OWNER has reset the kernel device, but the devices
+ * are stopped, and vhost_net_start() sets everything up again when the VM
+ * resumes.
+ */
+void vhost_net_cpr_set_owners(void)
+{
+ NetClientState *nc;
+
+ QTAILQ_FOREACH(nc, &net_clients, next) {
+ VHostNetState *net = get_vhost_net(nc);
+
+ if (net && net->cpr_owner_reset) {
+ int r = vhost_dev_set_owner(&net->dev);
+
+ if (r < 0) {
+ warn_report("cpr: failed to set vhost-net owner for %s: %s",
+ nc->name, strerror(-r));
+ } else {
+ net->cpr_owner_reset = false;
}
}
}
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index a92439d3443..421f7139cfb 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -151,6 +151,8 @@ struct vhost_net {
SaveAcketFeatures *save_acked_features;
bool is_vhost_user;
NetClientState *nc;
+ /* released for cpr-exec, take back if it fails */
+ bool cpr_owner_reset;
};
/**
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 39ce7ad0e3d..cdfefa81500 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -90,4 +90,5 @@ int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
void vhost_net_save_acked_features(NetClientState *nc);
void vhost_net_cpr_reset_owners(void);
+void vhost_net_cpr_set_owners(void);
#endif
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index dd2ff9b26ff..21caa614a13 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -251,6 +251,8 @@ fail:
if (cpr_ram_rdonly) {
cpr_ram_restore_rw();
}
+ /* Still the same mm, take back the vhost-net devices we released */
+ vhost_net_cpr_set_owners();
/* Note, we can go from state COMPLETED to FAILED */
migration_call_notifiers(MIG_EVENT_PRECOPY_FAILED, NULL);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 25/28] VZ: migration/cpr-exec: take the vhost-net devices back if exec fails
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 25/28] VZ: migration/cpr-exec: take the vhost-net devices back if exec fails Andrey Drobyshev
@ 2026-10-16 15:58 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 15:58 UTC (permalink / raw)
To: svt-core, Andrey Drobyshev; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] vhost-net: cpr-exec failure path restores only SET_OWNER, not the vring err/call fds
- [Low] migration: cpr-exec calls vhost_net_cpr_set_owners() with no stub in vhost_net-stub.c
--
commit 4559492d0874959294013f874fff0e018375087a
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
VZ: migration/cpr-exec: take the vhost-net devices back if exec fails
When execvp() fails in cpr_exec_cb(), the old QEMU keeps running. This
patch records which vhost-net devices were released with
VHOST_RESET_OWNER in vhost_net_cpr_reset_owners(). It then takes them
back with VHOST_SET_OWNER through the new vhost_net_cpr_set_owners() on
the fail path.
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
[ ... ]
> +/*
> + * Take the ownership back when the exec fails and the VM goes on in the
> + * old QEMU. VHOST_RESET_OWNER has reset the kernel device, but the devices
> + * are stopped, and vhost_net_start() sets everything up again when the VM
> + * resumes.
> + */
> +void vhost_net_cpr_set_owners(void)
> +{
> + NetClientState *nc;
> +
> + QTAILQ_FOREACH(nc, &net_clients, next) {
> + VHostNetState *net = get_vhost_net(nc);
> +
> + if (net && net->cpr_owner_reset) {
> + int r = vhost_dev_set_owner(&net->dev);
[Severity: Medium]
Does restoring only the owner here leave the vhost device without its
per-virtqueue error and call eventfds? vhost_dev_init() registers them
right after SET_OWNER, through vhost_virtqueue_init():
r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
...
r = dev->vhost_ops->vhost_set_vring_err(dev, &file);
As far as I can tell, vhost_set_vring_err is called only from
vhost_virtqueue_init(), and that runs once from vhost_dev_init(). Neither
vhost_dev_start() nor vhost_virtqueue_start() calls it again. Would the
error fd therefore stay unregistered after VHOST_RESET_OWNER followed by
VHOST_SET_OWNER?
The call fd looks similar. In vhost_virtqueue_start() it is re-set only
when guest notifier masking is not in use:
if (!vdev->use_guest_notifier_mask) {
/* TODO: check and handle errors. */
vhost_virtqueue_mask(dev, vdev, idx, false);
}
With use_guest_notifier_mask and MSI-X, a vector that the guest keeps
masked seems to get no call fd after the reset. The initial setup had
pointed it at masked_notifier. Could a pending interrupt on such a vector
then be dropped, and go unseen by virtio_pci_vector_poll()?
I have not checked the kernel side. This assumes VHOST_RESET_OWNER clears
error_ctx and call_ctx, as vhost_dev_cleanup() does.
The comment above and the commit message say "vhost_net_start() sets up
the rest again when the VM resumes". Is that accurate for the vring err fd
and for the call fd of masked vectors?
> +
> + if (r < 0) {
> + warn_report("cpr: failed to set vhost-net owner for %s: %s",
> + nc->name, strerror(-r));
> + } else {
> + net->cpr_owner_reset = false;
> }
> }
> }
[ ... ]
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
[ ... ]
> void vhost_net_cpr_reset_owners(void);
> +void vhost_net_cpr_set_owners(void);
[Severity: Low]
Is a stub needed for this function? hw/net/vhost_net-stub.c has no
vhost_net_cpr_reset_owners() or vhost_net_cpr_set_owners(). hw/net/meson.build
builds that stub file in place of vhost_net.c when vhost_net is not
available.
> diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
> --- a/migration/cpr-exec.c
> +++ b/migration/cpr-exec.c
[ ... ]
> + /* Still the same mm, take back the vhost-net devices we released */
> + vhost_net_cpr_set_owners();
[Severity: Low]
cpr_exec_cb() in migration/cpr-exec.c is always built and calls
vhost_net_cpr_set_owners() with no guard. In configurations that use
vhost_net-stub.c, would this give an undefined reference at link time?
The existing vhost_net_cpr_reset_owners() call in cpr_exec_cb() appears
to have the same problem, and this patch adds a second undefined symbol of
that kind.
>
> /* Note, we can go from state COMPLETED to FAILED */
> migration_call_notifiers(MIG_EVENT_PRECOPY_FAILED, NULL);
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=25
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 26/28] VZ: migration/cpr-exec: don't exec if a vhost-net device can't be released
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (24 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 25/28] VZ: migration/cpr-exec: take the vhost-net devices back if exec fails Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:56 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 27/28] VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860 Andrey Drobyshev
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 28/28] VZ: migration/cpr-exec: allow cpr-exec with VFIO devices #VSTOR-103860 Andrey Drobyshev
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
vhost_net_cpr_reset_owners() only warns when VHOST_RESET_OWNER fails for
a device, and the exec goes on. But the new QEMU can't take such a
device: it is still owned by the old mm, so VHOST_SET_OWNER fails with
EBUSY, and the guest is left without network after the update.
Stop at the first device that can't be released and abort the cpr-exec
instead. The VM goes on in the old QEMU, with the devices released so
far taken back. vhost-vsock already does that in its pre_save.
Fixes: 1278e4830052 ("VZ: migration/cpr-exec: vhost-net: send RESET_OWNER before cpr-exec")
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/net/vhost_net.c | 19 +++++++++++++------
include/net/vhost_net.h | 2 +-
migration/cpr-exec.c | 7 ++++++-
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ae98574fcfe..a556b41f5f7 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -579,8 +579,12 @@ void vhost_net_config_mask(VHostNetState *net, VirtIODevice *dev, bool mask)
* EBUSY. Called right before execve(), when the VM is stopped and the
* devices have already been stopped, so nothing is in flight. The device is
* fully re-initialized from the migrated state by the new QEMU.
+ *
+ * Stops at the first device that can't be released, since the new QEMU
+ * couldn't take it. The caller then takes back the ones already released
+ * with vhost_net_cpr_set_owners().
*/
-void vhost_net_cpr_reset_owners(void)
+void vhost_net_cpr_reset_owners(Error **errp)
{
NetClientState *nc;
@@ -591,12 +595,15 @@ void vhost_net_cpr_reset_owners(void)
net->dev.vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL) {
int r = vhost_dev_reset_owner(&net->dev);
- if (r < 0 && r != -ENOSYS) {
- warn_report("cpr: failed to reset vhost-net owner for %s: %s",
- nc->name, strerror(-r));
- } else if (r == 0) {
- net->cpr_owner_reset = true;
+ if (r == -ENOSYS) {
+ continue;
+ }
+ if (r < 0) {
+ error_setg_errno(errp, -r, "cpr: failed to reset vhost-net "
+ "owner for %s", nc->name);
+ return;
}
+ net->cpr_owner_reset = true;
}
}
}
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index cdfefa81500..679edefa1d3 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -89,6 +89,6 @@ int vhost_net_virtqueue_restart(VirtIODevice *vdev, NetClientState *nc,
void vhost_net_save_acked_features(NetClientState *nc);
-void vhost_net_cpr_reset_owners(void);
+void vhost_net_cpr_reset_owners(Error **errp);
void vhost_net_cpr_set_owners(void);
#endif
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index 21caa614a13..a206ca44a4b 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -228,7 +228,10 @@ static void cpr_exec_cb(void *opaque)
* Hand the preserved vhost-net devices over to the new QEMU: they are
* still owned by this mm, which is about to be replaced by execve().
*/
- vhost_net_cpr_reset_owners();
+ vhost_net_cpr_reset_owners(&err);
+ if (err) {
+ goto fail_with_err;
+ }
trace_cpr_exec();
execvp(argv[0], argv);
@@ -238,6 +241,8 @@ static void cpr_exec_cb(void *opaque)
* or the system is very short on resources.
*/
error_setg_errno(&err, errno, "execvp %s failed", argv[0]);
+
+fail_with_err:
error_report_err(error_copy(err));
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
migrate_set_error(s, err);
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH QEMU HCI-8.1 v3 27/28] VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (25 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 26/28] VZ: migration/cpr-exec: don't exec if a vhost-net device can't be released Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 16:02 ` no-reply
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 28/28] VZ: migration/cpr-exec: allow cpr-exec with VFIO devices #VSTOR-103860 Andrey Drobyshev
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
After cpr-exec, new QEMU mmaps the device BARs again on the preserved
device fd. Some drivers don't allow that: the NVIDIA vGPU VFIO driver
refuses to map a VF BAR that is still recorded as mapped by old QEMU,
and that record is dropped only when the device fd is closed. So BAR1
and BAR3 of the vGPU can't be mapped, and the guest loses the device.
Keep the mappings instead. Save each BAR mapping in the CPR ramblock
list, so cpr_exec_cb() marks it with MADV_DOEXEC together with guest RAM,
and new QEMU finds it there and reuses it instead of calling mmap().
Same find-or-create pattern as for other preserved objects, e.g. vector
eventfds in vfio_notifier_init(). No saved mapping means old QEMU didn't
map the region, e.g. its mmap failed or the region is mapped lazily, so
new QEMU maps it afresh.
The ID is "<device name> region <index> mmaps[<index>]". It doesn't
depend on the device path, which is gone when the display region is
finalized, so the entry is dropped together with the mapping.
The saved addresses only make sense for cpr-exec, which keeps the
address space. A cpr-transfer target is another process and would have
to map the regions anew, which the drivers above refuse while the old
mappings exist. So block cpr-transfer for VFIO devices.
This needs a kernel which supports MADV_DOEXEC on shared PFN mappings of
device memory. Without it cpr-exec of a VM with VFIO devices fails.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/vfio/device.c | 24 ++++++++++++++++++++++--
hw/vfio/region.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
include/hw/vfio/vfio-cpr.h | 1 +
3 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 5a14a7d5c0a..2a7073ccec8 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -464,12 +464,32 @@ bool vfio_device_attach(char *name, VFIODevice *vbasedev,
TYPE_VFIO_IOMMU_IOMMUFD :
TYPE_VFIO_IOMMU_LEGACY;
- return vfio_device_attach_by_iommu_type(iommu_type, name, vbasedev,
- as, errp);
+ /*
+ * Only cpr-exec keeps the address space and so the region mappings,
+ * see vfio_region_mmap(). A cpr-transfer target would have to map the
+ * regions anew, which some drivers (NVIDIA vGPU) refuse.
+ */
+ error_setg(&vbasedev->cpr.mmap_blocker,
+ "cpr-transfer is not supported with VFIO devices; "
+ "use the cpr-exec mode");
+ if (migrate_add_blocker_modes(&vbasedev->cpr.mmap_blocker, errp,
+ MIG_MODE_CPR_TRANSFER, -1)) {
+ return false;
+ }
+
+ if (!vfio_device_attach_by_iommu_type(iommu_type, name, vbasedev,
+ as, errp)) {
+ migrate_del_blocker(&vbasedev->cpr.mmap_blocker);
+ return false;
+ }
+
+ return true;
}
void vfio_device_detach(VFIODevice *vbasedev)
{
+ migrate_del_blocker(&vbasedev->cpr.mmap_blocker);
+
if (!vbasedev->bcontainer) {
return;
}
diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index b0332f27fcd..4d58794b3cf 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -30,6 +30,7 @@
#include "qemu/units.h"
#include "monitor/monitor.h"
#include "system/ramblock.h"
+#include "migration/cpr.h"
#include "vfio-helpers.h"
/*
@@ -227,8 +228,43 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
return 0;
}
+static char *vfio_region_cpr_name(VFIORegion *region, int index)
+{
+ return g_strdup_printf("%s region %d mmaps[%d]", region->vbasedev->name,
+ region->nr, index);
+}
+
+/*
+ * A saved mapping is one preserved by cpr-exec, reuse it. None means old
+ * QEMU didn't map the region, e.g. its mmap failed or it's mapped lazily,
+ * so map it afresh.
+ */
+static void *vfio_region_cpr_find(VFIORegion *region, int index)
+{
+ g_autofree char *name = vfio_region_cpr_name(region, index);
+ cpr_ramblock_t ram = cpr_find_ramblock(name);
+
+ if (ram.addr) {
+ assert(ram.length == region->mmaps[index].size);
+ }
+
+ return (void *)(uintptr_t)ram.addr;
+}
+
+static void vfio_region_cpr_save(VFIORegion *region, int index)
+{
+ g_autofree char *name = vfio_region_cpr_name(region, index);
+ void *addr = region->mmaps[index].mmap;
+ size_t size = region->mmaps[index].size;
+
+ cpr_save_ramblock(name, (cpr_ramblock_t) { (uintptr_t)addr, size });
+}
+
static void vfio_subregion_unmap(VFIORegion *region, int index)
{
+ g_autofree char *cpr_name = vfio_region_cpr_name(region, index);
+
+ cpr_delete_ramblock(cpr_name);
trace_vfio_region_unmap(memory_region_name(®ion->mmaps[index].mem),
region->mmaps[index].offset,
region->mmaps[index].offset +
@@ -314,6 +350,11 @@ int vfio_region_mmap(VFIORegion *region)
size_t align = MIN(1ULL << ctz64(region->mmaps[i].size), 1 * GiB);
void *map_base, *map_align;
+ region->mmaps[i].mmap = vfio_region_cpr_find(region, i);
+ if (region->mmaps[i].mmap) {
+ goto mapped;
+ }
+
/*
* Align the mmap for more efficient mapping in the kernel. Ideally
* we'd know the PMD and PUD mapping sizes to use as discrete alignment
@@ -347,7 +388,9 @@ int vfio_region_mmap(VFIORegion *region)
ret = -errno;
goto no_mmap;
}
+ vfio_region_cpr_save(region, i);
+mapped:
name = g_strdup_printf("%s mmaps[%d]",
memory_region_name(region->mem), i);
memory_region_init_ram_device_ptr(®ion->mmaps[i].mem,
@@ -427,6 +470,8 @@ void vfio_region_finalize(VFIORegion *region)
for (i = 0; i < region->nr_mmaps; i++) {
if (region->mmaps[i].mmap) {
+ g_autofree char *cpr_name = vfio_region_cpr_name(region, i);
+ cpr_delete_ramblock(cpr_name);
munmap(region->mmaps[i].mmap, region->mmaps[i].size);
}
}
diff --git a/include/hw/vfio/vfio-cpr.h b/include/hw/vfio/vfio-cpr.h
index 4606da500a7..fa1699857ca 100644
--- a/include/hw/vfio/vfio-cpr.h
+++ b/include/hw/vfio/vfio-cpr.h
@@ -34,6 +34,7 @@ typedef struct VFIOContainerCPR {
typedef struct VFIODeviceCPR {
Error *mdev_blocker;
Error *id_blocker;
+ Error *mmap_blocker;
uint32_t hwpt_id;
uint32_t ioas_id;
} VFIODeviceCPR;
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH QEMU HCI-8.1 v3 27/28] VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 27/28] VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860 Andrey Drobyshev
@ 2026-10-16 16:02 ` no-reply
0 siblings, 0 replies; 57+ messages in thread
From: no-reply @ 2026-10-16 16:02 UTC (permalink / raw)
To: svt-core, Andrey Drobyshev; +Cc: andrey.drobyshev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] vfio: vfio_device_attach() fails during migration/snapshot due to new blocker
--
commit c60b9a0f169a17999aea7d76e3213dbee63fd377
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860
This patch saves each VFIO BAR mapping in the CPR ramblock list. After
cpr-exec, new QEMU reuses the saved mapping instead of calling mmap()
again. It also adds a blocker so cpr-transfer is refused for VFIO devices.
> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
[ ... ]
> @@ -464,12 +464,32 @@ bool vfio_device_attach(char *name, VFIODevice *vbasedev,
> TYPE_VFIO_IOMMU_IOMMUFD :
> TYPE_VFIO_IOMMU_LEGACY;
>
> - return vfio_device_attach_by_iommu_type(iommu_type, name, vbasedev,
> - as, errp);
> + /*
> + * Only cpr-exec keeps the address space and so the region mappings,
> + * see vfio_region_mmap(). A cpr-transfer target would have to map the
> + * regions anew, which some drivers (NVIDIA vGPU) refuse.
> + */
> + error_setg(&vbasedev->cpr.mmap_blocker,
> + "cpr-transfer is not supported with VFIO devices; "
> + "use the cpr-exec mode");
> + if (migrate_add_blocker_modes(&vbasedev->cpr.mmap_blocker, errp,
> + MIG_MODE_CPR_TRANSFER, -1)) {
> + return false;
> + }
[Severity: Low]
Can this make vfio_device_attach() fail while a migration or savevm is
running? migrate_add_blocker_modes() calls is_busy() regardless of which
modes were requested:
migration/migration.c:is_busy() {
if (runstate_check(RUN_STATE_SAVE_VM) || migration_is_running()) {
error_propagate_prepend(errp, *reasonp, ...);
*reasonp = NULL;
return true;
}
}
migrate_add_blocker_modes() then returns -EBUSY, and vfio_device_attach()
returns false. So hotplugging a VFIO device during a migration or savevm
would now be refused, even though the new blocker only applies to
cpr-transfer. Before this patch, attach did not depend on migration state
for devices that needed no blocker.
Is that behaviour intended for this blocker?
> +
> + if (!vfio_device_attach_by_iommu_type(iommu_type, name, vbasedev,
> + as, errp)) {
> + migrate_del_blocker(&vbasedev->cpr.mmap_blocker);
> + return false;
> + }
> +
> + return true;
> }
[ ... ]
--
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=27
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH QEMU HCI-8.1 v3 28/28] VZ: migration/cpr-exec: allow cpr-exec with VFIO devices #VSTOR-103860
2026-10-16 15:25 [PATCH QEMU HCI-8.1 v3 00/28] vfio: support qemu-update with passed through GPU's and vGPU's #VSTOR-103860 Andrey Drobyshev
` (26 preceding siblings ...)
2026-10-16 15:26 ` [PATCH QEMU HCI-8.1 v3 27/28] VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860 Andrey Drobyshev
@ 2026-10-16 15:26 ` Andrey Drobyshev
2026-10-16 15:59 ` no-reply
27 siblings, 1 reply; 57+ messages in thread
From: Andrey Drobyshev @ 2026-10-16 15:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev
Commit bb8da93ddfc4 ("VZ: migration/cpr-exec: enable checks prohibiting
cpr-exec #VSTOR-108564") introduced restrictions for cpr-exec migration
mode. Among them was prohibition to do such migration in case of
presence of any VFIO devices. Now we want to allow migration with such
devices in scope of VSTOR-103860. Therefore, let's lift that restiction.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
hw/vfio/device.c | 5 -----
include/hw/vfio/vfio-device.h | 2 --
migration/migration.c | 3 ---
3 files changed, 10 deletions(-)
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 2a7073ccec8..d1b595adb71 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -36,11 +36,6 @@
VFIODeviceList vfio_device_list =
QLIST_HEAD_INITIALIZER(vfio_device_list);
-bool vfio_devices_present(void)
-{
- return !QLIST_EMPTY(&vfio_device_list);
-}
-
/*
* We want to differentiate hot reset of multiple in-use devices vs
* hot reset of a single in-use device. VFIO_DEVICE_RESET will already
diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 935000f0074..fc28f580ba3 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -162,8 +162,6 @@ VFIODevice *vfio_get_vfio_device(Object *obj);
typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
extern VFIODeviceList vfio_device_list;
-bool vfio_devices_present(void);
-
#ifdef CONFIG_LINUX
/*
* How devices communicate with the server. The default option is through
diff --git a/migration/migration.c b/migration/migration.c
index f48e75e8fad..b7c93e2e5ec 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2103,9 +2103,6 @@ static bool migrate_cpr_is_blocked(Error **errp)
} else if (cbw_filter_present()) {
error_setg(errp, "CPR is prohibited: active block snapshots (CBW "
"filter) present");
- } else if (vfio_devices_present()) {
- error_setg(errp, "CPR is prohibited: passthrough (VFIO) "
- "devices present");
} else {
res = false;
}
--
2.47.1
^ permalink raw reply [flat|nested] 57+ messages in thread