[libvirt PATCH v2 0/5] 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. Eugenio Pérez (4): qemu_monitor_json: Add qemuMonitorJSONGetMigrationBlockers qemu_monitor: add support for get qemu migration blockers qemu_migration: get migration blockers before hardcoded checks qemu_migrate: Do not forbif vDPA devices if can query blockers Jonathon Jongsma (1): qemu: introduce capability QEMU_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 | 35 +++++++++++++++++ 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, 104 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> --- 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 11:11:50 +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>
s/QEMU_MIGRATION_BLOCKED_REASONS/QEMU_CAPS_MIGRATION_BLOCKED_REASONS/ in the subject. Jirka
This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- src/qemu/qemu_monitor_json.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 38 insertions(+) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..a53d721720 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,41 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; } +int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + virJSONValue *jblockers; + size_t i; + + 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 -1; + + /* NULL terminated array */ + *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 11:11:51 +0200, Eugenio Pérez wrote:
This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- src/qemu/qemu_monitor_json.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 38 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..a53d721720 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,41 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; }
+int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + virJSONValue *jblockers; + size_t i; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL))) + return -1;
We already have a function which calls query-migrate in JSON monitor, but I actually agree with adding this separate function as it serves a completely different purpose.
+ + 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 -1;
This is not an error (not to mention that you would return -1 without a proper error message) as missing blocked-reasons means there's no migration blocker and the domain can be migrated (as long as the QEMU_CAPS_MIGRATION_BLOCKED_REASONS capability is set). I think we should return 0 and set *blockers = NULL in this case.
+ + /* NULL terminated array */
This comment would be better as part of a proper documentation above the function.
+ *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);
Jirka
On Wed, Jul 20, 2022 at 12:14 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:51 +0200, Eugenio Pérez wrote:
This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- src/qemu/qemu_monitor_json.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 38 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..a53d721720 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,41 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; }
+int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + virJSONValue *jblockers; + size_t i; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL))) + return -1;
We already have a function which calls query-migrate in JSON monitor, but I actually agree with adding this separate function as it serves a completely different purpose.
I'm totally ok if anybody prefers to merge both too.
+ + 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 -1;
This is not an error (not to mention that you would return -1 without a proper error message) as missing blocked-reasons means there's no migration blocker and the domain can be migrated (as long as the QEMU_CAPS_MIGRATION_BLOCKED_REASONS capability is set).
Right, I'll fix it. Regarding the message on failure, a lot of the functions in this file returns -1 without a message. How can I print it properly here or propagate to the caller?
I think we should return 0 and set *blockers = NULL in this case.
Sure, I'll change that.
+ + /* NULL terminated array */
This comment would be better as part of a proper documentation above the function.
I'll move to the function doc. Thanks!
+ *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);
Jirka
On Wed, Jul 20, 2022 at 12:43:44 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 12:14 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:51 +0200, Eugenio Pérez wrote:
This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- src/qemu/qemu_monitor_json.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 38 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..a53d721720 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,41 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; }
+int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + virJSONValue *jblockers; + size_t i; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL))) + return -1;
We already have a function which calls query-migrate in JSON monitor, but I actually agree with adding this separate function as it serves a completely different purpose.
I'm totally ok if anybody prefers to merge both too.
I don't think anyone would prefer that as the existing function is a big beast which would get even more complicated if generalized for this use case.
+ + 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 -1;
This is not an error (not to mention that you would return -1 without a proper error message) as missing blocked-reasons means there's no migration blocker and the domain can be migrated (as long as the QEMU_CAPS_MIGRATION_BLOCKED_REASONS capability is set).
Right, I'll fix it.
Regarding the message on failure, a lot of the functions in this file returns -1 without a message. How can I print it properly here or propagate to the caller?
Well, returning -1 is fine if the function called (qemuMonitorJSONCommand, qemuMonitorJSONCheckReply) already reports an error. But this is not the case of virJSONValueObjectGetArray. Reporting an error would be done just by calling virReportError() here instead of having it in the caller. Anyway, nothing to worry about here as you will be returning 0. Jirka
On Wed, Jul 20, 2022 at 1:13 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 12:43:44 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 12:14 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:51 +0200, Eugenio Pérez wrote:
This will allow qemuMigrationSrcIsAllowed to dynamically ask for migration blockers, reducing duplication.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- src/qemu/qemu_monitor_json.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 38 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e4a86e5ad..a53d721720 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3338,6 +3338,41 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, return 0; }
+int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon, + char ***blockers) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data; + virJSONValue *jblockers; + size_t i; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL))) + return -1;
We already have a function which calls query-migrate in JSON monitor, but I actually agree with adding this separate function as it serves a completely different purpose.
I'm totally ok if anybody prefers to merge both too.
I don't think anyone would prefer that as the existing function is a big beast which would get even more complicated if generalized for this use case.
+ + 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 -1;
This is not an error (not to mention that you would return -1 without a proper error message) as missing blocked-reasons means there's no migration blocker and the domain can be migrated (as long as the QEMU_CAPS_MIGRATION_BLOCKED_REASONS capability is set).
Right, I'll fix it.
Regarding the message on failure, a lot of the functions in this file returns -1 without a message. How can I print it properly here or propagate to the caller?
Well, returning -1 is fine if the function called (qemuMonitorJSONCommand, qemuMonitorJSONCheckReply) already reports an error. But this is not the case of virJSONValueObjectGetArray. Reporting an error would be done just by calling virReportError() here instead of having it in the caller. Anyway, nothing to worry about here as you will be returning 0.
Got it. Thanks!
Jirka
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. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- src/qemu/qemu_monitor.c | 11 +++++++++++ src/qemu/qemu_monitor.h | 4 ++++ 2 files changed, 15 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); -- 2.31.1
On Wed, Jul 20, 2022 at 11:11:52 +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.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- src/qemu/qemu_monitor.c | 11 +++++++++++ src/qemu/qemu_monitor.h | 4 ++++ 2 files changed, 15 insertions(+)
You can just squash this into the previous patch. Jirka
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> --- src/qemu/qemu_migration.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..4224339f39 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,25 @@ 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[0]) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain, %s"), blockers[0]); + return false; + } + } /* perform these checks only when migrating to remote hosts */ if (remote) { -- 2.31.1
On Wed, Jul 20, 2022 at 11:11:53 +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> --- src/qemu/qemu_migration.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..4224339f39 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,25 @@ 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"));
If qemuDomainGetMigrationBlockers returned -1 a better error message should already be set and you would overwrite it here. Also with your current patch you would report this error even in case there was no migration blocker reported by QEMU. But this part would be fixed by letting qemuDomainGetMigrationBlockers return 0 when the blocked-reasons field is missing.
+ return false; + } + + if (blockers[0]) {
blockers && blockers[0]
+ virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain, %s"), blockers[0]);
I would prefer a colon there: "cannot migrate domain: %s". And we got a list of blockers from QEMU, but only use the first one? Please, join them with g_strjoinv().
+ return false; + } + }
/* perform these checks only when migrating to remote hosts */ if (remote) {
Jirka
On Wed, Jul 20, 2022 at 12:31 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:53 +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> --- src/qemu/qemu_migration.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..4224339f39 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,25 @@ 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"));
If qemuDomainGetMigrationBlockers returned -1 a better error message should already be set and you would overwrite it here. Also with your current patch you would report this error even in case there was no migration blocker reported by QEMU. But this part would be fixed by letting qemuDomainGetMigrationBlockers return 0 when the blocked-reasons field is missing.
Got it.
+ return false; + } + + if (blockers[0]) {
blockers && blockers[0]
+ virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain, %s"), blockers[0]);
I would prefer a colon there: "cannot migrate domain: %s". And we got a list of blockers from QEMU, but only use the first one? Please, join them with g_strjoinv().
So the resulting message should be: "cannot migrate domain: <error1>, <error2>, ..., <errorN>", right? Thanks!
+ return false; + } + }
/* perform these checks only when migrating to remote hosts */ if (remote) {
Jirka
On Wed, Jul 20, 2022 at 13:25:45 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 12:31 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:53 +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> --- src/qemu/qemu_migration.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b12cb518ee..4224339f39 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,25 @@ 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"));
If qemuDomainGetMigrationBlockers returned -1 a better error message should already be set and you would overwrite it here. Also with your current patch you would report this error even in case there was no migration blocker reported by QEMU. But this part would be fixed by letting qemuDomainGetMigrationBlockers return 0 when the blocked-reasons field is missing.
Got it.
+ return false; + } + + if (blockers[0]) {
blockers && blockers[0]
+ virReportError(VIR_ERR_OPERATION_INVALID, + _("cannot migrate domain, %s"), blockers[0]);
I would prefer a colon there: "cannot migrate domain: %s". And we got a list of blockers from QEMU, but only use the first one? Please, join them with g_strjoinv().
So the resulting message should be: "cannot migrate domain: <error1>, <error2>, ..., <errorN>", right?
Yeah, something like that... or with "; " as a separator to avoid confusion in case any migration blocker contains ','. Jirka
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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4224339f39..2f5c1d8121 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) { @@ -1579,7 +1581,8 @@ 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 11:11:54 +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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4224339f39..2f5c1d8121 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);
Wrong indentation of the second line (QEMU... should be aligned with priv).
/* 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) { @@ -1579,7 +1581,8 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainNetDef *net = vm->def->nets[i]; qemuSlirp *slirp;
- if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) { +
Extra empty line.
+ if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
So possibly we could skip more checks in the function if migration blockers are supported by QEMU, but this is a good conservative approach. The other checks (if any) can be taken care of separately.
virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("vDPA devices cannot be migrated")); return false;
Jirka
On Wed, Jul 20, 2022 at 12:36:16 +0200, Jiri Denemark wrote:
On Wed, Jul 20, 2022 at 11:11:54 +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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Oh and I forgot to mention s/qemu_migrate/qemu_migration/ in the subject. Jirka
On Wed, Jul 20, 2022 at 12:37 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 12:36:16 +0200, Jiri Denemark wrote:
On Wed, Jul 20, 2022 at 11:11:54 +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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Oh and I forgot to mention s/qemu_migrate/qemu_migration/ in the subject.
I'll fix it in the next version. Thanks!
Jirka
On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:54 +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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4224339f39..2f5c1d8121 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);
Wrong indentation of the second line (QEMU... should be aligned with priv).
I'm fine with that indentation, but that will be a line with more than 80 chars.
/* 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) { @@ -1579,7 +1581,8 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainNetDef *net = vm->def->nets[i]; qemuSlirp *slirp;
- if (net->type == VIR_DOMAIN_NET_TYPE_VDPA) { +
Extra empty line.
I'll delete it in the next version.
+ if (!blockedReasonsCap && net->type == VIR_DOMAIN_NET_TYPE_VDPA) {
So possibly we could skip more checks in the function if migration blockers are supported by QEMU, but this is a good conservative approach. The other checks (if any) can be taken care of separately.
I did a fast search but I didn't see something super obvious to me. Thanks!
virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("vDPA devices cannot be migrated")); return false;
Jirka
On Wed, Jul 20, 2022 at 13:11:57 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:54 +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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4224339f39..2f5c1d8121 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);
Wrong indentation of the second line (QEMU... should be aligned with priv).
I'm fine with that indentation, but that will be a line with more than 80 chars.
The max 80 columns limitation is only a soft limit. We prefer way more to keep alignment and readability of the code rather than stick to the antiquated 80 columns limit.
On Wed, Jul 20, 2022 at 1:13 PM Peter Krempa <pkrempa@redhat.com> wrote:
On Wed, Jul 20, 2022 at 13:11:57 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:54 +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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4224339f39..2f5c1d8121 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);
Wrong indentation of the second line (QEMU... should be aligned with priv).
I'm fine with that indentation, but that will be a line with more than 80 chars.
The max 80 columns limitation is only a soft limit. We prefer way more to keep alignment and readability of the code rather than stick to the antiquated 80 columns limit.
Got it, I'll indent properly for the next version then. Thanks!
On Wed, Jul 20, 2022 at 13:11:57 +0200, Eugenio Perez Martin wrote:
On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark <jdenemar@redhat.com> wrote:
On Wed, Jul 20, 2022 at 11:11:54 +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> --- src/qemu/qemu_migration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4224339f39..2f5c1d8121 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);
Wrong indentation of the second line (QEMU... should be aligned with priv).
I'm fine with that indentation, but that will be a line with more than 80 chars.
No problem, it's not a hard requirement anymore in cases where it would make the result look worse. Jirka
participants (4)
-
Eugenio Perez Martin -
Eugenio Pérez
-
Jiri Denemark -
Peter Krempa