[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]