From 966d7977c755326bf0b7f0954213cb0477ed2cde Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 8 Apr 2024 01:34:12 +0100 Subject: [PATCH] portablectl: add --clean parameter for detaching Calls CleanUnit on each portable service being removed, after it has stopped --- man/portablectl.xml | 9 +++++ shell-completion/bash/portablectl | 3 +- src/portable/portablectl.c | 65 ++++++++++++++++++++++++++++--- test/units/testsuite-29.sh | 6 ++- 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/man/portablectl.xml b/man/portablectl.xml index bcfaf7ced1..5e88eb309d 100644 --- a/man/portablectl.xml +++ b/man/portablectl.xml @@ -391,6 +391,15 @@ + + + + When detaching ensure the configuration, state, logs, cache, and runtime data + directories of the portable service are removed from the host system. + + + + diff --git a/shell-completion/bash/portablectl b/shell-completion/bash/portablectl index 61a9610aa3..cd6db324fc 100644 --- a/shell-completion/bash/portablectl +++ b/shell-completion/bash/portablectl @@ -36,7 +36,8 @@ _portablectl() { local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]} local -A OPTS=( [STANDALONE]='-q --quiet --runtime --no-reload --cat --no-pager --no-legend - --no-ask-password --enable --now -h --help --version' + --no-ask-password --enable --now -h --help --version + --clean' [ARG]='-p --profile --copy -H --host -M --machine --extension' ) diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index 0f67e4432f..57b930d6cb 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -50,6 +50,7 @@ static bool arg_now = false; static bool arg_no_block = false; static char **arg_extension_images = NULL; static bool arg_force = false; +static bool arg_clean = false; STATIC_DESTRUCTOR_REGISTER(arg_extension_images, strv_freep); @@ -769,13 +770,49 @@ static int maybe_stop_enable_restart(sd_bus *bus, sd_bus_message *reply) { return 0; } -static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) { - _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *wait = NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_strv_free_ char **matches = NULL; +static int maybe_clean_units(sd_bus *bus, char **units) { int r; - if (!arg_enable && !arg_now) + assert(bus); + + if (!arg_clean) + return 0; + + STRV_FOREACH(name, units) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; + + r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "CleanUnit"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append(m, "s", *name); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append_strv(m, STRV_MAKE("all", "fdstore")); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_call(bus, m, 0, &error, NULL); + if (r < 0) + return log_error_errno( + r, + "Failed to call CleanUnit on portable service %s: %s", + *name, + bus_error_message(&error, r)); + } + + return 0; +} + +static int maybe_stop_disable_clean(sd_bus *bus, char *image, char *argv[]) { + _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *wait = NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_strv_free_ char **matches = NULL, **units = NULL; + int r; + + if (!arg_enable && !arg_now && !arg_clean) return 0; r = determine_matches(argv[1], argv + 2, true, &matches); @@ -829,6 +866,10 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) { (void) maybe_start_stop_restart(bus, name, "StopUnit", wait); (void) maybe_enable_disable(bus, name, false); + + r = strv_extend(&units, name); + if (r < 0) + return log_oom(); } r = sd_bus_message_exit_container(reply); @@ -840,6 +881,9 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) { if (r < 0) return r; + /* Need to ensure all units are stopped before calling CleanUnit, as files might be in use. */ + (void) maybe_clean_units(bus, units); + return 0; } @@ -942,7 +986,7 @@ static int detach_image(int argc, char *argv[], void *userdata) { (void) polkit_agent_open_if_enabled(arg_transport, arg_ask_password); - (void) maybe_stop_disable(bus, image, argv); + (void) maybe_stop_disable_clean(bus, image, argv); method = strv_isempty(arg_extension_images) && !arg_force ? "DetachImage" : "DetachImageWithExtensions"; @@ -1265,6 +1309,9 @@ static int help(int argc, char *argv[], void *userdata) { " --extension=PATH Extend the image with an overlay\n" " --force Skip 'already active' check when attaching or\n" " detaching an image (with extensions)\n" + " --clean When detaching, also remove configuration, state,\n" + " cache, logs or runtime data of the portable\n" + " service(s)\n" "\nSee the %s for details.\n", program_invocation_short_name, ansi_highlight(), @@ -1291,6 +1338,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_NO_BLOCK, ARG_EXTENSION, ARG_FORCE, + ARG_CLEAN, }; static const struct option options[] = { @@ -1312,6 +1360,7 @@ static int parse_argv(int argc, char *argv[]) { { "no-block", no_argument, NULL, ARG_NO_BLOCK }, { "extension", required_argument, NULL, ARG_EXTENSION }, { "force", no_argument, NULL, ARG_FORCE }, + { "clean", no_argument, NULL, ARG_CLEAN }, {} }; @@ -1421,6 +1470,10 @@ static int parse_argv(int argc, char *argv[]) { arg_force = true; break; + case ARG_CLEAN: + arg_clean = true; + break; + case '?': return -EINVAL; diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index 7af0ed4578..977d76e22f 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -304,7 +304,11 @@ grep -q -F "LogExtraFields=PORTABLE_EXTENSION_NAME_AND_VERSION=app" /run/systemd grep -q -F "LogExtraFields=PORTABLE_EXTENSION=app1" /run/systemd/system.attached/app1.service.d/20-portable.conf grep -q -F "LogExtraFields=PORTABLE_EXTENSION_NAME_AND_VERSION=app_1" /run/systemd/system.attached/app1.service.d/20-portable.conf -portablectl detach --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1 +portablectl detach --clean --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1 + +# Ensure --clean remove state and other directories belonging to the portable image being detached +test ! -d /var/lib/app0 +test ! -d /run/app0 # Ensure that mixed mode copies the images and units (client-owned) but symlinks the profile (OS owned) portablectl "${ARGS[@]}" attach --copy=mixed --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1