From f4a3acb6ed214b183e1931a81dbf6eb0af913aa8 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 21 May 2026 11:14:01 +0100 Subject: [PATCH 1/3] sysupdate: Convert an internal bool argument to flags This commit makes no functional changes, but the new flags will be used and extended in the following commit. We need a flags variable to avoid having two bool arguments, which would be confusing. Signed-off-by: Philip Withnall Helps: https://github.com/systemd/systemd/issues/41501 --- src/sysupdate/sysupdate.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 425460a319..159755e8b4 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -171,7 +171,11 @@ static int read_definitions( return 0; } -static int context_read_definitions(Context *c, const char* node, bool requires_enabled_transfers) { +typedef enum ReadDefinitionsFlags { + READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS = 1 << 0, +} ReadDefinitionsFlags; + +static int context_read_definitions(Context *c, const char* node, ReadDefinitionsFlags flags) { _cleanup_strv_free_ char **dirs = NULL; int r; @@ -244,7 +248,7 @@ static int context_read_definitions(Context *c, const char* node, bool requires_ log_warning("As of v257, transfer definitions should have the '.transfer' extension."); } - if (c->n_transfers + (requires_enabled_transfers ? 0 : c->n_disabled_transfers) == 0) { + if (c->n_transfers + (FLAGS_SET(flags, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS) ? 0 : c->n_disabled_transfers) == 0) { if (arg_component) return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "No transfer definitions for component '%s' found.", @@ -925,7 +929,7 @@ static int context_vacuum( return 0; } -static int context_make_offline(Context **ret, const char *node, bool requires_enabled_transfers) { +static int context_make_offline(Context **ret, const char *node, ReadDefinitionsFlags read_definitions_flags) { _cleanup_(context_freep) Context* context = NULL; int r; @@ -938,7 +942,7 @@ static int context_make_offline(Context **ret, const char *node, bool requires_e if (!context) return log_oom(); - r = context_read_definitions(context, node, requires_enabled_transfers); + r = context_read_definitions(context, node, read_definitions_flags); if (r < 0) return r; @@ -959,7 +963,7 @@ static int context_make_online(Context **ret, const char *node) { /* Like context_make_offline(), but also communicates with the update source looking for new * versions (as long as --offline is not specified on the command line). */ - r = context_make_offline(&context, node, /* requires_enabled_transfers= */ true); + r = context_make_offline(&context, node, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS); if (r < 0) return r; @@ -1365,7 +1369,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata if (r < 0) return r; - r = context_make_offline(&context, loop_device ? loop_device->node : NULL, /* requires_enabled_transfers= */ false); + r = context_make_offline(&context, loop_device ? loop_device->node : NULL, 0); if (r < 0) return r; @@ -1633,7 +1637,7 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) if (r < 0) return r; - r = context_make_offline(&context, loop_device ? loop_device->node : NULL, /* requires_enabled_transfers= */ false); + r = context_make_offline(&context, loop_device ? loop_device->node : NULL, 0); if (r < 0) return r; @@ -1655,7 +1659,7 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --root=/--image= switches may not be combined with the '%s' operation.", argv[0]); - r = context_make_offline(&context, /* node= */ NULL, /* requires_enabled_transfers= */ true); + r = context_make_offline(&context, /* node= */ NULL, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS); if (r < 0) return r; From a7a1bc8678050ae922d6de688bfa6ea5f241b44f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 21 May 2026 11:17:56 +0100 Subject: [PATCH 2/3] sysupdate: Add a flag to control error behaviour in internal function Optionally prevent `context_read_definitions()` erroring out if zero transfer definitions were found. This commit makes no functional changes (the flag is always passed to calls to `context_make_offline()` for the moment), but the new flag will be used in the following commit. Signed-off-by: Philip Withnall Helps: https://github.com/systemd/systemd/issues/41501 --- src/sysupdate/sysupdate.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 159755e8b4..7852ac3878 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -173,6 +173,7 @@ static int read_definitions( typedef enum ReadDefinitionsFlags { READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS = 1 << 0, + READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS = 1 << 1, } ReadDefinitionsFlags; static int context_read_definitions(Context *c, const char* node, ReadDefinitionsFlags flags) { @@ -248,7 +249,8 @@ static int context_read_definitions(Context *c, const char* node, ReadDefinition log_warning("As of v257, transfer definitions should have the '.transfer' extension."); } - if (c->n_transfers + (FLAGS_SET(flags, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS) ? 0 : c->n_disabled_transfers) == 0) { + if (FLAGS_SET(flags, READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS) && + c->n_transfers + (FLAGS_SET(flags, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS) ? 0 : c->n_disabled_transfers) == 0) { if (arg_component) return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "No transfer definitions for component '%s' found.", @@ -963,7 +965,8 @@ static int context_make_online(Context **ret, const char *node) { /* Like context_make_offline(), but also communicates with the update source looking for new * versions (as long as --offline is not specified on the command line). */ - r = context_make_offline(&context, node, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS); + r = context_make_offline(&context, node, + READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS | READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; @@ -1369,7 +1372,8 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata if (r < 0) return r; - r = context_make_offline(&context, loop_device ? loop_device->node : NULL, 0); + r = context_make_offline(&context, loop_device ? loop_device->node : NULL, + READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; @@ -1637,7 +1641,8 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) if (r < 0) return r; - r = context_make_offline(&context, loop_device ? loop_device->node : NULL, 0); + r = context_make_offline(&context, loop_device ? loop_device->node : NULL, + READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; @@ -1659,7 +1664,8 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --root=/--image= switches may not be combined with the '%s' operation.", argv[0]); - r = context_make_offline(&context, /* node= */ NULL, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS); + r = context_make_offline(&context, /* node= */ NULL, + READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS | READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; From 8de5a7b94d3af30b0686f14c4cc96a0575982aba Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 19 May 2026 15:46:36 +0100 Subject: [PATCH 3/3] sysupdate: List default component only if transfer definition exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sysupdate --json=short components` lists the components known to sysupdate; these are the components which something like `updatectl` will try to update. The `default` component represents the host, and is meant to be listed if transfer definitions exist in (for example) `/etc/sysupdate.d` corresponding to the host OS. This then corresponds to `TARGET_HOST` in `updatectl` and causes it to try updating that target. The logic for working out whether the `default` component was present essentially boiled down to “does `{/run,/etc,/usr/lib}/sysupdate.d` exist”, and it didn’t check whether a `.transfer` or `.conf` file actually existed in the config directory. This is quite the corner case, but becomes more evident on systems where sysupdate is being used to update a portable service but not the main OS. At that point, if `/etc/sysupdate.d` exists empty (for some reason), `updatectl` falls over because it starts trying to update the host OS without any configuration to do so. So, modify `sysupdate` to more fully load the available configuration when listing components, and query it a bit more deeply to check whether a default component exists. If `sysupdate` is called with various command line arguments to affect how its configuration is loaded, do *not* say that a default component exists, as these arguments essentially anull the possibility of a default being used in that process. Add an integration test based on the reproducer provided by the issue reporter. This test has been tested to fail if the changes to `sysupdate.c` aren’t applied — if so, the second call to `sysupdate components` would return `{"default":true,"components":["some-component"]}`. Signed-off-by: Philip Withnall Fixes: https://github.com/systemd/systemd/issues/41501 --- src/sysupdate/sysupdate.c | 14 +++++++++++++- test/units/TEST-72-SYSUPDATE.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 7852ac3878..92c40ddf71 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -1731,6 +1731,7 @@ static int component_name_valid(const char *c) { VERB_NOARG(verb_components, "components", "Show list of components"); static int verb_components(int argc, char *argv[], uintptr_t _data, void *userdata) { + _cleanup_(context_freep) Context* context = NULL; _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; _cleanup_set_free_ Set *names = NULL; @@ -1743,6 +1744,10 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda if (r < 0) return r; + r = context_make_offline(&context, loop_device ? loop_device->node : NULL, 0); + if (r < 0) + return r; + ConfFile **directories = NULL; size_t n_directories = 0; @@ -1757,7 +1762,6 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda ConfFile *e = *i; if (streq(e->filename, "sysupdate.d")) { - has_default_component = true; continue; } @@ -1785,6 +1789,14 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda TAKE_PTR(n); } + /* Does the system have at least one transfer file in /etc/sysupdate.d, which can be considered a + * TARGET_HOST? See target_get_argument() in sysupdated.c */ + has_default_component = (!arg_definitions && + !arg_component && + !arg_root && + !arg_image && + context->n_transfers > 0); + /* We use simple free() rather than strv_free() here, since set_free() will free the strings for us */ _cleanup_free_ char **z = set_get_strv(names); if (!z) diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index 21a3ea00a5..e501a7647f 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -552,6 +552,35 @@ EOF done done +# Regression test for https://github.com/systemd/systemd/issues/41501 — check +# that a ‘default’ component is only listed by sysupdate if it’s fully configured +mv "$CONFIGDIR" "$CONFIGDIR.backup" +mkdir -p /run/sysupdate.some-component.d +tee /run/sysupdate.some-component.d/portable.transfer << EOF +[Transfer] +ChangeLog=https://example.com/changelog/@v +Verify=no + +[Source] +Type=url-tar +Path=https://example.com/does-not-matter/@v.tar.xz +MatchPattern=some-component_@v-portable.tar.xz + +[Target] +Type=directory +Path=/var/lib/portables +MatchPattern=some-component_@v +CurrentSymlink=some-component +EOF +"$SYSUPDATE" --json=short components | grep -F '{"default":false,"components":["some-component"]}' >/dev/null +mkdir /run/sysupdate.d +"$SYSUPDATE" --json=short components | grep -F '{"default":false,"components":["some-component"]}' >/dev/null + +# Clean up regression test +rmdir /run/sysupdate.d +rm -rf /run/sysupdate.some-component.d +mv "$CONFIGDIR.backup" "$CONFIGDIR" + # Make sure the processing of compressed streams still handles uncompressed streams shorter than # COMPRESSION_MAGIC_BYTES_MAX correctly. rm -rf "$CONFIGDIR" "$WORKDIR/blobs"