[libvirt] [RFC PATCH v2 12/12] qemu_driver: detect threads corresponding to Vcpus

After hotplugging the CPUs, we need to re-detect threads corresponding to Vcpus. Signed-off-by: Zhu Guihua <zhugh.fnst(a)cn.fujitsu.com&gt; --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 274 ++++++++++++++++++++++++----------------------- src/qemu/qemu_driver.h | 8 ++ src/qemu/qemu_hotplug.c | 7 ++ src/util/virbitmap.c | 2 +- src/util/virbitmap.h | 2 + 6 files changed, 160 insertions(+), 134 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b23c45c..4399b58 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1047,6 +1047,7 @@ virBitmapFree; virBitmapGetBit; virBitmapIsAllClear; virBitmapIsAllSet; +virBitmapIsSet; virBitmapLastSetBit; virBitmapNew; virBitmapNewCopy; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bf33d7a..745bedd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4357,89 +4357,49 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); } -static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +/* After hotplugging the CPUs we need to re-detect threads corresponding + * * to the virtual CPUs. Some older versions don't provide the thread ID + * * or don't have the "info cpus" command (and they don't support multiple + * * CPUs anyways), so errors in the re-detection will not be treated + * * fatal */ +int +qemuDomainDetectVcpu(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int apic_id, + bool plug) { qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; - int rc = 1; - int ret = -1; int oldvcpus = vm->def->vcpus; int vcpus = oldvcpus; pid_t *cpupids = NULL; int ncpupids; + int ret = -1; virCgroupPtr cgroup_vcpu = NULL; char *mem_mask = NULL; - uint32_t apic_id; - - qemuDomainObjEnterMonitor(driver, vm); - - /* We need different branches here, because we want to offline - * in reverse order to onlining, so any partial fail leaves us in a - * reasonably sensible state */ - if (nvcpus > vcpus) { - for (i = vcpus; i < nvcpus; i++) { - /* Online new CPU */ - apic_id = virDomainCPUGetFreeApicID(vm->def); - rc = qemuMonitorSetCPU(priv->mon, apic_id, true); - if (rc == 0) - goto unsupported; - if (rc < 0) - goto exit_monitor; - - vcpus++; - ignore_value(virBitmapSetBit(vm->def->apic_id_map, apic_id)); - } - } else { - for (i = vcpus - 1; i >= nvcpus; i--) { - /* Offline old CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, false); - if (rc == 0) - goto unsupported; - if (rc < 0) - goto exit_monitor; - - vcpus--; - } - } - - /* hotplug succeeded */ + int idx = 0; + size_t i; + int *thread = NULL; ret = 0; + if (plug) + vcpus++; + else + vcpus--; + + qemuDomainObjEnterMonitor(driver, vm); - /* After hotplugging the CPUs we need to re-detect threads corresponding - * to the virtual CPUs. Some older versions don't provide the thread ID - * or don't have the "info cpus" command (and they don't support multiple - * CPUs anyways), so errors in the re-detection will not be treated - * fatal */ if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { virResetLastError(); - goto exit_monitor; - } - if (qemuDomainObjExitMonitor(driver, vm) < 0) { - ret = -1; goto cleanup; } - /* check if hotplug has failed */ - if (vcpus < oldvcpus && ncpupids == oldvcpus) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("qemu didn't unplug the vCPUs properly")); - vcpus = oldvcpus; - ret = -1; - goto cleanup; + for (i = 0; i < apic_id; i++) { + if (virBitmapIsSet(vm->def->apic_id_map, i)) + idx++; } - if (ncpupids != vcpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("got wrong number of vCPU pids from QEMU monitor. " - "got %d, wanted %d"), - ncpupids, vcpus); - vcpus = oldvcpus; - ret = -1; + if (VIR_ALLOC_N(thread, vcpus) < 0) goto cleanup; - } if (virDomainNumatuneGetMode(vm->def->numatune, -1) == VIR_DOMAIN_NUMATUNE_MEM_STRICT && @@ -4448,105 +4408,153 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, &mem_mask, -1) < 0) goto cleanup; - if (nvcpus > oldvcpus) { - for (i = oldvcpus; i < nvcpus; i++) { - if (priv->cgroup) { - int rv = -1; - /* Create cgroup for the onlined vcpu */ - if (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0) - goto cleanup; - - if (mem_mask && - virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) - goto cleanup; + if (vcpus > oldvcpus) { + if (priv->cgroup) { + int rv = -1; + if (virCgroupNewVcpu(priv->cgroup, apic_id, true, &cgroup_vcpu) < 0) + goto cleanup; - /* Add vcpu thread to the cgroup */ - rv = virCgroupAddTask(cgroup_vcpu, cpupids[i]); - if (rv < 0) { - virReportSystemError(-rv, - _("unable to add vcpu %zu task %d to cgroup"), - i, cpupids[i]); - virCgroupRemove(cgroup_vcpu); - goto cleanup; - } + if (mem_mask && + virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) + goto cleanup; + rv = virCgroupAddTask(cgroup_vcpu, cpupids[idx]); + if (rv < 0) { + virReportSystemError(-rv, + _("unable to add vcpu %d task %d to cgroup"), + apic_id, cpupids[idx]); + virCgroupRemove(cgroup_vcpu); + goto cleanup; } + } - /* Inherit def->cpuset */ - if (vm->def->cpumask) { - /* vm->def->cputune.vcpupin can't be NULL if - * vm->def->cpumask is not NULL. - */ - virDomainVcpuPinDefPtr vcpupin = NULL; + if (vm->def->cpumask) { + virDomainVcpuPinDefPtr vcpupin = NULL; - if (VIR_ALLOC(vcpupin) < 0) - goto cleanup; + if (VIR_ALLOC(vcpupin) < 0) + goto cleanup; - vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN); - virBitmapCopy(vcpupin->cpumask, vm->def->cpumask); - vcpupin->vcpuid = i; - if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin, vcpupin) < 0) { - virBitmapFree(vcpupin->cpumask); - VIR_FREE(vcpupin); + vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN); + virBitmapCopy(vcpupin->cpumask, vm->def->cpumask); + vcpupin->vcpuid = apic_id; + if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, + vm->def->cputune.nvcpupin, vcpupin) < 0) { + virBitmapFree(vcpupin->cpumask); + VIR_FREE(vcpupin); + ret = -1; + goto cleanup; + } + + if (cgroup_vcpu) { + if (qemuSetupCgroupVcpuPin(cgroup_vcpu, + vm->def->cputune.vcpupin, + vm->def->cputune.nvcpupin, apic_id) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("failed to set cpuset.cpus in cgroup" + " for vcpu %d"), apic_id); ret = -1; goto cleanup; } - - if (cgroup_vcpu) { - if (qemuSetupCgroupVcpuPin(cgroup_vcpu, - vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin, i) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("failed to set cpuset.cpus in cgroup" - " for vcpu %zu"), i); - ret = -1; - goto cleanup; - } - } else { - if (virProcessSetAffinity(cpupids[i], - vcpupin->cpumask) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("failed to set cpu affinity for vcpu %zu"), - i); - ret = -1; - goto cleanup; - } + } else { + if (virProcessSetAffinity(cpupids[idx], + vcpupin->cpumask) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to set cpu affinity for vcpu %d"), + apic_id); + ret = -1; + goto cleanup; } } virCgroupFree(&cgroup_vcpu); } } else { - for (i = oldvcpus - 1; i >= nvcpus; i--) { - if (priv->cgroup) { - if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu) < 0) - goto cleanup; - - /* Remove cgroup for the offlined vcpu */ - virCgroupRemove(cgroup_vcpu); - virCgroupFree(&cgroup_vcpu); - } + if (priv->cgroup) { + if (virCgroupNewVcpu(priv->cgroup, apic_id, false, &cgroup_vcpu) < 0) + goto cleanup; - /* Free vcpupin setting */ - virDomainVcpuPinDel(vm->def, i); + virCgroupRemove(cgroup_vcpu); + virCgroupFree(&cgroup_vcpu); } + + virDomainVcpuPinDel(vm->def, apic_id); } priv->nvcpupids = ncpupids; VIR_FREE(priv->vcpupids); priv->vcpupids = cpupids; cpupids = NULL; + thread = NULL; cleanup: + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + vm->def->vcpus = vcpus; VIR_FREE(cpupids); + VIR_FREE(thread); VIR_FREE(mem_mask); - if (virDomainObjIsActive(vm)) - vm->def->vcpus = vcpus; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); + virDomainAuditVcpu(vm, oldvcpus, vcpus, "update", true); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); return ret; +} + +static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int nvcpus) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + int rc = 1; + int ret = -1; + int oldvcpus = vm->def->vcpus; + int vcpus = oldvcpus; + uint32_t apic_id; + + /* We need different branches here, because we want to offline + * in reverse order to onlining, so any partial fail leaves us in a + * reasonably sensible state */ + if (nvcpus > vcpus) { + for (i = vcpus; i < nvcpus; i++) { + /* Online new CPU */ + qemuDomainObjEnterMonitor(driver, vm); + apic_id = virDomainCPUGetFreeApicID(vm->def); + rc = qemuMonitorSetCPU(priv->mon, apic_id, true); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto exit_monitor; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } + + vcpus++; + ignore_value(virBitmapSetBit(vm->def->apic_id_map, apic_id)); + + if (qemuDomainDetectVcpu(driver, vm, apic_id, true) < 0) + goto cleanup; + } + } else { + for (i = vcpus - 1; i >= nvcpus; i--) { + /* Offline old CPU */ + rc = qemuMonitorSetCPU(priv->mon, i, false); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto exit_monitor; + + vcpus--; + } + } + + /* hotplug succeeded */ + + ret = 0; + + cleanup: + return ret; unsupported: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_driver.h b/src/qemu/qemu_driver.h index df7533a..55ce618 100644 --- a/src/qemu/qemu_driver.h +++ b/src/qemu/qemu_driver.h @@ -20,10 +20,18 @@ * * Author: Daniel P. Berrange <berrange(a)redhat.com&gt; */ +#include "qemu_conf.h" +#include "domain_conf.h" #ifndef __QEMU_DRIVER_H__ # define __QEMU_DRIVER_H__ int qemuRegister(void); +int +qemuDomainDetectVcpu(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int apic_id, + bool plug); + #endif /* __QEMU_DRIVER_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 975ed1d..90ba17a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -31,6 +31,7 @@ #include "qemu_command.h" #include "qemu_hostdev.h" #include "qemu_interface.h" +#include "qemu_driver.h" #include "domain_audit.h" #include "netdev_bandwidth_conf.h" #include "domain_nwfilter.h" @@ -1595,6 +1596,9 @@ int qemuDomainAttachCPUDevice(virQEMUDriverPtr driver, ignore_value(virBitmapSetBit(vm->def->apic_id_map, cpu->apic_id)); ret = 0; + if (qemuDomainDetectVcpu(driver, vm, cpu->apic_id, true) < 0) + ret = -1; + cleanup: VIR_FREE(devstr); return ret; @@ -3914,6 +3918,9 @@ int qemuDomainDetachCPUDevice(virQEMUDriverPtr driver, else goto cleanup; + if (qemuDomainDetectVcpu(driver, vm, tmpCPU->apic_id, false) < 0) + ret = -1; + cleanup: qemuDomainResetDeviceRemoval(vm); return ret; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 05c50e4..168b8db 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -153,7 +153,7 @@ int virBitmapClearBit(virBitmapPtr bitmap, size_t b) } /* Helper function. caller must ensure b < bitmap->max_bit */ -static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) +bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) { return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b)); } diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 565264c..57fb195 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -60,6 +60,8 @@ int virBitmapSetBit(virBitmapPtr bitmap, size_t b) int virBitmapClearBit(virBitmapPtr bitmap, size_t b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +bool virBitmapIsSet(virBitmapPtr bitmap, size_t b); + /* * Get setting of bit position @b in @bitmap and store in @result */ -- 1.9.3