Re: [libvirt] [PATCH v3 4/6] qemu: Implement period and quota tunable XML configuration and parsing
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