[libvirt PATCH 0/3] Fix migration with host-passthrough CPU
Fixes a regression in libvirt 6.5.0 caused by commit 201bd5db63. See patches 2 and 3 for details. Jiri Denemark (3): qemu_monitor: Add API for checking CPU migratable property qemu: Do not set //cpu/@migratable for running domains in post-parse qemu: Properly set //cpu/@migratable default value for running domains src/qemu/qemu_domain.c | 8 ++++-- src/qemu/qemu_monitor.c | 18 +++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 27 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ src/qemu/qemu_process.c | 49 ++++++++++++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 2 deletions(-) -- 2.27.0
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 18 ++++++++++++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++++ 4 files changed, 53 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2c66397f8b..637361d24d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4526,6 +4526,24 @@ qemuMonitorGetJobInfo(qemuMonitorPtr mon, } +/* qemuMonitorGetCPUMigratable: + * + * Get the migratable property of the CPU object. + * + * Returns -1 on error, + * 1 when the property is not supported, + * 0 on success (@migratable is set accordingly). + */ +int +qemuMonitorGetCPUMigratable(qemuMonitorPtr mon, + bool *migratable) +{ + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetCPUMigratable(mon, migratable); +} + + int qemuMonitorTransactionBitmapAdd(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 50a337715d..1c1b0c9b89 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1377,6 +1377,10 @@ int qemuMonitorGetJobInfo(qemuMonitorPtr mon, qemuMonitorJobInfoPtr **jobs, size_t *njobs); +int +qemuMonitorGetCPUMigratable(qemuMonitorPtr mon, + bool *migratable); + int qemuMonitorTransactionBitmapAdd(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d808c4b55b..83f169e31b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9388,3 +9388,30 @@ qemuMonitorJSONGetJobInfo(qemuMonitorPtr mon, return 0; } + + +int +qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon, + bool *migratable) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", QOM_CPU_PATH, + "s:property", "migratable", + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONHasError(reply, "GenericError")) + return 1; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_BOOLEAN) < 0) + return -1; + + return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"), + migratable); +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 83c5e25ca5..84fea25983 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -690,3 +690,7 @@ int qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon, const char *vmstatepath, const char **list) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +int +qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon, + bool *migratable); -- 2.27.0
Commit v6.4.0-61-g201bd5db63 started to fill the default value for //cpu/@migratable attribute according to QEMU support. However, active domains either have the migratable attribute already set or the capabilities we use for checking the QEMU support were created by older libvirt which didn't probe for this specific capability. Thus we should leave active domains alone when parsing their XMLs. https://bugzilla.redhat.com/show_bug.cgi?id=1857967 Reported-by: Mark Mielke <mark.mielke@gmail.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4a2daffc0a..2c944ce051 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3986,9 +3986,13 @@ qemuDomainDefCPUPostParse(virDomainDefPtr def, } } - if (qemuCaps && + /* Running domains were either started before QEMU_CAPS_CPU_MIGRATABLE was + * introduced and thus we can't rely on it or they already have the + * migratable default set. */ + if (def->id == -1 && + qemuCaps && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH && - !def->cpu->migratable) { + def->cpu->migratable == VIR_TRISTATE_SWITCH_ABSENT) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_MIGRATABLE)) def->cpu->migratable = VIR_TRISTATE_SWITCH_ON; else if (ARCH_IS_X86(def->os.arch)) -- 2.27.0
Since active domains which do not have the attribute already set were not started by libvirt that probed for CPU migratable property, we need to check this property on reconnect and update the domain definition accordingly. https://bugzilla.redhat.com/show_bug.cgi?id=1857967 Reported-by: Mark Mielke <mark.mielke@gmail.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 49 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 70fc24b993..d5ac8af37e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7755,6 +7755,52 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver, } +static int +qemuProcessRefreshCPUMigratability(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr def = vm->def; + bool migratable; + int rc; + + if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) + return 0; + + /* If the cpu.migratable capability is present, the migratable attribute + * is set correctly. */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CPU_MIGRATABLE)) + return 0; + + if (!ARCH_IS_X86(def->os.arch)) + return 0; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rc = qemuMonitorGetCPUMigratable(priv->mon, &migratable); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; + + if (rc == 1) + migratable = false; + + /* Libvirt 6.5.0 would set migratable='off' for running domains even though + * the actual default used by QEMU was 'on'. */ + if (def->cpu->migratable == VIR_TRISTATE_SWITCH_OFF && migratable) { + VIR_DEBUG("Fixing CPU migratable attribute"); + def->cpu->migratable = VIR_TRISTATE_SWITCH_ON; + } + + if (def->cpu->migratable == VIR_TRISTATE_SWITCH_ABSENT) + def->cpu->migratable = virTristateSwitchFromBool(migratable); + + return 0; +} + + static int qemuProcessRefreshCPU(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -7770,6 +7816,9 @@ qemuProcessRefreshCPU(virQEMUDriverPtr driver, if (!vm->def->cpu) return 0; + if (qemuProcessRefreshCPUMigratability(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + if (!(host = virQEMUDriverGetHostCPU(driver))) { virResetLastError(); return 0; -- 2.27.0
On 7/16/20 5:08 PM, Jiri Denemark wrote:
Fixes a regression in libvirt 6.5.0 caused by commit 201bd5db63. See patches 2 and 3 for details.
Jiri Denemark (3): qemu_monitor: Add API for checking CPU migratable property qemu: Do not set //cpu/@migratable for running domains in post-parse qemu: Properly set //cpu/@migratable default value for running domains
Series: Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
src/qemu/qemu_domain.c | 8 ++++-- src/qemu/qemu_monitor.c | 18 +++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 27 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ src/qemu/qemu_process.c | 49 ++++++++++++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 2 deletions(-)
participants (2)
-
Daniel Henrique Barboza -
Jiri Denemark