From 4838d6eb809319e60c990fcbc4ae6b7bddeb2801 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 10:45:14 +0200 Subject: [PATCH 01/11] conmon: Rename all commandline option variables opt_* This makes it easier to figure out where they come from Signed-off-by: Alexander Larsson --- conmon/conmon.c | 122 ++++++++++++++++++++++++------------------------ 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index 4106f93f..4547cdce 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -94,32 +94,32 @@ static inline void strv_cleanup(char ***strv) #define CMD_SIZE 1024 #define MAX_EVENTS 10 -static bool terminal = false; +static bool opt_terminal = false; static bool opt_stdin = false; -static char *cid = NULL; -static char *cuuid = NULL; -static char *runtime_path = NULL; -static char *bundle_path = NULL; -static char *pid_file = NULL; -static bool systemd_cgroup = false; -static char *exec_process_spec = NULL; -static bool exec = false; -static char *log_path = NULL; -static int timeout = 0; -static GOptionEntry entries[] = +static char *opt_cid = NULL; +static char *opt_cuuid = NULL; +static char *opt_runtime_path = NULL; +static char *opt_bundle_path = NULL; +static char *opt_pid_file = NULL; +static bool opt_systemd_cgroup = false; +static char *opt_exec_process_spec = NULL; +static bool opt_exec = false; +static char *opt_log_path = NULL; +static int opt_timeout = 0; +static GOptionEntry opt_entries[] = { - { "terminal", 't', 0, G_OPTION_ARG_NONE, &terminal, "Terminal", NULL }, + { "terminal", 't', 0, G_OPTION_ARG_NONE, &opt_terminal, "Terminal", NULL }, { "stdin", 'i', 0, G_OPTION_ARG_NONE, &opt_stdin, "Stdin", NULL }, - { "cid", 'c', 0, G_OPTION_ARG_STRING, &cid, "Container ID", NULL }, - { "cuuid", 'u', 0, G_OPTION_ARG_STRING, &cuuid, "Container UUID", NULL }, - { "runtime", 'r', 0, G_OPTION_ARG_STRING, &runtime_path, "Runtime path", NULL }, - { "bundle", 'b', 0, G_OPTION_ARG_STRING, &bundle_path, "Bundle path", NULL }, - { "pidfile", 'p', 0, G_OPTION_ARG_STRING, &pid_file, "PID file", NULL }, - { "systemd-cgroup", 's', 0, G_OPTION_ARG_NONE, &systemd_cgroup, "Enable systemd cgroup manager", NULL }, - { "exec", 'e', 0, G_OPTION_ARG_NONE, &exec, "Exec a command in a running container", NULL }, - { "exec-process-spec", 0, 0, G_OPTION_ARG_STRING, &exec_process_spec, "Path to the process spec for exec", NULL }, - { "log-path", 'l', 0, G_OPTION_ARG_STRING, &log_path, "Log file path", NULL }, - { "timeout", 'T', 0, G_OPTION_ARG_INT, &timeout, "Timeout in seconds", NULL }, + { "cid", 'c', 0, G_OPTION_ARG_STRING, &opt_cid, "Container ID", NULL }, + { "cuuid", 'u', 0, G_OPTION_ARG_STRING, &opt_cuuid, "Container UUID", NULL }, + { "runtime", 'r', 0, G_OPTION_ARG_STRING, &opt_runtime_path, "Runtime path", NULL }, + { "bundle", 'b', 0, G_OPTION_ARG_STRING, &opt_bundle_path, "Bundle path", NULL }, + { "pidfile", 'p', 0, G_OPTION_ARG_STRING, &opt_pid_file, "PID file", NULL }, + { "systemd-cgroup", 's', 0, G_OPTION_ARG_NONE, &opt_systemd_cgroup, "Enable systemd cgroup manager", NULL }, + { "exec", 'e', 0, G_OPTION_ARG_NONE, &opt_exec, "Exec a command in a running container", NULL }, + { "exec-process-spec", 0, 0, G_OPTION_ARG_STRING, &opt_exec_process_spec, "Path to the process spec for exec", NULL }, + { "log-path", 'l', 0, G_OPTION_ARG_STRING, &opt_log_path, "Log file path", NULL }, + { "timeout", 'T', 0, G_OPTION_ARG_INT, &opt_timeout, "Timeout in seconds", NULL }, { NULL } }; @@ -742,7 +742,7 @@ static void write_sync_fd(int sync_pipe_fd, int res, const char *message) if (sync_pipe_fd == -1) return; - if (exec) + if (opt_exec) res_key = "exit_code"; else res_key = "pid"; @@ -799,26 +799,26 @@ int main(int argc, char *argv[]) /* Command line parameters */ context = g_option_context_new("- conmon utility"); - g_option_context_add_main_entries(context, entries, "conmon"); + g_option_context_add_main_entries(context, opt_entries, "conmon"); if (!g_option_context_parse(context, &argc, &argv, &error)) { g_print("option parsing failed: %s\n", error->message); exit(1); } - if (cid == NULL) + if (opt_cid == NULL) nexit("Container ID not provided. Use --cid"); - if (!exec && cuuid == NULL) + if (!opt_exec && opt_cuuid == NULL) nexit("Container UUID not provided. Use --cuuid"); - if (runtime_path == NULL) + if (opt_runtime_path == NULL) nexit("Runtime path not provided. Use --runtime"); - if (bundle_path == NULL && !exec) { + if (opt_bundle_path == NULL && !opt_exec) { if (getcwd(cwd, sizeof(cwd)) == NULL) { nexit("Failed to get working directory"); } - bundle_path = cwd; + opt_bundle_path = cwd; } dev_null_r = open("/dev/null", O_RDONLY | O_CLOEXEC); @@ -829,19 +829,19 @@ int main(int argc, char *argv[]) if (dev_null_w < 0) pexit("Failed to open /dev/null"); - if (exec && exec_process_spec == NULL) { + if (opt_exec && opt_exec_process_spec == NULL) { nexit("Exec process spec path not provided. Use --exec-process-spec"); } - if (pid_file == NULL) { + if (opt_pid_file == NULL) { if (snprintf(default_pid_file, sizeof(default_pid_file), - "%s/pidfile-%s", cwd, cid) < 0) { + "%s/pidfile-%s", cwd, opt_cid) < 0) { nexit("Failed to generate the pidfile path"); } - pid_file = default_pid_file; + opt_pid_file = default_pid_file; } - if (log_path == NULL) + if (opt_log_path == NULL) nexit("Log file path not provided. Use --log-path"); start_pipe = getenv("_OCI_STARTPIPE"); @@ -892,7 +892,7 @@ int main(int argc, char *argv[]) } /* Open the log path file. */ - logfd = open(log_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0600); + logfd = open(opt_log_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0600); if (logfd < 0) pexit("Failed to open log file"); @@ -905,7 +905,7 @@ int main(int argc, char *argv[]) pexit("Failed to set as subreaper"); } - if (terminal) { + if (opt_terminal) { struct sockaddr_un addr = {0}; /* @@ -972,38 +972,38 @@ int main(int argc, char *argv[]) slavefd_stderr = fds[1]; runtime_argv = g_ptr_array_new(); - g_ptr_array_add(runtime_argv, runtime_path); + g_ptr_array_add(runtime_argv, opt_runtime_path); /* Generate the cmdline. */ - if (!exec && systemd_cgroup) + if (!opt_exec && opt_systemd_cgroup) g_ptr_array_add(runtime_argv, "--systemd-cgroup"); - if (exec) { + if (opt_exec) { g_ptr_array_add (runtime_argv, "exec"); g_ptr_array_add (runtime_argv, "-d"); g_ptr_array_add (runtime_argv, "--pid-file"); - g_ptr_array_add (runtime_argv, pid_file); + g_ptr_array_add (runtime_argv, opt_pid_file); } else { g_ptr_array_add (runtime_argv, "create"); g_ptr_array_add (runtime_argv, "--bundle"); - g_ptr_array_add (runtime_argv, bundle_path); + g_ptr_array_add (runtime_argv, opt_bundle_path); g_ptr_array_add (runtime_argv, "--pid-file"); - g_ptr_array_add (runtime_argv, pid_file); + g_ptr_array_add (runtime_argv, opt_pid_file); } - if (terminal) { + if (opt_terminal) { g_ptr_array_add(runtime_argv, "--console-socket"); g_ptr_array_add(runtime_argv, csname); } /* Set the exec arguments. */ - if (exec) { + if (opt_exec) { g_ptr_array_add(runtime_argv, "--process"); - g_ptr_array_add(runtime_argv, exec_process_spec); + g_ptr_array_add(runtime_argv, opt_exec_process_spec); } /* Container name comes last. */ - g_ptr_array_add(runtime_argv, cid); + g_ptr_array_add(runtime_argv, opt_cid); g_ptr_array_add(runtime_argv, NULL); /* @@ -1047,7 +1047,7 @@ int main(int argc, char *argv[]) close(slavefd_stderr); ninfo("about to waitpid: %d", create_pid); - if (terminal) { + if (opt_terminal) { guint terminal_watch = g_unix_fd_add (csfd, G_IO_IN, terminal_accept_cb, csname); g_child_watch_add (create_pid, runtime_exit_cb, NULL); g_main_loop_run (main_loop); @@ -1058,7 +1058,7 @@ int main(int argc, char *argv[]) int old_errno = errno; kill(create_pid, SIGKILL); errno = old_errno; - pexit("Failed to wait for `runtime %s`", exec ? "exec" : "create"); + pexit("Failed to wait for `runtime %s`", opt_exec ? "exec" : "create"); } } @@ -1077,11 +1077,11 @@ int main(int argc, char *argv[]) nexit("Failed to create container: exit status %d", WEXITSTATUS(runtime_status)); } - if (terminal && masterfd_stdout == -1) + if (opt_terminal && masterfd_stdout == -1) nexit("Runtime did not set up terminal"); /* Read the pid so we can wait for the process to exit */ - g_file_get_contents(pid_file, &contents, NULL, &err); + g_file_get_contents(opt_pid_file, &contents, NULL, &err); if (err) { nwarn("Failed to read pidfile: %s", err->message); g_error_free(err); @@ -1095,21 +1095,21 @@ int main(int argc, char *argv[]) char attach_symlink_dir_path[PATH_MAX] = { 0 }; struct sockaddr_un attach_addr = {0}; - if (!exec) { + if (!opt_exec) { attach_addr.sun_family = AF_UNIX; /* * Create a symlink so we don't exceed unix domain socket * path length limit. */ - snprintf(attach_symlink_dir_path, PATH_MAX, "/var/run/crio/%s", cuuid); + snprintf(attach_symlink_dir_path, PATH_MAX, "/var/run/crio/%s", opt_cuuid); if (unlink(attach_symlink_dir_path) == -1 && errno != ENOENT) { pexit("Failed to remove existing symlink for attach socket directory"); } - if (symlink(bundle_path, attach_symlink_dir_path) == -1) + if (symlink(opt_bundle_path, attach_symlink_dir_path) == -1) pexit("Failed to create symlink for attach socket"); - snprintf(attach_sock_path, PATH_MAX, "/var/run/crio/%s/attach", cuuid); + snprintf(attach_sock_path, PATH_MAX, "/var/run/crio/%s/attach", opt_cuuid); ninfo("attach sock path: %s", attach_sock_path); strncpy(attach_addr.sun_path, attach_sock_path, sizeof(attach_addr.sun_path) - 1); @@ -1137,8 +1137,8 @@ int main(int argc, char *argv[]) /* Setup fifo for reading in terminal resize and other stdio control messages */ _cleanup_close_ int ctlfd = -1; _cleanup_close_ int dummyfd = -1; - if (!exec) { - snprintf(ctl_fifo_path, PATH_MAX, "%s/ctl", bundle_path); + if (!opt_exec) { + snprintf(ctl_fifo_path, PATH_MAX, "%s/ctl", opt_bundle_path); ninfo("ctl fifo path: %s", ctl_fifo_path); if (mkfifo(ctl_fifo_path, 0666) == -1) @@ -1160,7 +1160,7 @@ int main(int argc, char *argv[]) } /* Send the container pid back to parent */ - if (!exec) { + if (!opt_exec) { write_sync_fd(sync_pipe_fd, cpid, NULL); } @@ -1215,8 +1215,8 @@ int main(int argc, char *argv[]) g_unix_fd_add (ctlfd, G_IO_IN, ctrl_cb, NULL); } - if (timeout > 0) { - g_timeout_add_seconds (timeout, timeout_cb, NULL); + if (opt_timeout > 0) { + g_timeout_add_seconds (opt_timeout, timeout_cb, NULL); } g_main_loop_run (main_loop); @@ -1242,7 +1242,7 @@ int main(int argc, char *argv[]) } } - if (!exec) { + if (!opt_exec) { _cleanup_free_ char *status_str = NULL; ret = asprintf(&status_str, "%d", exit_status); if (ret < 0) { From 6aa1075ab6df4e3b92b88d919b1f3594cdf933cb Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 10:57:27 +0200 Subject: [PATCH 02/11] conmon: Add (and use) get_pipe_fd_from_env helper This avoids duplicating this code in two places. Signed-off-by: Alexander Larsson --- conmon/conmon.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index 4547cdce..b7cb82be 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -440,6 +440,24 @@ static char *escape_json_string(const char *str) return g_string_free (escaped, FALSE); } +static int get_pipe_fd_from_env(const char *envname) +{ + char *pipe_str, *endptr; + int pipe_fd; + + pipe_str = getenv(envname); + if (pipe_str == NULL) + return -1; + + errno = 0; + pipe_fd = strtol(pipe_str, &endptr, 10); + if (errno != 0 || *endptr != '\0') + pexit("unable to parse %s", envname); + if (fcntl(pipe_fd, F_SETFD, FD_CLOEXEC) == -1) + pexit("unable to make %s CLOEXEC", envname); + + return pipe_fd; +} /* Global state */ @@ -783,7 +801,6 @@ int main(int argc, char *argv[]) int num_read; int sync_pipe_fd = -1; int start_pipe_fd = -1; - char *start_pipe, *sync_pipe, *endptr; int len; GError *error = NULL; GOptionContext *context; @@ -844,12 +861,8 @@ int main(int argc, char *argv[]) if (opt_log_path == NULL) nexit("Log file path not provided. Use --log-path"); - start_pipe = getenv("_OCI_STARTPIPE"); - if (start_pipe) { - errno = 0; - start_pipe_fd = strtol(start_pipe, &endptr, 10); - if (errno != 0 || *endptr != '\0') - pexit("unable to parse _OCI_STARTPIPE"); + start_pipe_fd = get_pipe_fd_from_env("_OCI_STARTPIPE"); + if (start_pipe_fd >= 0) { /* Block for an initial write to the start pipe before spawning any childred or exiting, to ensure the parent can put us in the right cgroup. */ @@ -881,15 +894,7 @@ int main(int argc, char *argv[]) setsid(); /* Environment variables */ - sync_pipe = getenv("_OCI_SYNCPIPE"); - if (sync_pipe) { - errno = 0; - sync_pipe_fd = strtol(sync_pipe, &endptr, 10); - if (errno != 0 || *endptr != '\0') - pexit("unable to parse _OCI_SYNCPIPE"); - if (fcntl(sync_pipe_fd, F_SETFD, FD_CLOEXEC) == -1) - pexit("unable to make _OCI_SYNCPIPE CLOEXEC"); - } + sync_pipe_fd = get_pipe_fd_from_env("_OCI_SYNCPIPE"); /* Open the log path file. */ logfd = open(opt_log_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0600); From 215ef485df551c2ef1065269f648c08b9197c0c9 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 11:30:19 +0200 Subject: [PATCH 03/11] conmon: Add add_argv() helper This makes adding the arguments to runtime_argv somewhat nicer. Signed-off-by: Alexander Larsson --- conmon/conmon.c | 58 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index b7cb82be..dd3b9ec9 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -459,6 +459,24 @@ static int get_pipe_fd_from_env(const char *envname) return pipe_fd; } +static void add_argv(GPtrArray *argv_array, ...) G_GNUC_NULL_TERMINATED; + +static void add_argv(GPtrArray *argv_array, ...) +{ + va_list args; + char *arg; + + va_start (args, argv_array); + while ((arg = va_arg (args, char *))) + g_ptr_array_add (argv_array, arg); + va_end (args); +} + +static void end_argv(GPtrArray *argv_array) +{ + g_ptr_array_add(argv_array, NULL); +} + /* Global state */ static int runtime_status = -1; @@ -977,39 +995,45 @@ int main(int argc, char *argv[]) slavefd_stderr = fds[1]; runtime_argv = g_ptr_array_new(); - g_ptr_array_add(runtime_argv, opt_runtime_path); + add_argv(runtime_argv, + opt_runtime_path, + NULL); /* Generate the cmdline. */ if (!opt_exec && opt_systemd_cgroup) - g_ptr_array_add(runtime_argv, "--systemd-cgroup"); + add_argv(runtime_argv, + "--systemd-cgroup", + NULL); if (opt_exec) { - g_ptr_array_add (runtime_argv, "exec"); - g_ptr_array_add (runtime_argv, "-d"); - g_ptr_array_add (runtime_argv, "--pid-file"); - g_ptr_array_add (runtime_argv, opt_pid_file); + add_argv(runtime_argv, + "exec", "-d", + "--pid-file", opt_pid_file, + NULL); } else { - g_ptr_array_add (runtime_argv, "create"); - g_ptr_array_add (runtime_argv, "--bundle"); - g_ptr_array_add (runtime_argv, opt_bundle_path); - g_ptr_array_add (runtime_argv, "--pid-file"); - g_ptr_array_add (runtime_argv, opt_pid_file); + add_argv(runtime_argv, + "create", + "--bundle", opt_bundle_path, + "--pid-file", opt_pid_file, + NULL); } if (opt_terminal) { - g_ptr_array_add(runtime_argv, "--console-socket"); - g_ptr_array_add(runtime_argv, csname); + add_argv(runtime_argv, + "--console-socket", csname, + NULL); } /* Set the exec arguments. */ if (opt_exec) { - g_ptr_array_add(runtime_argv, "--process"); - g_ptr_array_add(runtime_argv, opt_exec_process_spec); + add_argv(runtime_argv, + "--process", opt_exec_process_spec, + NULL); } /* Container name comes last. */ - g_ptr_array_add(runtime_argv, opt_cid); - g_ptr_array_add(runtime_argv, NULL); + add_argv(runtime_argv, opt_cid, NULL); + end_argv(runtime_argv); /* * We have to fork here because the current runC API dups the stdio of the From a7c61e4f9fa94dc01d68c1dc349fa898385c7d3a Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 11:34:50 +0200 Subject: [PATCH 04/11] conmon: Remove unused variables Signed-off-by: Alexander Larsson --- conmon/conmon.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index dd3b9ec9..ecffd79f 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -808,7 +808,6 @@ int main(int argc, char *argv[]) int cpid = -1; int status; pid_t pid, main_pid, create_pid; - _cleanup_close_ int epfd = -1; _cleanup_close_ int csfd = -1; /* Used for !terminal cases. */ int slavefd_stdin = -1; @@ -819,7 +818,6 @@ int main(int argc, char *argv[]) int num_read; int sync_pipe_fd = -1; int start_pipe_fd = -1; - int len; GError *error = NULL; GOptionContext *context; GPtrArray *runtime_argv = NULL; From b26996921639dc7f7ac59eae51e858a9fd4e4249 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 11:57:47 +0200 Subject: [PATCH 05/11] conmon: Don't use fixed size string buffers We build paths using g_build_filename and g_strdup_printf() instead which means we don't have any arbitrary pathname lenght issue, and the code becomes cleaner. We also convert asprintf to g_strdup_printf so that we can use the glib OOM checker instead of open coding it everywhere. Signed-off-by: Alexander Larsson --- conmon/conmon.c | 67 +++++++++++++++---------------------------------- 1 file changed, 20 insertions(+), 47 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index ecffd79f..2de84cc7 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -352,14 +352,7 @@ next: * Returns NULL on error. */ static char *process_cgroup_subsystem_path(int pid, const char *subsystem) { - _cleanup_free_ char *cgroups_file_path = NULL; - int rc; - rc = asprintf(&cgroups_file_path, "/proc/%d/cgroup", pid); - if (rc < 0) { - nwarn("Failed to allocate memory for cgroups file path"); - return NULL; - } - + _cleanup_free_ char *cgroups_file_path = g_strdup_printf("/proc/%d/cgroup", pid); _cleanup_fclose_ FILE *fp = NULL; fp = fopen(cgroups_file_path, "re"); if (fp == NULL) { @@ -398,12 +391,7 @@ static char *process_cgroup_subsystem_path(int pid, const char *subsystem) { *subpath = 0; } - rc = asprintf(&subsystem_path, "%s/%s%s", CGROUP_ROOT, subpath, path); - if (rc < 0) { - nwarn("Failed to allocate memory for subsystemd path"); - return NULL; - } - + subsystem_path = g_strdup_printf("%s/%s%s", CGROUP_ROOT, subpath, path); subsystem_path[strlen(subsystem_path) - 1] = '\0'; return subsystem_path; } @@ -800,11 +788,11 @@ int main(int argc, char *argv[]) { int ret; char cwd[PATH_MAX]; - char default_pid_file[PATH_MAX]; - char attach_sock_path[PATH_MAX]; - char ctl_fifo_path[PATH_MAX]; + _cleanup_free_ char *default_pid_file = NULL; + _cleanup_free_ char *attach_sock_path = NULL; + _cleanup_free_ char *ctl_fifo_path = NULL; GError *err = NULL; - _cleanup_free_ char *contents; + _cleanup_free_ char *contents = NULL; int cpid = -1; int status; pid_t pid, main_pid, create_pid; @@ -867,10 +855,7 @@ int main(int argc, char *argv[]) } if (opt_pid_file == NULL) { - if (snprintf(default_pid_file, sizeof(default_pid_file), - "%s/pidfile-%s", cwd, opt_cid) < 0) { - nexit("Failed to generate the pidfile path"); - } + default_pid_file = g_strdup_printf ("%s/pidfile-%s", cwd, opt_cid); opt_pid_file = default_pid_file; } @@ -1119,7 +1104,7 @@ int main(int argc, char *argv[]) ninfo("container PID: %d", cpid); /* Setup endpoint for attach */ - char attach_symlink_dir_path[PATH_MAX] = { 0 }; + _cleanup_free_ char *attach_symlink_dir_path = NULL; struct sockaddr_un attach_addr = {0}; if (!opt_exec) { @@ -1129,14 +1114,14 @@ int main(int argc, char *argv[]) * Create a symlink so we don't exceed unix domain socket * path length limit. */ - snprintf(attach_symlink_dir_path, PATH_MAX, "/var/run/crio/%s", opt_cuuid); + attach_symlink_dir_path = g_build_filename("/var/run/crio", opt_cuuid, NULL); if (unlink(attach_symlink_dir_path) == -1 && errno != ENOENT) { pexit("Failed to remove existing symlink for attach socket directory"); } if (symlink(opt_bundle_path, attach_symlink_dir_path) == -1) pexit("Failed to create symlink for attach socket"); - snprintf(attach_sock_path, PATH_MAX, "/var/run/crio/%s/attach", opt_cuuid); + attach_sock_path = g_build_filename("/var/run/crio", opt_cuuid, "attach", NULL); ninfo("attach sock path: %s", attach_sock_path); strncpy(attach_addr.sun_path, attach_sock_path, sizeof(attach_addr.sun_path) - 1); @@ -1165,7 +1150,7 @@ int main(int argc, char *argv[]) _cleanup_close_ int ctlfd = -1; _cleanup_close_ int dummyfd = -1; if (!opt_exec) { - snprintf(ctl_fifo_path, PATH_MAX, "%s/ctl", opt_bundle_path); + ctl_fifo_path = g_build_filename(opt_bundle_path, "ctl", NULL); ninfo("ctl fifo path: %s", ctl_fifo_path); if (mkfifo(ctl_fifo_path, 0666) == -1) @@ -1198,17 +1183,16 @@ int main(int argc, char *argv[]) } bool oom_handling_enabled = true; - char memory_cgroup_file_path[PATH_MAX]; - snprintf(memory_cgroup_file_path, PATH_MAX, "%s/cgroup.event_control", memory_cgroup_path); + _cleanup_free_ char *memory_cgroup_file_path = g_build_filename(memory_cgroup_path, "cgroup.event_control", NULL); if ((cfd = open(memory_cgroup_file_path, O_WRONLY | O_CLOEXEC)) == -1) { nwarn("Failed to open %s", memory_cgroup_file_path); oom_handling_enabled = false; } if (oom_handling_enabled) { - snprintf(memory_cgroup_file_path, PATH_MAX, "%s/memory.oom_control", memory_cgroup_path); - if ((ofd = open(memory_cgroup_file_path, O_RDONLY | O_CLOEXEC)) == -1) - pexit("Failed to open %s", memory_cgroup_file_path); + _cleanup_free_ char *memory_cgroup_file_oom_path = g_build_filename(memory_cgroup_path, "memory.oom_control", NULL); + if ((ofd = open(memory_cgroup_file_oom_path, O_RDONLY | O_CLOEXEC)) == -1) + pexit("Failed to open %s", memory_cgroup_file_oom_path); if ((oom_efd = eventfd(0, EFD_CLOEXEC)) == -1) pexit("Failed to create eventfd"); @@ -1270,27 +1254,16 @@ int main(int argc, char *argv[]) } if (!opt_exec) { - _cleanup_free_ char *status_str = NULL; - ret = asprintf(&status_str, "%d", exit_status); - if (ret < 0) { - pexit("Failed to allocate memory for status"); - } - g_file_set_contents("exit", status_str, - strlen(status_str), &err); - if (err) { - fprintf(stderr, - "Failed to write %s to exit file: %s\n", - status_str, err->message); - g_error_free(err); - exit(1); - } - + _cleanup_free_ char *status_str = g_strdup_printf("%d", exit_status); + if (!g_file_set_contents("exit", status_str, -1, &err)) + nexit("Failed to write %s to exit file: %s\n", + status_str, err->message); } else { /* Send the command exec exit code back to the parent */ write_sync_fd(sync_pipe_fd, exit_status, exit_message); } - if (attach_symlink_dir_path[0] != 0 && + if (attach_symlink_dir_path != NULL && unlink(attach_symlink_dir_path) == -1 && errno != ENOENT) { pexit("Failed to remove symlink for attach socket directory"); } From cc3a1615fb93a3b77dffb9994fa802cfae2de531 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 12:13:16 +0200 Subject: [PATCH 06/11] conmon: Break out connection socket setup to a separate function Signed-off-by: Alexander Larsson --- conmon/conmon.c | 75 +++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index 2de84cc7..dafa6a7d 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -485,6 +485,7 @@ static int afd = -1; static int cfd = -1; /* Used for OOM notification API */ static int ofd = -1; +static int csfd = -1; static bool timed_out = FALSE; @@ -784,6 +785,42 @@ static void write_sync_fd(int sync_pipe_fd, int res, const char *message) } } +static char *setup_console_socket(void) +{ + struct sockaddr_un addr = {0}; + char csname[PATH_MAX] = "/tmp/conmon-term.XXXXXXXX"; + /* + * Generate a temporary name. Is this unsafe? Probably, but we can + * replace it with a rename(2) setup if necessary. + */ + + int unusedfd = g_mkstemp(csname); + if (unusedfd < 0) + pexit("Failed to generate random path for console-socket"); + close(unusedfd); + + addr.sun_family = AF_UNIX; + strncpy(addr.sun_path, csname, sizeof(addr.sun_path)-1); + + ninfo("addr{sun_family=AF_UNIX, sun_path=%s}", addr.sun_path); + + /* Bind to the console socket path. */ + csfd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); + if (csfd < 0) + pexit("Failed to create console-socket"); + if (fchmod(csfd, 0700)) + pexit("Failed to change console-socket permissions"); + /* XXX: This should be handled with a rename(2). */ + if (unlink(csname) < 0) + pexit("Failed to unlink temporary ranom path"); + if (bind(csfd, (struct sockaddr *) &addr, sizeof(addr)) < 0) + pexit("Failed to bind to console-socket"); + if (listen(csfd, 128) < 0) + pexit("Failed to listen on console-socket"); + + return g_strdup(csname); +} + int main(int argc, char *argv[]) { int ret; @@ -791,17 +828,16 @@ int main(int argc, char *argv[]) _cleanup_free_ char *default_pid_file = NULL; _cleanup_free_ char *attach_sock_path = NULL; _cleanup_free_ char *ctl_fifo_path = NULL; + _cleanup_free_ char *csname = NULL; GError *err = NULL; _cleanup_free_ char *contents = NULL; int cpid = -1; int status; pid_t pid, main_pid, create_pid; - _cleanup_close_ int csfd = -1; /* Used for !terminal cases. */ int slavefd_stdin = -1; int slavefd_stdout = -1; int slavefd_stderr = -1; - char csname[PATH_MAX] = "/tmp/conmon-term.XXXXXXXX"; char buf[BUF_SIZE]; int num_read; int sync_pipe_fd = -1; @@ -912,36 +948,7 @@ int main(int argc, char *argv[]) } if (opt_terminal) { - struct sockaddr_un addr = {0}; - - /* - * Generate a temporary name. Is this unsafe? Probably, but we can - * replace it with a rename(2) setup if necessary. - */ - - int unusedfd = g_mkstemp(csname); - if (unusedfd < 0) - pexit("Failed to generate random path for console-socket"); - close(unusedfd); - - addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, csname, sizeof(addr.sun_path)-1); - - ninfo("addr{sun_family=AF_UNIX, sun_path=%s}", addr.sun_path); - - /* Bind to the console socket path. */ - csfd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); - if (csfd < 0) - pexit("Failed to create console-socket"); - if (fchmod(csfd, 0700)) - pexit("Failed to change console-socket permissions"); - /* XXX: This should be handled with a rename(2). */ - if (unlink(csname) < 0) - pexit("Failed to unlink temporary ranom path"); - if (bind(csfd, (struct sockaddr *) &addr, sizeof(addr)) < 0) - pexit("Failed to bind to console-socket"); - if (listen(csfd, 128) < 0) - pexit("Failed to listen on console-socket"); + csname = setup_console_socket(); } else { /* @@ -1001,7 +1008,7 @@ int main(int argc, char *argv[]) NULL); } - if (opt_terminal) { + if (csname != NULL) { add_argv(runtime_argv, "--console-socket", csname, NULL); @@ -1059,7 +1066,7 @@ int main(int argc, char *argv[]) close(slavefd_stderr); ninfo("about to waitpid: %d", create_pid); - if (opt_terminal) { + if (csname != NULL) { guint terminal_watch = g_unix_fd_add (csfd, G_IO_IN, terminal_accept_cb, csname); g_child_watch_add (create_pid, runtime_exit_cb, NULL); g_main_loop_run (main_loop); From 640ebeafb3eb81875d14a5f63a8b710ea92addae Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 12:18:09 +0200 Subject: [PATCH 07/11] conmon: Break out attach socket setup to helper function Signed-off-by: Alexander Larsson --- conmon/conmon.c | 85 ++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index dafa6a7d..e67ef8e2 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -821,12 +821,56 @@ static char *setup_console_socket(void) return g_strdup(csname); } +static char *setup_attach_socket(void) +{ + _cleanup_free_ char *attach_sock_path = NULL; + char *attach_symlink_dir_path; + struct sockaddr_un attach_addr = {0}; + attach_addr.sun_family = AF_UNIX; + + /* + * Create a symlink so we don't exceed unix domain socket + * path length limit. + */ + attach_symlink_dir_path = g_build_filename("/var/run/crio", opt_cuuid, NULL); + if (unlink(attach_symlink_dir_path) == -1 && errno != ENOENT) + pexit("Failed to remove existing symlink for attach socket directory"); + + if (symlink(opt_bundle_path, attach_symlink_dir_path) == -1) + pexit("Failed to create symlink for attach socket"); + + attach_sock_path = g_build_filename("/var/run/crio", opt_cuuid, "attach", NULL); + ninfo("attach sock path: %s", attach_sock_path); + + strncpy(attach_addr.sun_path, attach_sock_path, sizeof(attach_addr.sun_path) - 1); + ninfo("addr{sun_family=AF_UNIX, sun_path=%s}", attach_addr.sun_path); + + /* + * We make the socket non-blocking to avoid a race where client aborts connection + * before the server gets a chance to call accept. In that scenario, the server + * accept blocks till a new client connection comes in. + */ + afd = socket(AF_UNIX, SOCK_SEQPACKET|SOCK_NONBLOCK|SOCK_CLOEXEC, 0); + if (afd == -1) + pexit("Failed to create attach socket"); + + if (fchmod(afd, 0700)) + pexit("Failed to change attach socket permissions"); + + if (bind(afd, (struct sockaddr *)&attach_addr, sizeof(struct sockaddr_un)) == -1) + pexit("Failed to bind attach socket: %s", attach_sock_path); + + if (listen(afd, 10) == -1) + pexit("Failed to listen on attach socket: %s", attach_sock_path); + + return attach_symlink_dir_path; +} + int main(int argc, char *argv[]) { int ret; char cwd[PATH_MAX]; _cleanup_free_ char *default_pid_file = NULL; - _cleanup_free_ char *attach_sock_path = NULL; _cleanup_free_ char *ctl_fifo_path = NULL; _cleanup_free_ char *csname = NULL; GError *err = NULL; @@ -1112,45 +1156,8 @@ int main(int argc, char *argv[]) /* Setup endpoint for attach */ _cleanup_free_ char *attach_symlink_dir_path = NULL; - struct sockaddr_un attach_addr = {0}; - if (!opt_exec) { - attach_addr.sun_family = AF_UNIX; - - /* - * Create a symlink so we don't exceed unix domain socket - * path length limit. - */ - attach_symlink_dir_path = g_build_filename("/var/run/crio", opt_cuuid, NULL); - if (unlink(attach_symlink_dir_path) == -1 && errno != ENOENT) { - pexit("Failed to remove existing symlink for attach socket directory"); - } - if (symlink(opt_bundle_path, attach_symlink_dir_path) == -1) - pexit("Failed to create symlink for attach socket"); - - attach_sock_path = g_build_filename("/var/run/crio", opt_cuuid, "attach", NULL); - ninfo("attach sock path: %s", attach_sock_path); - - strncpy(attach_addr.sun_path, attach_sock_path, sizeof(attach_addr.sun_path) - 1); - ninfo("addr{sun_family=AF_UNIX, sun_path=%s}", attach_addr.sun_path); - - /* - * We make the socket non-blocking to avoid a race where client aborts connection - * before the server gets a chance to call accept. In that scenario, the server - * accept blocks till a new client connection comes in. - */ - afd = socket(AF_UNIX, SOCK_SEQPACKET|SOCK_NONBLOCK|SOCK_CLOEXEC, 0); - if (afd == -1) - pexit("Failed to create attach socket"); - - if (fchmod(afd, 0700)) - pexit("Failed to change attach socket permissions"); - - if (bind(afd, (struct sockaddr *)&attach_addr, sizeof(struct sockaddr_un)) == -1) - pexit("Failed to bind attach socket: %s", attach_sock_path); - - if (listen(afd, 10) == -1) - pexit("Failed to listen on attach socket: %s", attach_sock_path); + attach_symlink_dir_path = setup_attach_socket(); } /* Setup fifo for reading in terminal resize and other stdio control messages */ From 34b75c20c23dffbfcff1c5742c3f9b8b31bb23ec Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 12:39:29 +0200 Subject: [PATCH 08/11] conmon: Move terminal control fifo setup to a helper function Signed-off-by: Alexander Larsson --- conmon/conmon.c | 50 ++++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index e67ef8e2..4b321852 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -486,6 +486,7 @@ static int cfd = -1; /* Used for OOM notification API */ static int ofd = -1; static int csfd = -1; +static int ctlfd = -1; static bool timed_out = FALSE; @@ -866,12 +867,36 @@ static char *setup_attach_socket(void) return attach_symlink_dir_path; } +static void setup_terminal_control_fifo() +{ + _cleanup_free_ char *ctl_fifo_path = g_build_filename(opt_bundle_path, "ctl", NULL); + ninfo("ctl fifo path: %s", ctl_fifo_path); + + /* Setup fifo for reading in terminal resize and other stdio control messages */ + + if (mkfifo(ctl_fifo_path, 0666) == -1) + pexit("Failed to mkfifo at %s", ctl_fifo_path); + + ctlfd = open(ctl_fifo_path, O_RDONLY|O_NONBLOCK|O_CLOEXEC); + if (ctlfd == -1) + pexit("Failed to open control fifo"); + + /* + * Open a dummy writer to prevent getting flood of POLLHUPs when + * last writer closes. + */ + int dummyfd = open(ctl_fifo_path, O_WRONLY|O_CLOEXEC); + if (dummyfd == -1) + pexit("Failed to open dummy writer for fifo"); + + ninfo("ctlfd: %d", ctlfd); +} + int main(int argc, char *argv[]) { int ret; char cwd[PATH_MAX]; _cleanup_free_ char *default_pid_file = NULL; - _cleanup_free_ char *ctl_fifo_path = NULL; _cleanup_free_ char *csname = NULL; GError *err = NULL; _cleanup_free_ char *contents = NULL; @@ -1160,29 +1185,8 @@ int main(int argc, char *argv[]) attach_symlink_dir_path = setup_attach_socket(); } - /* Setup fifo for reading in terminal resize and other stdio control messages */ - _cleanup_close_ int ctlfd = -1; - _cleanup_close_ int dummyfd = -1; if (!opt_exec) { - ctl_fifo_path = g_build_filename(opt_bundle_path, "ctl", NULL); - ninfo("ctl fifo path: %s", ctl_fifo_path); - - if (mkfifo(ctl_fifo_path, 0666) == -1) - pexit("Failed to mkfifo at %s", ctl_fifo_path); - - ctlfd = open(ctl_fifo_path, O_RDONLY|O_NONBLOCK|O_CLOEXEC); - if (ctlfd == -1) - pexit("Failed to open control fifo"); - - /* - * Open a dummy writer to prevent getting flood of POLLHUPs when - * last writer closes. - */ - dummyfd = open(ctl_fifo_path, O_WRONLY|O_CLOEXEC); - if (dummyfd == -1) - pexit("Failed to open dummy writer for fifo"); - - ninfo("ctlfd: %d", ctlfd); + setup_terminal_control_fifo(); } /* Send the container pid back to parent */ From 4cb4de6cda1b8e48225a65fcb6f31acacf4901d9 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 12:40:35 +0200 Subject: [PATCH 09/11] conmon: Move OOM setup to helper function Signed-off-by: Alexander Larsson --- conmon/conmon.c | 58 ++++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index 4b321852..ca7e2758 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -892,6 +892,33 @@ static void setup_terminal_control_fifo() ninfo("ctlfd: %d", ctlfd); } +static void setup_oom_handling(int cpid) +{ + /* Setup OOM notification for container process */ + _cleanup_free_ char *memory_cgroup_path = process_cgroup_subsystem_path(cpid, "memory"); + if (!memory_cgroup_path) { + nexit("Failed to get memory cgroup path"); + } + + _cleanup_free_ char *memory_cgroup_file_path = g_build_filename(memory_cgroup_path, "cgroup.event_control", NULL); + + if ((cfd = open(memory_cgroup_file_path, O_WRONLY | O_CLOEXEC)) == -1) { + nwarn("Failed to open %s", memory_cgroup_file_path); + return; + } + + _cleanup_free_ char *memory_cgroup_file_oom_path = g_build_filename(memory_cgroup_path, "memory.oom_control", NULL); + if ((ofd = open(memory_cgroup_file_oom_path, O_RDONLY | O_CLOEXEC)) == -1) + pexit("Failed to open %s", memory_cgroup_file_oom_path); + + if ((oom_efd = eventfd(0, EFD_CLOEXEC)) == -1) + pexit("Failed to create eventfd"); + + _cleanup_free_ char *data = g_strdup_printf("%d %d", oom_efd, ofd); + if (write_all(cfd, data, strlen(data)) < 0) + pexit("Failed to write to cgroup.event_control"); +} + int main(int argc, char *argv[]) { int ret; @@ -918,9 +945,6 @@ int main(int argc, char *argv[]) _cleanup_close_ int dev_null_w = -1; int fds[2]; - _cleanup_free_ char *memory_cgroup_path = NULL; - int wb; - main_loop = g_main_loop_new (NULL, FALSE); /* Command line parameters */ @@ -1194,31 +1218,7 @@ int main(int argc, char *argv[]) write_sync_fd(sync_pipe_fd, cpid, NULL); } - /* Setup OOM notification for container process */ - memory_cgroup_path = process_cgroup_subsystem_path(cpid, "memory"); - if (!memory_cgroup_path) { - nexit("Failed to get memory cgroup path"); - } - - bool oom_handling_enabled = true; - _cleanup_free_ char *memory_cgroup_file_path = g_build_filename(memory_cgroup_path, "cgroup.event_control", NULL); - if ((cfd = open(memory_cgroup_file_path, O_WRONLY | O_CLOEXEC)) == -1) { - nwarn("Failed to open %s", memory_cgroup_file_path); - oom_handling_enabled = false; - } - - if (oom_handling_enabled) { - _cleanup_free_ char *memory_cgroup_file_oom_path = g_build_filename(memory_cgroup_path, "memory.oom_control", NULL); - if ((ofd = open(memory_cgroup_file_oom_path, O_RDONLY | O_CLOEXEC)) == -1) - pexit("Failed to open %s", memory_cgroup_file_oom_path); - - if ((oom_efd = eventfd(0, EFD_CLOEXEC)) == -1) - pexit("Failed to create eventfd"); - - wb = snprintf(buf, BUF_SIZE, "%d %d", oom_efd, ofd); - if (write_all(cfd, buf, wb) < 0) - pexit("Failed to write to cgroup.event_control"); - } + setup_oom_handling(cpid); if (masterfd_stdout >= 0) { g_unix_fd_add (masterfd_stdout, G_IO_IN, stdio_cb, GINT_TO_POINTER(STDOUT_PIPE)); @@ -1230,7 +1230,7 @@ int main(int argc, char *argv[]) } /* Add the OOM event fd to epoll */ - if (oom_handling_enabled) { + if (oom_efd >= 0) { g_unix_fd_add (oom_efd, G_IO_IN, oom_cb, NULL); } From 7b91005b363a025e61aae516f0522d59ff971fb8 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 13:03:51 +0200 Subject: [PATCH 10/11] conmon: Rename global fd variables to longer names Since these are global, its nice if they are a bit more descriptive. Signed-off-by: Alexander Larsson --- conmon/conmon.c | 69 ++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index ca7e2758..e9c9e267 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -479,14 +479,11 @@ static int conn_sock = -1; static int conn_sock_readable; static int conn_sock_writable; -static int logfd = -1; -static int oom_efd = -1; -static int afd = -1; -static int cfd = -1; -/* Used for OOM notification API */ -static int ofd = -1; -static int csfd = -1; -static int ctlfd = -1; +static int log_fd = -1; +static int oom_event_fd = -1; +static int attach_socket_fd = -1; +static int console_socket_fd = -1; +static int terminal_ctrl_fd = -1; static bool timed_out = FALSE; @@ -525,7 +522,7 @@ static gboolean stdio_cb(int fd, GIOCondition condition, gpointer user_data) } if (num_read > 0) { - if (write_k8s_log(logfd, pipe, buf, num_read) < 0) { + if (write_k8s_log(log_fd, pipe, buf, num_read) < 0) { nwarn("write_k8s_log failed"); return G_SOURCE_CONTINUE; } @@ -588,7 +585,7 @@ static gboolean oom_cb(int fd, GIOCondition condition, G_GNUC_UNUSED gpointer us /* End of input */ close (fd); - oom_efd = -1; + oom_event_fd = -1; return G_SOURCE_REMOVE; } @@ -713,7 +710,7 @@ static gboolean terminal_accept_cb(int fd, G_GNUC_UNUSED GIOCondition condition, int connfd = -1; struct termios tset; - ninfo("about to accept from csfd: %d", fd); + ninfo("about to accept from console_socket_fd: %d", fd); connfd = accept4(fd, NULL, NULL, SOCK_CLOEXEC); if (connfd < 0) { nwarn("Failed to accept console-socket connection"); @@ -806,17 +803,17 @@ static char *setup_console_socket(void) ninfo("addr{sun_family=AF_UNIX, sun_path=%s}", addr.sun_path); /* Bind to the console socket path. */ - csfd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); - if (csfd < 0) + console_socket_fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); + if (console_socket_fd < 0) pexit("Failed to create console-socket"); - if (fchmod(csfd, 0700)) + if (fchmod(console_socket_fd, 0700)) pexit("Failed to change console-socket permissions"); /* XXX: This should be handled with a rename(2). */ if (unlink(csname) < 0) pexit("Failed to unlink temporary ranom path"); - if (bind(csfd, (struct sockaddr *) &addr, sizeof(addr)) < 0) + if (bind(console_socket_fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) pexit("Failed to bind to console-socket"); - if (listen(csfd, 128) < 0) + if (listen(console_socket_fd, 128) < 0) pexit("Failed to listen on console-socket"); return g_strdup(csname); @@ -851,17 +848,17 @@ static char *setup_attach_socket(void) * before the server gets a chance to call accept. In that scenario, the server * accept blocks till a new client connection comes in. */ - afd = socket(AF_UNIX, SOCK_SEQPACKET|SOCK_NONBLOCK|SOCK_CLOEXEC, 0); - if (afd == -1) + attach_socket_fd = socket(AF_UNIX, SOCK_SEQPACKET|SOCK_NONBLOCK|SOCK_CLOEXEC, 0); + if (attach_socket_fd == -1) pexit("Failed to create attach socket"); - if (fchmod(afd, 0700)) + if (fchmod(attach_socket_fd, 0700)) pexit("Failed to change attach socket permissions"); - if (bind(afd, (struct sockaddr *)&attach_addr, sizeof(struct sockaddr_un)) == -1) + if (bind(attach_socket_fd, (struct sockaddr *)&attach_addr, sizeof(struct sockaddr_un)) == -1) pexit("Failed to bind attach socket: %s", attach_sock_path); - if (listen(afd, 10) == -1) + if (listen(attach_socket_fd, 10) == -1) pexit("Failed to listen on attach socket: %s", attach_sock_path); return attach_symlink_dir_path; @@ -877,8 +874,8 @@ static void setup_terminal_control_fifo() if (mkfifo(ctl_fifo_path, 0666) == -1) pexit("Failed to mkfifo at %s", ctl_fifo_path); - ctlfd = open(ctl_fifo_path, O_RDONLY|O_NONBLOCK|O_CLOEXEC); - if (ctlfd == -1) + terminal_ctrl_fd = open(ctl_fifo_path, O_RDONLY|O_NONBLOCK|O_CLOEXEC); + if (terminal_ctrl_fd == -1) pexit("Failed to open control fifo"); /* @@ -889,13 +886,15 @@ static void setup_terminal_control_fifo() if (dummyfd == -1) pexit("Failed to open dummy writer for fifo"); - ninfo("ctlfd: %d", ctlfd); + ninfo("terminal_ctrl_fd: %d", terminal_ctrl_fd); } static void setup_oom_handling(int cpid) { /* Setup OOM notification for container process */ _cleanup_free_ char *memory_cgroup_path = process_cgroup_subsystem_path(cpid, "memory"); + _cleanup_close_ int cfd = -1; + int ofd = -1; /* Not closed */ if (!memory_cgroup_path) { nexit("Failed to get memory cgroup path"); } @@ -911,10 +910,10 @@ static void setup_oom_handling(int cpid) if ((ofd = open(memory_cgroup_file_oom_path, O_RDONLY | O_CLOEXEC)) == -1) pexit("Failed to open %s", memory_cgroup_file_oom_path); - if ((oom_efd = eventfd(0, EFD_CLOEXEC)) == -1) + if ((oom_event_fd = eventfd(0, EFD_CLOEXEC)) == -1) pexit("Failed to create eventfd"); - _cleanup_free_ char *data = g_strdup_printf("%d %d", oom_efd, ofd); + _cleanup_free_ char *data = g_strdup_printf("%d %d", oom_event_fd, ofd); if (write_all(cfd, data, strlen(data)) < 0) pexit("Failed to write to cgroup.event_control"); } @@ -1027,8 +1026,8 @@ int main(int argc, char *argv[]) sync_pipe_fd = get_pipe_fd_from_env("_OCI_SYNCPIPE"); /* Open the log path file. */ - logfd = open(opt_log_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0600); - if (logfd < 0) + log_fd = open(opt_log_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0600); + if (log_fd < 0) pexit("Failed to open log file"); /* @@ -1160,7 +1159,7 @@ int main(int argc, char *argv[]) ninfo("about to waitpid: %d", create_pid); if (csname != NULL) { - guint terminal_watch = g_unix_fd_add (csfd, G_IO_IN, terminal_accept_cb, csname); + guint terminal_watch = g_unix_fd_add (console_socket_fd, G_IO_IN, terminal_accept_cb, csname); g_child_watch_add (create_pid, runtime_exit_cb, NULL); g_main_loop_run (main_loop); g_source_remove (terminal_watch); @@ -1230,18 +1229,18 @@ int main(int argc, char *argv[]) } /* Add the OOM event fd to epoll */ - if (oom_efd >= 0) { - g_unix_fd_add (oom_efd, G_IO_IN, oom_cb, NULL); + if (oom_event_fd >= 0) { + g_unix_fd_add (oom_event_fd, G_IO_IN, oom_cb, NULL); } /* Add the attach socket to epoll */ - if (afd > 0) { - g_unix_fd_add (afd, G_IO_IN, attach_cb, NULL); + if (attach_socket_fd > 0) { + g_unix_fd_add (attach_socket_fd, G_IO_IN, attach_cb, NULL); } /* Add control fifo fd to epoll */ - if (ctlfd > 0) { - g_unix_fd_add (ctlfd, G_IO_IN, ctrl_cb, NULL); + if (terminal_ctrl_fd > 0) { + g_unix_fd_add (terminal_ctrl_fd, G_IO_IN, ctrl_cb, NULL); } if (opt_timeout > 0) { From c39868ad55a3324e3bf6d6270f3ce9b7c4e8071b Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 22 Jun 2017 13:30:02 +0200 Subject: [PATCH 11/11] conmon: Add fds to mainloop where they are created Signed-off-by: Alexander Larsson --- conmon/conmon.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/conmon/conmon.c b/conmon/conmon.c index e9c9e267..4e1d57ed 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -861,6 +861,8 @@ static char *setup_attach_socket(void) if (listen(attach_socket_fd, 10) == -1) pexit("Failed to listen on attach socket: %s", attach_sock_path); + g_unix_fd_add (attach_socket_fd, G_IO_IN, attach_cb, NULL); + return attach_symlink_dir_path; } @@ -886,6 +888,8 @@ static void setup_terminal_control_fifo() if (dummyfd == -1) pexit("Failed to open dummy writer for fifo"); + g_unix_fd_add (terminal_ctrl_fd, G_IO_IN, ctrl_cb, NULL); + ninfo("terminal_ctrl_fd: %d", terminal_ctrl_fd); } @@ -916,6 +920,8 @@ static void setup_oom_handling(int cpid) _cleanup_free_ char *data = g_strdup_printf("%d %d", oom_event_fd, ofd); if (write_all(cfd, data, strlen(data)) < 0) pexit("Failed to write to cgroup.event_control"); + + g_unix_fd_add (oom_event_fd, G_IO_IN, oom_cb, NULL); } int main(int argc, char *argv[]) @@ -1228,21 +1234,6 @@ int main(int argc, char *argv[]) num_stdio_fds++; } - /* Add the OOM event fd to epoll */ - if (oom_event_fd >= 0) { - g_unix_fd_add (oom_event_fd, G_IO_IN, oom_cb, NULL); - } - - /* Add the attach socket to epoll */ - if (attach_socket_fd > 0) { - g_unix_fd_add (attach_socket_fd, G_IO_IN, attach_cb, NULL); - } - - /* Add control fifo fd to epoll */ - if (terminal_ctrl_fd > 0) { - g_unix_fd_add (terminal_ctrl_fd, G_IO_IN, ctrl_cb, NULL); - } - if (opt_timeout > 0) { g_timeout_add_seconds (opt_timeout, timeout_cb, NULL); }