4:11 a.m.
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
4:11 a.m.
New subject: [libvirt PATCH v2 1/5] qemu: introduce capability QEMU_MIGRATION_BLOCKED_REASONS
From: Jonathon Jongsma <jjongsma(a)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(a)redhat.com>
Signed-off-by: Eugenio Pérez <eperezma(a)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
5:15 a.m.
New subject: [libvirt PATCH v2 1/5] qemu: introduce capability QEMU_MIGRATION_BLOCKED_REASONS
On Wed, Jul 20, 2022 at 11:11:50 +0200, Eugenio Pérez wrote:
From: Jonathon Jongsma <jjongsma(a)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(a)redhat.com>
Signed-off-by: Eugenio Pérez <eperezma(a)redhat.com>
s/QEMU_MIGRATION_BLOCKED_REASONS/QEMU_CAPS_MIGRATION_BLOCKED_REASONS/ in
the subject.
Jirka
4:11 a.m.
New subject: [libvirt PATCH v2 2/5] qemu_monitor_json: Add qemuMonitorJSONGetMigrationBlockers
This will allow qemuMigrationSrcIsAllowed to dynamically ask for
migration blockers, reducing duplication.
Signed-off-by: Eugenio Pérez <eperezma(a)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
5:14 a.m.
New subject: [libvirt PATCH v2 2/5] qemu_monitor_json: Add qemuMonitorJSONGetMigrationBlockers
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(a)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
5:43 a.m.
New subject: [libvirt PATCH v2 2/5] qemu_monitor_json: Add qemuMonitorJSONGetMigrationBlockers
On Wed, Jul 20, 2022 at 12:14 PM Jiri Denemark <jdenemar(a)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(a)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
6:13 a.m.
New subject: [libvirt PATCH v2 2/5] qemu_monitor_json: Add qemuMonitorJSONGetMigrationBlockers
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(a)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(a)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
6:14 a.m.
New subject: [libvirt PATCH v2 2/5] qemu_monitor_json: Add qemuMonitorJSONGetMigrationBlockers
On Wed, Jul 20, 2022 at 1:13 PM Jiri Denemark <jdenemar(a)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(a)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(a)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
4:11 a.m.
New subject: [libvirt PATCH v2 3/5] qemu_monitor: add support for get qemu migration blockers
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(a)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
5:16 a.m.
New subject: [libvirt PATCH v2 3/5] qemu_monitor: add support for get qemu migration blockers
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(a)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
4:11 a.m.
New subject: [libvirt PATCH v2 4/5] qemu_migration: get migration blockers before hardcoded checks
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(a)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
5:31 a.m.
New subject: [libvirt PATCH v2 4/5] qemu_migration: get migration blockers before hardcoded checks
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(a)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
6:25 a.m.
New subject: [libvirt PATCH v2 4/5] qemu_migration: get migration blockers before hardcoded checks
On Wed, Jul 20, 2022 at 12:31 PM Jiri Denemark <jdenemar(a)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(a)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
6:29 a.m.
New subject: [libvirt PATCH v2 4/5] qemu_migration: get migration blockers before hardcoded checks
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(a)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(a)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
4:11 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
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(a)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
5:36 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
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(a)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
5:37 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
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(a)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
6:10 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 12:37 PM Jiri Denemark <jdenemar(a)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(a)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
6:11 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 12:36 PM Jiri Denemark <jdenemar(a)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(a)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
6:13 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
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(a)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(a)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.
6:15 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
On Wed, Jul 20, 2022 at 1:13 PM Peter Krempa <pkrempa(a)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(a)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(a)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!
6:16 a.m.
New subject: [libvirt PATCH v2 5/5] qemu_migrate: Do not forbif vDPA devices if can query blockers
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(a)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(a)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
856
days inactive
856
days old
21 comments
4 participants
participants (4)
-
Eugenio Perez Martin -
Eugenio Pérez
-
Jiri Denemark -
Peter Krempa