Skip to content

Commit 8f3ec30

Browse files
committed
portablectl: fix regression when using --force without extension parameters
c18f4eb 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 c18f4eb (cherry picked from commit bdfa3f3) (cherry picked from commit 0f27d7b)
1 parent 4d79082 commit 8f3ec30

File tree

2 files changed

+43
-10
lines changed

2 files changed

+43
-10
lines changed

src/portable/portablectl.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,14 @@ static int determine_image(const char *image, bool permit_non_existing, char **r
9191
return 0;
9292
}
9393

94-
static int attach_extensions_to_message(sd_bus_message *m, char **extensions) {
94+
static int attach_extensions_to_message(sd_bus_message *m, const char *method, char **extensions) {
9595
int r;
9696

9797
assert(m);
98+
assert(method);
9899

99-
if (strv_isempty(extensions))
100+
/* The new methods also have flags parameters that are independent of the extensions */
101+
if (strv_isempty(extensions) && !endswith(method, "WithExtensions"))
100102
return 0;
101103

102104
r = sd_bus_message_open_container(m, 'a', "s");
@@ -280,15 +282,15 @@ static int get_image_metadata(sd_bus *bus, const char *image, char **matches, sd
280282
if (r < 0)
281283
return bus_log_create_error(r);
282284

283-
r = attach_extensions_to_message(m, arg_extension_images);
285+
r = attach_extensions_to_message(m, method, arg_extension_images);
284286
if (r < 0)
285287
return r;
286288

287289
r = sd_bus_message_append_strv(m, matches);
288290
if (r < 0)
289291
return bus_log_create_error(r);
290292

291-
if (!strv_isempty(arg_extension_images)) {
293+
if (streq(method, "GetImageMetadataWithExtensions")) {
292294
r = sd_bus_message_append(m, "t", flags);
293295
if (r < 0)
294296
return bus_log_create_error(r);
@@ -783,8 +785,9 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) {
783785
if (r < 0)
784786
return bus_log_parse_error(r);
785787

786-
/* If we specified any extensions, we'll first an array of extension-release metadata. */
787-
if (!strv_isempty(arg_extension_images)) {
788+
/* If we specified any extensions or --force (which makes the request go through the new
789+
* WithExtensions calls), we'll first get an array of extension-release metadata. */
790+
if (!strv_isempty(arg_extension_images) || arg_force) {
788791
r = sd_bus_message_skip(reply, "a{say}");
789792
if (r < 0)
790793
return bus_log_parse_error(r);
@@ -864,7 +867,7 @@ static int attach_reattach_image(int argc, char *argv[], const char *method) {
864867
if (r < 0)
865868
return bus_log_create_error(r);
866869

867-
r = attach_extensions_to_message(m, arg_extension_images);
870+
r = attach_extensions_to_message(m, method, arg_extension_images);
868871
if (r < 0)
869872
return r;
870873

@@ -942,11 +945,11 @@ static int detach_image(int argc, char *argv[], void *userdata) {
942945
if (r < 0)
943946
return bus_log_create_error(r);
944947

945-
r = attach_extensions_to_message(m, arg_extension_images);
948+
r = attach_extensions_to_message(m, method, arg_extension_images);
946949
if (r < 0)
947950
return r;
948951

949-
if (strv_isempty(arg_extension_images))
952+
if (streq(method, "DetachImage"))
950953
r = sd_bus_message_append(m, "b", arg_runtime);
951954
else {
952955
uint64_t flags = (arg_runtime ? PORTABLE_RUNTIME : 0) | (arg_force ? PORTABLE_FORCE_ATTACH | PORTABLE_FORCE_SYSEXT : 0);
@@ -1154,7 +1157,7 @@ static int is_image_attached(int argc, char *argv[], void *userdata) {
11541157
if (r < 0)
11551158
return bus_log_create_error(r);
11561159

1157-
r = attach_extensions_to_message(m, arg_extension_images);
1160+
r = attach_extensions_to_message(m, method, arg_extension_images);
11581161
if (r < 0)
11591162
return r;
11601163

test/units/testsuite-29.sh

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ EOF
4545

4646
portablectl "${ARGS[@]}" attach --now --runtime /usr/share/minimal_0.raw minimal-app0
4747

48+
portablectl is-attached minimal-app0
49+
portablectl inspect /usr/share/minimal_0.raw minimal-app0.service
4850
systemctl is-active minimal-app0.service
4951
systemctl is-active minimal-app0-foo.service
5052
systemctl is-active minimal-app0-bar.service && exit 1
@@ -53,6 +55,8 @@ systemctl is-active minimal-app0-bar.service && exit 1
5355
# Let's set timeout to improve performance.
5456
timeout "$TIMEOUT" portablectl "${ARGS[@]}" reattach --now --runtime /usr/share/minimal_1.raw minimal-app0
5557

58+
portablectl is-attached minimal-app0
59+
portablectl inspect /usr/share/minimal_0.raw minimal-app0.service
5660
systemctl is-active minimal-app0.service
5761
systemctl is-active minimal-app0-bar.service
5862
systemctl is-active minimal-app0-foo.service && exit 1
@@ -65,6 +69,32 @@ portablectl detach --now --runtime /usr/share/minimal_1.raw minimal-app0
6569
portablectl list | grep -q -F "No images."
6670
busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1' && exit 1
6771

72+
# Ensure we don't regress (again) when using --force
73+
74+
portablectl "${ARGS[@]}" attach --force --now --runtime /usr/share/minimal_0.raw minimal-app0
75+
76+
portablectl is-attached --force minimal-app0
77+
portablectl inspect --force /usr/share/minimal_0.raw minimal-app0.service
78+
systemctl is-active minimal-app0.service
79+
systemctl is-active minimal-app0-foo.service
80+
systemctl is-active minimal-app0-bar.service && exit 1
81+
82+
portablectl "${ARGS[@]}" reattach --force --now --runtime /usr/share/minimal_1.raw minimal-app0
83+
84+
portablectl is-attached --force minimal-app0
85+
portablectl inspect --force /usr/share/minimal_0.raw minimal-app0.service
86+
systemctl is-active minimal-app0.service
87+
systemctl is-active minimal-app0-bar.service
88+
systemctl is-active minimal-app0-foo.service && exit 1
89+
90+
portablectl list | grep -q -F "minimal_1"
91+
busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1'
92+
93+
portablectl detach --force --now --runtime /usr/share/minimal_1.raw minimal-app0
94+
95+
portablectl list | grep -q -F "No images."
96+
busctl tree org.freedesktop.portable1 --no-pager | grep -q -F '/org/freedesktop/portable1/image/minimal_5f1' && exit 1
97+
6898
# portablectl also works with directory paths rather than images
6999

70100
unsquashfs -dest /tmp/minimal_0 /usr/share/minimal_0.raw

0 commit comments

Comments
 (0)