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

[Wen Congyang]

At 07/19/2011 04:35 AM, Adam Litke Write:
> 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

This logic is the same as the logic in the similar function qemuSetupCgroup().

> 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,
>