diff options
Diffstat (limited to '0012-fix-cpu-rt-disable-after-reboot-machine.patch')
| -rw-r--r-- | 0012-fix-cpu-rt-disable-after-reboot-machine.patch | 823 |
1 files changed, 823 insertions, 0 deletions
diff --git a/0012-fix-cpu-rt-disable-after-reboot-machine.patch b/0012-fix-cpu-rt-disable-after-reboot-machine.patch new file mode 100644 index 0000000..5ae07d1 --- /dev/null +++ b/0012-fix-cpu-rt-disable-after-reboot-machine.patch @@ -0,0 +1,823 @@ +From 166edf2093b2c35fe4e479ca4b6568be8c98f907 Mon Sep 17 00:00:00 2001 +From: haozi007 <liuhao27@huawei.com> +Date: Wed, 15 Feb 2023 17:47:12 +0800 +Subject: [PATCH 12/53] fix cpu-rt disable after reboot machine + +1. ensure parent cgroup cpu-rt of container, should do in start container; +2. current do in create container, will cause failed of start container with cpu-rt after reboot machine + +Signed-off-by: haozi007 <liuhao27@huawei.com> +--- + src/daemon/common/sysinfo.c | 46 +++- + src/daemon/common/sysinfo.h | 2 + + src/daemon/executor/container_cb/execution.c | 158 ++++++++++++- + .../executor/container_cb/execution_create.c | 222 +----------------- + src/daemon/modules/api/specs_api.h | 2 +- + src/daemon/modules/spec/specs.c | 84 ++++--- + test/specs/specs/specs_ut.cc | 40 ++-- + 7 files changed, 280 insertions(+), 274 deletions(-) + +diff --git a/src/daemon/common/sysinfo.c b/src/daemon/common/sysinfo.c +index 38416db4..7559d653 100644 +--- a/src/daemon/common/sysinfo.c ++++ b/src/daemon/common/sysinfo.c +@@ -24,8 +24,10 @@ + #include <linux/magic.h> + #include <sys/stat.h> + ++#include <isula_libutils/auto_cleanup.h> ++#include <isula_libutils/log.h> ++ + #include "err_msg.h" +-#include "isula_libutils/log.h" + #include "utils.h" + #include "utils_array.h" + #include "utils_file.h" +@@ -1627,3 +1629,45 @@ free_out: + } + return minfos; + } ++ ++char *sysinfo_cgroup_controller_cpurt_mnt_path() ++{ ++ int nret = 0; ++ __isula_auto_free char *mnt = NULL; ++ __isula_auto_free char *root = NULL; ++ char fpath[PATH_MAX] = { 0 }; ++ sysinfo_t *sysinfo = NULL; ++ ++ sysinfo = get_sys_info(true); ++ if (sysinfo == NULL) { ++ ERROR("Can not get system info"); ++ return NULL; ++ } ++ ++ if (!(sysinfo->cgcpuinfo.cpu_rt_period)) { ++ ERROR("Daemon-scoped cpu-rt-period and cpu-rt-runtime are not supported by kernel"); ++ isulad_set_error_message("Daemon-scoped cpu-rt-period and cpu-rt-runtime are not supported by kernel"); ++ return NULL; ++ } ++ ++ nret = find_cgroup_mountpoint_and_root("cpu", &mnt, &root); ++ if (nret != 0 || mnt == NULL || root == NULL) { ++ ERROR("Can not find cgroup mnt and root path for subsystem 'cpu'"); ++ isulad_set_error_message("Can not find cgroup mnt and root path for subsystem 'cpu'"); ++ return NULL; ++ } ++ ++ // When iSulad is run inside docker, the root is based of the host cgroup. ++ // Replace root to "/" ++ if (strncmp(root, "/docker/", strlen("/docker/")) == 0) { ++ root[1] = '\0'; ++ } ++ ++ nret = snprintf(fpath, sizeof(fpath), "%s/%s", mnt, root); ++ if (nret < 0 || (size_t)nret >= sizeof(fpath)) { ++ ERROR("Failed to print string"); ++ return NULL; ++ } ++ ++ return util_strdup_s(fpath); ++} +\ No newline at end of file +diff --git a/src/daemon/common/sysinfo.h b/src/daemon/common/sysinfo.h +index 8468e00a..bbb3c6b5 100644 +--- a/src/daemon/common/sysinfo.h ++++ b/src/daemon/common/sysinfo.h +@@ -139,6 +139,8 @@ mountinfo_t *find_mount_info(mountinfo_t **minfos, const char *dir); + + void free_mounts_info(mountinfo_t **minfos); + ++char *sysinfo_cgroup_controller_cpurt_mnt_path(); ++ + #ifdef __cplusplus + } + #endif +diff --git a/src/daemon/executor/container_cb/execution.c b/src/daemon/executor/container_cb/execution.c +index 7b18a8e1..ed70fc14 100644 +--- a/src/daemon/executor/container_cb/execution.c ++++ b/src/daemon/executor/container_cb/execution.c +@@ -18,6 +18,12 @@ + #include <pthread.h> + #include <malloc.h> + #include <sys/eventfd.h> ++#include <stdbool.h> ++#include <stdint.h> ++#include <stdlib.h> ++#include <string.h> ++#include <libgen.h> ++ + #include <isula_libutils/container_config.h> + #include <isula_libutils/container_config_v2.h> + #include <isula_libutils/container_delete_request.h> +@@ -34,13 +40,13 @@ + #include <isula_libutils/container_stop_request.h> + #include <isula_libutils/container_stop_response.h> + #include <isula_libutils/json_common.h> +-#include <stdbool.h> +-#include <stdint.h> +-#include <stdlib.h> +-#include <string.h> ++#include <isula_libutils/auto_cleanup.h> ++#include <isula_libutils/log.h> + +-#include "isula_libutils/log.h" ++#include "isulad_config.h" ++#include "sysinfo.h" + #include "container_api.h" ++#include "specs_api.h" + #include "execution_extend.h" + #include "execution_information.h" + #include "execution_stream.h" +@@ -302,6 +308,135 @@ static void pack_start_response(container_start_response *response, uint32_t cc, + } + } + ++static int do_init_cpurt_cgroups_path(const char *path, int recursive_depth, const char *mnt_root, ++ int64_t cpu_rt_period, int64_t cpu_rt_runtime); ++ ++/* maybe create cpu realtime file */ ++static int maybe_create_cpu_realtime_file(int64_t value, const char *file, const char *path) ++{ ++ int ret; ++ __isula_auto_close int fd = -1; ++ ssize_t nwrite; ++ char fpath[PATH_MAX] = { 0 }; ++ char buf[ISULAD_NUMSTRLEN64] = { 0 }; ++ ++ if (value == 0) { ++ return 0; ++ } ++ ++ ret = util_mkdir_p(path, CONFIG_DIRECTORY_MODE); ++ if (ret != 0) { ++ ERROR("Failed to mkdir: %s", path); ++ return -1; ++ } ++ ++ ret = snprintf(fpath, sizeof(fpath), "%s/%s", path, file); ++ if (ret < 0 || ret >= sizeof(fpath)) { ++ ERROR("Failed to print string"); ++ return -1; ++ } ++ ret = snprintf(buf, sizeof(buf), "%lld", (long long int)value); ++ if (ret < 0 || (size_t)ret >= sizeof(buf)) { ++ ERROR("Failed to print string"); ++ return -1; ++ } ++ ++ fd = util_open(fpath, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, 0700); ++ if (fd < 0) { ++ ERROR("Failed to open file: %s: %s", fpath, strerror(errno)); ++ isulad_set_error_message("Failed to open file: %s: %s", fpath, strerror(errno)); ++ return -1; ++ } ++ nwrite = util_write_nointr(fd, buf, strlen(buf)); ++ if (nwrite < 0 || nwrite != strlen(buf)) { ++ ERROR("Failed to write %s to %s: %s", buf, fpath, strerror(errno)); ++ isulad_set_error_message("Failed to write '%s' to '%s': %s", buf, fpath, strerror(errno)); ++ return -1; ++ } ++ ++ return 0; ++} ++ ++static int recursively_create_cgroup(const char *path, const char *mnt_root, int recursive_depth, int64_t cpu_rt_period, ++ int64_t cpu_rt_runtime) ++{ ++ int ret = 0; ++ __isula_auto_free char *dup = NULL; ++ char *dirpath = NULL; ++ char fpath[PATH_MAX] = { 0 }; ++ ++ dup = util_strdup_s(path); ++ dirpath = dirname(dup); ++ ret = do_init_cpurt_cgroups_path(dirpath, (recursive_depth + 1), mnt_root, cpu_rt_period, cpu_rt_runtime); ++ if (ret != 0) { ++ return ret; ++ } ++ ++ int nret = snprintf(fpath, sizeof(fpath), "%s/%s", mnt_root, path); ++ if (nret < 0 || (size_t)nret >= sizeof(fpath)) { ++ ERROR("Failed to print string"); ++ return ret; ++ } ++ ++ ret = maybe_create_cpu_realtime_file(cpu_rt_period, "cpu.rt_period_us", fpath); ++ if (ret != 0) { ++ return ret; ++ } ++ ++ return maybe_create_cpu_realtime_file(cpu_rt_runtime, "cpu.rt_runtime_us", fpath); ++} ++ ++/* init cgroups path */ ++static int do_init_cpurt_cgroups_path(const char *path, int recursive_depth, const char *mnt_root, ++ int64_t cpu_rt_period, int64_t cpu_rt_runtime) ++{ ++ if ((recursive_depth + 1) > MAX_PATH_DEPTH) { ++ ERROR("Reach the max cgroup depth:%s", path); ++ return -1; ++ } ++ ++ if (path == NULL || strcmp(path, "/") == 0 || strcmp(path, ".") == 0) { ++ return 0; ++ } ++ ++ // Recursively create cgroup to ensure that the system and all parent cgroups have values set ++ // for the period and runtime as this limits what the children can be set to. ++ return recursively_create_cgroup(path, mnt_root, recursive_depth, cpu_rt_period, cpu_rt_runtime); ++} ++ ++// TODO: maybe we should adapt to cgroup v2 ++static int cpurt_controller_init(const char *id, const host_config *host_spec) ++{ ++ __isula_auto_free char *mnt_root = NULL; ++ __isula_auto_free char *cgroups_path = NULL; ++ char *dirpath = NULL; ++ int64_t cpu_rt_period = 0; ++ int64_t cpu_rt_runtime = 0; ++ ++ cgroups_path = merge_container_cgroups_path(id, host_spec); ++ if (cgroups_path == NULL || strcmp(cgroups_path, "/") == 0 || strcmp(cgroups_path, ".") == 0) { ++ return 0; ++ } ++ ++ if (conf_get_cgroup_cpu_rt(&cpu_rt_period, &cpu_rt_runtime)) { ++ return -1; ++ } ++ ++ if (cpu_rt_period == 0 && cpu_rt_runtime == 0) { ++ return 0; ++ } ++ ++ mnt_root = sysinfo_cgroup_controller_cpurt_mnt_path(); ++ if (mnt_root == NULL) { ++ ERROR("Failed to get cpu rt controller mnt root path"); ++ return -1; ++ } ++ ++ dirpath = dirname(cgroups_path); ++ ++ return do_init_cpurt_cgroups_path(dirpath, 0, mnt_root, cpu_rt_period, cpu_rt_runtime); ++} ++ + static int container_start_prepare(container_t *cont, const container_start_request *request, int stdinfd, + struct io_write_wrapper *stdout_handler, struct io_write_wrapper *stderr_handler, + char **fifopath, char *fifos[], int *sync_fd, pthread_t *thread_id) +@@ -314,6 +449,19 @@ static int container_start_prepare(container_t *cont, const container_start_requ + return -1; + } + ++ // init cgroup path for cpu_rt_runtime and cpu_rt_period ++ // we should do this in start container, not create container ++ // because, in scenarios: ++ // 1. enable cpu-rt of isulad; ++ // 2. then run container with --cpu-rt-runtime ++ // 3. then reboot machine; ++ // 4. finally, start before container, it will failed... ++ // cause of no one to set value into cgroup/isulad/cpu-rt-runtime and cpu-rt-period. ++ if (cpurt_controller_init(id, cont->hostconfig) != 0) { ++ isulad_set_error_message("Failed to init controller of cpu-rt for container \"%s\".", id); ++ return -1; ++ } ++ + if (prepare_start_io(cont, request, fifopath, fifos, stdinfd, stdout_handler, stderr_handler, sync_fd, thread_id) != + 0) { + return -1; +diff --git a/src/daemon/executor/container_cb/execution_create.c b/src/daemon/executor/container_cb/execution_create.c +index 4cc333fd..4abc89c7 100644 +--- a/src/daemon/executor/container_cb/execution_create.c ++++ b/src/daemon/executor/container_cb/execution_create.c +@@ -19,6 +19,14 @@ + #include <errno.h> + #include <sys/stat.h> + #include <malloc.h> ++#include <limits.h> ++#include <stdbool.h> ++#include <stdint.h> ++#include <stdlib.h> ++#include <string.h> ++ ++#include <isula_libutils/log.h> ++#include <isula_libutils/auto_cleanup.h> + #include <isula_libutils/container_config.h> + #include <isula_libutils/container_config_v2.h> + #include <isula_libutils/defs.h> +@@ -26,14 +34,7 @@ + #include <isula_libutils/isulad_daemon_configs.h> + #include <isula_libutils/json_common.h> + #include <isula_libutils/oci_runtime_spec.h> +-#include <limits.h> +-#include <stdbool.h> +-#include <stdint.h> +-#include <stdlib.h> +-#include <string.h> +-#include <libgen.h> + +-#include "isula_libutils/log.h" + #include "isulad_config.h" + #include "specs_api.h" + #include "verify.h" +@@ -58,9 +59,6 @@ + #include "opt_log.h" + #include "runtime_api.h" + +-static int do_init_cpurt_cgroups_path(const char *path, int recursive_depth, const char *mnt_root, +- int64_t cpu_rt_period, int64_t cpu_rt_runtime); +- + static int create_request_check(const container_create_request *request) + { + int ret = 0; +@@ -1323,203 +1321,6 @@ static int save_container_config_before_create(const char *id, const char *runti + return 0; + } + +-/* maybe create cpu realtime file */ +-static int maybe_create_cpu_realtime_file(int64_t value, const char *file, const char *path) +-{ +- int ret; +- int fd = -1; +- ssize_t nwrite; +- char fpath[PATH_MAX] = { 0 }; +- char buf[ISULAD_NUMSTRLEN64] = { 0 }; +- +- if (value == 0) { +- return 0; +- } +- +- ret = util_mkdir_p(path, CONFIG_DIRECTORY_MODE); +- if (ret != 0) { +- ERROR("Failed to mkdir: %s", path); +- return -1; +- } +- +- ret = snprintf(fpath, sizeof(fpath), "%s/%s", path, file); +- if (ret < 0 || ret >= sizeof(fpath)) { +- ERROR("Failed to print string"); +- return -1; +- } +- ret = snprintf(buf, sizeof(buf), "%lld", (long long int)value); +- if (ret < 0 || (size_t)ret >= sizeof(buf)) { +- ERROR("Failed to print string"); +- return -1; +- } +- +- fd = util_open(fpath, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, 0700); +- if (fd < 0) { +- ERROR("Failed to open file: %s: %s", fpath, strerror(errno)); +- isulad_set_error_message("Failed to open file: %s: %s", fpath, strerror(errno)); +- return -1; +- } +- nwrite = util_write_nointr(fd, buf, strlen(buf)); +- if (nwrite < 0) { +- ERROR("Failed to write %s to %s: %s", buf, fpath, strerror(errno)); +- isulad_set_error_message("Failed to write '%s' to '%s': %s", buf, fpath, strerror(errno)); +- close(fd); +- return -1; +- } +- close(fd); +- +- return 0; +-} +- +-static int recursively_create_cgroup(const char *path, const char *mnt_root, int recursive_depth, int64_t cpu_rt_period, +- int64_t cpu_rt_runtime) +-{ +- int ret = 0; +- char *dup = NULL; +- char *dirpath = NULL; +- char fpath[PATH_MAX] = { 0 }; +- +- dup = util_strdup_s(path); +- dirpath = dirname(dup); +- ret = do_init_cpurt_cgroups_path(dirpath, (recursive_depth + 1), mnt_root, cpu_rt_period, cpu_rt_runtime); +- free(dup); +- if (ret != 0) { +- return ret; +- } +- +- int nret = snprintf(fpath, sizeof(fpath), "%s/%s", mnt_root, path); +- if (nret < 0 || (size_t)nret >= sizeof(fpath)) { +- ERROR("Failed to print string"); +- ret = -1; +- goto out; +- } +- +- ret = maybe_create_cpu_realtime_file(cpu_rt_period, "cpu.rt_period_us", fpath); +- if (ret != 0) { +- goto out; +- } +- +- ret = maybe_create_cpu_realtime_file(cpu_rt_runtime, "cpu.rt_runtime_us", fpath); +- if (ret != 0) { +- goto out; +- } +- +-out: +- return ret; +-} +- +-/* init cgroups path */ +-static int do_init_cpurt_cgroups_path(const char *path, int recursive_depth, const char *mnt_root, +- int64_t cpu_rt_period, int64_t cpu_rt_runtime) +-{ +- if ((recursive_depth + 1) > MAX_PATH_DEPTH) { +- ERROR("Reach the max cgroup depth:%s", path); +- return -1; +- } +- +- if (path == NULL || strcmp(path, "/") == 0 || strcmp(path, ".") == 0) { +- return 0; +- } +- +- // Recursively create cgroup to ensure that the system and all parent cgroups have values set +- // for the period and runtime as this limits what the children can be set to. +- if (recursively_create_cgroup(path, mnt_root, recursive_depth, cpu_rt_period, cpu_rt_runtime)) { +- return -1; +- } +- +- return 0; +-} +- +-static char *get_cpurt_controller_mnt_path() +-{ +- char *res = NULL; +- int nret = 0; +- char *mnt = NULL; +- char *root = NULL; +- char fpath[PATH_MAX] = { 0 }; +- +- nret = find_cgroup_mountpoint_and_root("cpu", &mnt, &root); +- if (nret != 0 || mnt == NULL || root == NULL) { +- ERROR("Can not find cgroup mnt and root path for subsystem 'cpu'"); +- isulad_set_error_message("Can not find cgroup mnt and root path for subsystem 'cpu'"); +- goto out; +- } +- +- // When iSulad is run inside docker, the root is based of the host cgroup. +- // Replace root to "/" +- if (strncmp(root, "/docker/", strlen("/docker/")) == 0) { +- root[1] = '\0'; +- } +- +- nret = snprintf(fpath, sizeof(fpath), "%s/%s", mnt, root); +- if (nret < 0 || (size_t)nret >= sizeof(fpath)) { +- ERROR("Failed to print string"); +- goto out; +- } +- +- res = util_strdup_s(fpath); +- +-out: +- free(mnt); +- free(root); +- return res; +-} +- +-static int cpurt_controller_init(const char *cgroups_path) +-{ +- int ret = 0; +- char *dup = NULL; +- char *dirpath = NULL; +- int64_t cpu_rt_period = 0; +- int64_t cpu_rt_runtime = 0; +- sysinfo_t *sysinfo = NULL; +- char *mnt_root = NULL; +- +- if (cgroups_path == NULL || strcmp(cgroups_path, "/") == 0 || strcmp(cgroups_path, ".") == 0) { +- return 0; +- } +- +- if (conf_get_cgroup_cpu_rt(&cpu_rt_period, &cpu_rt_runtime)) { +- return -1; +- } +- +- if (cpu_rt_period == 0 && cpu_rt_runtime == 0) { +- return 0; +- } +- +- sysinfo = get_sys_info(true); +- if (sysinfo == NULL) { +- ERROR("Can not get system info"); +- ret = -1; +- goto out; +- } +- +- if (!(sysinfo->cgcpuinfo.cpu_rt_period)) { +- ERROR("Daemon-scoped cpu-rt-period and cpu-rt-runtime are not supported by kernel"); +- isulad_set_error_message("Daemon-scoped cpu-rt-period and cpu-rt-runtime are not supported by kernel"); +- ret = -1; +- goto out; +- } +- +- mnt_root = get_cpurt_controller_mnt_path(); +- if (mnt_root == NULL) { +- ERROR("Failed to get cpu rt controller mnt root path"); +- isulad_set_error_message("Failed to get cpu rt controller mnt root path"); +- ret = -1; +- goto out; +- } +- +- dup = util_strdup_s(cgroups_path); +- dirpath = dirname(dup); +- +- ret = do_init_cpurt_cgroups_path(dirpath, 0, mnt_root, cpu_rt_period, cpu_rt_runtime); +- +-out: +- free(mnt_root); +- free(dup); +- return ret; +-} +- + /* + * request -> host_spec + container_spec + * container_spec + image config +@@ -1680,13 +1481,6 @@ int container_create_cb(const container_create_request *request, container_creat + goto umount_channel; + } + +- // init cgroup path for cpu_rt_runtime and cpu_rt_period +- if (cpurt_controller_init(oci_spec->linux->cgroups_path) != 0) { +- ERROR("Unable to init CPU RT controller %s", oci_spec->linux->cgroups_path); +- cc = ISULAD_ERR_EXEC; +- goto umount_channel; +- } +- + if (container_v2_spec_merge_contaner_spec(v2_spec) != 0) { + ERROR("Failed to merge container settings"); + cc = ISULAD_ERR_EXEC; +diff --git a/src/daemon/modules/api/specs_api.h b/src/daemon/modules/api/specs_api.h +index 4c132108..e0a73f55 100644 +--- a/src/daemon/modules/api/specs_api.h ++++ b/src/daemon/modules/api/specs_api.h +@@ -28,7 +28,7 @@ extern "C" { + + int merge_all_specs(host_config *host_spec, const char *real_rootfs, container_config_v2_common_config *v2_spec, + oci_runtime_spec *oci_spec); +-int merge_oci_cgroups_path(const char *id, oci_runtime_spec *oci_spec, const host_config *host_spec); ++char *merge_container_cgroups_path(const char *id, const host_config *host_spec); + int merge_global_config(oci_runtime_spec *oci_spec); + oci_runtime_spec *load_oci_config(const char *rootpath, const char *name); + oci_runtime_spec *default_spec(bool system_container); +diff --git a/src/daemon/modules/spec/specs.c b/src/daemon/modules/spec/specs.c +index 12d9b96d..857fc3dc 100644 +--- a/src/daemon/modules/spec/specs.c ++++ b/src/daemon/modules/spec/specs.c +@@ -26,6 +26,7 @@ + #include <isula_libutils/oci_runtime_hooks.h> + #include <isula_libutils/host_config.h> + #include <isula_libutils/log.h> ++#include <isula_libutils/auto_cleanup.h> + #include <limits.h> + #include <stdint.h> + +@@ -165,36 +166,43 @@ out: + return ret; + } + +-static int make_annotations_cgroup_dir(const container_config *container_spec, const host_config *host_spec) ++static char *do_get_container_cgroup_path(const host_config *host_spec) + { +- int ret = 0; +- char cleaned[PATH_MAX] = { 0 }; +- char *default_cgroup_parent = NULL; + char *path = NULL; + +- default_cgroup_parent = conf_get_isulad_cgroup_parent(); + if (host_spec->cgroup_parent != NULL) { +- path = host_spec->cgroup_parent; +- } else if (default_cgroup_parent != NULL) { +- path = default_cgroup_parent; ++ // first, use user setting ++ path = util_strdup_s(host_spec->cgroup_parent); ++ } else { ++ // second, if user donot set, use setting from daemon config ++ path = conf_get_isulad_cgroup_parent(); + } ++ + if (path == NULL) { +- path = "/isulad"; ++ // third, all faild, just use default '/isulad' ++ path = util_strdup_s("/isulad"); + } ++ ++ return path; ++} ++ ++static int make_annotations_cgroup_dir(const container_config *container_spec, const host_config *host_spec) ++{ ++ char cleaned[PATH_MAX] = { 0 }; ++ __isula_auto_free char *path = NULL; ++ ++ path = do_get_container_cgroup_path(host_spec); + if (util_clean_path(path, cleaned, sizeof(cleaned)) == NULL) { + ERROR("Failed to clean path: %s", path); +- ret = -1; +- goto out; ++ return -1; + } ++ + if (append_json_map_string_string(container_spec->annotations, "cgroup.dir", cleaned)) { + ERROR("Realloc annotations failed"); +- ret = -1; +- goto out; ++ return -1; + } + +-out: +- free(default_cgroup_parent); +- return ret; ++ return 0; + } + + static int make_annotations_oom_score_adj(const container_config *container_spec, const host_config *host_spec) +@@ -2058,42 +2066,40 @@ out: + return ret; + } + +-int merge_oci_cgroups_path(const char *id, oci_runtime_spec *oci_spec, const host_config *host_spec) ++char *merge_container_cgroups_path(const char *id, const host_config *host_spec) + { +- int ret = 0; +- char *default_cgroup_parent = NULL; +- char *path = NULL; ++ __isula_auto_free char *path = NULL; + ++ if (id == NULL || host_spec == NULL) { ++ ERROR("Invalid arguments"); ++ return NULL; ++ } ++ ++ path = do_get_container_cgroup_path(host_spec); ++ ++ return util_path_join(path, id); ++} ++ ++static int merge_oci_cgroups_path(const char *id, oci_runtime_spec *oci_spec, const host_config *host_spec) ++{ + if (id == NULL || oci_spec == NULL || host_spec == NULL) { + ERROR("Invalid arguments"); +- ret = -1; +- goto out; ++ return -1; + } + + if (make_sure_oci_spec_linux(oci_spec) != 0) { + ERROR("Failed to make oci spec linux"); +- ret = -1; +- goto out; ++ return -1; + } + +- default_cgroup_parent = conf_get_isulad_cgroup_parent(); +- path = default_cgroup_parent; +- if (host_spec->cgroup_parent != NULL) { +- path = host_spec->cgroup_parent; +- } ++ free(oci_spec->linux->cgroups_path); ++ oci_spec->linux->cgroups_path = merge_container_cgroups_path(id, host_spec); + +- if (path == NULL) { +- free(oci_spec->linux->cgroups_path); +- oci_spec->linux->cgroups_path = util_path_join("/isulad", id); +- return 0; ++ if (oci_spec->linux->cgroups_path == NULL) { ++ WARN("OCI spec cgroups path is NULL"); + } + +- free(oci_spec->linux->cgroups_path); +- oci_spec->linux->cgroups_path = util_path_join(path, id); +- +-out: +- free(default_cgroup_parent); +- return ret; ++ return 0; + } + + int merge_all_specs(host_config *host_spec, const char *real_rootfs, container_config_v2_common_config *v2_spec, +diff --git a/test/specs/specs/specs_ut.cc b/test/specs/specs/specs_ut.cc +index c4014e2e..96aa1c63 100644 +--- a/test/specs/specs/specs_ut.cc ++++ b/test/specs/specs/specs_ut.cc +@@ -232,15 +232,16 @@ char *invoke_conf_get_isulad_cgroup_parent() + return util_strdup_s("/var/lib/isulad/engines/lcr"); + } + +-TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_1) ++TEST_F(SpecsUnitTest, test_merge_container_cgroups_path_1) + { +- ASSERT_EQ(merge_oci_cgroups_path(nullptr, nullptr, nullptr), -1); ++ ASSERT_EQ(merge_container_cgroups_path(nullptr, nullptr), nullptr); + } + +-TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_2) ++TEST_F(SpecsUnitTest, test_merge_container_cgroups_path_2) + { + oci_runtime_spec *oci_spec = nullptr; + host_config *host_spec = nullptr; ++ char *merged_cp = nullptr; + + oci_spec = (oci_runtime_spec *)util_common_calloc_s(sizeof(oci_runtime_spec)); + ASSERT_TRUE(oci_spec != nullptr); +@@ -250,20 +251,23 @@ TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_2) + + EXPECT_CALL(m_isulad_conf, GetCgroupParent()).WillRepeatedly(Invoke(invoke_conf_get_isulad_cgroup_parent_null)); + +- ASSERT_EQ(merge_oci_cgroups_path("123", oci_spec, host_spec), 0); ++ merged_cp = merge_container_cgroups_path("123", host_spec); ++ ASSERT_NE(merged_cp, nullptr); + +- ASSERT_STREQ(oci_spec->linux->cgroups_path, "/isulad/123"); ++ ASSERT_STREQ(merged_cp, "/isulad/123"); + + free_oci_runtime_spec(oci_spec); + free_host_config(host_spec); ++ free(merged_cp); + + testing::Mock::VerifyAndClearExpectations(&m_isulad_conf); + } + +-TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_3) ++TEST_F(SpecsUnitTest, test_merge_container_cgroups_path_3) + { + oci_runtime_spec *oci_spec = nullptr; + host_config *host_spec = nullptr; ++ char *merged_cp = nullptr; + + oci_spec = (oci_runtime_spec *)util_common_calloc_s(sizeof(oci_runtime_spec)); + ASSERT_TRUE(oci_spec != nullptr); +@@ -275,20 +279,23 @@ TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_3) + + EXPECT_CALL(m_isulad_conf, GetCgroupParent()).WillRepeatedly(Invoke(invoke_conf_get_isulad_cgroup_parent_null)); + +- ASSERT_EQ(merge_oci_cgroups_path("123", oci_spec, host_spec), 0); ++ merged_cp = merge_container_cgroups_path("123", host_spec); ++ ASSERT_NE(merged_cp, nullptr); + +- ASSERT_STREQ(oci_spec->linux->cgroups_path, "/test/123"); ++ ASSERT_STREQ(merged_cp, "/test/123"); + + free_oci_runtime_spec(oci_spec); + free_host_config(host_spec); ++ free(merged_cp); + + testing::Mock::VerifyAndClearExpectations(&m_isulad_conf); + } + +-TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_4) ++TEST_F(SpecsUnitTest, test_merge_container_cgroups_path_4) + { + oci_runtime_spec *oci_spec = nullptr; + host_config *host_spec = nullptr; ++ char *merged_cp = nullptr; + + oci_spec = (oci_runtime_spec *)util_common_calloc_s(sizeof(oci_runtime_spec)); + ASSERT_TRUE(oci_spec != nullptr); +@@ -298,20 +305,23 @@ TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_4) + + EXPECT_CALL(m_isulad_conf, GetCgroupParent()).WillRepeatedly(Invoke(invoke_conf_get_isulad_cgroup_parent)); + +- ASSERT_EQ(merge_oci_cgroups_path("123", oci_spec, host_spec), 0); ++ merged_cp = merge_container_cgroups_path("123", host_spec); ++ ASSERT_NE(merged_cp, nullptr); + +- ASSERT_STREQ(oci_spec->linux->cgroups_path, "/var/lib/isulad/engines/lcr/123"); ++ ASSERT_STREQ(merged_cp, "/var/lib/isulad/engines/lcr/123"); + + free_oci_runtime_spec(oci_spec); + free_host_config(host_spec); ++ free(merged_cp); + + testing::Mock::VerifyAndClearExpectations(&m_isulad_conf); + } + +-TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_5) ++TEST_F(SpecsUnitTest, test_merge_container_cgroups_path_5) + { + oci_runtime_spec *oci_spec = nullptr; + host_config *host_spec = nullptr; ++ char *merged_cp = nullptr; + + oci_spec = (oci_runtime_spec *)util_common_calloc_s(sizeof(oci_runtime_spec)); + ASSERT_TRUE(oci_spec != nullptr); +@@ -323,12 +333,14 @@ TEST_F(SpecsUnitTest, test_merge_oci_cgroups_path_5) + + EXPECT_CALL(m_isulad_conf, GetCgroupParent()).WillRepeatedly(Invoke(invoke_conf_get_isulad_cgroup_parent)); + +- ASSERT_EQ(merge_oci_cgroups_path("123", oci_spec, host_spec), 0); ++ merged_cp = merge_container_cgroups_path("123", host_spec); ++ ASSERT_NE(merged_cp, nullptr); + +- ASSERT_STREQ(oci_spec->linux->cgroups_path, "/test/123"); ++ ASSERT_STREQ(merged_cp, "/test/123"); + + free_oci_runtime_spec(oci_spec); + free_host_config(host_spec); ++ free(merged_cp); + + testing::Mock::VerifyAndClearExpectations(&m_isulad_conf); + } +-- +2.25.1 + |