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

[Adam Litke]

On 07/18/2011 07:46 PM, Wen Congyang wrote:
> 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().

Yes, I am aware.  However, the introduction of cpu quotas is a major
feature that defines the amount of computing capacity a domain has
available.  In my opinion, if a domain XML file specifies a quota then
'virsh create' should fail of that quota cannot be set up for any reason.

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