[libvirt] [PATCH 1/1] reset vcpu pin info from config for zero mask

The option for removing vcpu pinning information from config was added in: '7ea9778 vcpupin: add vcpupin resetting feature to qemu driver' and removed in: 'a02a161 qemu: libxl: vcpupin: Don't reset pinning when pinning to all pcpus' by some reasons. So, for now there is no way to remove vcpu pinning from config. This patch returns options for remove vcpu/emulator pinning settings from both configs if zero mask(mask filled by zeros) was specified. Signed-off-by: Konstantin Neumoin <kneumoin@virtuozzo.com> --- src/qemu/qemu_driver.c | 74 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bec7a38..7aa64a4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4969,7 +4969,8 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, virQEMUDriverConfigPtr cfg, virBitmapPtr cpumap) { - virBitmapPtr tmpmap = NULL; + virBitmapPtr effective_cpumap = NULL; + virBitmapPtr allcpu_map = NULL; virDomainVcpuInfoPtr vcpuinfo; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_vcpu = NULL; @@ -4980,6 +4981,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, int eventNparams = 0; int eventMaxparams = 0; int ret = -1; + int hostcpus = 0; if (!qemuDomainHasVcpuPids(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4994,29 +4996,38 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, goto cleanup; } - if (!(tmpmap = virBitmapNewCopy(cpumap))) - goto cleanup; + if (vcpuinfo->online) { + if (cpumap) { + effective_cpumap = cpumap; + } else if (def->cpumask) { + effective_cpumap = def->cpumask; + } else { + if ((hostcpus = nodeGetCPUCount(NULL)) < 0) + goto cleanup; - if (!(str = virBitmapFormat(cpumap))) - goto cleanup; + if (!(allcpu_map = virBitmapNew(hostcpus))) + goto cleanup; + virBitmapSetAll(allcpu_map); + effective_cpumap = allcpu_map; + } - if (vcpuinfo->online) { /* Configure the corresponding cpuset cgroup before set affinity. */ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu, false, &cgroup_vcpu) < 0) goto cleanup; - if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0) + if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, effective_cpumap) < 0) goto cleanup; } - if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), cpumap) < 0) + if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), effective_cpumap) < 0) goto cleanup; } virBitmapFree(vcpuinfo->cpumask); - vcpuinfo->cpumask = tmpmap; - tmpmap = NULL; + vcpuinfo->cpumask = NULL; + if (cpumap && !(vcpuinfo->cpumask = virBitmapNewCopy(cpumap))) + goto cleanup; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) goto cleanup; @@ -5026,6 +5037,9 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, goto cleanup; } + if (!(str = virBitmapFormat(effective_cpumap))) + goto cleanup; + if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams, paramField, str) < 0) goto cleanup; @@ -5035,7 +5049,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, ret = 0; cleanup: - virBitmapFree(tmpmap); + virBitmapFree(allcpu_map); virCgroupFree(&cgroup_vcpu); VIR_FREE(str); qemuDomainEventQueue(driver, event); @@ -5089,9 +5103,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, goto endjob; if (virBitmapIsAllClear(pcpumap)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Empty cpu list for pinning")); - goto endjob; + virBitmapFree(pcpumap); + pcpumap = NULL; } if (def && @@ -5177,12 +5190,15 @@ qemuDomainPinEmulator(virDomainPtr dom, int ret = -1; qemuDomainObjPrivatePtr priv; virBitmapPtr pcpumap = NULL; + virBitmapPtr allcpu_map = NULL; + virBitmapPtr effective_pcpumap = NULL; virQEMUDriverConfigPtr cfg = NULL; virObjectEventPtr event = NULL; char *str = NULL; virTypedParameterPtr eventParams = NULL; int eventNparams = 0; int eventMaxparams = 0; + int hostcpus = 0; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -5207,18 +5223,31 @@ qemuDomainPinEmulator(virDomainPtr dom, goto endjob; if (virBitmapIsAllClear(pcpumap)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Empty cpu list for pinning")); - goto endjob; + virBitmapFree(pcpumap); + pcpumap = NULL; } if (def) { + if (pcpumap) { + effective_pcpumap = pcpumap; + } else if (def->cpumask) { + effective_pcpumap = def->cpumask; + } else { + if ((hostcpus = nodeGetCPUCount(NULL)) < 0) + goto cleanup; + + if (!(allcpu_map = virBitmapNew(hostcpus))) + goto cleanup; + virBitmapSetAll(allcpu_map); + effective_pcpumap = allcpu_map; + } + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, false, &cgroup_emulator) < 0) goto endjob; - if (qemuSetupCgroupCpusetCpus(cgroup_emulator, pcpumap) < 0) { + if (qemuSetupCgroupCpusetCpus(cgroup_emulator, effective_pcpumap) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("failed to set cpuset.cpus in cgroup" " for emulator threads")); @@ -5226,19 +5255,19 @@ qemuDomainPinEmulator(virDomainPtr dom, } } - if (virProcessSetAffinity(vm->pid, pcpumap) < 0) + if (virProcessSetAffinity(vm->pid, effective_pcpumap) < 0) goto endjob; virBitmapFree(def->cputune.emulatorpin); def->cputune.emulatorpin = NULL; - if (!(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) + if (pcpumap && !(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) goto endjob; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) goto endjob; - str = virBitmapFormat(pcpumap); + str = virBitmapFormat(effective_pcpumap); if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams, VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN, @@ -5252,7 +5281,7 @@ qemuDomainPinEmulator(virDomainPtr dom, virBitmapFree(persistentDef->cputune.emulatorpin); persistentDef->cputune.emulatorpin = NULL; - if (!(persistentDef->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) + if (pcpumap && !(persistentDef->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) goto endjob; ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef); @@ -5270,6 +5299,7 @@ qemuDomainPinEmulator(virDomainPtr dom, qemuDomainEventQueue(driver, event); VIR_FREE(str); virBitmapFree(pcpumap); + virBitmapFree(allcpu_map); virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; -- 2.5.5

On Wed, Oct 19, 2016 at 02:39:46PM +0300, Konstantin Neumoin wrote:
The option for removing vcpu pinning information from config was added in: '7ea9778 vcpupin: add vcpupin resetting feature to qemu driver'
By the way, this hash is ambiguous, 7ea9778c is better, but I rather use 7ea9778c8a3d =)
and removed in: 'a02a161 qemu: libxl: vcpupin: Don't reset pinning when pinning to all pcpus' by some reasons.
The reason is written in the config. Pinning to all pCPUs is sometimes wanted. Also you cannot easily reset the pinning on a live domain. You can pin to all the CPUs, but after CPU is hotplugged (or becomes online) in the host, you are yet again not pinned to all of them, and more importantly, you are returning inconsistent information.
So, for now there is no way to remove vcpu pinning from config.
Well, there is. But it's just a workaround. You can pin it to all and then edit the config. I agree that it's not fun to do, though.
This patch returns options for remove vcpu/emulator pinning settings from both configs if zero mask(mask filled by zeros) was specified.
This is not documented anywhere. You should add that in the docs. Although I would rather prefer maplength == 0 to trigger this, or cpumask == NULL. But that would have to be taken care of in all drivers, so it's probably not a good idea. Bottom line for me is, that I don't think we should allow an API for which we cannot guarantee the results. Until there is a way to reset pinning reliably that the kernel provides, that is. Martin
participants (2)
-
Konstantin Neumoin
-
Martin Kletzander