[libvirt] [PATCH 1/3] qemu: Abstract code for the cpu controller setting into a helper

[Osier Yang]

On 04/06/13 22:23, Martin Kletzander wrote:
> On 05/24/2013 11:08 AM, Osier Yang wrote:
>> ---
>>   src/qemu/qemu_cgroup.c | 55 +++++++++++++++++++++++++++++++++-----------------
>>   1 file changed, 36 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index bb1b235..cf46993 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -676,6 +676,36 @@ cleanup:
>>   }
>>   
>>   
>> +static int
>> +qemuSetupCpuCgroup(virDomainObjPtr vm)
>> +{
>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>> +    int rc = -1;
>> +
>> +    if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
>> +       if (vm->def->cputune.shares) {
>> +           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                          _("CPU tuning is not available on this host"));
>> +           return -1;
>> +       } else {
>> +           return 0;
>> +       }
>> +    }
>> +
>> +    if (vm->def->cputune.shares) {
>> +        rc = virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares);
>> +        if (rc != 0) {
>> +            virReportSystemError(-rc,
>> +                                 _("Unable to set io cpu shares for domain %s"),
>> +                                 vm->def->name);
>> +            return -1;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
> I think this would be a bit more readable:
>
> if (!vm->def->cputune.shares)
>      return 0;
>
> if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
>      virReportError(...);
>      return -1;
> }
>
> rc = virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares);
> if (rc != 0) {
> ...
>
> but that's a matter of opinion.  ACK either way.
>
>
More clear indeed, but it's same case for other helpers, I can change 
them together
with later patch..

Pushed this as-is. Thanks.

Osier