From 23bfc4b9e8211719e9b379e4c9c42b473017653c Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 24 Jun 2026 13:10:35 +0100 Subject: [PATCH 01/15] sysupdate: Fix an early return path return value If the FD has already been opened, return 1 as if opening was successful, rather than returning 0 as if it gave `ENOENT`. This fixes doing multiple installdb operations on a single `Context`. Signed-off-by: Philip Withnall --- src/sysupdate/sysupdate-cleanup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sysupdate/sysupdate-cleanup.c b/src/sysupdate/sysupdate-cleanup.c index fac2a0b1b3..74e2a38119 100644 --- a/src/sysupdate/sysupdate-cleanup.c +++ b/src/sysupdate/sysupdate-cleanup.c @@ -57,7 +57,7 @@ static int context_installdb_acquire_fd(Context *c, bool make) { assert(c); if (c->installdb_fd >= 0) - return 0; + return 1; _cleanup_free_ char *j = NULL; const char *p; From 7f3c27128b4db379c397904defdb9b2d5ac3f05b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 23 Jun 2026 16:23:54 +0100 Subject: [PATCH 02/15] sysupdate: Factor context creation out of installdb_cleanup_component() This makes it like all the other verbs and therefore easier to refactor. At the same time, remove the separate `component` argument and instead use the `component` set on the `Context`. This guards against bugs, as various parts of the `Context` state depend on the component (for example, `installdb_fd`) and overriding the component without also overriding its dependent variables will lead to bugs. --- src/sysupdate/sysupdate-cleanup.c | 15 ++++---------- src/sysupdate/sysupdate-cleanup.h | 2 +- src/sysupdate/sysupdate.c | 33 +++++++++++++++++++++++++++---- src/sysupdate/sysupdate.h | 7 ------- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/sysupdate/sysupdate-cleanup.c b/src/sysupdate/sysupdate-cleanup.c index 74e2a38119..5c517e401a 100644 --- a/src/sysupdate/sysupdate-cleanup.c +++ b/src/sysupdate/sysupdate-cleanup.c @@ -352,30 +352,23 @@ static int context_installdb_process_entry( return context_installdb_process_directory(c, path, /* relpath= */ NULL, dir_fd, de, pattern); } -int installdb_cleanup_component(const char *node, const char *component) { +int installdb_cleanup_component(Context *context) { int r; - _cleanup_(context_freep) Context* context = NULL; - r = context_make_offline( - &context, - node, - component, - /* read_definitions_flags= */ 0); - if (r < 0) - return r; + assert(context); r = context_installdb_acquire_fd(context, /* make= */ false); if (r < 0) return r; if (r == 0) { - log_debug("Not cleaning up component '%s', install database is empty.", strna(component)); + log_debug("Not cleaning up component '%s', install database is empty.", strna(context->component)); return 0; } _cleanup_free_ DirectoryEntries *de = NULL; r = readdir_all(context->installdb_fd, RECURSE_DIR_ENSURE_TYPE|RECURSE_DIR_MUST_BE_SYMLINK, &de); if (r < 0) - return log_error_errno(r, "Failed to enumerate install database for component '%s': %m", strna(component)); + return log_error_errno(r, "Failed to enumerate install database for component '%s': %m", strna(context->component)); int ret = 0; FOREACH_ARRAY(_d, de->entries, de->n_entries) { diff --git a/src/sysupdate/sysupdate-cleanup.h b/src/sysupdate/sysupdate-cleanup.h index 98fbb0a83b..cb3c7b6444 100644 --- a/src/sysupdate/sysupdate-cleanup.h +++ b/src/sysupdate/sysupdate-cleanup.h @@ -5,5 +5,5 @@ int context_installdb_record(Context *c, const char *path, char **patterns); -int installdb_cleanup_component(const char *node, const char *component); +int installdb_cleanup_component(Context *context); int installdb_list_components(char ***ret); diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 9f78ee78bf..151aa24d57 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -171,6 +171,11 @@ static int read_definitions( return 0; } +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) { _cleanup_strv_free_ char **dirs = NULL; int r; @@ -926,7 +931,7 @@ static int context_vacuum( return 0; } -int context_make_offline( +static int context_make_offline( Context **ret, const char *node, const char *component, @@ -1929,6 +1934,7 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda VERB_NOARG(verb_cleanup, "cleanup", "Clean up orphaned files"); static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) { + _cleanup_(context_freep) Context* context = NULL; int r; assert(argc <= 1); @@ -1944,8 +1950,16 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) const char *node = loop_device ? loop_device->node : NULL; + r = context_make_offline( + &context, + node, + arg_component, + /* read_definitions_flags= */ 0); + if (r < 0) + return r; + int ret = 0; - RET_GATHER(ret, installdb_cleanup_component(node, arg_component)); + RET_GATHER(ret, installdb_cleanup_component(context)); if (arg_component_all) { _cleanup_strv_free_ char **z = NULL; @@ -1953,8 +1967,19 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) if (r < 0) return log_error_errno(r, "Failed to enumerate components: %m"); - STRV_FOREACH(i, z) - RET_GATHER(ret, installdb_cleanup_component(node, *i)); + STRV_FOREACH(i, z) { + _cleanup_(context_freep) Context* component_context = NULL; + + r = context_make_offline( + &component_context, + node, + *i, + /* read_definitions_flags= */ 0); + if (r < 0) + return r; + + RET_GATHER(ret, installdb_cleanup_component(component_context)); + } } return ret; diff --git a/src/sysupdate/sysupdate.h b/src/sysupdate/sysupdate.h index b925d4f5a2..68b8a79687 100644 --- a/src/sysupdate/sysupdate.h +++ b/src/sysupdate/sysupdate.h @@ -28,13 +28,6 @@ typedef struct Context { Context* context_free(Context *c); DEFINE_TRIVIAL_CLEANUP_FUNC(Context*, context_free); -typedef enum ReadDefinitionsFlags { - READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS = 1 << 0, - READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS = 1 << 1, -} ReadDefinitionsFlags; - -int context_make_offline(Context **ret, const char *node, const char *component, ReadDefinitionsFlags read_definitions_flags); - extern bool arg_sync; extern uint64_t arg_instances_max; extern char *arg_root; From 49cf024c0be1f660e9600499074758aa03ff4b48 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 29 May 2026 12:59:54 +0100 Subject: [PATCH 03/15] sysupdate: Factor process_image() into context_make_{offline,online}() `process_image()` is always called immediately before (almost) every `context_make_online()` or `context_make_offline()`, and the structures it allocates have the same lifetime as `Context`, so we might as well factor them all together to reduce duplication. This will also simplify the following commit, which changes heap allocation of `Context`s, and simplify upcoming changes to factor out `arg_*` handling. The call in `verb_pending_or_reboot()` is safe because it already validates that `arg_image` is `NULL`, hence `process_image()` will bail out early. This introduces no functional changes. --- src/sysupdate/sysupdate.c | 182 ++++++++++++++++---------------------- src/sysupdate/sysupdate.h | 3 + 2 files changed, 77 insertions(+), 108 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 151aa24d57..3fe03cd6a4 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -78,6 +78,9 @@ Context* context_free(Context *c) { free(c->component); + c->mounted_dir = umount_and_rmdir_and_free(c->mounted_dir); + c->loop_device = loop_device_unref(c->loop_device); + FOREACH_ARRAY(tr, c->transfers, c->n_transfers) transfer_free(*tr); free(c->transfers); @@ -931,10 +934,59 @@ static int context_vacuum( return 0; } +typedef enum ProcessImageFlags { + PROCESS_IMAGE_READ_ONLY = 1 << 0, +} ProcessImageFlags; + +static int process_image( + Context *c, + ProcessImageFlags flags) { + + _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; + _cleanup_(umount_and_freep) char *mounted_dir = NULL; + int r; + + assert(c); + + if (!arg_image) + return 0; + + assert(!arg_root); + assert(!c->mounted_dir); + assert(!c->loop_device); + + r = mount_image_privately_interactively( + arg_image, + arg_image_policy, + (FLAGS_SET(flags, PROCESS_IMAGE_READ_ONLY) ? DISSECT_IMAGE_READ_ONLY : 0) | + DISSECT_IMAGE_FSCK | + DISSECT_IMAGE_MKDIR | + DISSECT_IMAGE_GROWFS | + DISSECT_IMAGE_RELAX_VAR_CHECK | + DISSECT_IMAGE_USR_NO_ROOT | + DISSECT_IMAGE_GENERIC_ROOT | + DISSECT_IMAGE_REQUIRE_ROOT | + DISSECT_IMAGE_ALLOW_USERSPACE_VERITY, + &mounted_dir, + /* ret_dir_fd= */ NULL, + &loop_device); + if (r < 0) + return r; + + arg_root = strdup(mounted_dir); + if (!arg_root) + return log_oom(); + + c->mounted_dir = TAKE_PTR(mounted_dir); + c->loop_device = TAKE_PTR(loop_device); + + return 0; +} + static int context_make_offline( Context **ret, - const char *node, const char *component, + ProcessImageFlags process_image_flags, ReadDefinitionsFlags read_definitions_flags) { _cleanup_(context_freep) Context* context = NULL; int r; @@ -952,7 +1004,11 @@ static int context_make_offline( if (r < 0) return r; - r = context_read_definitions(context, node, read_definitions_flags); + r = process_image(context, process_image_flags); + if (r < 0) + return r; + + r = context_read_definitions(context, context->loop_device ? context->loop_device->node : NULL, read_definitions_flags); if (r < 0) return r; @@ -966,8 +1022,8 @@ static int context_make_offline( static int context_make_online( Context **ret, - const char *node, - const char *component) { + const char *component, + ProcessImageFlags process_image_flags) { _cleanup_(context_freep) Context* context = NULL; int r; @@ -979,8 +1035,8 @@ static int context_make_online( r = context_make_offline( &context, - node, component, + process_image_flags, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS|READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; @@ -1333,56 +1389,9 @@ static int context_install( return 1; } -static int process_image( - bool ro, - char **ret_mounted_dir, - LoopDevice **ret_loop_device) { - - _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_freep) char *mounted_dir = NULL; - int r; - - assert(ret_mounted_dir); - assert(ret_loop_device); - - if (!arg_image) - return 0; - - assert(!arg_root); - - r = mount_image_privately_interactively( - arg_image, - arg_image_policy, - (ro ? DISSECT_IMAGE_READ_ONLY : 0) | - DISSECT_IMAGE_FSCK | - DISSECT_IMAGE_MKDIR | - DISSECT_IMAGE_GROWFS | - DISSECT_IMAGE_RELAX_VAR_CHECK | - DISSECT_IMAGE_USR_NO_ROOT | - DISSECT_IMAGE_GENERIC_ROOT | - DISSECT_IMAGE_REQUIRE_ROOT | - DISSECT_IMAGE_ALLOW_USERSPACE_VERITY, - &mounted_dir, - /* ret_dir_fd= */ NULL, - &loop_device); - if (r < 0) - return r; - - arg_root = strdup(mounted_dir); - if (!arg_root) - return log_oom(); - - *ret_mounted_dir = TAKE_PTR(mounted_dir); - *ret_loop_device = TAKE_PTR(loop_device); - - return 0; -} - VERB(verb_list, "list", "[VERSION]", VERB_ANY, 2, VERB_DEFAULT, "Show installed and available versions"); static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; _cleanup_(context_freep) Context* context = NULL; _cleanup_strv_free_ char **appstream_urls = NULL; const char *version; @@ -1394,14 +1403,10 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = process_image(/* ro= */ true, &mounted_dir, &loop_device); - if (r < 0) - return r; - r = context_make_online( &context, - loop_device ? loop_device->node : NULL, - arg_component); + arg_component, + PROCESS_IMAGE_READ_ONLY); if (r < 0) return r; @@ -1457,8 +1462,6 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { VERB(verb_features, "features", "[FEATURE]", VERB_ANY, 2, 0, "Show optional features"); static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; _cleanup_(context_freep) Context* context = NULL; _cleanup_(table_unrefp) Table *table = NULL; const char *feature_id; @@ -1471,14 +1474,10 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = process_image(/* ro= */ true, &mounted_dir, &loop_device); - if (r < 0) - return r; - r = context_make_offline( &context, - loop_device ? loop_device->node : NULL, arg_component, + PROCESS_IMAGE_READ_ONLY, READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; @@ -1600,8 +1599,6 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata VERB_NOARG(verb_check_new, "check-new", "Check if there's a new version available"); static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; _cleanup_(context_freep) Context* context = NULL; int r; @@ -1610,14 +1607,10 @@ static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdat if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = process_image(/* ro= */ true, &mounted_dir, &loop_device); - if (r < 0) - return r; - r = context_make_online( &context, - loop_device ? loop_device->node : NULL, - arg_component); + arg_component, + PROCESS_IMAGE_READ_ONLY); if (r < 0) return r; @@ -1652,8 +1645,6 @@ typedef enum { } UpdateActionFlags; static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flags) { - _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; _cleanup_(context_freep) Context* context = NULL; _cleanup_free_ char *booted_version = NULL; UpdateSet *applied = NULL; @@ -1680,18 +1671,13 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag return log_error_errno(SYNTHETIC_ERRNO(ENODATA), "/etc/os-release lacks IMAGE_VERSION field."); } - r = process_image(/* ro= */ false, &mounted_dir, &loop_device); - if (r < 0) - return r; - - const char *node = loop_device ? loop_device->node : NULL; bool installed = false; int ret = 0; r = context_make_online( &context, - node, - arg_component); + arg_component, + /* process_image_flags= */ 0); if (r < 0) { if (r != -ENOENT) return r; @@ -1727,7 +1713,7 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag } if (arg_cleanup > 0) - RET_GATHER(ret, installdb_cleanup_component(node, arg_component)); + RET_GATHER(ret, installdb_cleanup_component(context)); if (installed) { /* We installed something, yay */ @@ -1771,8 +1757,6 @@ static int verb_acquire(int argc, char *argv[], uintptr_t _data, void *userdata) VERB_NOARG(verb_vacuum, "vacuum", "Make room, by deleting old versions"); static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; _cleanup_(context_freep) Context* context = NULL; int r; @@ -1785,14 +1769,10 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --instances-max argument must be >= 1 while vacuuming"); - r = process_image(/* ro= */ false, &mounted_dir, &loop_device); - if (r < 0) - return r; - r = context_make_offline( &context, - loop_device ? loop_device->node : NULL, arg_component, + /* process_image_flags= */ 0, READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; @@ -1821,8 +1801,8 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void r = context_make_offline( &context, - /* node= */ NULL, arg_component, + /* process_image_flags= */ 0, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS|READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; @@ -1869,8 +1849,6 @@ 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; bool has_default_component = false; int r; @@ -1879,14 +1857,10 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = process_image(/* ro= */ false, &mounted_dir, &loop_device); - if (r < 0) - return r; - r = context_make_offline( &context, - loop_device ? loop_device->node : NULL, arg_component, + /* process_image_flags= */ 0, /* read_definitions_flags= */ 0); if (r < 0) return r; @@ -1942,18 +1916,10 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) if (arg_cleanup == 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invocation of 'cleanup' with --cleanup=no is contradictory, refusing."); - _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; - r = process_image(/* ro= */ false, &mounted_dir, &loop_device); - if (r < 0) - return r; - - const char *node = loop_device ? loop_device->node : NULL; - r = context_make_offline( &context, - node, arg_component, + /* process_image_flags= */ 0, /* read_definitions_flags= */ 0); if (r < 0) return r; @@ -1972,8 +1938,8 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) r = context_make_offline( &component_context, - node, *i, + /* process_image_flags= */ 0, /* read_definitions_flags= */ 0); if (r < 0) return r; diff --git a/src/sysupdate/sysupdate.h b/src/sysupdate/sysupdate.h index 68b8a79687..ccf1b2dc12 100644 --- a/src/sysupdate/sysupdate.h +++ b/src/sysupdate/sysupdate.h @@ -7,6 +7,9 @@ typedef struct Context { char *component; + LoopDevice *loop_device; + char *mounted_dir; + Transfer **transfers; size_t n_transfers; From ed088f820e1586d1283e4f7ac02c0e064761a71c Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 29 May 2026 13:26:47 +0100 Subject: [PATCH 04/15] sysupdate: Change Context to be stack allocated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There’s no need for it to be heap allocated — there’s only ever one instance of it, and it’s allocated for the lifetime of a `verb_*()` function. Simplify things a bit by making it stack allocated. This will also help with upcoming commits where we introduce derived context structs to help with varlinkifying sysupdate. By allowing `Context` to be stack allocated we can include it in the derived context structs. As part of this, rename `context_make_{offline,online}()` to `context_load_{offline,online}()` for clarity (since they no longer init the struct). This introduces no functional changes. --- src/sysupdate/sysupdate.c | 164 +++++++++++++++++--------------------- src/sysupdate/sysupdate.h | 3 +- 2 files changed, 75 insertions(+), 92 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 3fe03cd6a4..74fe267036 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -72,46 +72,39 @@ const Specifier specifier_table[] = { {} }; -Context* context_free(Context *c) { - if (!c) - return NULL; +#define CONTEXT_NULL \ + (Context) { \ + .installdb_fd = -EBADF, \ + } - free(c->component); +void context_done(Context *c) { + assert(c); + + c->component = mfree(c->component); c->mounted_dir = umount_and_rmdir_and_free(c->mounted_dir); c->loop_device = loop_device_unref(c->loop_device); FOREACH_ARRAY(tr, c->transfers, c->n_transfers) transfer_free(*tr); - free(c->transfers); + c->transfers = mfree(c->transfers); + c->n_transfers = 0; FOREACH_ARRAY(tr, c->disabled_transfers, c->n_disabled_transfers) transfer_free(*tr); - free(c->disabled_transfers); + c->disabled_transfers = mfree(c->disabled_transfers); + c->n_disabled_transfers = 0; - hashmap_free(c->features); + c->features = hashmap_free(c->features); FOREACH_ARRAY(us, c->update_sets, c->n_update_sets) update_set_free(*us); - free(c->update_sets); + c->update_sets = mfree(c->update_sets); + c->n_update_sets = 0; - hashmap_free(c->web_cache); + c->web_cache = hashmap_free(c->web_cache); - safe_close(c->installdb_fd); - - return mfree(c); -} - -static Context* context_new(void) { - Context *c = new(Context, 1); - if (!c) - return NULL; - - *c = (Context) { - .installdb_fd = -EBADF, - }; - - return c; + c->installdb_fd = safe_close(c->installdb_fd); } static DEFINE_POINTER_ARRAY_FREE_FUNC(Transfer*, transfer_free); @@ -983,23 +976,18 @@ static int process_image( return 0; } -static int context_make_offline( - Context **ret, +static int context_load_offline( + Context *context, const char *component, ProcessImageFlags process_image_flags, ReadDefinitionsFlags read_definitions_flags) { - _cleanup_(context_freep) Context* context = NULL; int r; - assert(ret); + assert(context); - /* Allocates a context object and initializes everything we can initialize offline, i.e. without + /* Sets up a context object and initializes everything we can initialize offline, i.e. without * checking on the update source (i.e. the Internet) what versions are available */ - context = context_new(); - if (!context) - return log_oom(); - r = free_and_strdup_warn(&context->component, component); if (r < 0) return r; @@ -1016,25 +1004,22 @@ static int context_make_offline( if (r < 0) return r; - *ret = TAKE_PTR(context); return 0; } -static int context_make_online( - Context **ret, +static int context_load_online( + Context *context, const char *component, ProcessImageFlags process_image_flags) { - - _cleanup_(context_freep) Context* context = NULL; int r; - assert(ret); + assert(context); - /* Like context_make_offline(), but also communicates with the update source looking for new + /* Like context_load_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, + r = context_load_offline( + context, component, process_image_flags, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS|READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); @@ -1051,7 +1036,6 @@ static int context_make_online( if (r < 0) return r; - *ret = TAKE_PTR(context); return 0; } @@ -1392,7 +1376,7 @@ static int context_install( VERB(verb_list, "list", "[VERSION]", VERB_ANY, 2, VERB_DEFAULT, "Show installed and available versions"); static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(context_freep) Context* context = NULL; + _cleanup_(context_done) Context context = CONTEXT_NULL; _cleanup_strv_free_ char **appstream_urls = NULL; const char *version; int r; @@ -1403,7 +1387,7 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = context_make_online( + r = context_load_online( &context, arg_component, PROCESS_IMAGE_READ_ONLY); @@ -1411,16 +1395,16 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { return r; if (version) - return context_show_version(context, version); + return context_show_version(&context, version); else if (!sd_json_format_enabled(arg_json_format_flags)) - return context_show_table(context); + return context_show_table(&context); else { _cleanup_(sd_json_variant_unrefp) sd_json_variant *json = NULL; _cleanup_strv_free_ char **versions = NULL; const char *current = NULL; bool current_is_pending = false; - FOREACH_ARRAY(update_set, context->update_sets, context->n_update_sets) { + FOREACH_ARRAY(update_set, context.update_sets, context.n_update_sets) { UpdateSet *us = *update_set; if (FLAGS_SET(us->flags, UPDATE_INSTALLED) && @@ -1434,7 +1418,7 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { return log_oom(); } - FOREACH_ARRAY(tr, context->transfers, context->n_transfers) + FOREACH_ARRAY(tr, context.transfers, context.n_transfers) STRV_FOREACH(appstream_url, (*tr)->appstream) { /* Avoid duplicates */ if (strv_contains(appstream_urls, *appstream_url)) @@ -1462,7 +1446,7 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { VERB(verb_features, "features", "[FEATURE]", VERB_ANY, 2, 0, "Show optional features"); static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(context_freep) Context* context = NULL; + _cleanup_(context_done) Context context = CONTEXT_NULL; _cleanup_(table_unrefp) Table *table = NULL; const char *feature_id; Feature *f; @@ -1474,7 +1458,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = context_make_offline( + r = context_load_offline( &context, arg_component, PROCESS_IMAGE_READ_ONLY, @@ -1485,7 +1469,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata if (feature_id) { _cleanup_strv_free_ char **transfers = NULL; - f = hashmap_get(context->features, feature_id); + f = hashmap_get(context.features, feature_id); if (!f) return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "Optional feature not found: %s", @@ -1495,7 +1479,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata if (!table) return log_oom(); - FOREACH_ARRAY(tr, context->transfers, context->n_transfers) { + FOREACH_ARRAY(tr, context.transfers, context.n_transfers) { Transfer *t = *tr; if (!strv_contains(t->features, f->id) && !strv_contains(t->requisite_features, f->id)) @@ -1506,7 +1490,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata return log_oom(); } - FOREACH_ARRAY(tr, context->disabled_transfers, context->n_disabled_transfers) { + FOREACH_ARRAY(tr, context.disabled_transfers, context.n_disabled_transfers) { Transfer *t = *tr; if (!strv_contains(t->features, f->id) && !strv_contains(t->requisite_features, f->id)) @@ -1561,7 +1545,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata if (!table) return log_oom(); - HASHMAP_FOREACH(f, context->features) { + HASHMAP_FOREACH(f, context.features) { r = table_add_many(table, TABLE_BOOLEAN_CHECKMARK, f->enabled, TABLE_SET_COLOR, ansi_highlight_green_red(f->enabled), @@ -1578,7 +1562,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata _cleanup_(sd_json_variant_unrefp) sd_json_variant *json = NULL; _cleanup_strv_free_ char **features = NULL; - HASHMAP_FOREACH(f, context->features) { + HASHMAP_FOREACH(f, context.features) { r = strv_extend(&features, f->id); if (r < 0) return log_oom(); @@ -1599,7 +1583,7 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata VERB_NOARG(verb_check_new, "check-new", "Check if there's a new version available"); static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(context_freep) Context* context = NULL; + _cleanup_(context_done) Context context = CONTEXT_NULL; int r; assert(argc <= 1); @@ -1607,7 +1591,7 @@ static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdat if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = context_make_online( + r = context_load_online( &context, arg_component, PROCESS_IMAGE_READ_ONLY); @@ -1615,17 +1599,17 @@ static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdat return r; if (!sd_json_format_enabled(arg_json_format_flags)) { - if (!context->candidate) { + if (!context.candidate) { log_debug("No candidate found."); return EXIT_FAILURE; } - puts(context->candidate->version); + puts(context.candidate->version); } else { _cleanup_(sd_json_variant_unrefp) sd_json_variant *json = NULL; - if (context->candidate) - r = sd_json_buildo(&json, SD_JSON_BUILD_PAIR_STRING("available", context->candidate->version)); + if (context.candidate) + r = sd_json_buildo(&json, SD_JSON_BUILD_PAIR_STRING("available", context.candidate->version)); else r = sd_json_buildo(&json, SD_JSON_BUILD_PAIR_NULL("available")); if (r < 0) @@ -1645,7 +1629,7 @@ typedef enum { } UpdateActionFlags; static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flags) { - _cleanup_(context_freep) Context* context = NULL; + _cleanup_(context_done) Context context = CONTEXT_NULL; _cleanup_free_ char *booted_version = NULL; UpdateSet *applied = NULL; const char *version; @@ -1674,7 +1658,7 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag bool installed = false; int ret = 0; - r = context_make_online( + r = context_load_online( &context, arg_component, /* process_image_flags= */ 0); @@ -1686,14 +1670,14 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag RET_GATHER(ret, r); } else { if (action_flags & UPDATE_ACTION_ACQUIRE) - r = context_acquire(context, version); + r = context_acquire(&context, version); else - r = context_process_partial_and_pending(context, version); + r = context_process_partial_and_pending(&context, version); if (r < 0) return r; if (FLAGS_SET(action_flags, UPDATE_ACTION_INSTALL) && r > 0) { /* installation of update indicated */ - r = context_install(context, version, &applied); + r = context_install(&context, version, &applied); if (r < 0) return r; @@ -1708,12 +1692,12 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag if (f < 0 && f != -ENXIO) log_debug_errno(f, "Failed to parse $SYSTEMD_SYSUPDATE_FORCE_NOTIFY, ignoring: %m"); if (f > 0) - (void) context_notify_subscribers(context, /* us= */ NULL); + (void) context_notify_subscribers(&context, /* us= */ NULL); } } if (arg_cleanup > 0) - RET_GATHER(ret, installdb_cleanup_component(context)); + RET_GATHER(ret, installdb_cleanup_component(&context)); if (installed) { /* We installed something, yay */ @@ -1757,7 +1741,7 @@ static int verb_acquire(int argc, char *argv[], uintptr_t _data, void *userdata) VERB_NOARG(verb_vacuum, "vacuum", "Make room, by deleting old versions"); static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(context_freep) Context* context = NULL; + _cleanup_(context_done) Context context = CONTEXT_NULL; int r; assert(argc <= 1); @@ -1769,7 +1753,7 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --instances-max argument must be >= 1 while vacuuming"); - r = context_make_offline( + r = context_load_offline( &context, arg_component, /* process_image_flags= */ 0, @@ -1777,7 +1761,7 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) if (r < 0) return r; - return context_vacuum(context, 0, NULL); + return context_vacuum(&context, 0, NULL); } VERB(verb_pending_or_reboot, "pending", NULL, 1, 1, 0, @@ -1785,7 +1769,7 @@ VERB(verb_pending_or_reboot, "pending", NULL, 1, 1, 0, VERB(verb_pending_or_reboot, "reboot", NULL, 1, 1, 0, "Reboot if a newer version is installed than booted"); static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(context_freep) Context* context = NULL; + _cleanup_(context_done) Context context = CONTEXT_NULL; _cleanup_free_ char *booted_version = NULL; int r; @@ -1799,7 +1783,7 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --component= and --component-all switches may not be combined with the '%s' operation, which only applies to the booted OS version.", argv[0]); - r = context_make_offline( + r = context_load_offline( &context, arg_component, /* process_image_flags= */ 0, @@ -1809,10 +1793,10 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void log_info("Determining installed update sets%s", glyph(GLYPH_ELLIPSIS)); - r = context_discover_update_sets_by_flag(context, UPDATE_INSTALLED); + r = context_discover_update_sets_by_flag(&context, UPDATE_INSTALLED); if (r < 0) return r; - if (!context->newest_installed) + if (!context.newest_installed) return log_error_errno(SYNTHETIC_ERRNO(ENODATA), "Couldn't find any suitable installed versions."); r = parse_os_release(arg_root, "IMAGE_VERSION", &booted_version); @@ -1822,10 +1806,10 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void if (!booted_version) return log_error_errno(SYNTHETIC_ERRNO(ENODATA), "/etc/os-release lacks IMAGE_VERSION= field."); - r = strverscmp_improved(context->newest_installed->version, booted_version); + r = strverscmp_improved(context.newest_installed->version, booted_version); if (r > 0) { log_notice("Newest installed version '%s' is newer than booted version '%s'.%s", - context->newest_installed->version, booted_version, + context.newest_installed->version, booted_version, streq(argv[0], "pending") ? " Reboot recommended." : ""); if (streq(argv[0], "reboot")) @@ -1834,10 +1818,10 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void return EXIT_SUCCESS; } else if (r == 0) log_info("Newest installed version '%s' matches booted version '%s'.", - context->newest_installed->version, booted_version); + context.newest_installed->version, booted_version); else log_warning("Newest installed version '%s' is older than booted version '%s'.", - context->newest_installed->version, booted_version); + context.newest_installed->version, booted_version); if (streq(argv[0], "pending")) /* When called as 'pending' tell the caller via failure exit code that there's nothing newer installed */ return EXIT_FAILURE; @@ -1848,7 +1832,7 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void 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_(context_done) Context context = CONTEXT_NULL; bool has_default_component = false; int r; @@ -1857,7 +1841,7 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda if (arg_component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = context_make_offline( + r = context_load_offline( &context, arg_component, /* process_image_flags= */ 0, @@ -1873,10 +1857,10 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda /* 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 && - !context->component && + !context.component && !arg_root && !arg_image && - context->n_transfers > 0); + context.n_transfers > 0); if (!sd_json_format_enabled(arg_json_format_flags)) { if (!has_default_component && strv_isempty(z)) { @@ -1908,7 +1892,7 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda VERB_NOARG(verb_cleanup, "cleanup", "Clean up orphaned files"); static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(context_freep) Context* context = NULL; + _cleanup_(context_done) Context context = CONTEXT_NULL; int r; assert(argc <= 1); @@ -1916,7 +1900,7 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) if (arg_cleanup == 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invocation of 'cleanup' with --cleanup=no is contradictory, refusing."); - r = context_make_offline( + r = context_load_offline( &context, arg_component, /* process_image_flags= */ 0, @@ -1925,7 +1909,7 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) return r; int ret = 0; - RET_GATHER(ret, installdb_cleanup_component(context)); + RET_GATHER(ret, installdb_cleanup_component(&context)); if (arg_component_all) { _cleanup_strv_free_ char **z = NULL; @@ -1934,9 +1918,9 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) return log_error_errno(r, "Failed to enumerate components: %m"); STRV_FOREACH(i, z) { - _cleanup_(context_freep) Context* component_context = NULL; + _cleanup_(context_done) Context component_context = CONTEXT_NULL; - r = context_make_offline( + r = context_load_offline( &component_context, *i, /* process_image_flags= */ 0, @@ -1944,7 +1928,7 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) if (r < 0) return r; - RET_GATHER(ret, installdb_cleanup_component(component_context)); + RET_GATHER(ret, installdb_cleanup_component(&component_context)); } } diff --git a/src/sysupdate/sysupdate.h b/src/sysupdate/sysupdate.h index ccf1b2dc12..be6b46720d 100644 --- a/src/sysupdate/sysupdate.h +++ b/src/sysupdate/sysupdate.h @@ -28,8 +28,7 @@ typedef struct Context { int installdb_fd; } Context; -Context* context_free(Context *c); -DEFINE_TRIVIAL_CLEANUP_FUNC(Context*, context_free); +void context_done(Context *c); extern bool arg_sync; extern uint64_t arg_instances_max; From 97db6fbfc79a9925509a05b2e5fcbdf87dcb58b5 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 29 May 2026 13:29:07 +0100 Subject: [PATCH 05/15] shared: Add a image_policy_copy() helper method This will be used in an upcoming commit to move global command line argument variables in sysupdate into its `Context` struct. --- src/shared/image-policy.c | 6 ++++++ src/shared/image-policy.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/src/shared/image-policy.c b/src/shared/image-policy.c index 59d1161d04..622b5c2aa1 100644 --- a/src/shared/image-policy.c +++ b/src/shared/image-policy.c @@ -948,6 +948,12 @@ ImagePolicy* image_policy_free(ImagePolicy *p) { return mfree(p); } +ImagePolicy* image_policy_copy(const ImagePolicy *p) { + assert(p); + + return memdup(p, offsetof(ImagePolicy, policies) + sizeof(PartitionPolicy) * p->n_policies); +} + int image_policy_ignore_designators(const ImagePolicy *p, const PartitionDesignator table[], size_t n_table, ImagePolicy **ret) { assert(p); assert(table || n_table == 0); diff --git a/src/shared/image-policy.h b/src/shared/image-policy.h index fe2bf0d200..b0c229e71e 100644 --- a/src/shared/image-policy.h +++ b/src/shared/image-policy.h @@ -119,6 +119,8 @@ ImagePolicy* image_policy_free(ImagePolicy *p); DEFINE_TRIVIAL_CLEANUP_FUNC(ImagePolicy*, image_policy_free); +ImagePolicy* image_policy_copy(const ImagePolicy *p); + CONFIG_PARSER_PROTOTYPE(config_parse_image_policy); int parse_image_policy_argument(const char *s, ImagePolicy **policy); From f91ac1ac260aafdbd0c0c2365f0ad705f3769018 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 29 May 2026 14:40:08 +0100 Subject: [PATCH 06/15] sysupdate: Move global arg_* variables into Context This is another step towards varlinkifying the program, as it means the various verb implementations are no longer relying on global state from the command line. As part of this, move init of the `Context` struct into a new `context_from_cmdline()` function. Additionally pass some context into config parsing `userdata` arguments, as various config parsers were using `arg_root` via a sneaky `extern`. This introduces no functional changes. --- src/sysupdate/sysupdate-cleanup.c | 15 +- src/sysupdate/sysupdate-cleanup.h | 2 +- src/sysupdate/sysupdate-feature.c | 9 +- src/sysupdate/sysupdate-feature.h | 2 +- src/sysupdate/sysupdate-transfer.c | 44 +++--- src/sysupdate/sysupdate.c | 211 +++++++++++++++++++---------- src/sysupdate/sysupdate.h | 19 ++- 7 files changed, 195 insertions(+), 107 deletions(-) diff --git a/src/sysupdate/sysupdate-cleanup.c b/src/sysupdate/sysupdate-cleanup.c index 5c517e401a..2d0229423d 100644 --- a/src/sysupdate/sysupdate-cleanup.c +++ b/src/sysupdate/sysupdate-cleanup.c @@ -77,14 +77,14 @@ static int context_installdb_acquire_fd(Context *c, bool make) { c->installdb_fd = chase_and_open( p, - arg_root, + c->root, flags, O_DIRECTORY|O_CLOEXEC|(make ? O_CREAT : 0), /* ret_path= */ NULL); if (c->installdb_fd == -ENOENT && !make) return 0; if (c->installdb_fd < 0) - return log_error_errno(c->installdb_fd, "Failed to open install database '%s%s': %m", empty_or_root(arg_root) ? "" : arg_root, p); + return log_error_errno(c->installdb_fd, "Failed to open install database '%s%s': %m", empty_or_root(c->root) ? "" : c->root, p); return 1; } @@ -128,8 +128,8 @@ int context_installdb_record( if (strv_isempty(patterns)) return 0; - /* The provided path comes with arg_root prefixed. Strip it here again */ - const char *p = arg_root ? ASSERT_PTR(path_startswith(path, arg_root)) : path; + /* The provided path comes with c->root prefixed. Strip it here again */ + const char *p = c->root ? ASSERT_PTR(path_startswith(path, c->root)) : path; r = context_installdb_acquire_fd(c, /* make= */ true); if (r < 0) @@ -336,7 +336,7 @@ static int context_installdb_process_entry( /* NB: We set CHASE_PROHIBIT_SYMLINKS because the path was normalized by the writer of the entry * already, and if it isn't anymore, then something is fishy. */ - _cleanup_close_ int dir_fd = chase_and_open(path, arg_root, CHASE_MUST_BE_DIRECTORY|CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, O_DIRECTORY|O_CLOEXEC, /* ret_path= */ NULL); + _cleanup_close_ int dir_fd = chase_and_open(path, c->root, CHASE_MUST_BE_DIRECTORY|CHASE_PREFIX_ROOT|CHASE_PROHIBIT_SYMLINKS, O_DIRECTORY|O_CLOEXEC, /* ret_path= */ NULL); if (dir_fd == -ENOENT) { log_debug("Install database path '%s' does not exist, expunging database entry.", path); return 0; @@ -399,14 +399,15 @@ int installdb_cleanup_component(Context *context) { return ret; } -int installdb_list_components(char ***ret) { +int installdb_list_components(Context *context, char ***ret) { int r; + assert(context); assert(ret); _cleanup_close_ int dir_fd = chase_and_open( "/var/lib/systemd/sysupdate", - arg_root, + context->root, CHASE_MUST_BE_DIRECTORY|CHASE_PREFIX_ROOT, O_DIRECTORY|O_CLOEXEC, /* ret_path= */ NULL); diff --git a/src/sysupdate/sysupdate-cleanup.h b/src/sysupdate/sysupdate-cleanup.h index cb3c7b6444..d7b261e35b 100644 --- a/src/sysupdate/sysupdate-cleanup.h +++ b/src/sysupdate/sysupdate-cleanup.h @@ -6,4 +6,4 @@ int context_installdb_record(Context *c, const char *path, char **patterns); int installdb_cleanup_component(Context *context); -int installdb_list_components(char ***ret); +int installdb_list_components(Context *context, char ***ret); diff --git a/src/sysupdate/sysupdate-feature.c b/src/sysupdate/sysupdate-feature.c index 795d4ef90a..d67b61859c 100644 --- a/src/sysupdate/sysupdate-feature.c +++ b/src/sysupdate/sysupdate-feature.c @@ -54,6 +54,7 @@ static int config_parse_url_specifiers( void *data, void *userdata) { char **s = ASSERT_PTR(data); + const char *root = ASSERT_PTR(userdata); _cleanup_free_ char *resolved = NULL; int r; @@ -64,7 +65,7 @@ static int config_parse_url_specifiers( return 0; } - r = specifier_printf(rvalue, NAME_MAX, specifier_table, arg_root, NULL, &resolved); + r = specifier_printf(rvalue, NAME_MAX, specifier_table, root, NULL, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to expand specifiers in %s=, ignoring: %s", lvalue, rvalue); @@ -80,7 +81,7 @@ static int config_parse_url_specifiers( return free_and_replace(*s, resolved); } -int feature_read_definition(Feature *f, const char *path, const char *const *dirs) { +int feature_read_definition(Feature *f, const char *root, const char *path, const char *const *dirs) { assert(f); ConfigTableItem table[] = { @@ -105,12 +106,12 @@ int feature_read_definition(Feature *f, const char *path, const char *const *dir STRV_MAKE_CONST(path), dirs, strjoina(filename, ".d"), - arg_root, + root, /* root_fd= */ -EBADF, "Feature\0", config_item_table_lookup, table, CONFIG_PARSE_WARN, - /* userdata= */ NULL, + (void *) root, /* stats_by_path= */ NULL, /* drop_in_files= */ NULL); if (r < 0) diff --git a/src/sysupdate/sysupdate-feature.h b/src/sysupdate/sysupdate-feature.h index b42df5c27e..7d7495087c 100644 --- a/src/sysupdate/sysupdate-feature.h +++ b/src/sysupdate/sysupdate-feature.h @@ -22,4 +22,4 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Feature*, feature_unref); extern const struct hash_ops feature_hash_ops; -int feature_read_definition(Feature *f, const char *path, const char *const *conf_file_dirs); +int feature_read_definition(Feature *f, const char *root, const char *path, const char *const *conf_file_dirs); diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index f7e257e041..f2a4377e1e 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -122,11 +122,12 @@ static int config_parse_protect_version( _cleanup_free_ char *resolved = NULL; char ***protected_versions = ASSERT_PTR(data); + Transfer *t = ASSERT_PTR(userdata); int r; assert(rvalue); - r = specifier_printf(rvalue, NAME_MAX, specifier_table, arg_root, NULL, &resolved); + r = specifier_printf(rvalue, NAME_MAX, specifier_table, t->context->root, NULL, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to expand specifiers in ProtectVersion=, ignoring: %s", rvalue); @@ -160,11 +161,12 @@ static int config_parse_min_version( _cleanup_free_ char *resolved = NULL; char **version = ASSERT_PTR(data); + Transfer *t = ASSERT_PTR(userdata); int r; assert(rvalue); - r = specifier_printf(rvalue, NAME_MAX, specifier_table, arg_root, NULL, &resolved); + r = specifier_printf(rvalue, NAME_MAX, specifier_table, t->context->root, NULL, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to expand specifiers in MinVersion=, ignoring: %s", rvalue); @@ -192,6 +194,7 @@ static int config_parse_url_specifiers( void *data, void *userdata) { char ***s = ASSERT_PTR(data); + Transfer *t = ASSERT_PTR(userdata); _cleanup_free_ char *resolved = NULL; int r; @@ -202,7 +205,7 @@ static int config_parse_url_specifiers( return 0; } - r = specifier_printf(rvalue, NAME_MAX, specifier_table, arg_root, NULL, &resolved); + r = specifier_printf(rvalue, NAME_MAX, specifier_table, t->context->root, NULL, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to expand specifiers in %s=, ignoring: %s", lvalue, rvalue); @@ -236,11 +239,12 @@ static int config_parse_current_symlink( _cleanup_free_ char *resolved = NULL; char **current_symlink = ASSERT_PTR(data); + Transfer *t = ASSERT_PTR(userdata); int r; assert(rvalue); - r = specifier_printf(rvalue, NAME_MAX, specifier_table, arg_root, NULL, &resolved); + r = specifier_printf(rvalue, NAME_MAX, specifier_table, t->context->root, NULL, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to expand specifiers in CurrentSymlink=, ignoring: %s", rvalue); @@ -307,6 +311,7 @@ static int config_parse_resource_pattern( void *userdata) { char ***patterns = ASSERT_PTR(data); + Transfer *t = ASSERT_PTR(userdata); int r; assert(rvalue); @@ -328,7 +333,7 @@ static int config_parse_resource_pattern( if (r == 0) break; - r = specifier_printf(word, NAME_MAX, specifier_table, arg_root, NULL, &resolved); + r = specifier_printf(word, NAME_MAX, specifier_table, t->context->root, NULL, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to expand specifiers in MatchPattern=, ignoring: %s", rvalue); @@ -361,6 +366,7 @@ static int config_parse_resource_path( void *userdata) { _cleanup_free_ char *resolved = NULL; Resource *rr = ASSERT_PTR(data); + Transfer *t = ASSERT_PTR(userdata); int r; assert(rvalue); @@ -371,7 +377,7 @@ static int config_parse_resource_path( return 0; } - r = specifier_printf(rvalue, PATH_MAX-1, specifier_table, arg_root, NULL, &resolved); + r = specifier_printf(rvalue, PATH_MAX-1, specifier_table, t->context->root, NULL, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to expand specifiers in Path=, ignoring: %s", rvalue); @@ -549,14 +555,14 @@ int transfer_read_definition(Transfer *t, const char *path, const char **dirs, H STRV_MAKE_CONST(path), dirs, strjoina(filename, ".d"), - arg_root, + t->context->root, /* root_fd= */ -EBADF, "Transfer\0" "Source\0" "Target\0", config_item_table_lookup, table, CONFIG_PARSE_WARN, - /* userdata= */ NULL, + t, /* stats_by_path= */ NULL, /* drop_in_files= */ NULL); if (r < 0) @@ -613,7 +619,7 @@ int transfer_read_definition(Transfer *t, const char *path, const char **dirs, H return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL), "Source specification lacks Path=."); - if (t->source.path_relative_to == PATH_RELATIVE_TO_EXPLICIT && !arg_transfer_source) + if (t->source.path_relative_to == PATH_RELATIVE_TO_EXPLICIT && !t->context->transfer_source) return log_syntax(NULL, LOG_ERR, path, 1, SYNTHETIC_ERRNO(EINVAL), "PathRelativeTo=explicit requires --transfer-source= to be specified."); @@ -682,7 +688,7 @@ int transfer_resolve_paths( assert(t); - r = resource_resolve_path(&t->source, root, arg_transfer_source, node); + r = resource_resolve_path(&t->source, root, t->context->transfer_source, node); if (r < 0) return r; @@ -849,7 +855,7 @@ int transfer_vacuum( /* Second, calculate how many instances to keep, based on the instance limit — but keep at least one */ - instances_max = arg_instances_max != UINT64_MAX ? arg_instances_max : t->instances_max; + instances_max = t->context->instances_max != UINT64_MAX ? t->context->instances_max : t->instances_max; assert(instances_max >= 1); if (instances_max == UINT64_MAX) /* Keep infinite instances? */ limit = UINT64_MAX; @@ -1369,7 +1375,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra SYSTEMD_IMPORT_PATH, "raw", "--direct", /* just copy/unpack the specified file, don't do anything else */ - arg_sync ? "--sync=yes" : "--sync=no", + t->context->sync ? "--sync=yes" : "--sync=no", i->path, t->temporary_partial_path), t, i, cb, userdata); @@ -1386,7 +1392,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra "--direct", /* just copy/unpack the specified file, don't do anything else */ "--offset", offset, "--size-max", max_size, - arg_sync ? "--sync=yes" : "--sync=no", + t->context->sync ? "--sync=yes" : "--sync=no", i->path, t->target.path), t, i, cb, userdata); @@ -1409,7 +1415,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra SYSTEMD_IMPORT_FS_PATH, "run", "--direct", /* just untar the specified file, don't do anything else */ - arg_sync ? "--sync=yes" : "--sync=no", + t->context->sync ? "--sync=yes" : "--sync=no", t->target.type == RESOURCE_SUBVOLUME ? "--btrfs-subvol=yes" : "--btrfs-subvol=no", i->path, t->temporary_partial_path), @@ -1426,7 +1432,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra SYSTEMD_IMPORT_PATH, "tar", "--direct", /* just untar the specified file, don't do anything else */ - arg_sync ? "--sync=yes" : "--sync=no", + t->context->sync ? "--sync=yes" : "--sync=no", t->target.type == RESOURCE_SUBVOLUME ? "--btrfs-subvol=yes" : "--btrfs-subvol=no", i->path, t->temporary_partial_path), @@ -1447,7 +1453,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra "raw", "--direct", /* just download the specified URL, don't download anything else */ "--verify", digest, /* validate by explicit SHA256 sum */ - arg_sync ? "--sync=yes" : "--sync=no", + t->context->sync ? "--sync=yes" : "--sync=no", i->path, t->temporary_partial_path), t, i, cb, userdata); @@ -1465,7 +1471,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra "--verify", digest, /* validate by explicit SHA256 sum */ "--offset", offset, "--size-max", max_size, - arg_sync ? "--sync=yes" : "--sync=no", + t->context->sync ? "--sync=yes" : "--sync=no", i->path, t->target.path), t, i, cb, userdata); @@ -1487,7 +1493,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra "--direct", /* just download the specified URL, don't download anything else */ "--verify", digest, /* validate by explicit SHA256 sum */ t->target.type == RESOURCE_SUBVOLUME ? "--btrfs-subvol=yes" : "--btrfs-subvol=no", - arg_sync ? "--sync=yes" : "--sync=no", + t->context->sync ? "--sync=yes" : "--sync=no", i->path, t->temporary_partial_path), t, i, cb, userdata); @@ -1528,7 +1534,7 @@ int transfer_acquire_instance(Transfer *t, Instance *i, InstanceMetadata *f, Tra } /* Synchronize */ - if (arg_sync && need_sync) { + if (t->context->sync && need_sync) { if (t->target.type == RESOURCE_REGULAR_FILE) r = fsync_path_and_parent_at(AT_FDCWD, t->temporary_partial_path); else { diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 74fe267036..3232ab6e37 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -43,12 +43,12 @@ #include "verbs.h" static char *arg_definitions = NULL; -bool arg_sync = true; -uint64_t arg_instances_max = UINT64_MAX; +static bool arg_sync = true; +static uint64_t arg_instances_max = UINT64_MAX; static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF; static PagerFlags arg_pager_flags = 0; static bool arg_legend = true; -char *arg_root = NULL; +static char *arg_root = NULL; static char *arg_image = NULL; static bool arg_reboot = false; static int arg_cleanup = -1; @@ -57,7 +57,7 @@ static bool arg_component_all = false; static int arg_verify = -1; static ImagePolicy *arg_image_policy = NULL; static bool arg_offline = false; -char *arg_transfer_source = NULL; +static char *arg_transfer_source = NULL; STATIC_DESTRUCTOR_REGISTER(arg_definitions, freep); STATIC_DESTRUCTOR_REGISTER(arg_root, freep); @@ -74,14 +74,16 @@ const Specifier specifier_table[] = { #define CONTEXT_NULL \ (Context) { \ + .sync = true, \ + .instances_max = UINT64_MAX, \ + .verify = -1, \ + .cleanup = -1, \ .installdb_fd = -EBADF, \ } void context_done(Context *c) { assert(c); - c->component = mfree(c->component); - c->mounted_dir = umount_and_rmdir_and_free(c->mounted_dir); c->loop_device = loop_device_unref(c->loop_device); @@ -105,6 +107,51 @@ void context_done(Context *c) { c->web_cache = hashmap_free(c->web_cache); c->installdb_fd = safe_close(c->installdb_fd); + + c->definitions = mfree(c->definitions); + c->root = mfree(c->root); + c->image = mfree(c->image); + c->component = mfree(c->component); + c->image_policy = image_policy_free(c->image_policy); + c->transfer_source = mfree(c->transfer_source); +} + +static int context_from_cmdline(Context *ret) { + assert(ret); + + _cleanup_(context_done) Context context = CONTEXT_NULL; + + context.instances_max = arg_instances_max; + context.sync = arg_sync; + context.reboot = arg_reboot; + context.verify = arg_verify; + context.offline = arg_offline; + context.cleanup = arg_cleanup; + context.component_all = arg_component_all; + + if (strdup_to(&context.definitions, arg_definitions) < 0) + return log_oom(); + + if (strdup_to(&context.root, arg_root) < 0) + return log_oom(); + + if (strdup_to(&context.image, arg_image) < 0) + return log_oom(); + + if (strdup_to(&context.component, arg_component) < 0) + return log_oom(); + + if (strdup_to(&context.transfer_source, arg_transfer_source) < 0) + return log_oom(); + + if (arg_image_policy) { + context.image_policy = image_policy_copy(arg_image_policy); + if (!context.image_policy) + return log_oom(); + } + + *ret = TAKE_GENERIC(context, Context, CONTEXT_NULL); + return 0; } static DEFINE_POINTER_ARRAY_FREE_FUNC(Transfer*, transfer_free); @@ -128,7 +175,7 @@ static int read_definitions( assert(dirs); assert(suffix); - r = conf_files_list_strv_full(suffix, arg_root, + r = conf_files_list_strv_full(suffix, c->root, CONF_FILES_REGULAR|CONF_FILES_FILTER_MASKED|CONF_FILES_WARN, dirs, &files, &n_files); if (r < 0) @@ -147,7 +194,7 @@ static int read_definitions( if (r < 0) return r; - r = transfer_resolve_paths(t, arg_root, node); + r = transfer_resolve_paths(t, c->root, node); if (r < 0) return r; @@ -178,8 +225,8 @@ static int context_read_definitions(Context *c, const char* node, ReadDefinition assert(c); - if (arg_definitions) - dirs = strv_new(arg_definitions); + if (c->definitions) + dirs = strv_new(c->definitions); else if (c->component) { char **l = CONF_PATHS_STRV(""); size_t i = 0; @@ -207,7 +254,7 @@ static int context_read_definitions(Context *c, const char* node, ReadDefinition CLEANUP_ARRAY(files, n_files, conf_file_free_array); - r = conf_files_list_strv_full(".feature", arg_root, + r = conf_files_list_strv_full(".feature", c->root, CONF_FILES_REGULAR|CONF_FILES_FILTER_MASKED|CONF_FILES_WARN, (const char**) dirs, &files, &n_files); if (r < 0) @@ -221,7 +268,7 @@ static int context_read_definitions(Context *c, const char* node, ReadDefinition if (!f) return log_oom(); - r = feature_read_definition(f, e->result, (const char**) dirs); + r = feature_read_definition(f, c->root, e->result, (const char**) dirs); if (r < 0) return r; @@ -271,7 +318,7 @@ static int context_load_installed_instances(Context *c) { r = resource_load_instances( &t->target, - arg_verify >= 0 ? arg_verify : t->verify, + c->verify >= 0 ? c->verify : t->verify, &c->web_cache); if (r < 0) return r; @@ -282,7 +329,7 @@ static int context_load_installed_instances(Context *c) { r = resource_load_instances( &t->target, - arg_verify >= 0 ? arg_verify : t->verify, + c->verify >= 0 ? c->verify : t->verify, &c->web_cache); if (r < 0) return r; @@ -303,7 +350,7 @@ static int context_load_available_instances(Context *c) { r = resource_load_instances( &t->source, - arg_verify >= 0 ? arg_verify : t->verify, + c->verify >= 0 ? c->verify : t->verify, &c->web_cache); if (r < 0) return r; @@ -527,7 +574,7 @@ static int context_discover_update_sets(Context *c) { if (r < 0) return r; - if (!arg_offline) { + if (!c->offline) { log_info("Determining available update sets%s", glyph(GLYPH_ELLIPSIS)); r = context_discover_update_sets_by_flag(c, UPDATE_AVAILABLE); @@ -941,16 +988,16 @@ static int process_image( assert(c); - if (!arg_image) + if (!c->image) return 0; - assert(!arg_root); + assert(!c->root); assert(!c->mounted_dir); assert(!c->loop_device); r = mount_image_privately_interactively( - arg_image, - arg_image_policy, + c->image, + c->image_policy, (FLAGS_SET(flags, PROCESS_IMAGE_READ_ONLY) ? DISSECT_IMAGE_READ_ONLY : 0) | DISSECT_IMAGE_FSCK | DISSECT_IMAGE_MKDIR | @@ -966,8 +1013,8 @@ static int process_image( if (r < 0) return r; - arg_root = strdup(mounted_dir); - if (!arg_root) + c->root = strdup(mounted_dir); + if (!c->root) return log_oom(); c->mounted_dir = TAKE_PTR(mounted_dir); @@ -978,7 +1025,6 @@ static int process_image( static int context_load_offline( Context *context, - const char *component, ProcessImageFlags process_image_flags, ReadDefinitionsFlags read_definitions_flags) { int r; @@ -988,10 +1034,6 @@ static int context_load_offline( /* Sets up a context object and initializes everything we can initialize offline, i.e. without * checking on the update source (i.e. the Internet) what versions are available */ - r = free_and_strdup_warn(&context->component, component); - if (r < 0) - return r; - r = process_image(context, process_image_flags); if (r < 0) return r; @@ -1009,7 +1051,6 @@ static int context_load_offline( static int context_load_online( Context *context, - const char *component, ProcessImageFlags process_image_flags) { int r; @@ -1020,13 +1061,12 @@ static int context_load_online( r = context_load_offline( context, - component, process_image_flags, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS|READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) return r; - if (!arg_offline) { + if (!context->offline) { r = context_load_available_instances(context); if (r < 0) return r; @@ -1146,7 +1186,7 @@ static int context_acquire( if (r < 0) return r; - if (arg_sync) + if (c->sync) sync(); (void) sd_notifyf(/* unset_environment= */ false, @@ -1171,7 +1211,7 @@ static int context_acquire( return r; } - if (arg_sync) + if (c->sync) sync(); return 1; @@ -1294,7 +1334,7 @@ static int context_notify_subscribers(Context *c, UpdateSet *us) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *params = NULL; r = sd_json_buildo( ¶ms, - SD_JSON_BUILD_PAIR_CONDITION(!!arg_component, "component", SD_JSON_BUILD_STRING(arg_component)), + SD_JSON_BUILD_PAIR_CONDITION(!!c->component, "component", SD_JSON_BUILD_STRING(c->component)), SD_JSON_BUILD_PAIR_CONDITION(!!us, "version", SD_JSON_BUILD_STRING(us ? us->version : NULL)), SD_JSON_BUILD_PAIR_CONDITION(!!resources, "resources", SD_JSON_BUILD_VARIANT(resources))); if (r < 0) @@ -1354,14 +1394,14 @@ static int context_install( !inst->is_pending) continue; - r = transfer_install_instance(t, inst, arg_root); + r = transfer_install_instance(t, inst, c->root); if (r < 0) return r; } log_info("%s Successfully installed update '%s'.", glyph(GLYPH_SPARKLES), us->version); - if (!arg_root) + if (!c->root) (void) context_notify_subscribers(c, us); (void) sd_notifyf(/* unset_environment= */ false, @@ -1384,13 +1424,14 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { assert(argc <= 2); version = argc >= 2 ? argv[1] : NULL; - if (arg_component_all) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - r = context_load_online( - &context, - arg_component, - PROCESS_IMAGE_READ_ONLY); + r = context_load_online(&context, PROCESS_IMAGE_READ_ONLY); if (r < 0) return r; @@ -1455,12 +1496,15 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata assert(argc <= 2); feature_id = argc >= 2 ? argv[1] : NULL; - if (arg_component_all) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); r = context_load_offline( &context, - arg_component, PROCESS_IMAGE_READ_ONLY, READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) @@ -1588,12 +1632,15 @@ static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdat assert(argc <= 1); - if (arg_component_all) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); r = context_load_online( &context, - arg_component, PROCESS_IMAGE_READ_ONLY); if (r < 0) return r; @@ -1638,17 +1685,21 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag assert(argc <= 2); version = argc >= 2 ? argv[1] : NULL; - if (arg_component_all) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - if (arg_instances_max < 2) + if (context.instances_max < 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --instances-max argument must be >= 2 while updating"); - if (arg_reboot) { + if (context.reboot) { /* If automatic reboot on completion is requested, let's first determine the currently booted image */ - r = parse_os_release(arg_root, "IMAGE_VERSION", &booted_version); + r = parse_os_release(context.root, "IMAGE_VERSION", &booted_version); if (r < 0) return log_error_errno(r, "Failed to parse /etc/os-release: %m"); if (!booted_version) @@ -1660,7 +1711,6 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag r = context_load_online( &context, - arg_component, /* process_image_flags= */ 0); if (r < 0) { if (r != -ENOENT) @@ -1696,13 +1746,13 @@ static int verb_update_impl(int argc, char **argv, UpdateActionFlags action_flag } } - if (arg_cleanup > 0) + if (context.cleanup > 0) RET_GATHER(ret, installdb_cleanup_component(&context)); if (installed) { /* We installed something, yay */ - if (arg_reboot) { + if (context.reboot) { assert(applied); assert(booted_version); @@ -1746,16 +1796,19 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) assert(argc <= 1); - if (arg_component_all) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); - if (arg_instances_max < 1) + if (context.instances_max < 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --instances-max argument must be >= 1 while vacuuming"); r = context_load_offline( &context, - arg_component, /* process_image_flags= */ 0, READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) @@ -1775,17 +1828,20 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void assert(argc == 1); - if (arg_image || arg_root) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.image || context.root) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --root=/--image= switches may not be combined with the '%s' operation.", argv[0]); - if (arg_component || arg_component_all) + if (context.component || context.component_all) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --component= and --component-all switches may not be combined with the '%s' operation, which only applies to the booted OS version.", argv[0]); r = context_load_offline( &context, - arg_component, /* process_image_flags= */ 0, READ_DEFINITIONS_REQUIRES_ENABLED_TRANSFERS|READ_DEFINITIONS_REQUIRES_ANY_TRANSFERS); if (r < 0) @@ -1799,8 +1855,8 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void if (!context.newest_installed) return log_error_errno(SYNTHETIC_ERRNO(ENODATA), "Couldn't find any suitable installed versions."); - r = parse_os_release(arg_root, "IMAGE_VERSION", &booted_version); - if (r < 0) /* yes, arg_root is NULL here, but we have to pass something, and it's a lot more readable + r = parse_os_release(context.root, "IMAGE_VERSION", &booted_version); + if (r < 0) /* yes, context.root is NULL here, but we have to pass something, and it's a lot more readable * if we see what the first argument is about */ return log_error_errno(r, "Failed to parse /etc/os-release: %m"); if (!booted_version) @@ -1838,28 +1894,31 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda assert(argc <= 1); - if (arg_component_all) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component_all) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); r = context_load_offline( &context, - arg_component, /* process_image_flags= */ 0, /* read_definitions_flags= */ 0); if (r < 0) return r; _cleanup_strv_free_ char **z = NULL; - r = get_component_list(arg_root, &z); + r = get_component_list(context.root, &z); if (r < 0) return log_error_errno(r, "Failed to enumerate components: %m"); /* 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 && + has_default_component = (!context.definitions && !context.component && - !arg_root && - !arg_image && + !context.root && + !context.image && context.n_transfers > 0); if (!sd_json_format_enabled(arg_json_format_flags)) { @@ -1897,12 +1956,15 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) assert(argc <= 1); - if (arg_cleanup == 0) + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.cleanup == 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invocation of 'cleanup' with --cleanup=no is contradictory, refusing."); r = context_load_offline( &context, - arg_component, /* process_image_flags= */ 0, /* read_definitions_flags= */ 0); if (r < 0) @@ -1911,18 +1973,27 @@ static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) int ret = 0; RET_GATHER(ret, installdb_cleanup_component(&context)); - if (arg_component_all) { + if (context.component_all) { _cleanup_strv_free_ char **z = NULL; - r = installdb_list_components(&z); + r = installdb_list_components(&context, &z); if (r < 0) return log_error_errno(r, "Failed to enumerate components: %m"); STRV_FOREACH(i, z) { _cleanup_(context_done) Context component_context = CONTEXT_NULL; + r = context_from_cmdline(&component_context); + if (r < 0) + return r; + + /* Override the component with our iter. This needs to be done in a fresh Context + * as the installdb_fd and other state are specific to the component. */ + r = free_and_strdup_warn(&component_context.component, *i); + if (r < 0) + return r; + r = context_load_offline( &component_context, - *i, /* process_image_flags= */ 0, /* read_definitions_flags= */ 0); if (r < 0) diff --git a/src/sysupdate/sysupdate.h b/src/sysupdate/sysupdate.h index be6b46720d..0d0a8e9fbf 100644 --- a/src/sysupdate/sysupdate.h +++ b/src/sysupdate/sysupdate.h @@ -5,8 +5,22 @@ #include "sysupdate-forward.h" typedef struct Context { + /* Parameters/Command line arguments: */ + char *definitions; + bool sync; + uint64_t instances_max; + char *root; + char *image; + bool reboot; + int cleanup; char *component; + bool component_all; + int verify; + ImagePolicy *image_policy; + bool offline; + char *transfer_source; + /* Loaded state: */ LoopDevice *loop_device; char *mounted_dir; @@ -30,9 +44,4 @@ typedef struct Context { void context_done(Context *c); -extern bool arg_sync; -extern uint64_t arg_instances_max; -extern char *arg_root; -extern char *arg_transfer_source; - extern const Specifier specifier_table[]; From 6ff8027ee1f9d443bc14b5ac2e91ec30d52a2d69 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 2 Jun 2026 12:59:04 +0100 Subject: [PATCH 07/15] sysupdate: Minor fix to a cleanup function on an error path `process_image()` has historically used `umount_and_freep` to clean up the mounted directory locally, but callers to it have used `umount_and_rmdir_and_freep`. No directory is created after any of the error return paths in `process_image()`, so it should probably be using `umount_and_rmdir_and_freep` too. --- src/sysupdate/sysupdate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 3232ab6e37..a4c68e48cf 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -983,7 +983,7 @@ static int process_image( ProcessImageFlags flags) { _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; - _cleanup_(umount_and_freep) char *mounted_dir = NULL; + _cleanup_(umount_and_rmdir_and_freep) char *mounted_dir = NULL; int r; assert(c); From 06a6111a3c38490731a493c38d3adbbeb5425dfc Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 2 Jun 2026 15:54:47 +0100 Subject: [PATCH 08/15] sysupdate: Factor out core of `components` verb This will be used in the following commit to add a varlink method for it. This introduces no functional changes. --- src/sysupdate/sysupdate.c | 43 +++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index a4c68e48cf..f0f4ccefc1 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -1885,10 +1885,36 @@ static int verb_pending_or_reboot(int argc, char *argv[], uintptr_t _data, void return EXIT_SUCCESS; } +static int context_list_components(Context *context, char ***component_names, bool *has_default_component) { + int r; + + assert(context); + + _cleanup_strv_free_ char **z = NULL; + r = get_component_list(context->root, &z); + if (r < 0) + return r; + + if (component_names) + *component_names = TAKE_PTR(z); + + /* 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 */ + if (has_default_component) + *has_default_component = (!context->definitions && + !context->component && + !context->root && + !context->image && + context->n_transfers > 0); + + return 0; +} + VERB_NOARG(verb_components, "components", "Show list of components"); static int verb_components(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(context_done) Context context = CONTEXT_NULL; + _cleanup_strv_free_ char **component_names = NULL; bool has_default_component = false; int r; @@ -1908,21 +1934,12 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda if (r < 0) return r; - _cleanup_strv_free_ char **z = NULL; - r = get_component_list(context.root, &z); + r = context_list_components(&context, &component_names, &has_default_component); if (r < 0) return log_error_errno(r, "Failed to enumerate components: %m"); - /* 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 = (!context.definitions && - !context.component && - !context.root && - !context.image && - context.n_transfers > 0); - if (!sd_json_format_enabled(arg_json_format_flags)) { - if (!has_default_component && strv_isempty(z)) { + if (!has_default_component && strv_isempty(component_names)) { log_info("No components defined."); return 0; } @@ -1931,13 +1948,13 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda printf("%s%s\n", ansi_highlight(), ansi_normal()); - STRV_FOREACH(i, z) + STRV_FOREACH(i, component_names) puts(*i); } else { _cleanup_(sd_json_variant_unrefp) sd_json_variant *json = NULL; r = sd_json_buildo(&json, SD_JSON_BUILD_PAIR_BOOLEAN("default", has_default_component), - SD_JSON_BUILD_PAIR_STRV("components", z)); + SD_JSON_BUILD_PAIR_STRV("components", component_names)); if (r < 0) return log_error_errno(r, "Failed to create JSON: %m"); From 28b9b5712ae80e11b17ae8d616ffbbea741b0c33 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 4 Jun 2026 16:06:15 +0100 Subject: [PATCH 09/15] sysupdate: Factor some Target handling code out of sysupdated This will be used in upcoming commits to varlinkify `systemd-sysupdate`; it will need a way to identify targets over varlink, and the existing way with a `Target` over D-Bus seems to work quite well. --- src/sysupdate/meson.build | 3 ++- src/sysupdate/sysupdate-target.c | 22 ++++++++++++++++++++++ src/sysupdate/sysupdate-target.h | 32 ++++++++++++++++++++++++++++++++ src/sysupdate/sysupdated.c | 31 +------------------------------ 4 files changed, 57 insertions(+), 31 deletions(-) create mode 100644 src/sysupdate/sysupdate-target.c create mode 100644 src/sysupdate/sysupdate-target.h diff --git a/src/sysupdate/meson.build b/src/sysupdate/meson.build index ce1f4c14e7..b6f9436606 100644 --- a/src/sysupdate/meson.build +++ b/src/sysupdate/meson.build @@ -8,6 +8,7 @@ systemd_sysupdate_sources = files( 'sysupdate-partition.c', 'sysupdate-pattern.c', 'sysupdate-resource.c', + 'sysupdate-target.c', 'sysupdate-transfer.c', 'sysupdate-update-set.c', 'sysupdate.c', @@ -37,7 +38,7 @@ executables += [ 'name' : 'systemd-sysupdated', 'dbus' : true, 'conditions' : ['ENABLE_SYSUPDATED'], - 'sources' : files('sysupdated.c'), + 'sources' : files('sysupdated.c', 'sysupdate-target.c'), }, executable_template + { 'name' : 'updatectl', diff --git a/src/sysupdate/sysupdate-target.c b/src/sysupdate/sysupdate-target.c new file mode 100644 index 0000000000..ef132960f3 --- /dev/null +++ b/src/sysupdate/sysupdate-target.c @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "string-table.h" +#include "sysupdate-target.h" + +static const char* const target_class_table[_TARGET_CLASS_MAX] = { + [TARGET_MACHINE] = "machine", + [TARGET_PORTABLE] = "portable", + [TARGET_SYSEXT] = "sysext", + [TARGET_CONFEXT] = "confext", + [TARGET_COMPONENT] = "component", + [TARGET_HOST] = "host", +}; + +DEFINE_STRING_TABLE_LOOKUP(target_class, TargetClass); + +void target_identifier_done(TargetIdentifier *t) { + assert(t); + + t->class = _TARGET_CLASS_INVALID; + t->name = mfree(t->name); +} diff --git a/src/sysupdate/sysupdate-target.h b/src/sysupdate/sysupdate-target.h new file mode 100644 index 0000000000..9c0a13bf4a --- /dev/null +++ b/src/sysupdate/sysupdate-target.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "os-util.h" + +typedef enum TargetClass { + /* These should try to match ImageClass from src/basic/os-util.h */ + TARGET_MACHINE = IMAGE_MACHINE, + TARGET_PORTABLE = IMAGE_PORTABLE, + TARGET_SYSEXT = IMAGE_SYSEXT, + TARGET_CONFEXT = IMAGE_CONFEXT, + _TARGET_CLASS_IS_IMAGE_CLASS_MAX, + + /* sysupdate-specific classes */ + TARGET_HOST = _TARGET_CLASS_IS_IMAGE_CLASS_MAX, + TARGET_COMPONENT, + + _TARGET_CLASS_MAX, + _TARGET_CLASS_INVALID = -EINVAL, +} TargetClass; + +/* Let's ensure when the number of classes is updated things are updated here too */ +assert_cc((int) _IMAGE_CLASS_MAX == (int) _TARGET_CLASS_IS_IMAGE_CLASS_MAX); + +DECLARE_STRING_TABLE_LOOKUP(target_class, TargetClass); + +typedef struct TargetIdentifier { + TargetClass class; + char *name; +} TargetIdentifier; + +void target_identifier_done(TargetIdentifier *t); diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index 359fb50b91..8022fde77e 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -40,6 +40,7 @@ #include "signal-util.h" #include "string-table.h" #include "strv.h" +#include "sysupdate-target.h" #include "sysupdate-util.h" #include "utf8.h" @@ -64,25 +65,6 @@ typedef struct Manager { /* Forward declare so that jobs can call it on exit */ static void manager_check_idle(Manager *m); -typedef enum TargetClass { - /* These should try to match ImageClass from src/basic/os-util.h */ - TARGET_MACHINE = IMAGE_MACHINE, - TARGET_PORTABLE = IMAGE_PORTABLE, - TARGET_SYSEXT = IMAGE_SYSEXT, - TARGET_CONFEXT = IMAGE_CONFEXT, - _TARGET_CLASS_IS_IMAGE_CLASS_MAX, - - /* sysupdate-specific classes */ - TARGET_HOST = _TARGET_CLASS_IS_IMAGE_CLASS_MAX, - TARGET_COMPONENT, - - _TARGET_CLASS_MAX, - _TARGET_CLASS_INVALID = -EINVAL, -} TargetClass; - -/* Let's ensure when the number of classes is updated things are updated here too */ -assert_cc((int) _IMAGE_CLASS_MAX == (int) _TARGET_CLASS_IS_IMAGE_CLASS_MAX); - typedef struct Target { Manager *manager; @@ -138,17 +120,6 @@ struct Job { JobReady detach_cb; /* Callback called when job has started. Detaches the job to run in the background */ }; -static const char* const target_class_table[_TARGET_CLASS_MAX] = { - [TARGET_MACHINE] = "machine", - [TARGET_PORTABLE] = "portable", - [TARGET_SYSEXT] = "sysext", - [TARGET_CONFEXT] = "confext", - [TARGET_COMPONENT] = "component", - [TARGET_HOST] = "host", -}; - -DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(target_class, TargetClass); - static const char* const job_type_table[_JOB_TYPE_MAX] = { [JOB_LIST] = "list", [JOB_DESCRIBE] = "describe", From 0b3eeb1547d87133aba73edf68f9403578b0c340 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 4 Jun 2026 16:47:37 +0100 Subject: [PATCH 10/15] sd-json: Fix validation of optional fields within a mandatory struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a varlink method takes a struct/object as a parameter, and it’s marked as `SD_JSON_MANDATORY`, and it has an optional field inside it which is *not* marked as `SD_JSON_MANDATORY`, we want to not require that field to be set. Previously, due to using the `merged_flags` from both the mandatory struct and the optional field, `SD_JSON_MANDATORY` was effectively always set on the optional field even if we didn’t want it. This resulted in an error being emitted if the mandatory struct was provided in a varlink call, but without the optional field. Fix that by validating the field’s presence only against its own flags and not also the flags of its parent. Adds a unit test to prevent regressions. --- src/libsystemd/sd-json/sd-json.c | 2 +- src/libsystemd/sd-varlink/test-varlink.c | 72 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-json/sd-json.c b/src/libsystemd/sd-json/sd-json.c index dad7a3174d..531429b51f 100644 --- a/src/libsystemd/sd-json/sd-json.c +++ b/src/libsystemd/sd-json/sd-json.c @@ -5359,7 +5359,7 @@ _public_ int sd_json_dispatch_full( for (const sd_json_dispatch_field *p = table; p && p->name; p++) { sd_json_dispatch_flags_t merged_flags = p->flags | flags; - if ((merged_flags & SD_JSON_MANDATORY) && !found[p-table]) { + if ((p->flags & SD_JSON_MANDATORY) && !found[p-table]) { json_log(v, merged_flags, 0, "Missing object field '%s'.", p->name); if ((merged_flags & SD_JSON_PERMISSIVE)) diff --git a/src/libsystemd/sd-varlink/test-varlink.c b/src/libsystemd/sd-varlink/test-varlink.c index 31b7cb46f9..a2a32b7f78 100644 --- a/src/libsystemd/sd-varlink/test-varlink.c +++ b/src/libsystemd/sd-varlink/test-varlink.c @@ -452,6 +452,78 @@ TEST(invalid_parameter) { ASSERT_OK(sd_event_loop(e)); } +typedef struct MandatoryTypeWithOptionalField { + const char *mandatory; + const char *optional; +} MandatoryTypeWithOptionalField; + +static int dispatch_mandatory_type_with_optional_field(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { + MandatoryTypeWithOptionalField *m = ASSERT_PTR(userdata); + static const sd_json_dispatch_field dispatch[] = { + { "mandatory", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(*m, mandatory), SD_JSON_MANDATORY }, + { "optional", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(*m, optional), 0 }, + {} + }; + + return sd_json_dispatch(variant, dispatch, flags, m); +} + +static int method_mandatory_type_with_optional_field(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { + MandatoryTypeWithOptionalField m = { NULL, NULL }; + int r; + + sd_json_dispatch_field table[] = { + { "mandatoryField", SD_JSON_VARIANT_OBJECT, dispatch_mandatory_type_with_optional_field, 0, SD_JSON_MANDATORY }, + {} + }; + + r = sd_varlink_dispatch(link, parameters, table, &m); + if (r != 0) + return r; + + _cleanup_free_ char *sum = strjoin(m.mandatory, "+", m.optional); + if (!sum) + return log_oom(); + + return sd_varlink_replybo(link, SD_JSON_BUILD_PAIR_STRING("result", sum)); +} + +static int reply_valid_hi(sd_varlink *link, sd_json_variant *parameters, const char *error_id, sd_varlink_reply_flags_t flags, void *userdata) { + ASSERT_NULL(error_id); + ASSERT_STREQ(sd_json_variant_string(sd_json_variant_by_key(parameters, "result")), "hi+"); + ASSERT_OK(sd_event_exit(sd_varlink_get_event(link), EXIT_SUCCESS)); + return 0; +} + +TEST(mandatory_type_with_optional_field) { + _cleanup_(sd_event_unrefp) sd_event *e = NULL; + ASSERT_OK(sd_event_default(&e)); + + _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *s = NULL; + ASSERT_OK(sd_varlink_server_new(&s, 0)); + + ASSERT_OK(sd_varlink_server_attach_event(s, e, 0)); + + ASSERT_OK(sd_varlink_server_bind_method(s, "foo.mytest.MandatoryTypeWithOptionalField", method_mandatory_type_with_optional_field)); + + int connfd[2]; + ASSERT_OK_ERRNO(socketpair(AF_UNIX, SOCK_STREAM|SOCK_NONBLOCK|SOCK_CLOEXEC, 0, connfd)); + ASSERT_OK(sd_varlink_server_add_connection(s, connfd[0], /* ret= */ NULL)); + + _cleanup_(sd_varlink_unrefp) sd_varlink *c = NULL; + ASSERT_OK(sd_varlink_connect_fd(&c, connfd[1])); + + ASSERT_OK(sd_varlink_attach_event(c, e, 0)); + + ASSERT_OK(sd_varlink_bind_reply(c, reply_valid_hi)); + + ASSERT_OK(sd_varlink_invokebo(c, "foo.mytest.MandatoryTypeWithOptionalField", + SD_JSON_BUILD_PAIR_OBJECT("mandatoryField", + SD_JSON_BUILD_PAIR_STRING("mandatory", "hi")))); + + ASSERT_OK(sd_event_loop(e)); +} + static int method_with_error_sentinel(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { /* Set an error sentinel and return without sending a reply. The sentinel error should be sent automatically. */ ASSERT_OK(sd_varlink_set_sentinel(link, "io.test.SentinelError")); From d0fbca06c5006ddf8857c6998b38e143ccd05e6d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 1 Jun 2026 13:14:29 +0100 Subject: [PATCH 11/15] test: Factor out check-new calls in sysupdate integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This introduces no functional changes. In a following commit we’ll use this to check for updates via varlink as well. --- test/units/TEST-72-SYSUPDATE.sh | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index 6cabcab5bb..928f110b69 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -121,6 +121,14 @@ new_version() { fi } +check_no_new_update_available() { + (! "$SYSUPDATE" --verify=no check-new) +} + +check_new_update_available() { + "$SYSUPDATE" --verify=no check-new +} + update_now() { local update_type="${1:?}" local checks="${2:-}" @@ -135,7 +143,7 @@ update_now() { # repairing an installation), so that can be overridden via the local. if [[ "$checks" != "no-checks" ]]; then - "$SYSUPDATE" --verify=no check-new + check_new_update_available fi if [[ "$update_type" == "monolithic" ]]; then @@ -159,7 +167,7 @@ update_now() { fi if [[ "$checks" != "no-checks" ]]; then - (! "$SYSUPDATE" --verify=no check-new) + check_no_new_update_available fi } @@ -394,7 +402,7 @@ EOF new_version "$sector_size" v4 rm "$WORKDIR/source/uki-extra-v4.efi" update_checksums - (! "$SYSUPDATE" --verify=no check-new) + check_no_new_update_available # Create a fifth version, that's complete on the server side. We should # completely skip the incomplete v4 and install v5 instead. @@ -429,7 +437,7 @@ EOF # And now let's disable it and make sure it gets cleaned up rm -r "$CONFIGDIR/optional.feature.d" - (! "$SYSUPDATE" --verify=no check-new) + check_no_new_update_available "$SYSUPDATE" vacuum "$SYSUPDATE" --offline list v5 | grep -v "incomplete" >/dev/null verify_version "$blockdev" "$sector_size" v3 1 @@ -441,7 +449,7 @@ EOF new_version "$sector_size" v6 if [[ -x "$UPDATECTL" ]]; then systemctl start systemd-sysupdated - "$SYSUPDATE" --verify=no check-new + check_new_update_available "$UPDATECTL" update |& tee "$WORKDIR"/updatectl-update-6 grep "Done" "$WORKDIR"/updatectl-update-6 (! grep "Already up-to-date" "$WORKDIR"/updatectl-update-6) @@ -451,7 +459,7 @@ EOF fi # User-facing updatectl returns 0 if there's no updates, so use the low-level # utility to make sure we did upgrade - (! "$SYSUPDATE" --verify=no check-new ) + check_no_new_update_available verify_version_current "$blockdev" "$sector_size" v6 1 verify_version "$blockdev" "$sector_size" v5 2 From 2d201a107bea2a407fe35f8ad34cbdc6e1551faf Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 29 May 2026 13:37:16 +0100 Subject: [PATCH 12/15] sysupdate: Add basic varlink interface scaffolding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds the scaffolding for being able to call sysupdate via varlink, but it doesn’t yet define or implement any methods. Those will come in following commits. The existing `systemd-sysupdate.service` and `systemd-sysupdate.timer` (which periodically ran `systemd-sysupdate update`) have been renamed to `systemd-sysupdate-update.{service,timer}` to make way for a new `systemd-sysupdate@.service` and `systemd-sysupdate.socket` file to handle varlink activation. Compatibility symlinks have been added for them. --- NEWS | 8 ++ man/rules/meson.build | 2 + man/systemd-sysupdate.xml | 15 ++-- src/libsystemd/sd-varlink/test-varlink-idl.c | 2 + src/shared/meson.build | 1 + src/shared/varlink-io.systemd.SysUpdate.c | 9 ++ src/shared/varlink-io.systemd.SysUpdate.h | 6 ++ src/sysupdate/sysupdate.c | 83 +++++++++++++++++++ units/meson.build | 15 +++- ...rvice => systemd-sysupdate-update.service} | 6 +- ...e.timer => systemd-sysupdate-update.timer} | 2 +- units/systemd-sysupdate.socket | 28 +++++++ units/systemd-sysupdate@.service | 31 +++++++ 13 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 src/shared/varlink-io.systemd.SysUpdate.c create mode 100644 src/shared/varlink-io.systemd.SysUpdate.h rename units/{systemd-sysupdate.service => systemd-sysupdate-update.service} (86%) rename units/{systemd-sysupdate.timer => systemd-sysupdate-update.timer} (94%) create mode 100644 units/systemd-sysupdate.socket create mode 100644 units/systemd-sysupdate@.service diff --git a/NEWS b/NEWS index e482c3f898..d34215377e 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,14 @@ CHANGES WITH 262: the symlink target. Previously, these were only expanded in the source. + * As part of changes to systemd-sysupdate, the existing + "systemd-sysupdate.service" and "systemd-sysupdate.timer" units – + which periodically ran "systemd-sysupdate" to update the host system – + have been renamed to "systemd-sysupdate-update.service" and + "systemd-sysupdate-update.timer" respectively. Compatibility symlinks + have been provided. This clears the way for a new + "systemd-sysupdate@.service" unit for varlink activation of sysupdate. + CHANGES WITH 261: Announcements of Future Feature Removals and Incompatible Changes: diff --git a/man/rules/meson.build b/man/rules/meson.build index 974d9eee4b..13025b8993 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -1267,6 +1267,8 @@ manpages = [ '8', ['systemd-sysupdate-reboot.service', 'systemd-sysupdate-reboot.timer', + 'systemd-sysupdate-update.service', + 'systemd-sysupdate-update.timer', 'systemd-sysupdate.service', 'systemd-sysupdate.timer'], 'ENABLE_SYSUPDATE'], diff --git a/man/systemd-sysupdate.xml b/man/systemd-sysupdate.xml index 7f4b089031..2855c650c8 100644 --- a/man/systemd-sysupdate.xml +++ b/man/systemd-sysupdate.xml @@ -18,10 +18,15 @@ systemd-sysupdate - systemd-sysupdate.service - systemd-sysupdate.timer + systemd-sysupdate-update.service + systemd-sysupdate-update.timer systemd-sysupdate-reboot.service systemd-sysupdate-reboot.timer + + + systemd-sysupdate.service + systemd-sysupdate.timer + Automatically Update OS or Other Resources @@ -31,7 +36,7 @@ OPTIONS - systemd-sysupdate.service + systemd-sysupdate-update.service @@ -73,9 +78,9 @@ embedded in the disk images. For the latter, see below. The latter is particularly interesting to update container images or portable service images. - The systemd-sysupdate.service system service will automatically update the + The systemd-sysupdate-update.service system service will automatically update the host OS based on the installed transfer files. It is triggered in regular intervals via - systemd-sysupdate.timer. The systemd-sysupdate-reboot.service + systemd-sysupdate-update.timer. The systemd-sysupdate-reboot.service will automatically reboot the system after a new version is installed. It is triggered via systemd-sysupdate-reboot.timer. The two services are separate from each other as it is typically advisable to download updates regularly while the system is up, but delay reboots until the diff --git a/src/libsystemd/sd-varlink/test-varlink-idl.c b/src/libsystemd/sd-varlink/test-varlink-idl.c index 0b3304a43c..a1ed9a0f42 100644 --- a/src/libsystemd/sd-varlink/test-varlink-idl.c +++ b/src/libsystemd/sd-varlink/test-varlink-idl.c @@ -51,6 +51,7 @@ #include "varlink-io.systemd.Resolve.Monitor.h" #include "varlink-io.systemd.Shutdown.h" #include "varlink-io.systemd.StorageProvider.h" +#include "varlink-io.systemd.SysUpdate.h" #include "varlink-io.systemd.SysUpdate.Notify.h" #include "varlink-io.systemd.Udev.h" #include "varlink-io.systemd.Unit.h" @@ -228,6 +229,7 @@ TEST(parse_format) { &vl_interface_io_systemd_Resolve_Monitor, &vl_interface_io_systemd_Shutdown, &vl_interface_io_systemd_StorageProvider, + &vl_interface_io_systemd_SysUpdate, &vl_interface_io_systemd_SysUpdate_Notify, &vl_interface_io_systemd_Udev, &vl_interface_io_systemd_Unit, diff --git a/src/shared/meson.build b/src/shared/meson.build index 6fa4216305..e2bef83bd2 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -251,6 +251,7 @@ shared_sources = files( 'varlink-io.systemd.Resolve.Monitor.c', 'varlink-io.systemd.Shutdown.c', 'varlink-io.systemd.StorageProvider.c', + 'varlink-io.systemd.SysUpdate.c', 'varlink-io.systemd.SysUpdate.Notify.c', 'varlink-io.systemd.Udev.c', 'varlink-io.systemd.Unit.c', diff --git a/src/shared/varlink-io.systemd.SysUpdate.c b/src/shared/varlink-io.systemd.SysUpdate.c new file mode 100644 index 0000000000..67595fe9ee --- /dev/null +++ b/src/shared/varlink-io.systemd.SysUpdate.c @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "bus-polkit.h" +#include "varlink-io.systemd.SysUpdate.h" + +SD_VARLINK_DEFINE_INTERFACE( + io_systemd_SysUpdate, + "io.systemd.SysUpdate", + SD_VARLINK_INTERFACE_COMMENT("APIs to manage system updates")); diff --git a/src/shared/varlink-io.systemd.SysUpdate.h b/src/shared/varlink-io.systemd.SysUpdate.h new file mode 100644 index 0000000000..8399cdfbc3 --- /dev/null +++ b/src/shared/varlink-io.systemd.SysUpdate.h @@ -0,0 +1,6 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "sd-varlink-idl.h" + +extern const sd_varlink_interface vl_interface_io_systemd_SysUpdate; diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index f0f4ccefc1..f1ba327c5d 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -2,11 +2,13 @@ #include +#include "sd-bus.h" #include "sd-daemon.h" #include "sd-json.h" #include "sd-varlink.h" #include "build.h" +#include "bus-polkit.h" #include "conf-files.h" #include "constants.h" #include "dissect-image.h" @@ -19,6 +21,7 @@ #include "help-util.h" #include "hexdecoct.h" #include "image-policy.h" +#include "json-util.h" #include "loop-util.h" #include "main-func.h" #include "mount-util.h" @@ -39,6 +42,7 @@ #include "sysupdate-transfer.h" #include "sysupdate-update-set.h" #include "sysupdate-util.h" +#include "varlink-io.systemd.SysUpdate.h" #include "varlink-util.h" #include "verbs.h" @@ -58,6 +62,7 @@ static int arg_verify = -1; static ImagePolicy *arg_image_policy = NULL; static bool arg_offline = false; static char *arg_transfer_source = NULL; +static bool arg_varlink = false; STATIC_DESTRUCTOR_REGISTER(arg_definitions, freep); STATIC_DESTRUCTOR_REGISTER(arg_root, freep); @@ -154,6 +159,25 @@ static int context_from_cmdline(Context *ret) { return 0; } +/* Stores any long-running server state which needs to persist between varlink calls, such as state for + * pending polkit requests */ +typedef struct Server { + sd_bus *system_bus; + Hashmap *polkit_registry; +} Server; + +#define SERVER_NULL \ + (Server) { \ + /* all fields fine with being initialised to NULL */ \ + } + +static void server_done(Server *s) { + assert(s); + + s->polkit_registry = hashmap_free(s->polkit_registry); + s->system_bus = sd_bus_flush_close_unref(s->system_bus); +} + static DEFINE_POINTER_ARRAY_FREE_FUNC(Transfer*, transfer_free); static int read_definitions( @@ -1413,6 +1437,29 @@ static int context_install( return 1; } +static int verify_polkit(Context *context, sd_varlink *link, const char *action, const char **details) { + int r; + Server *s = ASSERT_PTR(sd_varlink_get_userdata(ASSERT_PTR(link))); + + assert(context); + + if (!s->system_bus) { + r = sd_bus_open_system_with_description(&s->system_bus, "sysupdate-system"); + if (r < 0) + return log_error_errno(r, "Failed to get system bus connection: %m"); + + r = sd_bus_attach_event(s->system_bus, sd_varlink_get_event(link), SD_EVENT_PRIORITY_NORMAL); + if (r < 0) + return log_error_errno(r, "Failed to attach system bus to event loop: %m"); + } + + return varlink_verify_polkit_async(link, + s->system_bus, + action, + details, + &s->polkit_registry); +} + VERB(verb_list, "list", "[VERSION]", VERB_ANY, 2, VERB_DEFAULT, "Show installed and available versions"); static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { @@ -2220,10 +2267,43 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) { if (arg_definitions && arg_component) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --definitions= and --component= switches may not be combined."); + r = sd_varlink_invocation(SD_VARLINK_ALLOW_ACCEPT); + if (r < 0) + return log_error_errno(r, "Failed to check if invoked in Varlink mode: %m"); + if (r > 0) + arg_varlink = true; + *remaining_args = option_parser_get_args(&opts); return 1; } +static int vl_server(void) { + _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *varlink_server = NULL; + _cleanup_(server_done) Server server = SERVER_NULL; + int r; + + r = varlink_server_new(&varlink_server, + SD_VARLINK_SERVER_ACCOUNT_UID|SD_VARLINK_SERVER_INHERIT_USERDATA, + &server); + if (r < 0) + return log_error_errno(r, "Failed to allocate Varlink server: %m"); + + r = sd_varlink_server_add_interface(varlink_server, &vl_interface_io_systemd_SysUpdate); + if (r < 0) + return log_error_errno(r, "Failed to add Varlink interface: %m"); + + r = sd_varlink_server_bind_method_many( + varlink_server); + if (r < 0) + return log_error_errno(r, "Failed to bind Varlink method: %m"); + + r = sd_varlink_server_loop_auto(varlink_server); + if (r < 0) + return log_error_errno(r, "Failed to run Varlink event loop: %m"); + + return 0; +} + static int run(int argc, char *argv[]) { int r; @@ -2234,6 +2314,9 @@ static int run(int argc, char *argv[]) { if (r <= 0) return r; + if (arg_varlink) + return vl_server(); /* Invocation as Varlink service */ + return dispatch_verb(args, NULL); } diff --git a/units/meson.build b/units/meson.build index 83dae605fa..569aa2e2a2 100644 --- a/units/meson.build +++ b/units/meson.build @@ -849,12 +849,23 @@ units = [ 'conditions' : ['ENABLE_SYSUPDATE'], }, { - 'file' : 'systemd-sysupdate.service', + 'file' : 'systemd-sysupdate-update.service', + 'conditions' : ['ENABLE_SYSUPDATE'], + 'symlinks' : ['systemd-sysupdate.service'], # compatibility after rename + }, + { + 'file' : 'systemd-sysupdate@.service', 'conditions' : ['ENABLE_SYSUPDATE'], }, { - 'file' : 'systemd-sysupdate.timer', + 'file' : 'systemd-sysupdate.socket', 'conditions' : ['ENABLE_SYSUPDATE'], + 'symlinks' : ['sockets.target.wants/'], + }, + { + 'file' : 'systemd-sysupdate-update.timer', + 'conditions' : ['ENABLE_SYSUPDATE'], + 'symlinks' : ['systemd-sysupdate.timer'], # compatibility after rename }, { 'file' : 'systemd-sysupdated.service.in', diff --git a/units/systemd-sysupdate.service b/units/systemd-sysupdate-update.service similarity index 86% rename from units/systemd-sysupdate.service rename to units/systemd-sysupdate-update.service index 92ec7266e8..e8a634bfff 100644 --- a/units/systemd-sysupdate.service +++ b/units/systemd-sysupdate-update.service @@ -9,7 +9,7 @@ [Unit] Description=Automatic System Update -Documentation=man:systemd-sysupdate.service(8) +Documentation=man:systemd-sysupdate-update.service(8) Wants=network-online.target After=network-online.target ConditionVirtualization=!container @@ -18,6 +18,8 @@ ConditionVirtualization=!container Type=simple NotifyAccess=main ExecStart=systemd-sysupdate update --cleanup=yes + +# Keep this sandboxing synchronised with systemd-sysupdate@.service CapabilityBoundingSet=CAP_CHOWN CAP_FOWNER CAP_FSETID CAP_MKNOD CAP_SETFCAP CAP_SYS_ADMIN CAP_SETPCAP CAP_DAC_OVERRIDE CAP_LINUX_IMMUTABLE NoNewPrivileges=yes MemoryDenyWriteExecute=yes @@ -31,4 +33,4 @@ SystemCallArchitectures=native LockPersonality=yes [Install] -Also=systemd-sysupdate.timer +Also=systemd-sysupdate-update.timer diff --git a/units/systemd-sysupdate.timer b/units/systemd-sysupdate-update.timer similarity index 94% rename from units/systemd-sysupdate.timer rename to units/systemd-sysupdate-update.timer index b2c7cd4808..d055948c29 100644 --- a/units/systemd-sysupdate.timer +++ b/units/systemd-sysupdate-update.timer @@ -9,7 +9,7 @@ [Unit] Description=Automatic System Update -Documentation=man:systemd-sysupdate.service(8) +Documentation=man:systemd-sysupdate-update.service(8) # For containers we assume that the manager will handle updates. And we likely # can't even access our backing block device anyway. diff --git a/units/systemd-sysupdate.socket b/units/systemd-sysupdate.socket new file mode 100644 index 0000000000..d91ec793e7 --- /dev/null +++ b/units/systemd-sysupdate.socket @@ -0,0 +1,28 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=System Updates +Documentation=man:systemd-sysupdate(8) +DefaultDependencies=no +Before=sockets.target + +[Socket] +ListenStream=/run/systemd/io.systemd.SysUpdate +Symlinks=/run/varlink/registry/io.systemd.SysUpdate +FileDescriptorName=varlink +SocketMode=0666 +Accept=yes +MaxConnectionsPerSource=16 +XAttrEntryPoint=user.varlink=entrypoint +XAttrListen=user.varlink=listen +XAttrAccept=user.varlink=server + +[Install] +WantedBy=sockets.target diff --git a/units/systemd-sysupdate@.service b/units/systemd-sysupdate@.service new file mode 100644 index 0000000000..f8e86dd2c4 --- /dev/null +++ b/units/systemd-sysupdate@.service @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=System Updates +Documentation=man:systemd-sysupdate(8) +DefaultDependencies=no +Conflicts=shutdown.target +Before=shutdown.target + +[Service] +ExecStart=-systemd-sysupdate + +# Keep this sandboxing synchronised with systemd-sysupdate-update.service +CapabilityBoundingSet=CAP_CHOWN CAP_FOWNER CAP_FSETID CAP_MKNOD CAP_SETFCAP CAP_SYS_ADMIN CAP_SETPCAP CAP_DAC_OVERRIDE CAP_LINUX_IMMUTABLE +NoNewPrivileges=yes +MemoryDenyWriteExecute=yes +ProtectHostname=yes +RestrictRealtime=yes +RestrictNamespaces=net +RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 +SystemCallFilter=@system-service @mount +SystemCallErrorNumber=EPERM +SystemCallArchitectures=native +LockPersonality=yes From 7757ec3317312f3ce0ceb69f394258bbf066c48f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 29 May 2026 15:34:56 +0100 Subject: [PATCH 13/15] sysupdate: Add varlink CheckNew() method This is the first varlink method added to sysupdate. The D-Bus interface (via sysupdated) will remain for now; the varlink interface will exist in parallel. This method can be called via: ``` varlinkctl call ./path/to/systemd-sysupdate \ io.systemd.SysUpdate.CheckNew \ '{"target":{"class":"host"}}' SYSTEMD_SYSUPDATE_NO_VERIFY=1 \ varlinkctl call ./path/to/systemd-sysupdate \ io.systemd.SysUpdate.CheckNew \ '{"target":{"class":"component","name":"some-component"}}' ``` This includes some changes to run the integration tests again using the varlink interface rather than running `systemd-sysupdate` directly, to test the new interface. --- src/shared/varlink-io.systemd.SysUpdate.c | 51 ++++- src/sysupdate/sysupdate.c | 220 +++++++++++++++++++++- src/sysupdate/sysupdate.h | 3 + test/units/TEST-72-SYSUPDATE.sh | 75 +++++--- 4 files changed, 324 insertions(+), 25 deletions(-) diff --git a/src/shared/varlink-io.systemd.SysUpdate.c b/src/shared/varlink-io.systemd.SysUpdate.c index 67595fe9ee..e80142f9c5 100644 --- a/src/shared/varlink-io.systemd.SysUpdate.c +++ b/src/shared/varlink-io.systemd.SysUpdate.c @@ -3,7 +3,56 @@ #include "bus-polkit.h" #include "varlink-io.systemd.SysUpdate.h" +static SD_VARLINK_DEFINE_ENUM_TYPE( + TargetClass, + SD_VARLINK_FIELD_COMMENT("Container or machine managed by systemd-machined.service(8)."), + SD_VARLINK_DEFINE_ENUM_VALUE(machine), + SD_VARLINK_FIELD_COMMENT("Portable service."), + SD_VARLINK_DEFINE_ENUM_VALUE(portable), + SD_VARLINK_FIELD_COMMENT("System extension managed by systemd-sysext.service(8)."), + SD_VARLINK_DEFINE_ENUM_VALUE(sysext), + SD_VARLINK_FIELD_COMMENT("Configuration extension managed by systemd-confext.service(8)."), + SD_VARLINK_DEFINE_ENUM_VALUE(confext), + SD_VARLINK_FIELD_COMMENT("Host system."), + SD_VARLINK_DEFINE_ENUM_VALUE(host), + SD_VARLINK_FIELD_COMMENT("Component managed by systemd-sysupdate.service(8)."), + SD_VARLINK_DEFINE_ENUM_VALUE(component)); + +static SD_VARLINK_DEFINE_STRUCT_TYPE( + TargetIdentifier, + SD_VARLINK_FIELD_COMMENT("Where the target was enumerated."), + SD_VARLINK_DEFINE_FIELD_BY_TYPE(class, TargetClass, 0), + SD_VARLINK_FIELD_COMMENT("Name of the target, unique within a class."), + SD_VARLINK_DEFINE_FIELD(name, SD_VARLINK_STRING, SD_VARLINK_NULLABLE)); + +static SD_VARLINK_DEFINE_METHOD( + CheckNew, + SD_VARLINK_FIELD_COMMENT("Target to check for updates for."), + SD_VARLINK_DEFINE_INPUT_BY_TYPE(target, TargetIdentifier, 0), + VARLINK_DEFINE_POLKIT_INPUT, + SD_VARLINK_FIELD_COMMENT("The version string for the new version which is available."), + SD_VARLINK_DEFINE_OUTPUT(available, SD_VARLINK_STRING, 0)); + +static SD_VARLINK_DEFINE_ERROR(NoUpdateNeeded); +static SD_VARLINK_DEFINE_ERROR(NoSuchTarget); + SD_VARLINK_DEFINE_INTERFACE( io_systemd_SysUpdate, "io.systemd.SysUpdate", - SD_VARLINK_INTERFACE_COMMENT("APIs to manage system updates")); + SD_VARLINK_INTERFACE_COMMENT("APIs to manage system updates"), + + /* Methods */ + SD_VARLINK_SYMBOL_COMMENT("Check if there’s a new version available"), + &vl_method_CheckNew, + + /* Types */ + SD_VARLINK_SYMBOL_COMMENT("Class of a Target."), + &vl_type_TargetClass, + SD_VARLINK_SYMBOL_COMMENT("Identifier for a component of the system (i.e. the host itself, a sysext, a confext, etc.) that can be updated by systemd-sysupdate(8)."), + &vl_type_TargetIdentifier, + + /* Errors */ + SD_VARLINK_SYMBOL_COMMENT("Error indicating that no update is currently available to update to."), + &vl_error_NoUpdateNeeded, + SD_VARLINK_SYMBOL_COMMENT("Error indicating the specified target doesn’t exist"), + &vl_error_NoSuchTarget); diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index f1ba327c5d..d3874c377d 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -11,6 +11,7 @@ #include "bus-polkit.h" #include "conf-files.h" #include "constants.h" +#include "discover-image.h" #include "dissect-image.h" #include "env-util.h" #include "errno-util.h" @@ -31,6 +32,7 @@ #include "parse-argument.h" #include "parse-util.h" #include "pretty-print.h" +#include "runtime-scope.h" #include "sort-util.h" #include "specifier.h" #include "string-util.h" @@ -39,6 +41,7 @@ #include "sysupdate-cleanup.h" #include "sysupdate-feature.h" #include "sysupdate-instance.h" +#include "sysupdate-target.h" #include "sysupdate-transfer.h" #include "sysupdate-update-set.h" #include "sysupdate-util.h" @@ -84,6 +87,7 @@ const Specifier specifier_table[] = { .verify = -1, \ .cleanup = -1, \ .installdb_fd = -EBADF, \ + .target_identifier.class = _TARGET_CLASS_INVALID, \ } void context_done(Context *c) { @@ -119,6 +123,8 @@ void context_done(Context *c) { c->component = mfree(c->component); c->image_policy = image_policy_free(c->image_policy); c->transfer_source = mfree(c->transfer_source); + + target_identifier_done(&c->target_identifier); } static int context_from_cmdline(Context *ret) { @@ -1047,6 +1053,8 @@ static int process_image( return 0; } +static int context_list_components(Context *context, char ***component_names, bool *has_default_component); + static int context_load_offline( Context *context, ProcessImageFlags process_image_flags, @@ -1103,6 +1111,144 @@ static int context_load_online( return 0; } +static bool image_type_can_sysupdate(ImageType image_type) { + switch (image_type) { + case IMAGE_DIRECTORY: + case IMAGE_SUBVOLUME: + case IMAGE_RAW: + case IMAGE_BLOCK: + return true; + + /* systemd-sysupdate doesn't support mstack images yet */ + case IMAGE_MSTACK: + default: + return false; + } +} + +static int context_load_paths_from_image(Context *context, Image *image) { + assert(context); + assert(image); + + assert(!context->root); + assert(!context->image); + + switch (image->type) { + case IMAGE_DIRECTORY: + case IMAGE_SUBVOLUME: + context->root = strdup(image->path); + if (!context->root) + return log_oom(); + return 0; + case IMAGE_RAW: + case IMAGE_BLOCK: + context->image = strdup(image->path); + if (!context->image) + return log_oom(); + return 0; + default: + assert_not_reached(); + } +} + +/* Load a Context to point to the target given by the TargetIdentifier. The TargetIdentifier will have been + * syntactically validated by dispatch_target_identifier(), but might still point to components which don’t + * exist, images which the user isn’t privileged to access, etc. This function validates the TargetIdentifier + * against an enumerated list of known targets, which are safe to update without additional permissions. */ +static int context_load_online_from_target(Context *context, ProcessImageFlags process_image_flags) { + int r; + + assert(context); + assert(context->target_identifier.class != _TARGET_CLASS_INVALID); + + /* These shouldn’t have been set up some other way first */ + assert(!context->component); + assert(!context->root); + assert(!context->image); + + switch (context->target_identifier.class) { + case TARGET_MACHINE: + case TARGET_PORTABLE: + case TARGET_SYSEXT: + case TARGET_CONFEXT: { + _cleanup_hashmap_free_ Hashmap *images = NULL; + Image *image, *selected_image = NULL; + + /* These are all image-based target classes, so first find the corresponding image. */ + r = image_discover(RUNTIME_SCOPE_SYSTEM, (ImageClass) context->target_identifier.class, NULL, &images); + if (r < 0) + return r; + + HASHMAP_FOREACH(image, images) { + bool have = false; + _cleanup_(context_done) Context image_context = CONTEXT_NULL; + + if (image_is_host(image)) + continue; /* We already enroll the host ourselves */ + + if (!image_type_can_sysupdate(image->type)) + continue; + + if (!streq(image->name, context->target_identifier.name)) + continue; + + r = context_load_paths_from_image(&image_context, image); + if (r < 0) + return r; + + /* Load the components in a separate Context specific to the given Image before + * committing to loading that state to the main Context. */ + r = context_load_offline(&image_context, 0, 0); + if (r < 0) + return r; + + r = context_list_components(&image_context, /* component_names= */ NULL, &have); + if (r < 0) + return r; + if (!have) { + log_debug("Skipping %s because it has no default component", image->path); + continue; + } + + /* This is the match we were looking for */ + selected_image = image; + break; + } + + if (!selected_image) + return -ENOENT; + + r = context_load_paths_from_image(context, selected_image); + if (r < 0) + return r; + + break; + } + case TARGET_HOST: + /* No additional setup needed */ + break; + case TARGET_COMPONENT: { + _cleanup_strv_free_ char **component_names = NULL; + + r = context_list_components(context, &component_names, /* has_default_component= */ NULL); + if (r < 0) + return r; + + if (!strv_contains(component_names, context->target_identifier.name)) + return -ENOENT; + + context->component = strdup(context->target_identifier.name); + if (!context->component) + return log_oom(); + break; + } + default: + assert_not_reached(); + } + + return context_load_online(context, process_image_flags); +} + static int context_on_acquire_progress(const Transfer *t, const Instance *inst, unsigned percentage) { const Context *c = ASSERT_PTR(t->context); size_t i, n = c->n_transfers; @@ -1437,6 +1583,31 @@ static int context_install( return 1; } +static JSON_DISPATCH_ENUM_DEFINE(dispatch_target_class, TargetClass, target_class_from_string); + +static int dispatch_target_identifier(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) { + TargetIdentifier *t = ASSERT_PTR(userdata); + static const sd_json_dispatch_field dispatch[] = { + { "class", SD_JSON_VARIANT_STRING, dispatch_target_class, voffsetof(*t, class), SD_JSON_MANDATORY }, + { "name", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, voffsetof(*t, name), SD_JSON_NULLABLE }, + {} + }; + int r; + + r = sd_json_dispatch(variant, dispatch, flags, t); + if (r < 0) + return r; + + /* Name is mandatory unless class is `host` */ + if ((t->class == TARGET_HOST) != (!t->name)) + return json_log(variant, flags, SYNTHETIC_ERRNO(ENXIO), "Target name does not match class."); + + if (t->class == TARGET_COMPONENT && !component_name_valid(t->name)) + return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "Component name invalid: %s", t->name); + + return 0; +} + static int verify_polkit(Context *context, sd_varlink *link, const char *action, const char **details) { int r; Server *s = ASSERT_PTR(sd_varlink_get_userdata(ASSERT_PTR(link))); @@ -1717,6 +1888,52 @@ static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdat return EXIT_SUCCESS; } +static int vl_method_check_new(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { + _cleanup_(context_done) Context context = CONTEXT_NULL; + int r; + + assert(link); + + static const sd_json_dispatch_field dispatch_table[] = { + { "target", SD_JSON_VARIANT_OBJECT, dispatch_target_identifier, voffsetof(context, target_identifier), SD_JSON_MANDATORY }, + VARLINK_DISPATCH_POLKIT_FIELD, + {}, + }; + + r = sd_varlink_dispatch(link, parameters, dispatch_table, &context); + if (r != 0) + return r; + + r = verify_polkit(&context, link, "org.freedesktop.sysupdate1.check", + (const char**) STRV_MAKE( + "class", target_class_to_string(context.target_identifier.class), + "offline", "0", + context.target_identifier.name ? "name" : NULL, context.target_identifier.name)); + if (r <= 0) + return r; + + if (getenv_bool("SYSTEMD_SYSUPDATE_NO_VERIFY") > 0) + context.verify = 0; + + /* CheckNew is always online */ + context.offline = false; + + r = context_load_online_from_target(&context, PROCESS_IMAGE_READ_ONLY); + if (r == -ENOENT) + return sd_varlink_error(link, "io.systemd.SysUpdate.NoSuchTarget", NULL); + else if (r < 0) + return r; + + if (context.candidate) + r = sd_varlink_replybo(link, SD_JSON_BUILD_PAIR_STRING("available", context.candidate->version)); + else + r = sd_varlink_error(link, "io.systemd.SysUpdate.NoUpdateNeeded", NULL); + if (r < 0) + return r; + + return 0; +} + typedef enum { UPDATE_ACTION_ACQUIRE = 1 << 0, UPDATE_ACTION_INSTALL = 1 << 1, @@ -2293,7 +2510,8 @@ static int vl_server(void) { return log_error_errno(r, "Failed to add Varlink interface: %m"); r = sd_varlink_server_bind_method_many( - varlink_server); + varlink_server, + "io.systemd.SysUpdate.CheckNew", vl_method_check_new); if (r < 0) return log_error_errno(r, "Failed to bind Varlink method: %m"); diff --git a/src/sysupdate/sysupdate.h b/src/sysupdate/sysupdate.h index 0d0a8e9fbf..02a9f7c8f3 100644 --- a/src/sysupdate/sysupdate.h +++ b/src/sysupdate/sysupdate.h @@ -3,6 +3,7 @@ #include "specifier.h" #include "sysupdate-forward.h" +#include "sysupdate-target.h" typedef struct Context { /* Parameters/Command line arguments: */ @@ -40,6 +41,8 @@ typedef struct Context { Hashmap *web_cache; /* Cache for downloaded resources, keyed by URL */ int installdb_fd; + + TargetIdentifier target_identifier; } Context; void context_done(Context *c); diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index 928f110b69..f8ba6035df 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -8,6 +8,7 @@ set -o pipefail SYSUPDATE=/usr/bin/systemd-sysupdate SYSUPDATED=/lib/systemd/systemd-sysupdated UPDATECTL="" +VARLINK_SOCKET=/run/systemd/io.systemd.SysUpdate SECTOR_SIZES=(512 4096) WORKDIR="$(mktemp -d /var/tmp/test-72-XXXXXX)" CONFIGDIR="/run/sysupdate.d" @@ -35,8 +36,8 @@ if [[ ! -e /dev/loop-control ]]; then SECTOR_SIZES=(512) fi -# Set up sysupdated drop-in pointing at the correct definitions and setting -# no verification of images. +# Set up sysupdated and varlink drop-ins pointing at the correct definitions and +# setting no verification of images. mkdir -p /run/systemd/system/systemd-sysupdated.service.d cat >/run/systemd/system/systemd-sysupdated.service.d/override.conf</run/systemd/system/systemd-sysupdate@.service.d/override.conf</dev/null + else + exit 1 + fi } check_new_update_available() { - "$SYSUPDATE" --verify=no check-new + local client="${1:?}" + + if [[ "$client" == "sysupdate-cli" ]]; then + "$SYSUPDATE" --verify=no check-new + elif [[ "$client" == "varlink" ]]; then + varlinkctl call "$VARLINK_SOCKET" io.systemd.SysUpdate.CheckNew '{"target":{"class":"host"}}' | grep available + else + exit 1 + fi } update_now() { local update_type="${1:?}" - local checks="${2:-}" + local client="${2:?}" + local checks="${3:-}" # Update to newest version. First there should be an update ready, then we # do the update, and then there should not be any ready anymore @@ -143,7 +170,7 @@ update_now() { # repairing an installation), so that can be overridden via the local. if [[ "$checks" != "no-checks" ]]; then - check_new_update_available + check_new_update_available "$client" fi if [[ "$update_type" == "monolithic" ]]; then @@ -167,7 +194,7 @@ update_now() { fi if [[ "$checks" != "no-checks" ]]; then - check_no_new_update_available + check_no_new_update_available "$client" fi } @@ -216,6 +243,7 @@ verify_object_fields() { } for sector_size in "${SECTOR_SIZES[@]}"; do +for client in sysupdate-cli varlink; do for update_type in monolithic split-offline split updatectl; do # Disk size of: # - 1MB for GPT @@ -379,18 +407,18 @@ EOF # Install initial version and verify new_version "$sector_size" v1 - update_now "$update_type" + update_now "$update_type" "$client" verify_version_current "$blockdev" "$sector_size" v1 1 # Create second version, update and verify that it is added new_version "$sector_size" v2 - update_now "$update_type" + update_now "$update_type" "$client" verify_version "$blockdev" "$sector_size" v1 1 verify_version_current "$blockdev" "$sector_size" v2 2 # Create third version, update and verify it replaced the first version new_version "$sector_size" v3 - update_now "$update_type" + update_now "$update_type" "$client" verify_version_current "$blockdev" "$sector_size" v3 1 verify_version "$blockdev" "$sector_size" v2 2 test ! -f "$WORKDIR/xbootldr/EFI/Linux/uki_v1+3-0.efi" @@ -402,12 +430,12 @@ EOF new_version "$sector_size" v4 rm "$WORKDIR/source/uki-extra-v4.efi" update_checksums - check_no_new_update_available + check_no_new_update_available "$client" # Create a fifth version, that's complete on the server side. We should # completely skip the incomplete v4 and install v5 instead. new_version "$sector_size" v5 - update_now "$update_type" + update_now "$update_type" "$client" verify_version "$blockdev" "$sector_size" v3 1 verify_version_current "$blockdev" "$sector_size" v5 2 @@ -417,7 +445,7 @@ EOF # Always do this as a monolithic update for the repair to work. rm -r "$WORKDIR/xbootldr/EFI/Linux/uki_v5.efi.extra.d" "$SYSUPDATE" --offline list v5 | grep "incomplete" >/dev/null - update_now "monolithic" + update_now "monolithic" "$client" "$SYSUPDATE" --offline list v5 | grep -v "incomplete" >/dev/null verify_version "$blockdev" "$sector_size" v3 1 verify_version_current "$blockdev" "$sector_size" v5 2 @@ -429,7 +457,7 @@ EOF mkdir "$CONFIGDIR/optional.feature.d" echo -e "[Feature]\nEnabled=true" > "$CONFIGDIR/optional.feature.d/enable.conf" "$SYSUPDATE" --offline list v5 | grep "incomplete" >/dev/null - update_now "$update_type" + update_now "$update_type" "$client" "$SYSUPDATE" --offline list v5 | grep -v "incomplete" >/dev/null verify_version "$blockdev" "$sector_size" v3 1 verify_version_current "$blockdev" "$sector_size" v5 2 @@ -437,7 +465,7 @@ EOF # And now let's disable it and make sure it gets cleaned up rm -r "$CONFIGDIR/optional.feature.d" - check_no_new_update_available + check_no_new_update_available "$client" "$SYSUPDATE" vacuum "$SYSUPDATE" --offline list v5 | grep -v "incomplete" >/dev/null verify_version "$blockdev" "$sector_size" v3 1 @@ -449,17 +477,17 @@ EOF new_version "$sector_size" v6 if [[ -x "$UPDATECTL" ]]; then systemctl start systemd-sysupdated - check_new_update_available + check_new_update_available "$client" "$UPDATECTL" update |& tee "$WORKDIR"/updatectl-update-6 grep "Done" "$WORKDIR"/updatectl-update-6 (! grep "Already up-to-date" "$WORKDIR"/updatectl-update-6) else # If no updatectl, gracefully fall back to systemd-sysupdate - update_now "$update_type" + update_now "$update_type" "$client" fi # User-facing updatectl returns 0 if there's no updates, so use the low-level # utility to make sure we did upgrade - check_no_new_update_available + check_no_new_update_available "$client" verify_version_current "$blockdev" "$sector_size" v6 1 verify_version "$blockdev" "$sector_size" v5 2 @@ -512,7 +540,7 @@ MatchPattern=dir-@v InstancesMax=3 EOF - update_now "$update_type" + update_now "$update_type" "$client" verify_version "$blockdev" "$sector_size" v6 1 verify_version_current "$blockdev" "$sector_size" v7 2 @@ -535,7 +563,7 @@ EOF # (what .transfer files were called before v257) for i in "$CONFIGDIR/"*.conf; do echo mv "$i" "${i%.conf}.transfer"; done new_version "$sector_size" v8 - update_now "$update_type" + update_now "$update_type" "$client" verify_version_current "$blockdev" "$sector_size" v8 1 verify_version "$blockdev" "$sector_size" v7 2 @@ -545,11 +573,11 @@ EOF # Vacuum the partial version, regenerate it on the server, try updating # again and it should succeed. new_version "$sector_size" v9 "corrupt-checksum" - (! update_now "$update_type") + (! update_now "$update_type" "$client") "$SYSUPDATE" --offline list v9 | grep "partial" >/dev/null verify_version_current "$blockdev" "$sector_size" v8 1 # don’t verify the other part of the block device as it’s in an indeterminate state - (! update_now "$update_type" "no-checks") |& tee "$WORKDIR"/update_now-9 + (! update_now "$update_type" "$client" "no-checks") |& tee "$WORKDIR"/update_now-9 cat "$WORKDIR"/update_now-9 grep "is already acquired and partially installed. Vacuum it to try installing again." "$WORKDIR"/update_now-9 "$SYSUPDATE" --offline vacuum |& grep "Removing old partial" >/dev/null @@ -557,7 +585,7 @@ EOF # don’t verify the other part of the block device as it’s in an indeterminate state "$SYSUPDATE" --verify=no list v9 | grep "candidate" >/dev/null new_version "$sector_size" v9 - update_now "$update_type" + update_now "$update_type" "$client" verify_version "$blockdev" "$sector_size" v8 1 verify_version_current "$blockdev" "$sector_size" v9 2 @@ -566,6 +594,7 @@ EOF rm "$BACKING_FILE" done 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 From dffff8be674ec4f9a963dc535cc0988b25684581 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 18 Jun 2026 13:57:35 +0100 Subject: [PATCH 14/15] test: Test error handling for non-existent targets in sysupdate Primarily I want to test the new varlink error, but this will also test the `systemd-sysupdate` CLI behaviour too. --- test/units/TEST-72-SYSUPDATE.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index f8ba6035df..a8bc7432c1 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -589,6 +589,18 @@ EOF verify_version "$blockdev" "$sector_size" v8 1 verify_version_current "$blockdev" "$sector_size" v9 2 + # Test that checking for an update on a non-existent target fails + # (for backwards compatibility reasons, the validation in sysupdate-cli is + # less strict) + if [[ "$client" == "sysupdate-cli" ]]; then + (! "$SYSUPDATE" --verify=no check-new --component=../) |& grep "Component name invalid" >/dev/null + elif [[ "$client" == "varlink" ]]; then + (! varlinkctl call "$VARLINK_SOCKET" io.systemd.SysUpdate.CheckNew '{"target":{"class":"component","name":"../"}}') |& grep org.varlink.service.InvalidParameter >/dev/null + (! varlinkctl call "$VARLINK_SOCKET" io.systemd.SysUpdate.CheckNew '{"target":{"class":"component","name":"doesnotexist"}}') |& grep io.systemd.SysUpdate.NoSuchTarget >/dev/null + else + exit 1 + fi + # Cleanup [[ -b "$blockdev" ]] && losetup --detach "$blockdev" rm "$BACKING_FILE" From e261b552c850221dd3fd510b8c32d2adf4e0044a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 26 Jun 2026 12:07:15 +0100 Subject: [PATCH 15/15] sysupdate: Fix some clang-tidy lint warnings for argument names Signed-off-by: Philip Withnall --- src/sysupdate/sysupdate-feature.c | 4 ++-- src/sysupdate/sysupdate-transfer.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sysupdate/sysupdate-feature.c b/src/sysupdate/sysupdate-feature.c index d67b61859c..0c9bb112fe 100644 --- a/src/sysupdate/sysupdate-feature.c +++ b/src/sysupdate/sysupdate-feature.c @@ -112,8 +112,8 @@ int feature_read_definition(Feature *f, const char *root, const char *path, cons config_item_table_lookup, table, CONFIG_PARSE_WARN, (void *) root, - /* stats_by_path= */ NULL, - /* drop_in_files= */ NULL); + /* ret_stats_by_path= */ NULL, + /* ret_drop_in_files= */ NULL); if (r < 0) return r; diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index f2a4377e1e..c79743cad6 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -563,8 +563,8 @@ int transfer_read_definition(Transfer *t, const char *path, const char **dirs, H config_item_table_lookup, table, CONFIG_PARSE_WARN, t, - /* stats_by_path= */ NULL, - /* drop_in_files= */ NULL); + /* ret_stats_by_path= */ NULL, + /* ret_drop_in_files= */ NULL); if (r < 0) return r;