From 7b1ba433d24ef74b82a276c17837600c5dfd5233 Mon Sep 17 00:00:00 2001 From: Mattias Axelsson Date: Fri, 22 Mar 2024 18:48:28 +0100 Subject: [PATCH 1/5] Pass around data root in settings struct This reverts commit ba7a3b381f2b0b01410ade1a7ebb711bcbe2161b. --- app/dockerdwrapper.c | 71 +++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/app/dockerdwrapper.c b/app/dockerdwrapper.c index 22417d6..ca709fd 100644 --- a/app/dockerdwrapper.c +++ b/app/dockerdwrapper.c @@ -26,7 +26,7 @@ #include struct settings { - bool use_sdcard; + char *data_root; bool use_tls; bool use_ipc_socket; }; @@ -208,10 +208,9 @@ get_filesystem_of_path(const char *path) * @return True if successful, false if setup failed. */ static bool -setup_sdcard(const char *dockerd_path) +setup_sdcard(const char *data_root) { g_autofree char *sd_file_system = NULL; - g_autofree char *data_root = g_strdup_printf("%s/data", dockerd_path); g_autofree char *create_droot_command = g_strdup_printf("mkdir -p %s", data_root); @@ -225,11 +224,11 @@ setup_sdcard(const char *dockerd_path) } // Confirm that the SD card is usable - sd_file_system = get_filesystem_of_path(dockerd_path); + sd_file_system = get_filesystem_of_path(data_root); if (sd_file_system == NULL) { syslog(LOG_ERR, "Couldn't identify the file system of the SD card at %s", - dockerd_path); + data_root); return false; } @@ -239,17 +238,17 @@ setup_sdcard(const char *dockerd_path) "The SD card at %s uses file system %s which does not support " "Unix file permissions. Please reformat to a file system that " "support Unix file permissions, such as ext4 or xfs.", - dockerd_path, + data_root, sd_file_system); return false; } - if (access(dockerd_path, F_OK) == 0 && access(dockerd_path, W_OK) != 0) { + if (access(data_root, F_OK) == 0 && access(data_root, W_OK) != 0) { syslog(LOG_ERR, "The application user does not have write permissions to the SD " "card directory at %s. Please change the directory permissions or " "remove the directory.", - dockerd_path); + data_root); return false; } @@ -272,23 +271,18 @@ is_parameter_yes(const char *name) * @return True if successful, false otherwise. */ static gboolean -get_and_verify_sd_card_selection(bool *use_sdcard_ret) +get_and_verify_sd_card_selection(char **data_root) { - gboolean return_value = false; - const bool use_sdcard = is_parameter_yes("SDCardSupport"); - - { - if (use_sdcard) { - if (!setup_sdcard(dockerd_path_on_sd_card)) { - syslog(LOG_ERR, "Failed to setup SD card."); - goto end; - } + if (is_parameter_yes("SDCardSupport")) { + *data_root = g_strdup_printf("%s/data", dockerd_path_on_sd_card); + if (!setup_sdcard(*data_root)) { + syslog(LOG_ERR, "Failed to setup SD card."); + return false; } - *use_sdcard_ret = use_sdcard; - return_value = true; + } else { + *data_root = NULL; } -end: - return return_value; + return true; } /** @@ -366,7 +360,7 @@ get_ipc_socket_selection(bool *use_ipc_socket_ret) static bool read_settings(struct settings *settings) { - if (!get_and_verify_sd_card_selection(&settings->use_sdcard)) { + if (!get_and_verify_sd_card_selection(&settings->data_root)) { syslog(LOG_ERR, "Failed to setup sd_card"); return false; } @@ -386,7 +380,7 @@ read_settings(struct settings *settings) static bool start_dockerd(const struct settings *settings) { - const bool use_sdcard = settings->use_sdcard; + const char *data_root = settings->data_root; const bool use_tls = settings->use_tls; const bool use_ipc_socket = settings->use_ipc_socket; @@ -440,25 +434,20 @@ start_dockerd(const struct settings *settings) g_strlcat(msg, " in unsecured mode", msg_len); } - if (use_sdcard) { - args_offset += - g_snprintf(args + args_offset, - args_len - args_offset, - " %s", - "--data-root /var/spool/storage/SD_DISK/dockerd/data"); - - g_strlcat(msg, " using SD card as storage", msg_len); - } else { - g_strlcat(msg, " using internal storage", msg_len); - } - - if (use_ipc_socket) { + g_autofree char *data_root_msg = g_strdup_printf( + " using %s as storage", data_root ? data_root : "/var/lib/docker"); + g_strlcat(msg, data_root_msg, msg_len); + if (data_root) args_offset += g_snprintf(args + args_offset, args_len - args_offset, - " %s", - "-H unix:///var/run/docker.sock"); + " --data-root %s", + data_root); + if (use_ipc_socket) { g_strlcat(msg, " with IPC socket.", msg_len); + args_offset += g_snprintf(args + args_offset, + args_len - args_offset, + " -H unix:///var/run/docker.sock"); } else { g_strlcat(msg, " without IPC socket.", msg_len); } @@ -505,7 +494,9 @@ static bool read_settings_and_start_dockerd(void) { struct settings settings = {0}; - return read_settings(&settings) && start_dockerd(&settings); + bool success = read_settings(&settings) && start_dockerd(&settings); + free(settings.data_root); + return success; } /** From 23ef43b257f0e401155c6cbeb3cdb371da018a32 Mon Sep 17 00:00:00 2001 From: Mattias Axelsson Date: Fri, 22 Mar 2024 20:56:15 +0100 Subject: [PATCH 2/5] Remove handling of unknown parameters in callback Callbacks from unknown parameters isn't something that happens, and if it did, it wouldn't be a big deal anyway. Removing is a step towards removing global variable 'restart_dockerd'. --- app/dockerdwrapper.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/app/dockerdwrapper.c b/app/dockerdwrapper.c index ca709fd..ceb8e91 100644 --- a/app/dockerdwrapper.c +++ b/app/dockerdwrapper.c @@ -592,24 +592,14 @@ parameter_changed_callback(const gchar *name, { const gchar *parname = name += strlen("root.dockerdwrapper."); - bool unknown_parameter = true; for (size_t i = 0; i < sizeof(ax_parameters) / sizeof(ax_parameters[0]); ++i) { if (strcmp(parname, ax_parameters[i]) == 0) { syslog(LOG_INFO, "%s changed to: %s", ax_parameters[i], value); restart_dockerd = true; - unknown_parameter = false; } } - if (unknown_parameter) { - syslog(LOG_WARNING, "Parameter %s is not recognized", name); - restart_dockerd = false; - - // No known parameter was changed, do not restart. - return; - } - // Stop the currently running process. if (!stop_dockerd()) { syslog(LOG_ERR, From 76a2276d455f7a3ff3af602c96c47102c92011ff Mon Sep 17 00:00:00 2001 From: Mattias Axelsson Date: Fri, 22 Mar 2024 21:22:54 +0100 Subject: [PATCH 3/5] Centralize restart of dockerd By running g_main_loop_run() in a while-loop, other parts of the program can retry starting dockerd by simply calling g_main_loop_quit(). To exit this loop, call quit_program(), which will also set the global acap_exit_code variable. This is needed then e.g. axstorage is used to wait for SD card to become available. --- app/dockerdwrapper.c | 76 ++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 49 deletions(-) diff --git a/app/dockerdwrapper.c b/app/dockerdwrapper.c index ceb8e91..03b09be 100644 --- a/app/dockerdwrapper.c +++ b/app/dockerdwrapper.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -42,8 +43,9 @@ static void dockerd_process_exited_callback(__attribute__((unused)) GPid pid, // Loop run on the main process static GMainLoop *loop = NULL; -// Exit code -static int exit_code = 0; +// Exit code of this program. Set using 'quit_program()'. +#define EX_KEEP_RUNNING -1 +static int application_exit_code = EX_KEEP_RUNNING; // Pid of the running dockerd process static pid_t dockerd_process_pid = -1; @@ -52,9 +54,6 @@ static pid_t dockerd_process_pid = -1; static const char *dockerd_path_on_sd_card = "/var/spool/storage/SD_DISK/dockerd"; -// True if the dockerd_exited_callback should restart dockerd -static bool restart_dockerd = false; - // All ax_parameters the acap has static const char *ax_parameters[] = {"IPCSocket", "SDCardSupport", "UseTLS"}; @@ -64,6 +63,13 @@ static const char *tls_certs[] = {"ca.pem", "server-cert.pem", "server-key.pem"}; +static void +quit_program(int exit_code) +{ + application_exit_code = exit_code; + g_main_loop_quit(loop); +} + /** * @brief Signals handling * @@ -76,7 +82,7 @@ handle_signals(__attribute__((unused)) int signal_num) case SIGINT: case SIGTERM: case SIGQUIT: - g_main_loop_quit(loop); + quit_program(EX_OK); } } @@ -478,8 +484,7 @@ start_dockerd(const struct settings *settings) if (!is_process_alive(dockerd_process_pid)) { syslog(LOG_ERR, "Starting dockerd failed: Process died unexpectedly during startup"); - exit_code = -1; - g_main_loop_quit(loop); + quit_program(EX_SOFTWARE); goto end; } return_value = true; @@ -514,7 +519,7 @@ stop_dockerd(void) goto end; } - // Send SIGTERM to the process + syslog(LOG_INFO, "Sending SIGTERM to dockerd."); bool sigterm_successfully_sent = kill(dockerd_process_pid, SIGTERM) == 0; if (!sigterm_successfully_sent) { syslog( @@ -555,8 +560,6 @@ dockerd_process_exited_callback(__attribute__((unused)) GPid pid, if (!g_spawn_check_exit_status(status, &error)) { syslog(LOG_ERR, "Dockerd process exited with error: %d", status); g_clear_error(&error); - - exit_code = -1; } dockerd_process_pid = -1; @@ -566,16 +569,7 @@ dockerd_process_exited_callback(__attribute__((unused)) GPid pid, // manner. Remove it manually. remove("/var/run/docker.pid"); - if (restart_dockerd) { - restart_dockerd = false; - if (!read_settings_and_start_dockerd()) { - exit_code = -1; - g_main_loop_quit(loop); - } - } else { - // We shouldn't restart, stop instead. - g_main_loop_quit(loop); - } + g_main_loop_quit(loop); // Trigger a restart of dockerd from main() } /** @@ -596,17 +590,9 @@ parameter_changed_callback(const gchar *name, ++i) { if (strcmp(parname, ax_parameters[i]) == 0) { syslog(LOG_INFO, "%s changed to: %s", ax_parameters[i], value); - restart_dockerd = true; + g_main_loop_quit(loop); // Trigger a restart of dockerd from main() } } - - // Stop the currently running process. - if (!stop_dockerd()) { - syslog(LOG_ERR, - "Failed to stop dockerd process. Please restart the acap " - "manually."); - exit_code = -1; - } } static AXParameter * @@ -651,11 +637,12 @@ int main(void) { AXParameter *ax_parameter = NULL; - exit_code = 0; openlog(NULL, LOG_PID, LOG_USER); syslog(LOG_INFO, "Started logging."); + loop = g_main_loop_new(NULL, FALSE); + // Setup signal handling. init_signals(); @@ -663,30 +650,21 @@ main(void) ax_parameter = setup_axparameter(); if (ax_parameter == NULL) { syslog(LOG_ERR, "Error in setup_axparameter"); - exit_code = -1; - goto end; + quit_program(EX_SOFTWARE); } - /* Create the GLib event loop. */ - loop = g_main_loop_new(NULL, FALSE); - loop = g_main_loop_ref(loop); + while (application_exit_code == EX_KEEP_RUNNING) { + if (dockerd_process_pid == -1 && !read_settings_and_start_dockerd()) + quit_program(EX_SOFTWARE); - if (!read_settings_and_start_dockerd()) { - exit_code = -1; - goto end; + g_main_loop_run(loop); + + if (!stop_dockerd()) + syslog(LOG_WARNING, "Failed to shut down dockerd."); } - /* Run the GLib event loop. */ - g_main_loop_run(loop); g_main_loop_unref(loop); -end: - if (stop_dockerd()) { - syslog(LOG_INFO, "Shutting down. dockerd shut down successfully."); - } else { - syslog(LOG_WARNING, "Shutting down. Failed to shut down dockerd."); - } - if (ax_parameter != NULL) { for (size_t i = 0; i < sizeof(ax_parameters) / sizeof(ax_parameters[0]); ++i) { @@ -698,5 +676,5 @@ main(void) ax_parameter_free(ax_parameter); } - return exit_code; + return application_exit_code; } From f632755b04fec61e6f1814bfc49f0e711bcd55f6 Mon Sep 17 00:00:00 2001 From: Mattias Axelsson Date: Fri, 22 Mar 2024 21:27:02 +0100 Subject: [PATCH 4/5] Pass around SD card area instead of using global constant --- app/dockerdwrapper.c | 83 +++++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/app/dockerdwrapper.c b/app/dockerdwrapper.c index 03b09be..d39d5b9 100644 --- a/app/dockerdwrapper.c +++ b/app/dockerdwrapper.c @@ -32,6 +32,10 @@ struct settings { bool use_ipc_socket; }; +struct app_state { + char *sd_card_area; +}; + /** * @brief Callback called when the dockerd process exits. */ @@ -50,10 +54,6 @@ static int application_exit_code = EX_KEEP_RUNNING; // Pid of the running dockerd process static pid_t dockerd_process_pid = -1; -// Full path to the SD card -static const char *dockerd_path_on_sd_card = - "/var/spool/storage/SD_DISK/dockerd"; - // All ax_parameters the acap has static const char *ax_parameters[] = {"IPCSocket", "SDCardSupport", "UseTLS"}; @@ -270,25 +270,31 @@ is_parameter_yes(const char *name) return value && strcmp(value, "yes") == 0; } -/** - * @brief Gets and verifies the SDCardSupport selection - * - * @param use_sdcard_ret selection to be updated. - * @return True if successful, false otherwise. - */ -static gboolean -get_and_verify_sd_card_selection(char **data_root) +// Return data root matching the current SDCardSupport selection. +// +// If SDCardSupport is "yes", data root will be located on the proved SD card +// area. Passing NULL as SD card area signals that the SD card is not availble. +static char * +prepare_data_root(const char *sd_card_area) { if (is_parameter_yes("SDCardSupport")) { - *data_root = g_strdup_printf("%s/data", dockerd_path_on_sd_card); - if (!setup_sdcard(*data_root)) { + if (!sd_card_area) { + syslog( + LOG_ERR, + "SD card was requested, but no SD card is available at the moment."); + return NULL; + } + char *data_root = g_strdup_printf("%s/data", sd_card_area); + if (!setup_sdcard(data_root)) { syslog(LOG_ERR, "Failed to setup SD card."); - return false; + free(data_root); + return NULL; } + return data_root; } else { - *data_root = NULL; + return strdup( + "/var/lib/docker"); // Same location as if --data-root is omitted } - return true; } /** @@ -364,12 +370,8 @@ get_ipc_socket_selection(bool *use_ipc_socket_ret) } static bool -read_settings(struct settings *settings) +read_settings(struct settings *settings, const struct app_state *app_state) { - if (!get_and_verify_sd_card_selection(&settings->data_root)) { - syslog(LOG_ERR, "Failed to setup sd_card"); - return false; - } if (!get_and_verify_tls_selection(&settings->use_tls)) { syslog(LOG_ERR, "Failed to verify tls selection"); return false; @@ -378,13 +380,17 @@ read_settings(struct settings *settings) syslog(LOG_ERR, "Failed to get ipc socket selection"); return false; } + if (!(settings->data_root = prepare_data_root(app_state->sd_card_area))) { + syslog(LOG_ERR, "Failed to set up dockerd data root"); + return false; + } return true; } // Return true if dockerd was successfully started. // Log an error and return false if it failed to start properly. static bool -start_dockerd(const struct settings *settings) +start_dockerd(const struct settings *settings, struct app_state *app_state) { const char *data_root = settings->data_root; const bool use_tls = settings->use_tls; @@ -440,14 +446,11 @@ start_dockerd(const struct settings *settings) g_strlcat(msg, " in unsecured mode", msg_len); } - g_autofree char *data_root_msg = g_strdup_printf( - " using %s as storage", data_root ? data_root : "/var/lib/docker"); + g_autofree char *data_root_msg = + g_strdup_printf(" using %s as storage", data_root); g_strlcat(msg, data_root_msg, msg_len); - if (data_root) - args_offset += g_snprintf(args + args_offset, - args_len - args_offset, - " --data-root %s", - data_root); + args_offset += g_snprintf( + args + args_offset, args_len - args_offset, " --data-root %s", data_root); if (use_ipc_socket) { g_strlcat(msg, " with IPC socket.", msg_len); @@ -479,7 +482,8 @@ start_dockerd(const struct settings *settings) } // Watch the child process. - g_child_watch_add(dockerd_process_pid, dockerd_process_exited_callback, NULL); + g_child_watch_add( + dockerd_process_pid, dockerd_process_exited_callback, app_state); if (!is_process_alive(dockerd_process_pid)) { syslog(LOG_ERR, @@ -496,10 +500,11 @@ start_dockerd(const struct settings *settings) } static bool -read_settings_and_start_dockerd(void) +read_settings_and_start_dockerd(struct app_state *app_state) { struct settings settings = {0}; - bool success = read_settings(&settings) && start_dockerd(&settings); + bool success = read_settings(&settings, app_state) && + start_dockerd(&settings, app_state); free(settings.data_root); return success; } @@ -552,10 +557,11 @@ stop_dockerd(void) * @brief Callback called when the dockerd process exits. */ static void -dockerd_process_exited_callback(__attribute__((unused)) GPid pid, +dockerd_process_exited_callback(GPid pid, gint status, - __attribute__((unused)) gpointer user_data) + gpointer app_state_void_ptr) { + struct app_state *app_state = app_state_void_ptr; GError *error = NULL; if (!g_spawn_check_exit_status(status, &error)) { syslog(LOG_ERR, "Dockerd process exited with error: %d", status); @@ -636,6 +642,9 @@ setup_axparameter(void) int main(void) { + struct app_state app_state = {0}; + app_state.sd_card_area = strdup("/var/spool/storage/SD_DISK/dockerd"); + AXParameter *ax_parameter = NULL; openlog(NULL, LOG_PID, LOG_USER); @@ -654,7 +663,8 @@ main(void) } while (application_exit_code == EX_KEEP_RUNNING) { - if (dockerd_process_pid == -1 && !read_settings_and_start_dockerd()) + if (dockerd_process_pid == -1 && + !read_settings_and_start_dockerd(&app_state)) quit_program(EX_SOFTWARE); g_main_loop_run(loop); @@ -676,5 +686,6 @@ main(void) ax_parameter_free(ax_parameter); } + free(app_state.sd_card_area); return application_exit_code; } From 31ac611113966ee68942b8d6e1909713745380e7 Mon Sep 17 00:00:00 2001 From: madelen-at-work Date: Tue, 26 Mar 2024 11:46:22 +0100 Subject: [PATCH 5/5] clang format --- app/dockerdwrapper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dockerdwrapper.c b/app/dockerdwrapper.c index d09d9a1..a5d124f 100644 --- a/app/dockerdwrapper.c +++ b/app/dockerdwrapper.c @@ -539,7 +539,7 @@ read_settings_and_start_dockerd(struct app_state *app_state) bool success = read_settings(&settings, app_state) && start_dockerd(&settings, app_state); - + free(settings.data_root); return success; }