On Thu, Jul 19, 2012 at 09:54:31AM +0100, Daniel P. Berrange wrote:
On Thu, Jul 19, 2012 at 04:37:14PM +0800, Daniel Veillard wrote:
On Wed, Jul 18, 2012 at 05:32:30PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Move the cgroup setup code out of the lxc_controller.c file and into lxc_cgroup.{c,h}. This reduces the size of the lxc_controller.c file and paves the way to invoke cgroup setup from lxc_driver.c instead of lxc_controller.c in the future
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- po/POTFILES.in | 1 + src/Makefile.am | 2 + src/lxc/lxc_cgroup.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++ src/lxc/lxc_cgroup.h | 29 +++++ src/lxc/lxc_controller.c | 232 +-------------------------------------- 5 files changed, 306 insertions(+), 230 deletions(-) create mode 100644 src/lxc/lxc_cgroup.c create mode 100644 src/lxc/lxc_cgroup.h
+int virLXCCgroupSetup(virDomainDefPtr def) +{ + virCgroupPtr driver = NULL; + virCgroupPtr cgroup = NULL; + int rc = -1; + + rc = virCgroupForDriver("lxc", &driver, 1, 0); + if (rc != 0) { + /* Skip all if no driver cgroup is configured */ + if (rc == -ENXIO || rc == -ENOENT) + return 0; + + virReportSystemError(-rc, "%s", + _("Unable to get cgroup for driver")); + return rc; + } + + rc = virCgroupForDomain(driver, def->name, &cgroup, 1); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to create cgroup for domain %s"), + def->name); + goto cleanup; + } + + if (virLXCCgroupSetupCpuTune(def, cgroup) < 0) + goto cleanup; + + if (virLXCCgroupSetupBlkioTune(def, cgroup) < 0) + goto cleanup; + + if (virLXCCgroupSetupMemTune(def, cgroup) < 0) + goto cleanup; + + if (virLXCCgroupSetupDeviceACL(def, cgroup) < 0) + goto cleanup; + + rc = virCgroupAddTask(cgroup, getpid()); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to add task %d to cgroup for domain %s"), + getpid(), def->name); + } + +cleanup: + virCgroupFree(&cgroup); + virCgroupFree(&driver); + + return rc; +}
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 7a1ce14..4777d51 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -58,6 +58,7 @@
#include "lxc_conf.h" #include "lxc_container.h" +#include "lxc_cgroup.h" #include "virnetdev.h" #include "virnetdevveth.h" #include "memory.h" @@ -72,13 +73,6 @@
#define VIR_FROM_THIS VIR_FROM_LXC
-struct cgroup_device_policy { - char type; - int major; - int minor; -}; - - typedef struct _virLXCControllerConsole virLXCControllerConsole; typedef virLXCControllerConsole *virLXCControllerConsolePtr; struct _virLXCControllerConsole { @@ -127,8 +121,6 @@ struct _virLXCController {
/* Server socket */ virNetServerPtr server; - - virCgroupPtr cgroup; };
static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -201,8 +193,6 @@ static void virLXCControllerStopInit(virLXCControllerPtr ctrl) virLXCControllerCloseLoopDevices(ctrl, true); virPidAbort(ctrl->initpid); ctrl->initpid = 0; - - virCgroupFree(&ctrl->cgroup); }
I'm a bit surprized there. The cgroup is moved out of that structure so that's fine, but i would have expected to see somewhere in the patch another chunk of code outside of the 2 new modules calling the free of the new structure, but it's nowhere to be found in src/lxc/lxc_controller.c diff. Something escapes me, where do we free those now ?
If you look at the virLXCControllerSetupResourceLimits() method below, you'll see we call virCgroupForDomain in the old code to create 'ctrl->cgroup', and free it in virLXCControllerStopInit().
In the new virLXCCgroupSetup() method above, we create & free the cgroup object within the scope of that method. So we don't need final cleanup when stopping the controler anymore.
okidoc, ACK :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/