[libvirt] [PATCH 0/2] qemu: hotplug: Write status XML only during successful vcpu hot(un)plug

See the second patch. Peter Krempa (2): qemu: hotplug: Unexport qemuDomainHotplugDelVcpu qemu: hotplug: Don't save status XML when monitor is closed src/qemu/qemu_hotplug.c | 31 +++++++++++++++++-------------- src/qemu/qemu_hotplug.h | 3 --- 2 files changed, 17 insertions(+), 17 deletions(-) -- 2.12.2

The function is used only in the hotplug module. --- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_hotplug.h | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a6dac6f09..8116d8cbb 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5384,7 +5384,7 @@ qemuDomainRemoveVcpuAlias(virQEMUDriverPtr driver, } -int +static int qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int vcpu) diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index f06f232bb..985c6733f 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -140,9 +140,6 @@ int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainRNGDefPtr rng); -int qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int vcpu); void qemuDomainRemoveVcpuAlias(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *alias); -- 2.12.2

In the vcpu hotplug code if exit from the monitor failed we would still attempt to save the status XML. When the daemon is terminated the monitor socket is closed. In such case, the written status XML would not contain the monitor path and thus be invalid. Avoid this issue by only saving status XML on success of the monitor command. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1439452 --- src/qemu/qemu_hotplug.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8116d8cbb..7213f70d8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5386,6 +5386,7 @@ qemuDomainRemoveVcpuAlias(virQEMUDriverPtr driver, static int qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, + virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, unsigned int vcpu) { @@ -5427,6 +5428,11 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, if (qemuDomainRemoveVcpu(driver, vm, vcpu) < 0) goto cleanup; + qemuDomainVcpuPersistOrder(vm->def); + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) + goto cleanup; + ret = 0; cleanup: @@ -5437,6 +5443,7 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, static int qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, + virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, unsigned int vcpu) { @@ -5497,6 +5504,11 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, if (qemuDomainValidateVcpuInfo(vm) < 0) goto cleanup; + qemuDomainVcpuPersistOrder(vm->def); + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) + goto cleanup; + ret = 0; cleanup: @@ -5611,7 +5623,6 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; qemuCgroupEmulatorAllNodesDataPtr emulatorCgroup = NULL; ssize_t nextvcpu = -1; - int rc = 0; int ret = -1; if (qemuCgroupEmulatorAllNodesAllow(priv->cgroup, &emulatorCgroup) < 0) @@ -5619,27 +5630,19 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, if (enable) { while ((nextvcpu = virBitmapNextSetBit(vcpumap, nextvcpu)) != -1) { - if ((rc = qemuDomainHotplugAddVcpu(driver, vm, nextvcpu)) < 0) - break; + if (qemuDomainHotplugAddVcpu(driver, cfg, vm, nextvcpu) < 0) + goto cleanup; } } else { for (nextvcpu = virDomainDefGetVcpusMax(vm->def) - 1; nextvcpu >= 0; nextvcpu--) { if (!virBitmapIsBitSet(vcpumap, nextvcpu)) continue; - if ((rc = qemuDomainHotplugDelVcpu(driver, vm, nextvcpu)) < 0) - break; + if (qemuDomainHotplugDelVcpu(driver, cfg, vm, nextvcpu) < 0) + goto cleanup; } } - qemuDomainVcpuPersistOrder(vm->def); - - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - goto cleanup; - - if (rc < 0) - goto cleanup; - ret = 0; cleanup: -- 2.12.2

On 04/13/2017 08:49 AM, Peter Krempa wrote:
See the second patch.
Peter Krempa (2): qemu: hotplug: Unexport qemuDomainHotplugDelVcpu qemu: hotplug: Don't save status XML when monitor is closed
src/qemu/qemu_hotplug.c | 31 +++++++++++++++++-------------- src/qemu/qemu_hotplug.h | 3 --- 2 files changed, 17 insertions(+), 17 deletions(-)
ACK series, Could be a performance hit depending on 'nextcpu' size, but still no way to "save" a partial alteration either without doing the PersistOrder and SaveStatus for each successful one. John
participants (2)
-
John Ferlan
-
Peter Krempa