[libvirt] [PATCH v3 4/6] qemu: Implement period and quota tunable XML configuration and parsing

Adam Litke agl at us.ibm.com
Mon Jul 18 20:35:17 UTC 2011


This is looking good to me.  I am pleased to see that the global case is
handled as expected when per-vcpu threads are not active.

On 07/18/2011 04:42 AM, Wen Congyang wrote:
<snip>
> +int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm)
> +{
> +    virCgroupPtr cgroup = NULL;
> +    virCgroupPtr cgroup_vcpu = NULL;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    int rc;
> +    unsigned int i;
> +    unsigned long long period = vm->def->cputune.period;
> +    long long quota = vm->def->cputune.quota;
> +
> +    if (driver->cgroup == NULL)
> +        return 0; /* Not supported, so claim success */

I just want to check: Is the above logic still valid?  Failure to apply
a quota setting (>0) could have fairly substantial implications for
system performance.  I am not convinced either way but I do think this
point merits some discussion.

> +
> +    rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0);
> +    if (rc != 0) {
> +        virReportSystemError(-rc,
> +                             _("Unable to find cgroup for %s"),
> +                             vm->def->name);
> +        goto cleanup;
> +    }
> +
> +    if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) {
> +        /* If we does not know VCPU<->PID mapping or all vcpu runs in the same
> +         * thread, we can not control each vcpu.
> +         */
> +        if (period || quota) {
> +            if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
> +                if (qemuSetupCgroupVcpuBW(cgroup, period, quota) < 0)
> +                    goto cleanup;
> +            }
> +        }
> +        return 0;
> +    }
> +
> +    for (i = 0; i < priv->nvcpupids; i++) {
> +        rc = virCgroupForVcpu(cgroup, i, &cgroup_vcpu, 1);
> +        if (rc < 0) {
> +            virReportSystemError(-rc,
> +                                 _("Unable to create vcpu cgroup for %s(vcpu:"
> +                                   " %d)"),
> +                                 vm->def->name, i);
> +            goto cleanup;
> +        }
> +
> +        /* move the thread for vcpu to sub dir */
> +        rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]);
> +        if (rc < 0) {
> +            virReportSystemError(-rc,
> +                                 _("unable to add vcpu %d task %d to cgroup"),
> +                                 i, priv->vcpupids[i]);
> +            goto cleanup;
> +        }
> +
> +        if (period || quota) {
> +            if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
> +                if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0)
> +                    goto cleanup;
> +            }
> +        }
> +
> +        virCgroupFree(&cgroup_vcpu);
> +    }
> +
> +    virCgroupFree(&cgroup_vcpu);
> +    virCgroupFree(&cgroup);
> +    return 0;
> +
> +cleanup:
> +    virCgroupFree(&cgroup_vcpu);
> +    if (cgroup) {
> +        virCgroupRemove(cgroup);
> +        virCgroupFree(&cgroup);
> +    }
> +
> +    return -1;
> +}
> +
> 
>  int qemuRemoveCgroup(struct qemud_driver *driver,
>                       virDomainObjPtr vm,

-- 
Adam Litke
IBM Linux Technology Center




More information about the libvir-list mailing list