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
diff --git a/po/POTFILES.in b/po/POTFILES.in index 6ca40d9..2d5735a 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -42,6 +42,7 @@ src/libvirt.c src/libvirt-qemu.c src/locking/lock_driver_sanlock.c src/locking/lock_manager.c +src/lxc/lxc_cgroup.c src/lxc/lxc_container.c src/lxc/lxc_conf.c src/lxc/lxc_controller.c diff --git a/src/Makefile.am b/src/Makefile.am index e18b0dc..9e16d06 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -350,12 +350,14 @@ endif LXC_DRIVER_SOURCES = \ lxc/lxc_conf.c lxc/lxc_conf.h \ lxc/lxc_container.c lxc/lxc_container.h \ + lxc/lxc_cgroup.c lxc/lxc_cgroup.h \ lxc/lxc_domain.c lxc/lxc_domain.h \ lxc/lxc_driver.c lxc/lxc_driver.h
LXC_CONTROLLER_SOURCES = \ lxc/lxc_conf.c lxc/lxc_conf.h \ lxc/lxc_container.c lxc/lxc_container.h \ + lxc/lxc_cgroup.c lxc/lxc_cgroup.h \ lxc/lxc_controller.c
SECURITY_DRIVER_APPARMOR_HELPER_SOURCES = \ diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c new file mode 100644 index 0000000..ddd4901 --- /dev/null +++ b/src/lxc/lxc_cgroup.c @@ -0,0 +1,272 @@ +/* + * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright IBM Corp. 2008 + * + * lxc_cgroup.c: LXC cgroup helpers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <config.h> + +#include "lxc_cgroup.h" +#include "lxc_container.h" +#include "virterror_internal.h" +#include "logging.h" +#include "memory.h" +#include "cgroup.h" + +#define VIR_FROM_THIS VIR_FROM_LXC + +static int virLXCCgroupSetupCpuTune(virDomainDefPtr def, + virCgroupPtr cgroup) +{ + int ret = -1; + if (def->cputune.shares != 0) { + int rc = virCgroupSetCpuShares(cgroup, def->cputune.shares); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set io cpu shares for domain %s"), + def->name); + goto cleanup; + } + } + if (def->cputune.quota != 0) { + int rc = virCgroupSetCpuCfsQuota(cgroup, def->cputune.quota); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set io cpu quota for domain %s"), + def->name); + goto cleanup; + } + } + if (def->cputune.period != 0) { + int rc = virCgroupSetCpuCfsPeriod(cgroup, def->cputune.period); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set io cpu period for domain %s"), + def->name); + goto cleanup; + } + } + ret = 0; +cleanup: + return ret; +} + + +static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, + virCgroupPtr cgroup) +{ + int ret = -1; + + if (def->blkio.weight) { + int rc = virCgroupSetBlkioWeight(cgroup, def->blkio.weight); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set Blkio weight for domain %s"), + def->name); + goto cleanup; + } + } + + ret = 0; +cleanup: + return ret; +} + + +static int virLXCCgroupSetupMemTune(virDomainDefPtr def, + virCgroupPtr cgroup) +{ + int ret = -1; + int rc; + + rc = virCgroupSetMemory(cgroup, def->mem.max_balloon); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set memory limit for domain %s"), + def->name); + goto cleanup; + } + + if (def->mem.hard_limit) { + rc = virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set memory hard limit for domain %s"), + def->name); + goto cleanup; + } + } + + if (def->mem.soft_limit) { + rc = virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set memory soft limit for domain %s"), + def->name); + goto cleanup; + } + } + + if (def->mem.swap_hard_limit) { + rc = virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set swap hard limit for domain %s"), + def->name); + goto cleanup; + } + } + + ret = 0; +cleanup: + return ret; +} + + +typedef struct _virLXCCgroupDevicePolicy virLXCCgroupDevicePolicy; +typedef virLXCCgroupDevicePolicy *virLXCCgroupDevicePolicyPtr; + +struct _virLXCCgroupDevicePolicy { + char type; + int major; + int minor; +}; + + + +static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, + virCgroupPtr cgroup) +{ + int ret = -1; + int rc; + size_t i; + static virLXCCgroupDevicePolicy devices[] = { + {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL}, + {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO}, + {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL}, + {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM}, + {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM}, + {'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_TTY}, + {'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX}, + {0, 0, 0}}; + + rc = virCgroupDenyAllDevices(cgroup); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to deny devices for domain %s"), + def->name); + goto cleanup; + } + + for (i = 0; devices[i].type != 0; i++) { + virLXCCgroupDevicePolicyPtr dev = &devices[i]; + rc = virCgroupAllowDevice(cgroup, + dev->type, + dev->major, + dev->minor, + VIR_CGROUP_DEVICE_RWM); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to allow device %c:%d:%d for domain %s"), + dev->type, dev->major, dev->minor, def->name); + goto cleanup; + } + } + + for (i = 0 ; i < def->nfss ; i++) { + if (def->fss[i]->type != VIR_DOMAIN_FS_TYPE_BLOCK) + continue; + + rc = virCgroupAllowDevicePath(cgroup, + def->fss[i]->src, + def->fss[i]->readonly ? + VIR_CGROUP_DEVICE_READ : + VIR_CGROUP_DEVICE_RW); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to allow device %s for domain %s"), + def->fss[i]->src, def->name); + goto cleanup; + } + } + + rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY, + VIR_CGROUP_DEVICE_RWM); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to allow PTY devices for domain %s"), + def->name); + goto cleanup; + } + + ret = 0; +cleanup: + return ret; +} + + +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_cgroup.h b/src/lxc/lxc_cgroup.h new file mode 100644 index 0000000..97bb12a --- /dev/null +++ b/src/lxc/lxc_cgroup.h @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright IBM Corp. 2008 + * + * lxc_cgroup.c: LXC cgroup helpers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __VIR_LXC_CGROUP_H__ +# define __VIR_LXC_CGROUP_H__ + +# include "domain_conf.h" + +int virLXCCgroupSetup(virDomainDefPtr def); + +#endif /* __VIR_LXC_CGROUP_H__ */ 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 ?
@@ -527,181 +517,6 @@ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) }
-static int virLXCControllerSetupCgroupsCpuTune(virLXCControllerPtr ctrl) -{ - int ret = -1; - if (ctrl->def->cputune.shares != 0) { - int rc = virCgroupSetCpuShares(ctrl->cgroup, ctrl->def->cputune.shares); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io cpu shares for domain %s"), - ctrl->def->name); - goto cleanup; - } - } - if (ctrl->def->cputune.quota != 0) { - int rc = virCgroupSetCpuCfsQuota(ctrl->cgroup, ctrl->def->cputune.quota); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io cpu quota for domain %s"), - ctrl->def->name); - goto cleanup; - } - } - if (ctrl->def->cputune.period != 0) { - int rc = virCgroupSetCpuCfsPeriod(ctrl->cgroup, ctrl->def->cputune.period); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io cpu period for domain %s"), - ctrl->def->name); - goto cleanup; - } - } - ret = 0; -cleanup: - return ret; -} - - -static int virLXCControllerSetupCgroupsBlkioTune(virLXCControllerPtr ctrl) -{ - int ret = -1; - - if (ctrl->def->blkio.weight) { - int rc = virCgroupSetBlkioWeight(ctrl->cgroup, ctrl->def->blkio.weight); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set Blkio weight for domain %s"), - ctrl->def->name); - goto cleanup; - } - } - - ret = 0; -cleanup: - return ret; -} - - -static int virLXCControllerSetupCgroupsMemTune(virLXCControllerPtr ctrl) -{ - int ret = -1; - int rc; - - rc = virCgroupSetMemory(ctrl->cgroup, ctrl->def->mem.max_balloon); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory limit for domain %s"), - ctrl->def->name); - goto cleanup; - } - - if (ctrl->def->mem.hard_limit) { - rc = virCgroupSetMemoryHardLimit(ctrl->cgroup, ctrl->def->mem.hard_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory hard limit for domain %s"), - ctrl->def->name); - goto cleanup; - } - } - - if (ctrl->def->mem.soft_limit) { - rc = virCgroupSetMemorySoftLimit(ctrl->cgroup, ctrl->def->mem.soft_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory soft limit for domain %s"), - ctrl->def->name); - goto cleanup; - } - } - - if (ctrl->def->mem.swap_hard_limit) { - rc = virCgroupSetMemSwapHardLimit(ctrl->cgroup, ctrl->def->mem.swap_hard_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set swap hard limit for domain %s"), - ctrl->def->name); - goto cleanup; - } - } - - ret = 0; -cleanup: - return ret; -} - - -static int virLXCControllerSetupCgroupsDeviceACL(virLXCControllerPtr ctrl) -{ - int ret = -1; - int rc; - size_t i; - static const struct cgroup_device_policy devices[] = { - {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL}, - {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO}, - {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL}, - {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM}, - {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM}, - {'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_TTY}, - {'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX}, - {0, 0, 0}}; - - rc = virCgroupDenyAllDevices(ctrl->cgroup); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to deny devices for domain %s"), - ctrl->def->name); - goto cleanup; - } - - for (i = 0; devices[i].type != 0; i++) { - const struct cgroup_device_policy *dev = &devices[i]; - rc = virCgroupAllowDevice(ctrl->cgroup, - dev->type, - dev->major, - dev->minor, - VIR_CGROUP_DEVICE_RWM); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow device %c:%d:%d for domain %s"), - dev->type, dev->major, dev->minor, ctrl->def->name); - goto cleanup; - } - } - - for (i = 0 ; i < ctrl->def->nfss ; i++) { - if (ctrl->def->fss[i]->type != VIR_DOMAIN_FS_TYPE_BLOCK) - continue; - - rc = virCgroupAllowDevicePath(ctrl->cgroup, - ctrl->def->fss[i]->src, - ctrl->def->fss[i]->readonly ? - VIR_CGROUP_DEVICE_READ : - VIR_CGROUP_DEVICE_RW); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow device %s for domain %s"), - ctrl->def->fss[i]->src, ctrl->def->name); - goto cleanup; - } - } - - rc = virCgroupAllowDeviceMajor(ctrl->cgroup, 'c', LXC_DEV_MAJ_PTY, - VIR_CGROUP_DEVICE_RWM); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow PTY devices for domain %s"), - ctrl->def->name); - goto cleanup; - } - - ret = 0; -cleanup: - return ret; -} - - /** * virLXCControllerSetupResourceLimits * @ctrl: the controller state @@ -713,8 +528,6 @@ cleanup: */ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl) { - virCgroupPtr driver; - int rc = -1;
if (virLXCControllerSetupCpuAffinity(ctrl) < 0) return -1; @@ -722,48 +535,7 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl) if (virLXCControllerSetupNUMAPolicy(ctrl) < 0) return -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, ctrl->def->name, &ctrl->cgroup, 1); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for domain %s"), - ctrl->def->name); - goto cleanup; - } - - if (virLXCControllerSetupCgroupsCpuTune(ctrl) < 0) - goto cleanup; - - if (virLXCControllerSetupCgroupsBlkioTune(ctrl) < 0) - goto cleanup; - - if (virLXCControllerSetupCgroupsMemTune(ctrl) < 0) - goto cleanup; - - if (virLXCControllerSetupCgroupsDeviceACL(ctrl) < 0) - goto cleanup; - - rc = virCgroupAddTask(ctrl->cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to add task %d to cgroup for domain %s"), - getpid(), ctrl->def->name); - } - -cleanup: - virCgroupFree(&driver); - - return rc; + return virLXCCgroupSetup(ctrl->def); }
So we do call the Setup() but never the free ... I'm missing something :-) ACK pending on clarification ! 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/