On 05/11/2012 12:48 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berra...@redhat.com>

Normal practice is for cgroups controllers to be mounted at
/sys/fs/cgroup. When setting up a container, /sys is mounted
with a new sysfs instance, thus we must re-mount all the
cgroups controllers. The complexity is that we must mount
them in the same layout as the host OS. ie if 'cpu' and 'cpuacct'
were mounted at the same location in the host we must preserve
this in the container. Also if any controllers are co-located
we must setup symlinks from the individual controller name to
the co-located mount-point


Signed-off-by: Daniel P. Berrange<berra...@redhat.com>
---
  src/lxc/lxc_container.c |  243 +++++++++++++++++++++++++++++++++++++++++++++--
  src/util/cgroup.h       |    2 +
  2 files changed, 236 insertions(+), 9 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index a3ca76c..79ecae8 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -35,6 +35,7 @@
  #include<sys/stat.h>
  #include<unistd.h>
  #include<mntent.h>
+#include<dirent.h>

  /* Yes, we want linux private one, for _syscall2() macro */
  #include<linux/unistd.h>

+static int lxcContainerIdentifyCGroups(struct lxcContainerCGroup **mountsret,
+                                       size_t *nmountsret)
+{
[...]
+    while ((dent = readdir(dh)) != NULL) {
+        ssize_t rv;
+        /* The cgroups links are just relative to the local
+         * dir so we don't need a large buf */
+        char linkbuf[100];

Nit: empty line here.

+        if (dent->d_name[0] == '.')
+            continue;
+
[...]
+cleanup:
+    closedir(dh);
+    endmntent(procmnt);
+    VIR_FREE(path);
+
+    if (ret<  0) {
+        for (i = 0 ; i<  nmounts ; i++) {
+            VIR_FREE(mounts[i].dir);
+            VIR_FREE(mounts[i].linkDest);
+        }
+        VIR_FREE(mounts);

You could put this into a function lxcContainerCGroupFree().

+
+cleanup:
+    for (i = 0 ; i<  nmounts ; i++) {
+        VIR_FREE(mounts[i].dir);
+        VIR_FREE(mounts[i].linkDest);
+    }
+    VIR_FREE(mounts);


... and call it again from here and save a few lines ...


@@ -1190,19 +1394,40 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr 
vmDef,
      if (lxcContainerMountAllFS(vmDef, "", false)<  0)
          return -1;

+    /* Before replacing /sys we need to identify any
+     * cgroups controllers that are mounted */
+    if (lxcContainerIdentifyCGroups(&mounts,&nmounts)<  0)
+        goto cleanup;
+
      /* Gets rid of any existing stuff under /proc, since we need new
       * namespace aware versions of those. We must do /proc second
       * otherwise we won't find /proc/mounts :-) */
      if (lxcContainerUnmountSubtree("/sys", false)<  0 ||
          lxcContainerUnmountSubtree("/proc", false)<  0)
-        return -1;
+        goto cleanup;

      /* Mounts the core /proc, /sys, etc filesystems */
      if (lxcContainerMountBasicFS(vmDef, false, securityDriver)<  0)
-        return -1;
+        goto cleanup;
+
+    /* Now we can re-mount the cgroups controllers in the
+     * same configuration as before */
+    if (lxcContainerMountCGroups(mounts, nmounts)<  0)
+        goto cleanup;

      VIR_DEBUG("Mounting completed");
      return 0;
+
+    ret = 0;
+
+cleanup:
+    for (i = 0 ; i<  nmounts ; i++) {
+        VIR_FREE(mounts[i].dir);
+        VIR_FREE(mounts[i].linkDest);
+    }
+    VIR_FREE(mounts);

... and save a few more lines here as well.

 With this nit, I'd ACK the series.

   Stefan

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

Reply via email to