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

2012-12-13 Thread Osier Yang 

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.


Correct me if I'm wrong.

Regards,
Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-12-13 Thread Hu Tao 
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.

Agreed. I don't think those directories should be created if not in use,
neither.

 
 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.

I'm thinking about refactoring cgroup code to create cgroup dir as
needed.

-- 
Hu Tao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-12-13 Thread 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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-12-12 Thread Hu Tao 
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;
+}
+
 /* We need to control cpu bandwidth for each vcpu now */
 if ((flags  VIR_CGROUP_VCPU) 
 (i != VIR_CGROUP_CONTROLLER_CPU 
-- 
1.8.0.1.240.ge8a1f5a

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list