From aaf26830526092805f9c6d87548edacff77727da Mon Sep 17 00:00:00 2001 From: Kien Truong Date: Sun, 5 Apr 2015 23:46:22 +0000 Subject: [PATCH 1/3] Sort the cgroup memory settings before applying. Add a function to sort the cgroup settings before applying. Currently, the function will put memory.memsw.limit_in_bytes after memory.limit_in_bytes setting so the container will start regardless of the order specified in the input. Fix #453 Signed-off-by: Kien Truong --- src/lxc/cgfs.c | 9 ++++++++- src/lxc/cgmanager.c | 9 ++++++++- src/lxc/conf.c | 35 +++++++++++++++++++++++++++++++++++ src/lxc/conf.h | 1 + 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c index 11a5925df..0b7f99818 100644 --- a/src/lxc/cgfs.c +++ b/src/lxc/cgfs.c @@ -1888,12 +1888,15 @@ static int do_setup_cgroup_limits(struct cgfs_data *d, { struct lxc_list *iterator; struct lxc_cgroup *cg; + struct lxc_list *sorted_cgroup_settings; int ret = -1; if (lxc_list_empty(cgroup_settings)) return 0; - lxc_list_for_each(iterator, cgroup_settings) { + sorted_cgroup_settings = sort_cgroup_settings(cgroup_settings); + + lxc_list_for_each(iterator, sorted_cgroup_settings) { cg = iterator->elem; if (do_devices == !strncmp("devices", cg->subsystem, 7)) { @@ -1916,6 +1919,10 @@ static int do_setup_cgroup_limits(struct cgfs_data *d, ret = 0; INFO("cgroup has been setup"); out: + lxc_list_for_each(iterator, sorted_cgroup_settings) { + lxc_list_del(iterator); + free(iterator); + } return ret; } diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c index c8d0745b4..51373913d 100644 --- a/src/lxc/cgmanager.c +++ b/src/lxc/cgmanager.c @@ -1221,6 +1221,7 @@ static bool cgm_setup_limits(void *hdata, struct lxc_list *cgroup_settings, bool struct lxc_list *iterator; struct lxc_cgroup *cg; bool ret = false; + struct lxc_list *sorted_cgroup_settings; if (lxc_list_empty(cgroup_settings)) return true; @@ -1233,7 +1234,9 @@ static bool cgm_setup_limits(void *hdata, struct lxc_list *cgroup_settings, bool return false; } - lxc_list_for_each(iterator, cgroup_settings) { + sorted_cgroup_settings = sort_cgroup_settings(cgroup_settings); + + lxc_list_for_each(iterator, sorted_cgroup_settings) { char controller[100], *p; cg = iterator->elem; if (do_devices != !strncmp("devices", cg->subsystem, 7)) @@ -1261,6 +1264,10 @@ static bool cgm_setup_limits(void *hdata, struct lxc_list *cgroup_settings, bool ret = true; INFO("cgroup limits have been setup"); out: + lxc_list_for_each(iterator, sorted_cgroup_settings) { + lxc_list_del(iterator); + free(iterator); + } cgm_dbus_disconnect(); return ret; } diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 3a7a52a0d..64ced0f84 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -4563,3 +4563,38 @@ void suggest_default_idmap(void) free(gname); free(uname); } + +/* + * Return the list of cgroup_settings sorted according to the following rules + * 1. Put memory.limit_in_bytes before memory.memsw.limit_in_bytes + */ +struct lxc_list *sort_cgroup_settings(struct lxc_list* cgroup_settings) +{ + struct lxc_list *result; + struct lxc_list *memsw_limit = NULL; + struct lxc_list *it = NULL; + struct lxc_cgroup *cg = NULL; + struct lxc_list *item = NULL; + + result = malloc(sizeof(*result)); + lxc_list_init(result); + + /*Iterate over the cgroup settings and copy them to the output list*/ + lxc_list_for_each(it, cgroup_settings) { + item = malloc(sizeof(*item)); + item->elem = it->elem; + cg = it->elem; + if (strcmp(cg->subsystem, "memory.memsw.limit_in_bytes") == 0) { + /* Store the memsw_limit location */ + memsw_limit = item; + } else if (strcmp(cg->subsystem, "memory.limit_in_bytes") == 0 && memsw_limit != NULL) { + /* lxc.cgroup.memory.memsw.limit_in_bytes is found before + * lxc.cgroup.memory.limit_in_bytes, swap these two items */ + item->elem = memsw_limit->elem; + memsw_limit->elem = it->elem; + } + lxc_list_add_tail(result, item); + } + + return result; +} \ No newline at end of file diff --git a/src/lxc/conf.h b/src/lxc/conf.h index 0c0475e02..48f7fa253 100644 --- a/src/lxc/conf.h +++ b/src/lxc/conf.h @@ -431,4 +431,5 @@ extern void tmp_proc_unmount(struct lxc_conf *lxc_conf); void remount_all_slave(void); extern void suggest_default_idmap(void); FILE *write_mount_file(struct lxc_list *mount); +struct lxc_list *sort_cgroup_settings(struct lxc_list* cgroup_settings); #endif From fac7c663865a706b521dcdb3182c92e93bf0c721 Mon Sep 17 00:00:00 2001 From: Kien Truong Date: Mon, 6 Apr 2015 17:05:20 +0100 Subject: [PATCH 2/3] Check malloc failure when sorting cgroup settings. Signed-off-by: Kien Truong --- src/lxc/cgfs.c | 3 +++ src/lxc/cgmanager.c | 3 +++ src/lxc/conf.c | 8 ++++++++ 3 files changed, 14 insertions(+) diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c index 0b7f99818..2ee9058ab 100644 --- a/src/lxc/cgfs.c +++ b/src/lxc/cgfs.c @@ -1895,6 +1895,9 @@ static int do_setup_cgroup_limits(struct cgfs_data *d, return 0; sorted_cgroup_settings = sort_cgroup_settings(cgroup_settings); + if (!sorted_cgroup_settings) { + return -1; + } lxc_list_for_each(iterator, sorted_cgroup_settings) { cg = iterator->elem; diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c index 51373913d..de0bf30f3 100644 --- a/src/lxc/cgmanager.c +++ b/src/lxc/cgmanager.c @@ -1235,6 +1235,9 @@ static bool cgm_setup_limits(void *hdata, struct lxc_list *cgroup_settings, bool } sorted_cgroup_settings = sort_cgroup_settings(cgroup_settings); + if (!sorted_cgroup_settings) { + return false; + } lxc_list_for_each(iterator, sorted_cgroup_settings) { char controller[100], *p; diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 64ced0f84..9255b3488 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -4577,11 +4577,19 @@ struct lxc_list *sort_cgroup_settings(struct lxc_list* cgroup_settings) struct lxc_list *item = NULL; result = malloc(sizeof(*result)); + if (!result) { + ERROR("failed to allocate memory to sort cgroup settings"); + return NULL; + } lxc_list_init(result); /*Iterate over the cgroup settings and copy them to the output list*/ lxc_list_for_each(it, cgroup_settings) { item = malloc(sizeof(*item)); + if (!item) { + ERROR("failed to allocate memory to sort cgroup settings"); + return NULL; + } item->elem = it->elem; cg = it->elem; if (strcmp(cg->subsystem, "memory.memsw.limit_in_bytes") == 0) { From 365d180a391f75002b091b2ff8de7b21bc3aa5e4 Mon Sep 17 00:00:00 2001 From: Kien Truong Date: Mon, 6 Apr 2015 17:20:43 +0100 Subject: [PATCH 3/3] Properly free memory of sorted cgroup settings We need to use lxc_list_for_each_safe, otherwise de-allocation will fail with a list size bigger than 2. The pointer to the head of the list also need freeing after we've freed all other elements of the list. Signed-off-by: Kien Truong --- src/lxc/cgfs.c | 6 +++--- src/lxc/cgmanager.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c index 2ee9058ab..fcb3cde15 100644 --- a/src/lxc/cgfs.c +++ b/src/lxc/cgfs.c @@ -1886,9 +1886,8 @@ static int do_cgroup_set(const char *cgroup_path, const char *sub_filename, static int do_setup_cgroup_limits(struct cgfs_data *d, struct lxc_list *cgroup_settings, bool do_devices) { - struct lxc_list *iterator; + struct lxc_list *iterator, *sorted_cgroup_settings, *next; struct lxc_cgroup *cg; - struct lxc_list *sorted_cgroup_settings; int ret = -1; if (lxc_list_empty(cgroup_settings)) @@ -1922,10 +1921,11 @@ static int do_setup_cgroup_limits(struct cgfs_data *d, ret = 0; INFO("cgroup has been setup"); out: - lxc_list_for_each(iterator, sorted_cgroup_settings) { + lxc_list_for_each_safe(iterator, sorted_cgroup_settings, next) { lxc_list_del(iterator); free(iterator); } + free(sorted_cgroup_settings); return ret; } diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c index de0bf30f3..7dddb89de 100644 --- a/src/lxc/cgmanager.c +++ b/src/lxc/cgmanager.c @@ -1218,10 +1218,9 @@ static bool cgm_unfreeze(void *hdata) static bool cgm_setup_limits(void *hdata, struct lxc_list *cgroup_settings, bool do_devices) { struct cgm_data *d = hdata; - struct lxc_list *iterator; + struct lxc_list *iterator, *sorted_cgroup_settings, *next; struct lxc_cgroup *cg; bool ret = false; - struct lxc_list *sorted_cgroup_settings; if (lxc_list_empty(cgroup_settings)) return true; @@ -1267,10 +1266,11 @@ static bool cgm_setup_limits(void *hdata, struct lxc_list *cgroup_settings, bool ret = true; INFO("cgroup limits have been setup"); out: - lxc_list_for_each(iterator, sorted_cgroup_settings) { + lxc_list_for_each_safe(iterator, sorted_cgroup_settings, next) { lxc_list_del(iterator); free(iterator); } + free(sorted_cgroup_settings); cgm_dbus_disconnect(); return ret; }