At 07/21/2011 12:34 PM, Daniel Veillard Write:
> 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
Okay, I will change the message level.
>
>> + 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
EINVAL means the value is invalid. If you think it is not enough, I will change the
error message.
Thanks
Wen Congyang
>
>> + goto cleanup;
>> + }
>> +
snip
>> ret = 0;
>>
>> cleanup:
>
> ACK but please add a commit message
>
> Daniel
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list