X Tutup
Skip to content

Commit 087a799

Browse files
keszybzbluca
keszybz
and
bluca
committed
portable: add return parameter to GetImageMetadataWithExtensions
The complaint was that the output array was used for two kinds of data, and the input flag decided whether this extra data should be included. The flag is removed, and instead the old method is changed to include the data always as a separate parameter. This breaks backward compatibility, but the old method is effectively broken and does not appear to be used yet, at least in open source code, by searching on codesearch.debian.net and github.com. Fixes systemd#22404. Co-authored-by: Luca Boccassi <bluca@debian.org>
1 parent 6d6104e commit 087a799

File tree

5 files changed

+110
-97
lines changed

5 files changed

+110
-97
lines changed

‎man/org.freedesktop.portable1.xml‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ node /org/freedesktop/portable1 {
5454
in t flags,
5555
out s image,
5656
out ay os_release,
57+
out a{say} extensions,
5758
out a{say} units);
5859
GetImageState(in s image,
5960
out s state);
@@ -189,19 +190,12 @@ node /org/freedesktop/portable1 {
189190
and a list of portable units contained in the image, in the form of a string (unit name) and
190191
an array of bytes with the content.</para>
191192

192-
<para><function>GetImageMetadataWithExtensions()</function> retrieves metadata associated with an image.
193-
This method is a superset of <function>GetImageMetadata()</function> with the addition of
194-
a list of extensions as input parameter, which were overlaid on top of the main
195-
image via <function>AttachImageWithExtensions()</function>.
196-
The <varname>flag</varname> parameter can be used to request that, before the units, the path of
197-
each extension and an array of bytes with the content of the respective extension-release file
198-
are sent. One such structure will be sent for each extension named in the input arguments. The
199-
flag value to enable this functionality is defined as follows:</para>
200-
201-
<programlisting>
202-
#define PORTABLE_INSPECT_EXTENSION_RELEASES (UINT64_C(1) &lt;&lt; 1)
203-
</programlisting>
204-
193+
<para><function>GetImageMetadataWithExtensions()</function> retrieves metadata associated with an
194+
image. This method is a superset of <function>GetImageMetadata()</function> with the addition of a list
195+
of extensions as input parameter, which were overlaid on top of the main image via
196+
<function>AttachImageWithExtensions()</function>. The path of each extension and an array of bytes with
197+
the content of the respective extension-release file are returned, one such structure for each
198+
extension named in the input arguments.</para>
205199

206200
<para><function>GetImageState()</function> retrieves the image state as one of the following
207201
strings:
@@ -352,6 +346,7 @@ node /org/freedesktop/portable1 {
352346
in t flags,
353347
out s image,
354348
out ay os_release,
349+
out a{say} extensions,
355350
out a{say} units);
356351
GetState(out s state);
357352
GetStateWithExtensions(in as extensions,

‎src/portable/portable.h‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,13 @@ typedef struct PortableMetadata {
2121
#define PORTABLE_METADATA_IS_UNIT(m) (!IN_SET((m)->name[0], 0, '/'))
2222

2323
typedef enum PortableFlags {
24-
PORTABLE_RUNTIME = 1 << 0,
25-
PORTABLE_INSPECT_EXTENSION_RELEASES = 1 << 1, /* Public API via DBUS, do not change */
26-
PORTABLE_PREFER_COPY = 1 << 2,
27-
PORTABLE_PREFER_SYMLINK = 1 << 3,
28-
PORTABLE_REATTACH = 1 << 4,
29-
_PORTABLE_MASK_PUBLIC = PORTABLE_RUNTIME | PORTABLE_INSPECT_EXTENSION_RELEASES,
24+
PORTABLE_RUNTIME = 1 << 0, /* Public API via DBUS, do not change */
25+
PORTABLE_PREFER_COPY = 1 << 1,
26+
PORTABLE_PREFER_SYMLINK = 1 << 2,
27+
PORTABLE_REATTACH = 1 << 3,
28+
_PORTABLE_MASK_PUBLIC = PORTABLE_RUNTIME,
3029
_PORTABLE_TYPE_MAX,
31-
_PORTABLE_TYPE_INVALID = -EINVAL,
30+
_PORTABLE_TYPE_INVALID = -EINVAL,
3231
} PortableFlags;
3332

3433
/* This enum is anonymous, since we usually store it in an 'int', as we overload it with negative errno

‎src/portable/portablectl.c‎

Lines changed: 76 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ static int maybe_reload(sd_bus **bus) {
260260
static int get_image_metadata(sd_bus *bus, const char *image, char **matches, sd_bus_message **reply) {
261261
_cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
262262
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
263-
PortableFlags flags = PORTABLE_INSPECT_EXTENSION_RELEASES;
263+
uint64_t flags = 0;
264264
const char *method;
265265
int r;
266266

@@ -362,78 +362,89 @@ static int inspect_image(int argc, char *argv[], void *userdata) {
362362
strna(pretty_os));
363363
}
364364

365-
r = sd_bus_message_enter_container(reply, 'a', "{say}");
366-
if (r < 0)
367-
return bus_log_parse_error(r);
368-
369-
/* If we specified any extensions, we'll first get back exactly the
370-
* paths (and extension-release content) for each one of the arguments. */
371-
for (size_t i = 0; i < strv_length(arg_extension_images); ++i) {
372-
const char *name;
365+
if (!strv_isempty(arg_extension_images)) {
366+
/* If we specified any extensions, we'll first get back exactly the paths (and
367+
* extension-release content) for each one of the arguments. */
373368

374-
r = sd_bus_message_enter_container(reply, 'e', "say");
369+
r = sd_bus_message_enter_container(reply, 'a', "{say}");
375370
if (r < 0)
376371
return bus_log_parse_error(r);
377-
if (r == 0)
378-
break;
379372

380-
r = sd_bus_message_read(reply, "s", &name);
381-
if (r < 0)
382-
return bus_log_parse_error(r);
373+
for (size_t i = 0; i < strv_length(arg_extension_images); ++i) {
374+
const char *name;
383375

384-
r = sd_bus_message_read_array(reply, 'y', &data, &sz);
385-
if (r < 0)
386-
return bus_log_parse_error(r);
376+
r = sd_bus_message_enter_container(reply, 'e', "say");
377+
if (r < 0)
378+
return bus_log_parse_error(r);
379+
if (r == 0)
380+
break;
387381

388-
if (arg_cat) {
389-
if (nl)
390-
fputc('\n', stdout);
382+
r = sd_bus_message_read(reply, "s", &name);
383+
if (r < 0)
384+
return bus_log_parse_error(r);
391385

392-
printf("%s-- Extension Release: %s --%s\n", ansi_highlight(), name, ansi_normal());
393-
fwrite(data, sz, 1, stdout);
394-
fflush(stdout);
395-
nl = true;
396-
} else {
397-
_cleanup_free_ char *pretty_portable = NULL, *pretty_os = NULL, *sysext_level = NULL,
398-
*id = NULL, *version_id = NULL, *sysext_scope = NULL, *portable_prefixes = NULL;
399-
_cleanup_fclose_ FILE *f = NULL;
400-
401-
f = fmemopen_unlocked((void*) data, sz, "re");
402-
if (!f)
403-
return log_error_errno(errno, "Failed to open extension-release buffer: %m");
404-
405-
r = parse_env_file(f, name,
406-
"ID", &id,
407-
"VERSION_ID", &version_id,
408-
"SYSEXT_SCOPE", &sysext_scope,
409-
"SYSEXT_LEVEL", &sysext_level,
410-
"PORTABLE_PRETTY_NAME", &pretty_portable,
411-
"PORTABLE_PREFIXES", &portable_prefixes,
412-
"PRETTY_NAME", &pretty_os);
386+
r = sd_bus_message_read_array(reply, 'y', &data, &sz);
387+
if (r < 0)
388+
return bus_log_parse_error(r);
389+
390+
if (arg_cat) {
391+
if (nl)
392+
fputc('\n', stdout);
393+
394+
printf("%s-- Extension Release: %s --%s\n", ansi_highlight(), name, ansi_normal());
395+
fwrite(data, sz, 1, stdout);
396+
fflush(stdout);
397+
nl = true;
398+
} else {
399+
_cleanup_free_ char *pretty_portable = NULL, *pretty_os = NULL, *sysext_level = NULL,
400+
*id = NULL, *version_id = NULL, *sysext_scope = NULL, *portable_prefixes = NULL;
401+
_cleanup_fclose_ FILE *f = NULL;
402+
403+
f = fmemopen_unlocked((void*) data, sz, "re");
404+
if (!f)
405+
return log_error_errno(errno, "Failed to open extension-release buffer: %m");
406+
407+
r = parse_env_file(f, name,
408+
"ID", &id,
409+
"VERSION_ID", &version_id,
410+
"SYSEXT_SCOPE", &sysext_scope,
411+
"SYSEXT_LEVEL", &sysext_level,
412+
"PORTABLE_PRETTY_NAME", &pretty_portable,
413+
"PORTABLE_PREFIXES", &portable_prefixes,
414+
"PRETTY_NAME", &pretty_os);
415+
if (r < 0)
416+
return log_error_errno(r, "Failed to parse extension release from '%s': %m", name);
417+
418+
printf("Extension:\n\t%s\n"
419+
"\tExtension Scope:\n\t\t%s\n"
420+
"\tExtension Compatibility Level:\n\t\t%s\n"
421+
"\tPortable Service:\n\t\t%s\n"
422+
"\tPortable Prefixes:\n\t\t%s\n"
423+
"\tOperating System:\n\t\t%s (%s %s)\n",
424+
name,
425+
strna(sysext_scope),
426+
strna(sysext_level),
427+
strna(pretty_portable),
428+
strna(portable_prefixes),
429+
strna(pretty_os),
430+
strna(id),
431+
strna(version_id));
432+
}
433+
434+
r = sd_bus_message_exit_container(reply);
413435
if (r < 0)
414-
return log_error_errno(r, "Failed to parse extension release from '%s': %m", name);
415-
416-
printf("Extension:\n\t%s\n"
417-
"\tExtension Scope:\n\t\t%s\n"
418-
"\tExtension Compatibility Level:\n\t\t%s\n"
419-
"\tPortable Service:\n\t\t%s\n"
420-
"\tPortable Prefixes:\n\t\t%s\n"
421-
"\tOperating System:\n\t\t%s (%s %s)\n",
422-
name,
423-
strna(sysext_scope),
424-
strna(sysext_level),
425-
strna(pretty_portable),
426-
strna(portable_prefixes),
427-
strna(pretty_os),
428-
strna(id),
429-
strna(version_id));
436+
return bus_log_parse_error(r);
430437
}
431438

432439
r = sd_bus_message_exit_container(reply);
433440
if (r < 0)
434441
return bus_log_parse_error(r);
435442
}
436443

444+
r = sd_bus_message_enter_container(reply, 'a', "{say}");
445+
if (r < 0)
446+
return bus_log_parse_error(r);
447+
437448
for (;;) {
438449
const char *name;
439450

@@ -764,18 +775,17 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) {
764775
if (r < 0)
765776
return bus_log_parse_error(r);
766777

767-
r = sd_bus_message_enter_container(reply, 'a', "{say}");
768-
if (r < 0)
769-
return bus_log_parse_error(r);
770-
771-
/* If we specified any extensions, we'll first get back exactly the
772-
* paths (and extension-release content) for each one of the arguments. */
773-
for (size_t i = 0; i < strv_length(arg_extension_images); ++i) {
774-
r = sd_bus_message_skip(reply, "{say}");
778+
/* If we specified any extensions, we'll first an array of extension-release metadata. */
779+
if (!strv_isempty(arg_extension_images)) {
780+
r = sd_bus_message_skip(reply, "a{say}");
775781
if (r < 0)
776782
return bus_log_parse_error(r);
777783
}
778784

785+
r = sd_bus_message_enter_container(reply, 'a', "{say}");
786+
if (r < 0)
787+
return bus_log_parse_error(r);
788+
779789
for (;;) {
780790
const char *name;
781791

‎src/portable/portabled-bus.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ const sd_bus_vtable manager_vtable[] = {
441441
"t", flags),
442442
SD_BUS_RESULT("s", image,
443443
"ay", os_release,
444+
"a{say}", extensions,
444445
"a{say}", units),
445446
method_get_image_metadata,
446447
SD_BUS_VTABLE_UNPRIVILEGED),

‎src/portable/portabled-image-bus.c‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ int bus_image_common_get_metadata(
108108
_cleanup_hashmap_free_ Hashmap *unit_files = NULL;
109109
_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
110110
_cleanup_free_ PortableMetadata **sorted = NULL;
111-
PortableFlags flags = 0;
112111
int r;
113112

114113
assert(name_or_path || image);
@@ -119,8 +118,10 @@ int bus_image_common_get_metadata(
119118
m = image->userdata;
120119
}
121120

122-
if (sd_bus_message_is_method_call(message, NULL, "GetImageMetadataWithExtensions") ||
123-
sd_bus_message_is_method_call(message, NULL, "GetMetadataWithExtensions")) {
121+
bool have_exti = sd_bus_message_is_method_call(message, NULL, "GetImageMetadataWithExtensions") ||
122+
sd_bus_message_is_method_call(message, NULL, "GetMetadataWithExtensions");
123+
124+
if (have_exti) {
124125
r = sd_bus_message_read_strv(message, &extension_images);
125126
if (r < 0)
126127
return r;
@@ -130,8 +131,7 @@ int bus_image_common_get_metadata(
130131
if (r < 0)
131132
return r;
132133

133-
if (sd_bus_message_is_method_call(message, NULL, "GetImageMetadataWithExtensions") ||
134-
sd_bus_message_is_method_call(message, NULL, "GetMetadataWithExtensions")) {
134+
if (have_exti) {
135135
uint64_t input_flags = 0;
136136

137137
r = sd_bus_message_read(message, "t", &input_flags);
@@ -142,7 +142,6 @@ int bus_image_common_get_metadata(
142142
return sd_bus_reply_method_errorf(message, SD_BUS_ERROR_INVALID_ARGS,
143143
"Invalid 'flags' parameter '%" PRIu64 "'",
144144
input_flags);
145-
flags |= input_flags;
146145
}
147146

148147
r = bus_image_acquire(m,
@@ -186,16 +185,16 @@ int bus_image_common_get_metadata(
186185
if (r < 0)
187186
return r;
188187

189-
r = sd_bus_message_open_container(reply, 'a', "{say}");
190-
if (r < 0)
191-
return r;
192-
193188
/* If it was requested, also send back the extension path and the content
194189
* of each extension-release file. Behind a flag, as it's an incompatible
195190
* change. */
196-
if (FLAGS_SET(flags, PORTABLE_INSPECT_EXTENSION_RELEASES)) {
191+
if (have_exti) {
197192
PortableMetadata *extension_release;
198193

194+
r = sd_bus_message_open_container(reply, 'a', "{say}");
195+
if (r < 0)
196+
return r;
197+
199198
ORDERED_HASHMAP_FOREACH(extension_release, extension_releases) {
200199

201200
r = sd_bus_message_open_container(reply, 'e', "say");
@@ -214,8 +213,16 @@ int bus_image_common_get_metadata(
214213
if (r < 0)
215214
return r;
216215
}
216+
217+
r = sd_bus_message_close_container(reply);
218+
if (r < 0)
219+
return r;
217220
}
218221

222+
r = sd_bus_message_open_container(reply, 'a', "{say}");
223+
if (r < 0)
224+
return r;
225+
219226
for (size_t i = 0; i < hashmap_size(unit_files); i++) {
220227

221228
r = sd_bus_message_open_container(reply, 'e', "say");
@@ -888,6 +895,7 @@ const sd_bus_vtable image_vtable[] = {
888895
"t", flags),
889896
SD_BUS_RESULT("s", image,
890897
"ay", os_release,
898+
"a{say}", extensions,
891899
"a{say}", units),
892900
bus_image_method_get_metadata,
893901
SD_BUS_VTABLE_UNPRIVILEGED),

0 commit comments

Comments
 (0)
X Tutup