From: Luca Boccassi Date: Fri, 4 Aug 2023 12:34:00 +0000 (+0100) Subject: portablectl: fix regression when using --force without extension parameters X-Git-Tag: v255-rc1~820 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bdfa3f3a5c6d16d56d432e7bc52be0c03a5ce6ad;p=thirdparty%2Fsystemd.git portablectl: fix regression when using --force without extension parameters c18f4eb9e96836a made it possible to use --force with various verbs, by going through the newer D-Bus methods. Except it didn't, as it regressed during PR review refactorings, and nobody noticed because there were no tests for it. Fix it, and add tests. Follow-up for c18f4eb9e96836a6a8285ec42fd8a34c8909f6d9 --- diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index e77ee50dab2..3a25624b088 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -91,12 +91,14 @@ static int determine_image(const char *image, bool permit_non_existing, char **r return 0; } -static int attach_extensions_to_message(sd_bus_message *m, char **extensions) { +static int attach_extensions_to_message(sd_bus_message *m, const char *method, char **extensions) { int r; assert(m); + assert(method); - if (strv_isempty(extensions)) + /* The new methods also have flags parameters that are independent of the extensions */ + if (strv_isempty(extensions) && !endswith(method, "WithExtensions")) return 0; r = sd_bus_message_open_container(m, 'a', "s"); @@ -263,7 +265,7 @@ static int get_image_metadata(sd_bus *bus, const char *image, char **matches, sd if (r < 0) return bus_log_create_error(r); - r = attach_extensions_to_message(m, arg_extension_images); + r = attach_extensions_to_message(m, method, arg_extension_images); if (r < 0) return r; @@ -271,7 +273,7 @@ static int get_image_metadata(sd_bus *bus, const char *image, char **matches, sd if (r < 0) return bus_log_create_error(r); - if (!strv_isempty(arg_extension_images)) { + if (streq(method, "GetImageMetadataWithExtensions")) { r = sd_bus_message_append(m, "t", flags); if (r < 0) return bus_log_create_error(r); @@ -774,8 +776,9 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) { if (r < 0) return bus_log_parse_error(r); - /* If we specified any extensions, we'll first an array of extension-release metadata. */ - if (!strv_isempty(arg_extension_images)) { + /* If we specified any extensions or --force (which makes the request go through the new + * WithExtensions calls), we'll first get an array of extension-release metadata. */ + if (!strv_isempty(arg_extension_images) || arg_force) { r = sd_bus_message_skip(reply, "a{say}"); if (r < 0) return bus_log_parse_error(r); @@ -855,7 +858,7 @@ static int attach_reattach_image(int argc, char *argv[], const char *method) { if (r < 0) return bus_log_create_error(r); - r = attach_extensions_to_message(m, arg_extension_images); + r = attach_extensions_to_message(m, method, arg_extension_images); if (r < 0) return r; @@ -933,11 +936,11 @@ static int detach_image(int argc, char *argv[], void *userdata) { if (r < 0) return bus_log_create_error(r); - r = attach_extensions_to_message(m, arg_extension_images); + r = attach_extensions_to_message(m, method, arg_extension_images); if (r < 0) return r; - if (strv_isempty(arg_extension_images)) + if (streq(method, "DetachImage")) r = sd_bus_message_append(m, "b", arg_runtime); else { uint64_t flags = (arg_runtime ? PORTABLE_RUNTIME : 0) | (arg_force ? PORTABLE_FORCE_ATTACH | PORTABLE_FORCE_SYSEXT : 0); @@ -1145,7 +1148,7 @@ static int is_image_attached(int argc, char *argv[], void *userdata) { if (r < 0) return bus_log_create_error(r); - r = attach_extensions_to_message(m, arg_extension_images); + r = attach_extensions_to_message(m, method, arg_extension_images); if (r < 0) return r; diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index 18ec41727d0..36e5cdc1263 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -43,12 +43,16 @@ EOF portablectl "${ARGS[@]}" attach --now --runtime /usr/share/minimal_0.raw minimal-app0 +portablectl is-attached minimal-app0 +portablectl inspect /usr/share/minimal_0.raw minimal-app0.service systemctl is-active minimal-app0.service systemctl is-active minimal-app0-foo.service systemctl is-active minimal-app0-bar.service && exit 1 portablectl "${ARGS[@]}" reattach --now --runtime /usr/share/minimal_1.raw minimal-app0 +portablectl is-attached minimal-app0 +portablectl inspect /usr/share/minimal_0.raw minimal-app0.service systemctl is-active minimal-app0.service systemctl is-active minimal-app0-bar.service systemctl is-active minimal-app0-foo.service && exit 1 @@ -61,6 +65,32 @@ portablectl detach --now --runtime /usr/share/minimal_1.raw minimal-app0 portablectl list | grep -q -F "No images." busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1' && exit 1 +# Ensure we don't regress (again) when using --force + +portablectl "${ARGS[@]}" attach --force --now --runtime /usr/share/minimal_0.raw minimal-app0 + +portablectl is-attached --force minimal-app0 +portablectl inspect --force /usr/share/minimal_0.raw minimal-app0.service +systemctl is-active minimal-app0.service +systemctl is-active minimal-app0-foo.service +systemctl is-active minimal-app0-bar.service && exit 1 + +portablectl "${ARGS[@]}" reattach --force --now --runtime /usr/share/minimal_1.raw minimal-app0 + +portablectl is-attached --force minimal-app0 +portablectl inspect --force /usr/share/minimal_0.raw minimal-app0.service +systemctl is-active minimal-app0.service +systemctl is-active minimal-app0-bar.service +systemctl is-active minimal-app0-foo.service && exit 1 + +portablectl list | grep -q -F "minimal_1" +busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1' + +portablectl detach --force --now --runtime /usr/share/minimal_1.raw minimal-app0 + +portablectl list | grep -q -F "No images." +busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1' && exit 1 + # portablectl also works with directory paths rather than images unsquashfs -dest /tmp/minimal_0 /usr/share/minimal_0.raw