[lxc-devel] [PATCH 1/1] cgmanager: implement setting of cgroup limits
S.Çağlar Onur
caglar at 10ur.org
Wed Jan 22 02:35:27 UTC 2014
Hey Serge,
On Mon, Jan 20, 2014 at 10:09 AM, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> Also replace a wrong free of nih-allocated variable with nih_free.
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
Looks like after this commit unprivileged containers are not starting
while using cgmanager v0.16
[caglar at qp:~/Projects/lxc(master)] sudo lxc-ls
[caglar at qp:~/Projects/lxc(master)] sudo lxc-create -q -t ubuntu -n lorem
[caglar at qp:~/Projects/lxc(master)] sudo lxc-ls
lorem
[caglar at qp:~/Projects/lxc(master)] sudo lxc-start -d -n lorem -l debug
-o lorem.log
[caglar at qp:~/Projects/lxc(master)] sudo lxc-stop -n lorem
[caglar at qp:~/Projects/lxc(master)] lxc-ls
[caglar at qp:~/Projects/lxc(master)] lxc-create -q -t download -n ipsum
-- -d ubuntu -r saucy -a amd64
[caglar at qp:~/Projects/lxc(master)] lxc-ls
ipsum
[caglar at qp:~/Projects/lxc(master)] lxc-start -d -n ipsum -l debug -o ipsum.log
lxc-start: command get_cgroup failed to receive response
cat ipsum.log[caglar at qp:~/Projects/lxc(master)] cat ipsum.log
lxc-start 1390357846.813 INFO lxc_start_ui - using rcfile
/home/caglar/.local/share/lxc/ipsum/config
lxc-start 1390357846.814 INFO lxc_confile - read uid map:
type u nsid 0 hostid 260000 range 10000
lxc-start 1390357846.814 INFO lxc_confile - read uid map:
type g nsid 0 hostid 260000 range 10000
lxc-start 1390357846.814 WARN lxc_log - lxc_log_init called
with log already initialized
lxc-start 1390357846.817 INFO lxc_lsm - LSM security driver AppArmor
lxc-start 1390357846.818 DEBUG lxc_conf - allocated pty
'/dev/pts/3' (5/6)
lxc-start 1390357846.818 DEBUG lxc_conf - allocated pty
'/dev/pts/7' (7/8)
lxc-start 1390357846.818 DEBUG lxc_conf - allocated pty
'/dev/pts/8' (9/10)
lxc-start 1390357846.818 DEBUG lxc_conf - allocated pty
'/dev/pts/9' (11/12)
lxc-start 1390357846.818 INFO lxc_conf - tty's configured
lxc-start 1390357846.818 DEBUG lxc_start - sigchild handler set
lxc-start 1390357846.819 DEBUG lxc_console - no console peer
lxc-start 1390357846.820 INFO lxc_monitor - using monitor
sock name lxc/845341109187512e//home/caglar/.local/share/lxc
lxc-start 1390357847.067 INFO lxc_start - 'ipsum' is initialized
lxc-start 1390357847.087 DEBUG lxc_start - Not dropping
cap_sys_boot or watching utmp
lxc-start 1390357847.087 INFO lxc_start - Cloning a new user namespace
lxc-start 1390357847.093 ERROR lxc_start - failed to setup
the cgroup limits for 'ipsum'
lxc-start 1390357847.127 ERROR lxc_start - failed to spawn 'ipsum'
lxc-start 1390357847.127 ERROR lxc_commands - command
get_cgroup failed to receive response
[caglar at qp:~/Projects/lxc(master)]
as long as I see compiling LXC without --enable-cgmanager works.
> ---
> src/lxc/cgmanager.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++-------
> src/lxc/cgroup.c | 30 ++++++++++-------------
> src/lxc/cgroup.h | 4 ++--
> src/lxc/start.c | 6 ++---
> 4 files changed, 78 insertions(+), 30 deletions(-)
>
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index 97bda2c..61f7f6f 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -55,6 +55,7 @@ lxc_log_define(lxc_cgmanager, lxc);
>
> #include <nih-dbus/dbus_connection.h>
> #include <cgmanager-client/cgmanager-client.h>
> +#include <nih/alloc.h>
> NihDBusProxy *cgroup_manager = NULL;
>
> extern struct cgroup_ops *active_cg_ops;
> @@ -269,13 +270,25 @@ int cgm_get(const char *filename, char *value, size_t len, const char *name, con
> strncpy(value, result, len);
> if (strlen(result) >= len)
> value[len-1] = '\0';
> - free(result);
> + nih_free(result);
> return len;
> }
>
> +static int cgm_do_set(const char *controller, const char *file,
> + const char *cgroup, const char *value)
> +{
> + int ret;
> + ret = cgmanager_set_value_sync(NULL, cgroup_manager, controller,
> + cgroup, file, value);
> + if (ret != 0)
> + ERROR("Error setting cgroup %s limit %s", file, cgroup);
> + return ret;
> +}
> +
> int cgm_set(const char *filename, const char *value, const char *name, const char *lxcpath)
> {
> char *controller, *key, *cgroup;
> + int ret;
>
> controller = alloca(strlen(filename)+1);
> strcpy(controller, filename);
> @@ -291,14 +304,9 @@ int cgm_set(const char *filename, const char *value, const char *name, const cha
> controller, lxcpath, name);
> return -1;
> }
> - if (cgmanager_set_value_sync(NULL, cgroup_manager, controller, cgroup, filename, value) != 0) {
> - ERROR("Error setting value for %s from cgmanager for cgroup %s (%s:%s)",
> - filename, cgroup, lxcpath, name);
> - free(cgroup);
> - return -1;
> - }
> + ret = cgm_do_set(controller, filename, cgroup, value);
> free(cgroup);
> - return 0;
> + return ret;
> }
>
> /*
> @@ -367,6 +375,49 @@ static int cgm_unfreeze_fromhandler(struct lxc_handler *handler)
> return 0;
> }
>
> +static bool setup_limits(struct lxc_handler *h, bool do_devices)
> +{
> + struct lxc_list *iterator;
> + struct lxc_cgroup *cg;
> + bool ret = false;
> + struct lxc_list *cgroup_settings = &h->conf->cgroup;
> + struct cgm_data *d = h->cgroup_info->data;
> +
> + if (lxc_list_empty(cgroup_settings))
> + return 0;
> +
> + lxc_list_for_each(iterator, cgroup_settings) {
> + char controller[100], *p;
> + cg = iterator->elem;
> + if (do_devices != !strncmp("devices", cg->subsystem, 7))
> + continue;
> + if (strlen(cg->subsystem) > 100) // i smell a rat
> + goto out;
> + strcpy(controller, cg->subsystem);
> + p = strchr(controller, '.');
> + if (p)
> + *p = '\0';
> + if (cgm_do_set(controller, cg->subsystem, d->cgroup_path
> + , cg->value) < 0) {
> + ERROR("Error setting %s to %s for %s\n",
> + cg->subsystem, cg->value, h->name);
> + goto out;
> + }
> +
> + DEBUG("cgroup '%s' set to '%s'", cg->subsystem, cg->value);
> + }
> +
> + ret = true;
> + INFO("cgroup limits have been setup");
> +out:
> + return ret;
> +}
> +
> +static bool cgm_setup_limits(struct lxc_handler *handler, bool with_devices)
> +{
> + return setup_limits(handler, with_devices);
> +}
> +
> static struct cgroup_ops cgmanager_ops = {
> .destroy = cgm_destroy,
> .init = cgm_init,
> @@ -377,6 +428,7 @@ static struct cgroup_ops cgmanager_ops = {
> .get = cgm_get,
> .set = cgm_set,
> .unfreeze_fromhandler = cgm_unfreeze_fromhandler,
> + .setup_limits = cgm_setup_limits,
> .name = "cgmanager"
> };
> #endif
> diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
> index 85384fc..4482b32 100644
> --- a/src/lxc/cgroup.c
> +++ b/src/lxc/cgroup.c
> @@ -70,7 +70,7 @@ static struct cgroup_process_info *find_info_for_subsystem(struct cgroup_process
> static int do_cgroup_get(const char *cgroup_path, const char *sub_filename, char *value, size_t len);
> static int do_cgroup_set(const char *cgroup_path, const char *sub_filename, const char *value);
> static bool cgroup_devices_has_allow_or_deny(struct lxc_handler *h, char *v, bool for_allow);
> -static int do_setup_cgroup(struct lxc_handler *h, struct lxc_list *cgroup_settings, bool do_devices);
> +static int do_setup_cgroup_limits(struct lxc_handler *h, struct lxc_list *cgroup_settings, bool do_devices);
> static int cgroup_recursive_task_count(const char *cgroup_path);
> static int count_lines(const char *fn);
> static int handle_cgroup_settings(struct cgroup_mount_point *mp, char *cgroup_path);
> @@ -1838,7 +1838,7 @@ static int do_cgroup_set(const char *cgroup_path, const char *sub_filename,
> return ret;
> }
>
> -static int do_setup_cgroup(struct lxc_handler *h,
> +static int do_setup_cgroup_limits(struct lxc_handler *h,
> struct lxc_list *cgroup_settings, bool do_devices)
> {
> struct lxc_list *iterator;
> @@ -2182,6 +2182,11 @@ static int cgfs_unfreeze_fromhandler(struct lxc_handler *handler)
> return ret;
> }
>
> +bool cgroupfs_setup_limits(struct lxc_handler *h, bool with_devices)
> +{
> + return do_setup_cgroup_limits(h, &h->conf->cgroup, with_devices) == 0;
> +}
> +
> static struct cgroup_ops cgfs_ops = {
> .destroy = cgfs_destroy,
> .init = cgfs_init,
> @@ -2192,6 +2197,7 @@ static struct cgroup_ops cgfs_ops = {
> .get = lxc_cgroupfs_get,
> .set = lxc_cgroupfs_set,
> .unfreeze_fromhandler = cgfs_unfreeze_fromhandler,
> + .setup_limits = cgroupfs_setup_limits,
> .name = "cgroupfs",
> };
> static void init_cg_ops(void)
> @@ -2251,21 +2257,6 @@ bool cgroup_create(struct lxc_handler *handler)
> }
>
> /*
> - * Set up per-controller configuration excluding the devices
> - * cgroup
> - */
> -bool cgroup_setup_without_devices(struct lxc_handler *handler)
> -{
> - return do_setup_cgroup(handler, &handler->conf->cgroup, false) == 0;
> -}
> -
> -/* Set up the devices cgroup configuration for the container */
> -bool cgroup_setup_devices(struct lxc_handler *handler)
> -{
> - return do_setup_cgroup(handler, &handler->conf->cgroup, true) == 0;
> -}
> -
> -/*
> * Enter the container init into its new cgroups for all
> * requested controllers */
> bool cgroup_enter(struct lxc_handler *handler)
> @@ -2301,3 +2292,8 @@ int lxc_unfreeze_fromhandler(struct lxc_handler *handler)
> {
> return active_cg_ops->unfreeze_fromhandler(handler);
> }
> +
> +bool cgroup_setup_limits(struct lxc_handler *handler, bool with_devices)
> +{
> + return active_cg_ops->setup_limits(handler, with_devices);
> +}
> diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
> index 76dcbd1..af5a5c8 100644
> --- a/src/lxc/cgroup.h
> +++ b/src/lxc/cgroup.h
> @@ -179,6 +179,7 @@ struct cgroup_ops {
> int (*set)(const char *filename, const char *value, const char *name, const char *lxcpath);
> int (*get)(const char *filename, char *value, size_t len, const char *name, const char *lxcpath);
> int (*unfreeze_fromhandler)(struct lxc_handler *handler);
> + bool (*setup_limits)(struct lxc_handler *handler, bool with_devices);
> const char *name;
> };
>
> @@ -207,8 +208,7 @@ struct cgfs_data {
> extern void cgroup_destroy(struct lxc_handler *handler);
> extern bool cgroup_init(struct lxc_handler *handler);
> extern bool cgroup_create(struct lxc_handler *handler);
> -extern bool cgroup_setup_without_devices(struct lxc_handler *handler);
> -extern bool cgroup_setup_devices(struct lxc_handler *handler);
> +extern bool cgroup_setup_limits(struct lxc_handler *handler, bool with_devices);
> extern bool cgroup_enter(struct lxc_handler *handler);
> extern void cgroup_cleanup(struct lxc_handler *handler);
> extern bool cgroup_create_legacy(struct lxc_handler *handler);
> diff --git a/src/lxc/start.c b/src/lxc/start.c
> index b09bd9b..2fcd845 100644
> --- a/src/lxc/start.c
> +++ b/src/lxc/start.c
> @@ -796,8 +796,8 @@ static int lxc_spawn(struct lxc_handler *handler)
> ERROR("failed to setup the legacy cgroups for %s", name);
> goto out_delete_net;
> }
> - if (!cgroup_setup_without_devices(handler)) {
> - ERROR("failed to setup the cgroups for '%s'", name);
> + if (!cgroup_setup_limits(handler, false)) {
> + ERROR("failed to setup the cgroup limits for '%s'", name);
> goto out_delete_net;
> }
>
> @@ -831,7 +831,7 @@ static int lxc_spawn(struct lxc_handler *handler)
> if (lxc_sync_barrier_child(handler, LXC_SYNC_POST_CONFIGURE))
> goto out_delete_net;
>
> - if (!cgroup_setup_devices(handler)) {
> + if (!cgroup_setup_limits(handler, true)) {
> ERROR("failed to setup the devices cgroup for '%s'", name);
> goto out_delete_net;
> }
> --
> 1.8.5.2
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
S.Çağlar Onur <caglar at 10ur.org>
More information about the lxc-devel
mailing list