On 05/23/2010 07:15 AM, Ryota Ozaki wrote: [Apologies for the delayed review, and for the fact that you had to ping us to remind us this had not been visited]
Through conversation with Kumar L Srikanth-B22348, I found that the function of getting memory usage (e.g., virsh dominfo) doesn't work for lxc with ns subsystem of cgroup enabled.
This is because of features of ns and memory subsystems. Ns creates child cgroup on every process fork and as a result processes in a container are not assigned in a cgroup for domain (e.g., libvirt/lxc/test1/). For example, libvirt_lxc and init (or somewhat specified in XML) are assigned into libvirt/lxc/test1/8839/ and libvirt/lxc/test1/8839/8849/, respectively. On the other hand, memory subsystem accounts memory usage within a group of processes by default, i.e., it does not take any child (and descendent) groups into account. With the two features, virsh dominfo which just checks memory usage of a cgroup for domain always returns zero because the cgroup has no process.
Setting memory.use_hierarchy of a group allows to account (and limit) memory usage of every descendent groups of the group.
s/descendent/descendant/
By setting it of a cgroup for domain, we can get proper memory usage of lxc with ns subsystem enabled. (To be exact, the setting is required only when memory and ns subsystems are enabled at the same time, e.g., mount -t cgroup none /cgroup.) --- src/util/cgroup.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b8b2eb5..f7d6b41 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -443,7 +443,38 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) return rc; }
-static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int create) +static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) +{ + int rc = 0; + unsigned long long value; + const char *filename = "memory.use_hierarchy"; + + rc = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, &value); + if (rc != 0) { + VIR_ERROR("Failed to read %s/%s (%d)", group->path, filename, rc);
Needs _() translation these days; 'make syntax-check' should have caught this.
+ return rc; + } + + /* Setting twice causes error, so if already enabled, skip setting */ + if (value == 1) + return 0; + + VIR_DEBUG("Setting up %s/%s", group->path, filename);
Debug remains untranslated...
+ rc = virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, 1); + + if (rc != 0) { + VIR_ERROR("Failed to set %s/%s (%d)", group->path, filename, rc);
...but another VIR_ERROR that needs translation.
+ } + + return rc; +} + +static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, + int create, int memory_hierarchy)
Based on how this is being used, it seems like you should use bool/true/false rather than int/0/1 for memory_hierarchy.
{ int i; int rc = 0; @@ -477,6 +508,20 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, int creat break; } } + /* + * Note that virCgroupSetMemoryUseHierarchy should always be + * called prior to creating subcgroups and attaching tasks. + */ + if (memory_hierarchy && + group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint != NULL && + (i == VIR_CGROUP_CONTROLLER_MEMORY || + STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) { + rc = virCgroupSetMemoryUseHierarchy(group); + if (rc != 0) { + VIR_FREE(path); + break; + } + } }
VIR_FREE(path); @@ -553,7 +598,7 @@ static int virCgroupAppRoot(int privileged, if (rc != 0) goto cleanup;
- rc = virCgroupMakeGroup(rootgrp, *group, create); + rc = virCgroupMakeGroup(rootgrp, *group, create, 0);
ACK with those nits addressed. Hopefully a v2 submission won't take as much time on the review side of things. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org