[libvirt] [PATCH v1 rebase 1/6] cgroup: Add a flag VIR_CGROUP_DISABLE_CPUSET

[Daniel P. Berrange]

On Thu, Dec 13, 2012 at 05:10:47PM +0800, Osier Yang wrote:
> On 2012年12月13日 15:01, Hu Tao wrote:
> >Add a flag VIR_CGROUP_DISABLE_CPUSET to disable cgroup cpuset.
> >This flag inhibits making of directory under cpuset.
> >---
> >  src/util/cgroup.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> >diff --git a/src/util/cgroup.c b/src/util/cgroup.c
> >index f867fb7..e955a22 100644
> >--- a/src/util/cgroup.c
> >+++ b/src/util/cgroup.c
> >@@ -72,6 +72,7 @@ typedef enum {
> >                                         */
> >      VIR_CGROUP_VCPU = 1<<  1, /* create subdir only under the cgroup cpu,
> >                                 * cpuacct and cpuset if possible. */
> >+    VIR_CGROUP_DISABLE_CPUSET = 1<<  2,
> >  } virCgroupFlags;
> >
> >  /**
> >@@ -542,6 +543,12 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
> >          if (!group->controllers[i].mountPoint)
> >              continue;
> >
> >+        if ((flags&  VIR_CGROUP_DISABLE_CPUSET)&&
> >+            i == VIR_CGROUP_CONTROLLER_CPUSET) {
> >+            group->controllers[i].mountPoint = NULL;
> >+            continue;
> >+        }
> 
> This actually exposes an existed bug, regardless of whether the user
> will configure qemu.conf without the controller or not. It will always
> be created.
> 
> As it iterates over the all the internal-defined controllers without
> honoring the configuration in qemu.conf. (see the 'for' loop)
> 
> However, It's expected to not create the cgroup as long as it's not
> configured in qemu.conf.
> 
> The right way is to fix virCgroupMakeCgroup to honor the configuration
> in qemu.conf instead, a new flag here only fixes the case "cpuset"
> is not configured, it doesn't fix the root cause, and the new flags
> is
> not needed if the root cause is fixed.
> 
> I see this set passes driver->cgroupContollers to virCgroupMakeCgroup,
> which is good, but it still iterates over the internal-defined all
> controllers.

Yep, this  is an existing bug.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|