[libvirt] [PATCH] qemu_driver: add virCgroupMounted

[Daniel P. Berrange]

On Fri, Oct 29, 2010 at 05:32:16PM +0800, Lai Jiangshan wrote:
> When wen mount any cgroup without "-o devices", we will fail to start vms:
> 
> # virsh start vm1
> error: Failed to start domain vm1
> error: Unable to deny all devices for vm1: No such file or directory
> 
> When wen mount any cgroup without "-o cpu", we will fail to get schedinfo:
> # virsh schedinfo vm1
> Scheduler      : posix
> error: unable to get cpu shares tunable: No such file or directory
> 
> We should only use the cgroup controllers which are mounted on host.
> So I add virCgroupMounted() for qemuCgroupControllerActive()
> 
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 3b127d6..5885ad8 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -61,6 +61,7 @@ virConfWriteMem;
>  # cgroup.h
>  virCgroupForDomain;
>  virCgroupForDriver;
> +virCgroupMounted;
>  virCgroupRemove;
>  virCgroupFree;
>  virCgroupAddTask;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 16f34f7..f09d029 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -724,6 +724,8 @@ static int qemuCgroupControllerActive(struct qemud_driver *driver,
>  {
>      if (driver->cgroup == NULL)
>          return 0;
> +    if (!virCgroupMounted(driver->cgroup, controller))
> +        return 0;
>      if (driver->cgroupControllers & (1 << controller))
>          return 1;
>      return 0;
> diff --git a/src/util/cgroup.c b/src/util/cgroup.c
> index 7e887dc..8fac389 100644
> --- a/src/util/cgroup.c
> +++ b/src/util/cgroup.c
> @@ -71,6 +71,17 @@ void virCgroupFree(virCgroupPtr *group)
>      VIR_FREE(*group);
>  }
>  
> +/**
> + * virCgroupMounted: query whether a cgroup subsystem is mounted or not
> + *
> + * @cgroup: The group structure to be queried
> + * @controller: cgroup subsystem id
> + */
> +bool virCgroupMounted(virCgroupPtr cgroup, int controller)
> +{
> +	return !!cgroup->controllers[controller].mountPoint;
> +}

IMHO its slightly clearer to read with  mountPoint != NULL, rather than !!

> +
>  #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
>  /*
>   * Process /proc/mounts figuring out what controllers are
> diff --git a/src/util/cgroup.h b/src/util/cgroup.h
> index b8f2d08..9e1c61f 100644
> --- a/src/util/cgroup.h
> +++ b/src/util/cgroup.h
> @@ -83,5 +83,6 @@ int virCgroupGetFreezerState(virCgroupPtr group, char **state);
>  int virCgroupRemove(virCgroupPtr group);
>  
>  void virCgroupFree(virCgroupPtr *group);
> +bool virCgroupMounted(virCgroupPtr cgroup, int controller);

ACK aside from that minor point.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|