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

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,