[libvirt] [PATCH 0/3] Fix output of virsh vcpuinfo after cpu hotplug
The informations about virtual processors stored by libvirt were not updated after a cpu hotplug. Peter Krempa (3): qemu: Refactor qemuDomainSetVcpusFlags qemu: Re-detect virtual cpu threads after cpu hot (un)plug. qemu: Don't skip detection of virtual cpu's on non KVM targets src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++++++--------------- src/qemu/qemu_process.c | 12 ----------- 2 files changed, 32 insertions(+), 28 deletions(-) -- 1.7.3.4
--- src/qemu/qemu_driver.c | 24 ++++++++---------------- 1 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c100a1a..7b1d1b6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3463,8 +3463,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - switch (flags) { - case VIR_DOMAIN_AFFECT_CONFIG: + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (maximum) { persistentDef->maxvcpus = nvcpus; if (nvcpus < persistentDef->vcpus) @@ -3472,24 +3471,17 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } else { persistentDef->vcpus = nvcpus; } - ret = 0; - break; - case VIR_DOMAIN_AFFECT_LIVE: - ret = qemudDomainHotplugVcpus(driver, vm, nvcpus); - break; + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + goto endjob; + } - case VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG: - ret = qemudDomainHotplugVcpus(driver, vm, nvcpus); - if (ret == 0) { - persistentDef->vcpus = nvcpus; - } - break; + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (qemudDomainHotplugVcpus(driver, vm, nvcpus) < 0) + goto endjob; } - /* Save the persistent config to disk */ - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - ret = virDomainSaveConfig(driver->configDir, persistentDef); + ret = 0; endjob: if (qemuDomainObjEndJob(driver, vm) == 0) -- 1.7.3.4
On 05/07/2012 06:16 AM, Peter Krempa wrote: Mentioning a summary of what you refactored might be nice for someone reading 'git log'
--- src/qemu/qemu_driver.c | 24 ++++++++---------------- 1 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c100a1a..7b1d1b6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3463,8 +3463,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; }
- switch (flags) { - case VIR_DOMAIN_AFFECT_CONFIG: + if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
Thank you for cleaning this up. I have no idea what I was thinking when I first coded this switch statement (it was back in the days when I wasn't quite sure how live vs. persistent definitions worked).
if (maximum) { persistentDef->maxvcpus = nvcpus; if (nvcpus < persistentDef->vcpus) @@ -3472,24 +3471,17 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } else { persistentDef->vcpus = nvcpus; } - ret = 0; - break;
- case VIR_DOMAIN_AFFECT_LIVE: - ret = qemudDomainHotplugVcpus(driver, vm, nvcpus); - break; + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + goto endjob; + }
- case VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG: - ret = qemudDomainHotplugVcpus(driver, vm, nvcpus); - if (ret == 0) { - persistentDef->vcpus = nvcpus; - } - break; + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (qemudDomainHotplugVcpus(driver, vm, nvcpus) < 0) + goto endjob; }
Alas, I see a flaw in your refactoring. You want to affect live before config, otherwise, if the live change fails, you would have to undo the config change (or, at the bare minimum, sink the virDomainSaveConfig(persistentDef) until after the affect_live portion, even if the rest of the persistent code is done first).
- /* Save the persistent config to disk */ - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - ret = virDomainSaveConfig(driver->configDir, persistentDef); + ret = 0;
endjob: if (qemuDomainObjEndJob(driver, vm) == 0)
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
After a cpu hotplug the qemu driver did not refresh information about virtual procesors used by qemu and their corresponding threads. This patch forces a re-detection as is done on start of QEMU. This ensures that correct informations are reported by the virDomainGetVcpus API and "virsh vcpuinfo". --- src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7b1d1b6..fcaf0d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3343,6 +3343,8 @@ static int qemudDomainHotplugVcpus(struct qemud_driver *driver, int ret = -1; int oldvcpus = vm->def->vcpus; int vcpus = oldvcpus; + pid_t *cpupids = NULL; + int ncpupids; qemuDomainObjEnterMonitor(driver, vm); @@ -3373,8 +3375,30 @@ static int qemudDomainHotplugVcpus(struct qemud_driver *driver, } } + /* after hotplugging the cpu we need to re-detect threads for the virtual + * cpus */ + if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) < 0) + goto cleanup; + ret = 0; + /* Treat failure to get VCPU<->PID mapping as non-fatal */ + if (ncpupids == 0) + goto cleanup; + + if (ncpupids != vcpus) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("got wrong number of vCPU pids from QEMU monitor. " + "got %d, wanted %d"), + ncpupids, vcpus); + VIR_FREE(cpupids); + ret = -1; + goto cleanup; + } + + priv->nvcpupids = ncpupids; + priv->vcpupids = cpupids; + cleanup: qemuDomainObjExitMonitor(driver, vm); vm->def->vcpus = vcpus; -- 1.7.3.4
On 05/07/2012 06:16 AM, Peter Krempa wrote:
After a cpu hotplug the qemu driver did not refresh information about virtual procesors used by qemu and their corresponding threads. This
s/procesors/processors/
patch forces a re-detection as is done on start of QEMU.
This ensures that correct informations are reported by the
s/informations are/information is/
virDomainGetVcpus API and "virsh vcpuinfo". --- src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++++ 1 files changed, 24 insertions(+), 0 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
QEMU adapted the info cpus command from the kvm branch. This patch removes the check for the KVM domain type to detect virtualprocessors also while using software emulation. The output of the "info cpus" command may vary across different targets but informations that are parsed by libvirt (cpu ID and thread pid) are (should be) present on those other targets too. --- src/qemu/qemu_process.c | 12 ------------ 1 files changed, 0 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f1401e1..b25cecb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1613,18 +1613,6 @@ qemuProcessDetectVcpuPIDs(struct qemud_driver *driver, int ncpupids; qemuDomainObjPrivatePtr priv = vm->privateData; - if (vm->def->virtType != VIR_DOMAIN_VIRT_KVM) { - priv->nvcpupids = 1; - if (VIR_ALLOC_N(priv->vcpupids, priv->nvcpupids) < 0) { - virReportOOMError(); - return -1; - } - priv->vcpupids[0] = vm->pid; - return 0; - } - - /* What follows is now all KVM specific */ - qemuDomainObjEnterMonitorWithDriver(driver, vm); if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); -- 1.7.3.4
On 05/07/2012 06:16 AM, Peter Krempa wrote:
QEMU adapted the info cpus command from the kvm branch. This patch removes the check for the KVM domain type to detect virtualprocessors also while using software emulation.
Which version of qemu adopted the command? Do we need a capability bit in qemu_capabilities.h to track whether the command exists and will be reliable enough?
The output of the "info cpus" command may vary across different targets but informations that are parsed by libvirt (cpu ID and thread pid) are
s/informations that are/information that is/
(should be) present on those other targets too. --- src/qemu/qemu_process.c | 12 ------------ 1 files changed, 0 insertions(+), 12 deletions(-)
Bug fix by deletion is always fun, but are we sure this works with, for example, non-kvm qemu 0.12? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/11/2012 01:12 AM, Eric Blake wrote:
On 05/07/2012 06:16 AM, Peter Krempa wrote:
QEMU adapted the info cpus command from the kvm branch. This patch removes the check for the KVM domain type to detect virtualprocessors also while using software emulation.
Which version of qemu adopted the command? Do we need a capability bit in qemu_capabilities.h to track whether the command exists and will be reliable enough?
The thread_id field was initialy added in the kvm fork of qemu and is present in the 0.12 version. Upstream qemu bacported this functionality in v0.15. Before 0.15 only the program counter was printed. I had to modify the patch (It's no longer pure deletion :-( ) to cope with this in a (hopefuly)-all-version-compatible(TM) manner. I'll post v2 soon. Peter
The output of the "info cpus" command may vary across different targets but informations that are parsed by libvirt (cpu ID and thread pid) are
s/informations that are/information that is/
(should be) present on those other targets too. --- src/qemu/qemu_process.c | 12 ------------ 1 files changed, 0 insertions(+), 12 deletions(-)
Bug fix by deletion is always fun, but are we sure this works with, for example, non-kvm qemu 0.12?
On 05/07/2012 06:16 AM, Peter Krempa wrote:
The informations about virtual processors stored by libvirt were not updated after a cpu hotplug.
Peter Krempa (3): qemu: Refactor qemuDomainSetVcpusFlags qemu: Re-detect virtual cpu threads after cpu hot (un)plug. qemu: Don't skip detection of virtual cpu's on non KVM targets
Sounds like a bug fix worth including in 0.9.12, if my review of the series is favorable.
src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++++++--------------- src/qemu/qemu_process.c | 12 ----------- 2 files changed, 32 insertions(+), 28 deletions(-)
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa