[libvirt] [PATCH] Don't return an error if fail to create blkio controller

[Eric Blake]

On 03/06/2011 08:49 PM, Hu Tao wrote:
> This patch enables cgroup controllers as much as possible by skipping
> the creation of blkio controller when running with old kenels that

s/kenels/kernels/

> doesn't support multi-level directory for blkio controller.
> 
> Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> ---
>  src/util/cgroup.c |   28 +++++++++++++++++++++++-----
>  1 files changed, 23 insertions(+), 5 deletions(-)
> 
> @@ -527,9 +527,19 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group,
>          if (access(path, F_OK) != 0) {
>              if (!create ||
>                  mkdir(path, 0755) < 0) {
> -                rc = -errno;
> -                VIR_FREE(path);
> -                break;
> +                /* With a kernel that doesn't support multi-level directory
> +                 * for blkio controller, libvirt will fail and disable all
> +                 * other controllers even though they are available. So skip
> +                 * blkio here if mkdir fails. */
> +                if (i == VIR_CGROUP_CONTROLLER_BLKIO) {
> +                    rc = 0;
> +                    VIR_FREE(path);
> +                    continue;
> +                } else {
> +                    rc = -errno;
> +                    VIR_FREE(path);
> +                    break;
> +                }
>              }
>              if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint != NULL &&
>                  (i == VIR_CGROUP_CONTROLLER_CPUSET ||
> @@ -751,8 +761,16 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid)
>              continue;
>  
>          rc = virCgroupSetValueU64(group, i, "tasks", (unsigned long long)pid);
> -        if (rc != 0)
> -            break;
> +
> +        /* See virCgroupMakeGroup() for the reason why check BLKIO here */

s/why check/why we special case/

But I don't like this hunk in the first place - it means that if you
have a newer kernel that does support subdirectories, you ignore the
write failure to add the process into the group.  Rather, it's easier to
just nuke the mount point once we detect the problem (by pretending that
blkio is not mounted, we don't ever try the SetValueU64 in the first
place on an old kernel, while a new kernel doesn't need any code change
here).

Here's what I squashed in before pushing just the first hunk:

diff --git i/src/util/cgroup.c w/src/util/cgroup.c
index d3855c4..afe8731 100644
--- i/src/util/cgroup.c
+++ w/src/util/cgroup.c
@@ -529,10 +529,11 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
virCgroupPtr group,
                 mkdir(path, 0755) < 0) {
                 /* With a kernel that doesn't support multi-level directory
                  * for blkio controller, libvirt will fail and disable all
-                 * other controllers even though they are available. So
skip
-                 * blkio here if mkdir fails. */
+                 * other controllers even though they are available. So
+                 * treat blkio as unmounted if mkdir fails. */
                 if (i == VIR_CGROUP_CONTROLLER_BLKIO) {
                     rc = 0;
+                    VIR_FREE(group->controllers[i].mountPoint);
                     VIR_FREE(path);
                     continue;
                 } else {

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110318/7f5267d4/attachment-0001.sig>