Re: [libvirt] [PATCH RESEND RFC v4 5/6] qemu: Implement cfs_period and cfs_quota's modification

On Thu, Jul 21, 2011 at 10:11:32AM +0800, Wen Congyang wrote: > --- > src/qemu/qemu_driver.c | 312 ++++++++++++++++++++++++++++++++++++++++++++---- > 1 files changed, 287 insertions(+), 25 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index cd65bce..fd80537 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -5139,11 +5139,46 @@ cleanup: > } > > > +/* > + * check whether the host supports CFS bandwidth > + * > + * Return 1 when CFS bandwidth is supported, 0 when CFS bandwidth is not > + * supported, -1 on error. > + */ > +static int qemuGetCpuBWStatus(virCgroupPtr cgroup) > +{ > + char *cfs_period_path = NULL; > + int ret = -1; > + > + if (!cgroup) > + return 0; > + > + if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, > + "cpu.cfs_period_us", &cfs_period_path) < 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + "%s", > + _("cannot get the path of cgroup CPU controller")); Hum, I'm not sure we should really flag this as an error here It should be made an INFO I think. What should get an error is if we try to start using cpu control on a guest while the host doesn't support it. In practice we need to check proper handling in 3 cases: - at guest startup - on migration when checking the target host - when activated at runtime

> + goto cleanup; > + } > + > + if (access(cfs_period_path, F_OK) < 0) { > + ret = 0; > + } else { > + ret = 1; > + } > + > +cleanup: > + VIR_FREE(cfs_period_path); > + return ret; > +} > + > + > static char *qemuGetSchedulerType(virDomainPtr dom, > int *nparams) > { > struct qemud_driver *driver = dom->conn->privateData; > char *ret = NULL; > + int rc; > > qemuDriverLock(driver); > if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { > @@ -5152,8 +5187,15 @@ static char *qemuGetSchedulerType(virDomainPtr dom, > goto cleanup; > } > > - if (nparams) > - *nparams = 1; > + if (nparams) { > + rc = qemuGetCpuBWStatus(driver->cgroup); > + if (rc < 0) > + goto cleanup; > + else if (rc == 0) > + *nparams = 1; > + else > + *nparams = 3; > + } > > ret = strdup("posix"); > if (!ret) > @@ -5786,6 +5828,58 @@ cleanup: > return ret; > } > > +static int > +qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, > + unsigned long long period, long long quota) > +{ > + int i; > + qemuDomainObjPrivatePtr priv = vm->privateData; > + virCgroupPtr cgroup_vcpu = NULL; > + int rc; > + > + if (period == 0 && quota == 0) > + return 0; > + > + 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. > + */ > + /* Ensure that we can multiply by vcpus without overflowing. */ > + if (quota > LLONG_MAX / vm->def->vcpus) { > + virReportSystemError(EINVAL, > + _("%s"), > + "Unable to set cpu bandwidth quota"); should probably give an hint of why in the error message

> + goto cleanup; > + } > +

> ret = 0; > > cleanup: ACK but please add a commit message Daniel