[PATCH 0/8] qemu: Fix regression with non-shared storage migration and clean out more dead code

Patch 1 fixes regression caused by improper refactor from the pre-blockdev removal series. The rest are cleanups noticed while investigating the code. Peter Krempa (8): qemu: migration: Fix setup of non-shared storage migration in qemuMigrationSrcBeginPhase NEWS: Mention that non-shared storage migration was broken in libvirt-8.7 qemu: migration: Always assume support for QEMU_CAPS_NBD_SERVER qemu: capabilities: Retire QEMU_CAPS_NBD_SERVER qemu: migration: Don't attempt to fall back to old-style storage migration qemu: monitor: Drop support for old-style non-shared storage migration qemu: migration: Remove QEMU_MONITOR_MIGRATE_BACKGROUND qemu: monitor: Renumber QEMU_MONITOR_MIGRATE_RESUME NEWS.rst | 6 ++ src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_migration.c | 78 ++++++++----------- src/qemu/qemu_monitor.h | 5 +- src/qemu/qemu_monitor_json.c | 7 +- .../caps_4.2.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 - .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 - .../caps_4.2.0.x86_64.xml | 1 - .../caps_5.0.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 - .../caps_5.0.0.riscv64.xml | 1 - .../caps_5.0.0.x86_64.xml | 1 - .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 - .../caps_5.1.0.x86_64.xml | 1 - .../caps_5.2.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 - .../caps_5.2.0.riscv64.xml | 1 - .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 - .../caps_5.2.0.x86_64.xml | 1 - .../caps_6.0.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 - .../caps_6.0.0.x86_64.xml | 1 - .../caps_6.1.0.x86_64.xml | 1 - .../caps_6.2.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 - .../caps_6.2.0.x86_64.xml | 1 - .../caps_7.0.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 - .../caps_7.0.0.x86_64.xml | 1 - .../caps_7.1.0.x86_64.xml | 1 - tests/qemumonitorjsontest.c | 4 +- 33 files changed, 42 insertions(+), 89 deletions(-) -- 2.37.1

In commit 6111b2352242e9 removing pre-blockdev code paths I've improperly refactored the setup of non-shared storage migration. Specifically the code checking that there are disks and setting up the NBD data in the migration cookie was originally outside of the loop checking the user provided list of specific disks to migrate, but became part of the block as it was not un-indented when a higher level block was being removed. The above caused that if non-shared storage migration is requested, but the user doesn't provide the list of disks to migrate (thus implying to migrate every appropriate disk) the code doesn't actually setup the migration and then later on falls back to the old-style migration which no longer works with blockdev. Move the check that there's anything to migrate out of the 'nmigrate_disks' block. Fixes: 6111b2352242e93c6d2c29f9549d596ed1056ce5 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2125111 Resolves: https://gitlab.com/libvirt/libvirt/-/issues/373 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 76a65bf298..5367b74de4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2609,14 +2609,14 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, return NULL; } } + } - priv->nbdPort = 0; + priv->nbdPort = 0; - if (qemuMigrationHasAnyStorageMigrationDisks(vm->def, - migrate_disks, - nmigrate_disks)) - cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; - } + if (qemuMigrationHasAnyStorageMigrationDisks(vm->def, + migrate_disks, + nmigrate_disks)) + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; } if (virDomainDefHasMemoryHotplug(vm->def) || -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 73d3c0054a..69c5b70c2f 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -27,6 +27,12 @@ v8.8.0 (unreleased) * **Bug fixes** + * qemu: Fix non-shared storage migration setup + + This release fixes a bug in setup of a migration with non-shared storage + ( ``virsh migrate --copy-storage-all``) which was broken by a refactor of + the code in libvirt-8.7. + v8.7.0 (2022-09-01) =================== -- 2.37.1

The NBD server (detected via 'nbd-server-start' qmp command) was added to qemu in v1.3 and can't be compiled out. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5367b74de4..995364da02 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3147,8 +3147,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, goto error; if (mig->nbd && - flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { + flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { const char *nbdTLSAlias = NULL; if (flags & VIR_MIGRATE_TLS) { @@ -3356,8 +3355,7 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, } if (mig->nbd && - flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) + flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; } -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_capabilities.h | 2 +- tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 - tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 - tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 - tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 - tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 - tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 - tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 - tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 - tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 - tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 - tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 - tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 - tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 - tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml | 1 - tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 - tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml | 1 - tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 - tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 1 - tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml | 1 - 28 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3e9983bb96..e84310c79a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -242,7 +242,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 125 */ "add-fd", /* X_QEMU_CAPS_ADD_FD */ - "nbd-server", /* QEMU_CAPS_NBD_SERVER */ + "nbd-server", /* X_QEMU_CAPS_NBD_SERVER */ "virtio-rng", /* QEMU_CAPS_DEVICE_VIRTIO_RNG */ "rng-random", /* QEMU_CAPS_OBJECT_RNG_RANDOM */ "rng-egd", /* QEMU_CAPS_OBJECT_RNG_EGD */ @@ -1207,7 +1207,6 @@ struct virQEMUCapsStringFlags { struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-spice", QEMU_CAPS_SPICE }, { "query-vnc", QEMU_CAPS_VNC }, - { "nbd-server-start", QEMU_CAPS_NBD_SERVER }, { "rtc-reset-reinjection", QEMU_CAPS_RTC_RESET_REINJECTION }, { "query-hotpluggable-cpus", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS }, { "query-cpu-model-expansion", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION }, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 395a3281d3..9cd00ba812 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -220,7 +220,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 125 */ X_QEMU_CAPS_ADD_FD, /* -add-fd */ - QEMU_CAPS_NBD_SERVER, /* nbd-server-start QMP command */ + X_QEMU_CAPS_NBD_SERVER, /* nbd-server-start QMP command */ QEMU_CAPS_DEVICE_VIRTIO_RNG, /* virtio-rng device */ QEMU_CAPS_OBJECT_RNG_RANDOM, /* the rng-random backend for virtio rng */ QEMU_CAPS_OBJECT_RNG_EGD, /* EGD protocol daemon for rng */ diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml index 8af26b9e7a..8223d13409 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml @@ -31,7 +31,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml index 24a885c279..96baa4e7bf 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml @@ -30,7 +30,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml index 2c4ba45d66..7bf0427077 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml @@ -16,7 +16,6 @@ <flag name='seccomp-sandbox'/> <flag name='vnc'/> <flag name='s390-sclp'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index f282f6d0ab..245a0bd2fb 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -40,7 +40,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index 50b42bca78..250ad9d914 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -31,7 +31,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml index 08146d2235..e356f431c2 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml @@ -30,7 +30,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml index 9376367a6b..19edb90927 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml @@ -34,7 +34,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index 0438554086..46bbfac186 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -40,7 +40,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml index 399d37218b..93ae1b6d0a 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml @@ -12,7 +12,6 @@ <flag name='scsi-disk.wwn'/> <flag name='seccomp-sandbox'/> <flag name='vnc'/> - <flag name='nbd-server'/> <flag name='rng-random'/> <flag name='rng-egd'/> <flag name='enable-fips'/> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index a94cf90d9b..9dbe8f1ca8 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -40,7 +40,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml index 20101d8c3b..f4c9ad1bc4 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml @@ -31,7 +31,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml index 3ea86de12a..b40b8a5735 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml @@ -30,7 +30,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml index 33efb8d367..7b4915b67f 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml @@ -34,7 +34,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml index 2b65c1a2f2..0523c8d22f 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml @@ -16,7 +16,6 @@ <flag name='seccomp-sandbox'/> <flag name='vnc'/> <flag name='s390-sclp'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index 0ca9043d73..c71275506a 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -40,7 +40,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml index 8502fca633..e38e8fa5f5 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml @@ -30,7 +30,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml index a3a6b57d65..4f009def58 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml @@ -16,7 +16,6 @@ <flag name='seccomp-sandbox'/> <flag name='vnc'/> <flag name='s390-sclp'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index 545df89eb1..876a5afe43 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -39,7 +39,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index 627eeb3395..f73c070c9e 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -39,7 +39,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml index fa329f2cbc..234f386736 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml @@ -35,7 +35,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml index 365dd8904d..4e90d3953a 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml @@ -30,7 +30,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml index cc15970f6d..fd57627ad5 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml @@ -39,7 +39,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml index 3e60c19a59..19a6c33353 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml @@ -35,7 +35,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml index 6f985a55d0..0d3c321a67 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml @@ -36,7 +36,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index d48c1f733c..dc7b041294 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -39,7 +39,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml index 9fb4ca58fa..5c60b7cf88 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml @@ -39,7 +39,6 @@ <flag name='cirrus-vga'/> <flag name='vmware-svga'/> <flag name='usb-serial'/> - <flag name='nbd-server'/> <flag name='virtio-rng'/> <flag name='rng-random'/> <flag name='rng-egd'/> -- 2.37.1

QEMU supported the NBD server required for the new-style migration for a long time already and when coupled with -blockdev the old style migration doesn't even work, thus remove support for it. This patch modifies the code to check that the destination returned data for the NBD migration and returns an error if it did not and deletes the fallback code paths which would not work. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 45 +++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 995364da02..7dc593d49f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4817,35 +4817,28 @@ qemuMigrationSrcRun(virQEMUDriver *driver, } if (storageMigration) { - if (mig->nbd) { - const char *host = ""; - const char *tlsHostname = qemuMigrationParamsGetTLSHostname(migParams); + const char *host = ""; + const char *tlsHostname = qemuMigrationParamsGetTLSHostname(migParams); - if (spec->destType == MIGRATION_DEST_HOST || - spec->destType == MIGRATION_DEST_CONNECT_HOST) { - host = spec->dest.host.name; - } - - if (qemuMigrationSrcNBDStorageCopy(driver, vm, mig, - host, - priv->migMaxBandwidth, - nmigrate_disks, - migrate_disks, - dconn, tlsAlias, tlsHostname, - nbdURI, flags) < 0) { - goto error; - } - } else { - /* Destination doesn't support NBD server. - * Fall back to previous implementation. */ - VIR_DEBUG("Destination doesn't support NBD server " - "Falling back to previous implementation."); + if (!mig->nbd) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("migration of non-shared disks requested but NBD is not set up")); + goto error; + } - if (flags & VIR_MIGRATE_NON_SHARED_DISK) - migrate_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_DISK; + if (spec->destType == MIGRATION_DEST_HOST || + spec->destType == MIGRATION_DEST_CONNECT_HOST) { + host = spec->dest.host.name; + } - if (flags & VIR_MIGRATE_NON_SHARED_INC) - migrate_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC; + if (qemuMigrationSrcNBDStorageCopy(driver, vm, mig, + host, + priv->migMaxBandwidth, + nmigrate_disks, + migrate_disks, + dconn, tlsAlias, tlsHostname, + nbdURI, flags) < 0) { + goto error; } } -- 2.37.1

Remove the support for enabling the 'blk' and 'inc' parameters of the 'migrate' command as there are no users any more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 2 -- src/qemu/qemu_monitor_json.c | 4 ---- tests/qemumonitorjsontest.c | 4 +--- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 63269e15bc..53ea26c58a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -826,8 +826,6 @@ int qemuMonitorGetSEVCapabilities(qemuMonitor *mon, typedef enum { QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, - QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared storage with full disk copy */ - QEMU_MONITOR_MIGRATE_NON_SHARED_INC = 1 << 2, /* migration with non-shared storage with incremental copy */ QEMU_MONITOR_MIGRATE_RESUME = 1 << 3, /* resume failed post-copy migration */ QEMU_MONITOR_MIGRATION_FLAGS_LAST } QEMU_MONITOR_MIGRATE; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 70fba50e6c..bf22cc64f8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3110,13 +3110,9 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, const char *uri) { bool detach = !!(flags & QEMU_MONITOR_MIGRATE_BACKGROUND); - bool blk = !!(flags & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK); - bool inc = !!(flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC); bool resume = !!(flags & QEMU_MONITOR_MIGRATE_RESUME); g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("migrate", "b:detach", detach, - "b:blk", blk, - "b:inc", inc, "b:resume", resume, "s:uri", uri, NULL); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index b508f63aea..fa5455e513 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1201,9 +1201,7 @@ GEN_TEST_FUNC(qemuMonitorJSONExpirePassword, "spice", "123456") GEN_TEST_FUNC(qemuMonitorJSONSetBalloon, 1024) GEN_TEST_FUNC(qemuMonitorJSONSaveVirtualMemory, 0, 1024, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONSavePhysicalMemory, 0, 1024, "/foo/bar") -GEN_TEST_FUNC(qemuMonitorJSONMigrate, QEMU_MONITOR_MIGRATE_BACKGROUND | - QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | - QEMU_MONITOR_MIGRATE_NON_SHARED_INC, "tcp:localhost:12345") +GEN_TEST_FUNC(qemuMonitorJSONMigrate, QEMU_MONITOR_MIGRATE_BACKGROUND, "tcp:localhost:12345") GEN_TEST_FUNC(qemuMonitorJSONMigrateRecover, "tcp://destination.host:54321"); GEN_TEST_FUNC(qemuMonitorJSONDump, "dummy_protocol", "elf", true) -- 2.37.1

'qemuMonitorJSONMigrate' is called from: - qemuMonitorMigrateToHost - qemuMonitorMigrateToSocket Both of the above function are called only from qemuMigrationSrcStart. - qemuMonitorMigrateToFd - called from: - qemuMigrationSrcToFile Both instances here pass QEMU_MONITOR_MIGRATE_BACKGROUND directly. - qemuMigrationSrcStart qemuMigrationSrcStart is then called from qemuMigrationSrcRun and qemuMigrationSrcResume, both of which always add QEMU_MONITOR_MIGRATE_BACKGROUND to the flags. Thus any caller always passes the flag so that we can remove the flag altogether. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 15 ++++----------- src/qemu/qemu_monitor.h | 1 - src/qemu/qemu_monitor_json.c | 3 +-- tests/qemumonitorjsontest.c | 2 +- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7dc593d49f..c63b00c922 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4678,7 +4678,6 @@ qemuMigrationSrcRun(virQEMUDriver *driver, const char *nbdURI) { int ret = -1; - unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND; qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(qemuMigrationCookie) mig = NULL; g_autofree char *tlsAlias = NULL; @@ -4867,7 +4866,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver, goto exit_monitor; } - rc = qemuMigrationSrcStart(vm, spec, migrate_flags, &fd); + rc = qemuMigrationSrcStart(vm, spec, 0, &fd); qemuDomainObjExitMonitor(vm); if (rc < 0) @@ -5033,8 +5032,6 @@ qemuMigrationSrcResume(virDomainObj *vm, qemuDomainObjPrivate *priv = vm->privateData; virQEMUDriver *driver = priv->driver; g_autoptr(qemuMigrationCookie) mig = NULL; - unsigned int migrateFlags = QEMU_MONITOR_MIGRATE_BACKGROUND | - QEMU_MONITOR_MIGRATE_RESUME; int rc; VIR_DEBUG("vm=%p", vm); @@ -5053,7 +5050,7 @@ qemuMigrationSrcResume(virDomainObj *vm, VIR_ASYNC_JOB_MIGRATION_OUT) < 0) return -1; - rc = qemuMigrationSrcStart(vm, spec, migrateFlags, NULL); + rc = qemuMigrationSrcStart(vm, spec, QEMU_MONITOR_MIGRATE_RESUME, NULL); qemuDomainObjExitMonitor(vm); if (rc < 0) @@ -6902,9 +6899,7 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, goto cleanup; if (!compressor) { - rc = qemuMonitorMigrateToFd(priv->mon, - QEMU_MONITOR_MIGRATE_BACKGROUND, - fd); + rc = qemuMonitorMigrateToFd(priv->mon, 0, fd); } else { virCommandSetInputFD(compressor, pipeFD[0]); virCommandSetOutputFD(compressor, &fd); @@ -6920,9 +6915,7 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, qemuDomainObjExitMonitor(vm); goto cleanup; } - rc = qemuMonitorMigrateToFd(priv->mon, - QEMU_MONITOR_MIGRATE_BACKGROUND, - pipeFD[1]); + rc = qemuMonitorMigrateToFd(priv->mon, 0, pipeFD[1]); if (VIR_CLOSE(pipeFD[0]) < 0 || VIR_CLOSE(pipeFD[1]) < 0) VIR_WARN("failed to close intermediate pipe"); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 53ea26c58a..c256e0f5ba 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -825,7 +825,6 @@ int qemuMonitorGetSEVCapabilities(qemuMonitor *mon, virSEVCapability **capabilities); typedef enum { - QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, QEMU_MONITOR_MIGRATE_RESUME = 1 << 3, /* resume failed post-copy migration */ QEMU_MONITOR_MIGRATION_FLAGS_LAST } QEMU_MONITOR_MIGRATE; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index bf22cc64f8..031238665f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3109,10 +3109,9 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, unsigned int flags, const char *uri) { - bool detach = !!(flags & QEMU_MONITOR_MIGRATE_BACKGROUND); bool resume = !!(flags & QEMU_MONITOR_MIGRATE_RESUME); g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("migrate", - "b:detach", detach, + "b:detach", true, "b:resume", resume, "s:uri", uri, NULL); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index fa5455e513..238c6c1813 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1201,7 +1201,7 @@ GEN_TEST_FUNC(qemuMonitorJSONExpirePassword, "spice", "123456") GEN_TEST_FUNC(qemuMonitorJSONSetBalloon, 1024) GEN_TEST_FUNC(qemuMonitorJSONSaveVirtualMemory, 0, 1024, "/foo/bar") GEN_TEST_FUNC(qemuMonitorJSONSavePhysicalMemory, 0, 1024, "/foo/bar") -GEN_TEST_FUNC(qemuMonitorJSONMigrate, QEMU_MONITOR_MIGRATE_BACKGROUND, "tcp:localhost:12345") +GEN_TEST_FUNC(qemuMonitorJSONMigrate, 0, "tcp:localhost:12345") GEN_TEST_FUNC(qemuMonitorJSONMigrateRecover, "tcp://destination.host:54321"); GEN_TEST_FUNC(qemuMonitorJSONDump, "dummy_protocol", "elf", true) -- 2.37.1

Now that all preceding flags were deleted we can fix the enum value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c256e0f5ba..4d770486be 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -825,7 +825,7 @@ int qemuMonitorGetSEVCapabilities(qemuMonitor *mon, virSEVCapability **capabilities); typedef enum { - QEMU_MONITOR_MIGRATE_RESUME = 1 << 3, /* resume failed post-copy migration */ + QEMU_MONITOR_MIGRATE_RESUME = 1 << 0, /* resume failed post-copy migration */ QEMU_MONITOR_MIGRATION_FLAGS_LAST } QEMU_MONITOR_MIGRATE; -- 2.37.1

On a Thursday in 2022, Peter Krempa wrote:
Patch 1 fixes regression caused by improper refactor from the pre-blockdev removal series. The rest are cleanups noticed while investigating the code.
Peter Krempa (8): qemu: migration: Fix setup of non-shared storage migration in qemuMigrationSrcBeginPhase NEWS: Mention that non-shared storage migration was broken in libvirt-8.7 qemu: migration: Always assume support for QEMU_CAPS_NBD_SERVER qemu: capabilities: Retire QEMU_CAPS_NBD_SERVER qemu: migration: Don't attempt to fall back to old-style storage migration qemu: monitor: Drop support for old-style non-shared storage migration qemu: migration: Remove QEMU_MONITOR_MIGRATE_BACKGROUND qemu: monitor: Renumber QEMU_MONITOR_MIGRATE_RESUME
NEWS.rst | 6 ++ src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_migration.c | 78 ++++++++----------- src/qemu/qemu_monitor.h | 5 +- src/qemu/qemu_monitor_json.c | 7 +- .../caps_4.2.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 - .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 - .../caps_4.2.0.x86_64.xml | 1 - .../caps_5.0.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 - .../caps_5.0.0.riscv64.xml | 1 - .../caps_5.0.0.x86_64.xml | 1 - .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 - .../caps_5.1.0.x86_64.xml | 1 - .../caps_5.2.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 - .../caps_5.2.0.riscv64.xml | 1 - .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 - .../caps_5.2.0.x86_64.xml | 1 - .../caps_6.0.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 - .../caps_6.0.0.x86_64.xml | 1 - .../caps_6.1.0.x86_64.xml | 1 - .../caps_6.2.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 - .../caps_6.2.0.x86_64.xml | 1 - .../caps_7.0.0.aarch64.xml | 1 - .../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 - .../caps_7.0.0.x86_64.xml | 1 - .../caps_7.1.0.x86_64.xml | 1 - tests/qemumonitorjsontest.c | 4 +- 33 files changed, 42 insertions(+), 89 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Peter Krempa