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

[Martin Kletzander]

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.

Martin