[libvirt] [PATCH v2 18/21] qemu: vcpu: Aggregate code to set vCPU tuning

[Ján Tomko]

On Fri, Jan 29, 2016 at 05:02:13PM +0100, Peter Krempa wrote:
> Rather than iterating 3 times for various settings this function
> aggregates all the code into single place. One of the other advantages
> is that it can then be reused for properly setting vCPU info on hotplug.

It would be also nice to mention that the autoCpuset is now also used
for affinity.

> ---
>  src/qemu/qemu_cgroup.c  |  95 ---------------------
>  src/qemu/qemu_cgroup.h  |   1 -
>  src/qemu/qemu_process.c | 216 +++++++++++++++++++++++++++++++++---------------
>  src/qemu/qemu_process.h |   4 +
>  4 files changed, 152 insertions(+), 164 deletions(-)

> @@ -4464,6 +4402,152 @@ qemuProcessInit(virQEMUDriverPtr driver,
> 
> 
>  /**
> + * qemuProcessSetupVcpu:
> + * @vm: domain object
> + * @vcpuid: id of VCPU to set defaults
> + *
> + * This function sets resource properities (affinity, cgroups, scheduler) for a
> + * vCPU. This function expects that the vCPU is online and the vCPU pids were
> + * correctly detected at the point when it's called.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int
> +qemuProcessSetupVcpu(virDomainObjPtr vm,
> +                     unsigned int vcpuid)
> +{
> +    pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid);
> +    virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    char *mem_mask = NULL;
> +    virDomainNumatuneMemMode mem_mode;
> +    unsigned long long period = vm->def->cputune.period;
> +    long long quota = vm->def->cputune.quota;
> +    virCgroupPtr cgroup_vcpu = NULL;
> +    virBitmapPtr cpumask;
> +    int ret = -1;
> +
> +    if (period || quota) {
> +        if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {

The condition should be negated, as noted by John in his review of v1.

> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("cgroup cpu is required for scheduler tuning"));
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) ||
> +        virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
> +
> +        if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
> +            mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
> +            virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
> +                                                priv->autoNodeset,
> +                                                &mem_mask, -1) < 0)
> +            goto cleanup;
> +
> +        if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpuid,
> +                               true, &cgroup_vcpu) < 0)
> +            goto cleanup;
> +
> +        if (period || quota) {
> +            if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0)
> +                goto cleanup;
> +        }
> +    }
> +
> +    /* infer which cpumask shall be used */
> +    if (vcpu->cpumask)
> +        cpumask = vcpu->cpumask;
> +    else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
> +        cpumask = priv->autoCpuset;
> +    else
> +        cpumask = vm->def->cpumask;
> +
> +    if (cpumask) {
> +        /* setup legacy affinty */
> +        if (virProcessSetAffinity(vcpupid, cpumask) < 0)
> +            goto cleanup;
> +

Previously we set up cgroups before setting the affinity.
This patch should preserve that order.

> +        /* setup  cgroups */

Extra space.

ACK with the fixes.

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160203/09b069ca/attachment-0001.sig>