[libvirt PATCH v3 0/4] Ask qemu about migration blockers
There are some hardcoded migration blockers in libvirt, like having a net vhost-vdpa device. While it's true that they cannot be migrated at the moment, there are plans to be able to migrate them soon. They'll put a migration blockers in the cases where you cannot migrate them. Ask qemu about then before rejecting the migration. In case the question is not possible, assume they're not migratable. v3: * Return ok in qemuMonitorJSONGetMigrationBlockers is there are no blockers. * Fix indentation * Report all blockers instead of only the first. * Squash some patches * Move note to function doc. * s/QEMU_MIGRATION_BLOCKED_REASONS/QEMU_CAPS_MIGRATION_BLOCKED_REASONS/ v2: * Ask qemu if it has some pending blockers before try the migration Eugenio Pérez (3): qemu_monitor: add support for get qemu migration blockers qemu_migration: get migration blockers before hardcoded checks qemu_migration: Do not forbid vDPA devices if can query blockers Jonathon Jongsma (1): qemu: introduce capability QEMU_CAPS_MIGRATION_BLOCKED_REASONS src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_migration.c | 38 +++++++++++++++- src/qemu/qemu_monitor.c | 11 +++++ src/qemu/qemu_monitor.h | 4 ++ src/qemu/qemu_monitor_json.c | 43 +++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ .../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 + 18 files changed, 112 insertions(+), 1 deletion(-) -- 2.31.1
From: Jonathon Jongsma <jjongsma@redhat.com> since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: s/QEMU_MIGRATION_BLOCKED_REASONS/QEMU_CAPS_MIGRATION_BLOCKED_REASONS/ --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 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 + 13 files changed, 14 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 30b396d32d..b002fb98ed 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -672,6 +672,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "display-dbus", /* QEMU_CAPS_DISPLAY_DBUS */ "iothread.thread-pool-max", /* QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX */ "usb-host.guest-resets-all", /* QEMU_CAPS_USB_HOST_GUESTS_RESETS_ALL */ + "migration.blocked-reasons", /* QEMU_CAPS_MIGRATION_BLOCKED_REASONS */ ); @@ -1622,6 +1623,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "chardev-add/arg-type/backend/+qemu-vdagent", QEMU_CAPS_CHARDEV_QEMU_VDAGENT }, { "query-display-options/ret-type/+dbus", QEMU_CAPS_DISPLAY_DBUS }, { "object-add/arg-type/+iothread/thread-pool-max", QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX }, + { "query-migrate/ret-type/blocked-reasons", QEMU_CAPS_MIGRATION_BLOCKED_REASONS }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d979a5ba3b..8f3090e2ce 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -651,6 +651,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DISPLAY_DBUS, /* -display dbus */ QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX, /* -object iothread.thread-pool-max */ QEMU_CAPS_USB_HOST_GUESTS_RESETS_ALL, /* -device usb-host.guest-resets-all */ + QEMU_CAPS_MIGRATION_BLOCKED_REASONS, /* query-migrate returns 'blocked-reasons */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml index 01e30f4e02..4afd7b26ce 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml @@ -187,6 +187,7 @@ <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml index aa7b5deab5..c9cb85daa0 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml @@ -144,6 +144,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migration.blocked-reasons'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index d9e385ab1d..508804521c 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -229,6 +229,7 @@ <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index 05f297dfa2..d4a540fafd 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -234,6 +234,7 @@ <flag name='virtio-net.rss'/> <flag name='chardev.qemu-vdagent'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>6001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml index 9cb1a32354..71697fac95 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml @@ -199,6 +199,7 @@ <flag name='virtio-net.rss'/> <flag name='chardev.qemu-vdagent'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>6001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml index 5df148d787..3f86e03f18 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml @@ -193,6 +193,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migration.blocked-reasons'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml index dd011f8408..1a1a9643d4 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml @@ -236,6 +236,7 @@ <flag name='virtio-net.rss'/> <flag name='chardev.qemu-vdagent'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml index 39a5cd154d..a3991e9853 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml @@ -207,6 +207,7 @@ <flag name='virtio-net.rss'/> <flag name='chardev.qemu-vdagent'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>6002092</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml index 6e872f4f85..67dff32f50 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml @@ -211,6 +211,7 @@ <flag name='virtio-net.rss'/> <flag name='chardev.qemu-vdagent'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>7000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index a8f46df1cd..8ce423557e 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -241,6 +241,7 @@ <flag name='chardev.qemu-vdagent'/> <flag name='display-dbus'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>7000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml index 8464909698..2b2f1aef52 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml @@ -242,6 +242,7 @@ <flag name='display-dbus'/> <flag name='iothread.thread-pool-max'/> <flag name='usb-host.guest-resets-all'/> + <flag name='migration.blocked-reasons'/> <version>7000050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> -- 2.31.1
On Wed, Jul 20, 2022 at 14:15:55 +0200, Eugenio Pérez wrote:
From: Jonathon Jongsma <jjongsma@redhat.com>
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: s/QEMU_MIGRATION_BLOCKED_REASONS/QEMU_CAPS_MIGRATION_BLOCKED_REASONS/ --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 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 + 13 files changed, 14 insertions(+)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc. Enable qemu monitor to send this query. This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Squash some patches * Return ok in qemuMonitorJSONGetMigrationBlockers is there are no blockers. * Move note to function doc. --- src/qemu/qemu_monitor.c | 11 +++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 43 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 61 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 109107eaae..e0939beecd 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4486,3 +4486,14 @@ qemuMonitorMigrateRecover(qemuMonitor *mon, return qemuMonitorJSONMigrateRecover(mon, uri); } + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + VIR_DEBUG("blockers=%p", blockers); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetMigrationBlockers(mon, blockers); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cc1a0bc8c9..b82f198285 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1543,3 +1543,7 @@ qemuMonitorChangeMemoryRequestedSize(qemuMonitor *mon, int qemuMonitorMigrateRecover(qemuMonitor *mon, const char *uri); + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..6d15a458a3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,49 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; } +/* + * Get the exposed migration blockers. + * + * This function assume qemu has the capability of request them. + * + * It returns a NULL terminated array on blockers if there are any, or it set + * it to NULL otherwise. + */ +int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + virJSONValue *jblockers; + size_t i; + + *blockers = NULL; + if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + return -1; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(jblockers = virJSONValueObjectGetArray(data, "blocked-reasons"))) + return 0; + + *blockers = g_new0(char *, virJSONValueArraySize(jblockers) + 1); + for (i = 0; i < virJSONValueArraySize(jblockers); i++) { + virJSONValue *jblocker = virJSONValueArrayGet(jblockers, i); + const char *blocker = virJSONValueGetString(jblocker); + + (*blockers)[i] = g_strdup(blocker); + } + + return 0; +} + int qemuMonitorJSONMigrateCancel(qemuMonitor *mon) { g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("migrate_cancel", NULL); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 2759566892..e4c65e250e 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -184,6 +184,9 @@ qemuMonitorJSONMigrate(qemuMonitor *mon, unsigned int flags, const char *uri); int +qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers); +int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitor *mon, bool *spice_migrated); -- 2.31.1
On Wed, Jul 20, 2022 at 14:15:56 +0200, Eugenio Pérez wrote:
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc.
Enable qemu monitor to send this query. This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Squash some patches * Return ok in qemuMonitorJSONGetMigrationBlockers is there are no blockers. * Move note to function doc. --- src/qemu/qemu_monitor.c | 11 +++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 43 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 61 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 109107eaae..e0939beecd 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4486,3 +4486,14 @@ qemuMonitorMigrateRecover(qemuMonitor *mon,
return qemuMonitorJSONMigrateRecover(mon, uri); } + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + VIR_DEBUG("blockers=%p", blockers); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetMigrationBlockers(mon, blockers); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cc1a0bc8c9..b82f198285 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1543,3 +1543,7 @@ qemuMonitorChangeMemoryRequestedSize(qemuMonitor *mon, int qemuMonitorMigrateRecover(qemuMonitor *mon, const char *uri); + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..6d15a458a3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,49 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; }
+/* + * Get the exposed migration blockers. + * + * This function assume qemu has the capability of request them. + * + * It returns a NULL terminated array on blockers if there are any, or it set + * it to NULL otherwise. + */ +int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon,
I'm sorry for not noticing this in v2, but the return type should be on a separate line (yeah, I know the three functions around the place you put yours do not follow this). I can change it before pushing to avoid trivial v4 if you don't mind. With the change Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On Wed, Jul 20, 2022 at 3:58 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 14:15:56 +0200, Eugenio Pérez wrote:
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc.
Enable qemu monitor to send this query. This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Squash some patches * Return ok in qemuMonitorJSONGetMigrationBlockers is there are no blockers. * Move note to function doc. --- src/qemu/qemu_monitor.c | 11 +++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 43 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 61 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 109107eaae..e0939beecd 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4486,3 +4486,14 @@ qemuMonitorMigrateRecover(qemuMonitor *mon,
return qemuMonitorJSONMigrateRecover(mon, uri); } + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + VIR_DEBUG("blockers=%p", blockers); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetMigrationBlockers(mon, blockers); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index cc1a0bc8c9..b82f198285 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1543,3 +1543,7 @@ qemuMonitorChangeMemoryRequestedSize(qemuMonitor *mon, int qemuMonitorMigrateRecover(qemuMonitor *mon, const char *uri); + +int +qemuMonitorGetMigrationBlockers(qemuMonitor *mon, + char ***blockers); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..6d15a458a3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,49 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; }
+/* + * Get the exposed migration blockers. + * + * This function assume qemu has the capability of request them. + * + * It returns a NULL terminated array on blockers if there are any, or it set + * it to NULL otherwise. + */ +int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon,
I'm sorry for not noticing this in v2, but the return type should be on a separate line (yeah, I know the three functions around the place you put yours do not follow this).
I'll send a new version with this.
I can change it before pushing to avoid trivial v4 if you don't mind.
With the change Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc. Enable qemuMigrationSrcIsAllowed to query it. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Report message with a colon. * Report all blockers instead of only the first. --- src/qemu/qemu_migration.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..6ac4ef150b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1414,6 +1414,20 @@ qemuMigrationSrcIsAllowedHostdev(const virDomainDef *def) return true; } +static int +qemuDomainGetMigrationBlockers(virQEMUDriver *driver, + virDomainObj *vm, + char ***blockers) +{ + qemuDomainObjPrivate *priv = vm->privateData; + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorGetMigrationBlockers(priv->mon, blockers); + qemuDomainObjExitMonitor(vm); + + return rc; +} /** * qemuMigrationSrcIsAllowed: @@ -1439,6 +1453,26 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int nsnapshots; int pauseReason; size_t i; + int r; + + /* Ask qemu if it have a migration blocker */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + g_auto(GStrv) blockers = NULL; + r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); + if (r != 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), + _("error getting blockers")); + return false; + } + + if (blockers && blockers[0]) { + g_autofree char *reasons = g_strjoinv(", ", blockers); + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), reasons); + return false; + } + } /* perform these checks only when migrating to remote hosts */ if (remote) { -- 2.31.1
On Wed, Jul 20, 2022 at 14:15:57 +0200, Eugenio Pérez wrote:
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc.
Enable qemuMigrationSrcIsAllowed to query it.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Report message with a colon. * Report all blockers instead of only the first. --- src/qemu/qemu_migration.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..6ac4ef150b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1414,6 +1414,20 @@ qemuMigrationSrcIsAllowedHostdev(const virDomainDef *def) return true; }
+static int +qemuDomainGetMigrationBlockers(virQEMUDriver *driver, + virDomainObj *vm, + char ***blockers) +{ + qemuDomainObjPrivate *priv = vm->privateData; + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorGetMigrationBlockers(priv->mon, blockers); + qemuDomainObjExitMonitor(vm); + + return rc; +}
/** * qemuMigrationSrcIsAllowed: @@ -1439,6 +1453,26 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int nsnapshots; int pauseReason; size_t i; + int r; + + /* Ask qemu if it have a migration blocker */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + g_auto(GStrv) blockers = NULL; + r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); + if (r != 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), + _("error getting blockers")); + return false; + }
As mentioned in v2 review the virReportError call should be dropped as it overwrites the error reported by qemuDomainGetMigrationBlockers. That is, you can just if (qemuDomainGetMigrationBlockers(driver, vm, &blockers) < 0) return false;
+ + if (blockers && blockers[0]) { + g_autofree char *reasons = g_strjoinv(", ", blockers);
In the following patch you change ", " to "; ". I don't mind that much either way, but it should be done in this patch :-)
+ virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), reasons); + return false; + } + }
/* perform these checks only when migrating to remote hosts */ if (remote) {
Hmm, easy but not trivial changes so I guess v4 would be better. Jirka
On Wed, Jul 20, 2022 at 4:01 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 14:15:57 +0200, Eugenio Pérez wrote:
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc.
Enable qemuMigrationSrcIsAllowed to query it.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Report message with a colon. * Report all blockers instead of only the first. --- src/qemu/qemu_migration.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..6ac4ef150b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1414,6 +1414,20 @@ qemuMigrationSrcIsAllowedHostdev(const virDomainDef *def) return true; }
+static int +qemuDomainGetMigrationBlockers(virQEMUDriver *driver, + virDomainObj *vm, + char ***blockers) +{ + qemuDomainObjPrivate *priv = vm->privateData; + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorGetMigrationBlockers(priv->mon, blockers); + qemuDomainObjExitMonitor(vm); + + return rc; +}
/** * qemuMigrationSrcIsAllowed: @@ -1439,6 +1453,26 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int nsnapshots; int pauseReason; size_t i; + int r; + + /* Ask qemu if it have a migration blocker */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + g_auto(GStrv) blockers = NULL; + r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); + if (r != 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), + _("error getting blockers")); + return false; + }
As mentioned in v2 review the virReportError call should be dropped as it overwrites the error reported by qemuDomainGetMigrationBlockers. That is, you can just
if (qemuDomainGetMigrationBlockers(driver, vm, &blockers) < 0) return false;
But there are a few conditions that don't report an error, like a bad JSON answer. For example, if "blockers" is not an array parsing the response JSON, libvirt would not print any error, isn't it?
+ + if (blockers && blockers[0]) { + g_autofree char *reasons = g_strjoinv(", ", blockers);
In the following patch you change ", " to "; ". I don't mind that much either way, but it should be done in this patch :-)
+ virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), reasons); + return false; + } + }
/* perform these checks only when migrating to remote hosts */ if (remote) {
Hmm, easy but not trivial changes so I guess v4 would be better.
Jirka
On Wed, Jul 20, 2022 at 16:29:45 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 4:01 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 14:15:57 +0200, Eugenio Pérez wrote:
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc.
Enable qemuMigrationSrcIsAllowed to query it.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Report message with a colon. * Report all blockers instead of only the first. --- src/qemu/qemu_migration.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..6ac4ef150b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1414,6 +1414,20 @@ qemuMigrationSrcIsAllowedHostdev(const virDomainDef *def) return true; }
+static int +qemuDomainGetMigrationBlockers(virQEMUDriver *driver, + virDomainObj *vm, + char ***blockers) +{ + qemuDomainObjPrivate *priv = vm->privateData; + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorGetMigrationBlockers(priv->mon, blockers); + qemuDomainObjExitMonitor(vm); + + return rc; +}
/** * qemuMigrationSrcIsAllowed: @@ -1439,6 +1453,26 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int nsnapshots; int pauseReason; size_t i; + int r; + + /* Ask qemu if it have a migration blocker */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + g_auto(GStrv) blockers = NULL; + r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); + if (r != 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), + _("error getting blockers")); + return false; + }
As mentioned in v2 review the virReportError call should be dropped as it overwrites the error reported by qemuDomainGetMigrationBlockers. That is, you can just
if (qemuDomainGetMigrationBlockers(driver, vm, &blockers) < 0) return false;
But there are a few conditions that don't report an error, like a bad JSON answer. For example, if "blockers" is not an array parsing the response JSON, libvirt would not print any error, isn't it?
Well in qemuDomainGetMigrationBlockers you now have if (!(jblockers = virJSONValueObjectGetArray(data, "blocked-reasons"))) return 0; so you would not get pass r != 0 in such case anyway. If you wanted to distinguish missing blocked-reasons (that is no blockers) and blocked-reasons which is not an array (invalid response), you would need to do something else: if (!(jblockers = virJSONValueObjectGet(data, "blocked-reasons"))) return 0; if (!virJSONValueIsArray(jblockers)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blocker-reasons is not an array")); return -1; } All this inside qemuDomainGetMigrationBlockers() to make sure the function reports an error when returning -1. The caller would not report any error anyway. Jirka
On Wed, Jul 20, 2022 at 4:48 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 16:29:45 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 4:01 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 14:15:57 +0200, Eugenio Pérez wrote:
since qemu 6.0, if migration is blocked for some reason, 'query-migrate' will return an array of error strings describing the migration blockers. This can be used to check whether there are any devices blocking migration, etc.
Enable qemuMigrationSrcIsAllowed to query it.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: * Report message with a colon. * Report all blockers instead of only the first. --- src/qemu/qemu_migration.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..6ac4ef150b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1414,6 +1414,20 @@ qemuMigrationSrcIsAllowedHostdev(const virDomainDef *def) return true; }
+static int +qemuDomainGetMigrationBlockers(virQEMUDriver *driver, + virDomainObj *vm, + char ***blockers) +{ + qemuDomainObjPrivate *priv = vm->privateData; + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorGetMigrationBlockers(priv->mon, blockers); + qemuDomainObjExitMonitor(vm); + + return rc; +}
/** * qemuMigrationSrcIsAllowed: @@ -1439,6 +1453,26 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int nsnapshots; int pauseReason; size_t i; + int r; + + /* Ask qemu if it have a migration blocker */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + g_auto(GStrv) blockers = NULL; + r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); + if (r != 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain: %s"), + _("error getting blockers")); + return false; + }
As mentioned in v2 review the virReportError call should be dropped as it overwrites the error reported by qemuDomainGetMigrationBlockers. That is, you can just
if (qemuDomainGetMigrationBlockers(driver, vm, &blockers) < 0) return false;
But there are a few conditions that don't report an error, like a bad JSON answer. For example, if "blockers" is not an array parsing the response JSON, libvirt would not print any error, isn't it?
Well in qemuDomainGetMigrationBlockers you now have
if (!(jblockers = virJSONValueObjectGetArray(data, "blocked-reasons"))) return 0;
so you would not get pass r != 0 in such case anyway. If you wanted to distinguish missing blocked-reasons (that is no blockers) and blocked-reasons which is not an array (invalid response), you would need to do something else:
if (!(jblockers = virJSONValueObjectGet(data, "blocked-reasons"))) return 0;
if (!virJSONValueIsArray(jblockers)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blocker-reasons is not an array")); return -1; }
All this inside qemuDomainGetMigrationBlockers() to make sure the function reports an error when returning -1. The caller would not report any error anyway.
That's right. Deleting error report then. Thanks!
vDPA devices will be migratable soon. Since they are not migratable before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking for migration blockers, let it hardcoded in that case. Otherwise, ask qemu about the explicit blocker. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: Fix indentation --- src/qemu/qemu_migration.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6ac4ef150b..45e16242f0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int pauseReason; size_t i; int r; + bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_MIGRATION_BLOCKED_REASONS); /* Ask qemu if it have a migration blocker */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + if (blockedReasonsCap) { g_auto(GStrv) blockers = NULL; r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); if (r != 0) { @@ -1467,7 +1469,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, } if (blockers && blockers[0]) { - g_autofree char *reasons = g_strjoinv(", ", blockers); + g_autofree char *reasons = g_strjoinv("; ", blockers); virReportError(VIR_ERR_OPERATION_INVALID, _("cannot migrate domain: %s"), reasons); return false; @@ -1580,7 +1582,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainNetDef *net = vm->def->nets[i]; qemuSlirp *slirp; - if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) { + if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("vDPA devices cannot be migrated")); return false; -- 2.31.1
On Wed, Jul 20, 2022 at 14:15:58 +0200, Eugenio Pérez wrote:
vDPA devices will be migratable soon. Since they are not migratable before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking for migration blockers, let it hardcoded in that case.
Otherwise, ask qemu about the explicit blocker.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: Fix indentation --- src/qemu/qemu_migration.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6ac4ef150b..45e16242f0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int pauseReason; size_t i; int r; + bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_MIGRATION_BLOCKED_REASONS);
/* Ask qemu if it have a migration blocker */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + if (blockedReasonsCap) { g_auto(GStrv) blockers = NULL; r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); if (r != 0) { @@ -1467,7 +1469,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, }
if (blockers && blockers[0]) { - g_autofree char *reasons = g_strjoinv(", ", blockers); + g_autofree char *reasons = g_strjoinv("; ", blockers); virReportError(VIR_ERR_OPERATION_INVALID, _("cannot migrate domain: %s"), reasons); return false;
This hunk should be squashed into the previous patch.
@@ -1580,7 +1582,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainNetDef *net = vm->def->nets[i]; qemuSlirp *slirp;
- if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) { + if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("vDPA devices cannot be migrated")); return false;
With that change Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On Wed, Jul 20, 2022 at 4:02 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 14:15:58 +0200, Eugenio Pérez wrote:
vDPA devices will be migratable soon. Since they are not migratable before qemu 6.0, and qemu pre-6.0 didn't have the capability of asking for migration blockers, let it hardcoded in that case.
Otherwise, ask qemu about the explicit blocker.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- v3: Fix indentation --- src/qemu/qemu_migration.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6ac4ef150b..45e16242f0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1454,9 +1454,11 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, int pauseReason; size_t i; int r; + bool blockedReasonsCap = virQEMUCapsGet(priv->qemuCaps, + QEMU_CAPS_MIGRATION_BLOCKED_REASONS);
/* Ask qemu if it have a migration blocker */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_BLOCKED_REASONS)) { + if (blockedReasonsCap) { g_auto(GStrv) blockers = NULL; r = qemuDomainGetMigrationBlockers(driver, vm, &blockers); if (r != 0) { @@ -1467,7 +1469,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, }
if (blockers && blockers[0]) { - g_autofree char *reasons = g_strjoinv(", ", blockers); + g_autofree char *reasons = g_strjoinv("; ", blockers); virReportError(VIR_ERR_OPERATION_INVALID, _("cannot migrate domain: %s"), reasons); return false;
This hunk should be squashed into the previous patch.
Sorry, I squashed in the wrong patch, I'll send v4 with this. Thanks!
@@ -1580,7 +1582,7 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainNetDef *net = vm->def->nets[i]; qemuSlirp *slirp;
- if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) { + if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("vDPA devices cannot be migrated")); return false;
With that change
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (3)
-
Eugenio Perez Martin -
Eugenio Pérez
-
Jiri Denemark