From: Lennart Poettering Date: Mon, 18 Aug 2025 14:45:16 +0000 (+0200) Subject: importd: clean up how we determine image root in importd backends X-Git-Tag: v259-rc1~267^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d9c10bf1d2a46d4d4bafb3eef25048d414573824;p=thirdparty%2Fsystemd.git importd: clean up how we determine image root in importd backends Let's introduce a single helper that determines where to download images to, taking all three primary parameters into account: the image class, the runtime scope and whether to do runtime or persistency. Then port everything over to this. This not only cleans things up, but makes sure the importd backends actually properly can deal with per-user downloads, as before we never took the runtime scope into account for determining download location. --- diff --git a/src/import/import-fs.c b/src/import/import-fs.c index 342c024d56f..537fe46853a 100644 --- a/src/import/import-fs.c +++ b/src/import/import-fs.c @@ -35,10 +35,12 @@ static bool arg_btrfs_subvol = true; static bool arg_btrfs_quota = true; static bool arg_sync = true; static bool arg_direct = false; -static const char *arg_image_root = NULL; +static char *arg_image_root = NULL; static ImageClass arg_class = IMAGE_MACHINE; static RuntimeScope arg_runtime_scope = _RUNTIME_SCOPE_INVALID; +STATIC_DESTRUCTOR_REGISTER(arg_image_root, freep); + typedef struct ProgressInfo { RateLimit limit; char *path; @@ -344,7 +346,10 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_IMAGE_ROOT: - arg_image_root = optarg; + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image_root); + if (r < 0) + return r; + break; case ARG_READ_ONLY: @@ -398,8 +403,11 @@ static int parse_argv(int argc, char *argv[]) { assert_not_reached(); } - if (!arg_image_root) - arg_image_root = image_root_to_string(arg_class); + if (!arg_image_root) { + r = image_root_pick(arg_runtime_scope < 0 ? RUNTIME_SCOPE_SYSTEM : arg_runtime_scope, arg_class, /* runtime= */ false, &arg_image_root); + if (r < 0) + return log_error_errno(r, "Failed to pick image root: %m"); + } return 1; } diff --git a/src/import/import-generator.c b/src/import/import-generator.c index bf5f41a7472..4aab1618d32 100644 --- a/src/import/import-generator.c +++ b/src/import/import-generator.c @@ -17,6 +17,7 @@ #include "parse-util.h" #include "path-util.h" #include "proc-cmdline.h" +#include "runtime-scope.h" #include "specifier.h" #include "string-util.h" #include "unit-name.h" @@ -200,7 +201,10 @@ static int parse_pull_expression(const char *v) { if (!GREEDY_REALLOC(arg_transfers, arg_n_transfers + 1)) return log_oom(); - const char *image_root = runtime ? image_root_runtime_to_string(class) : image_root_to_string(class); + _cleanup_free_ char *image_root = NULL; + r = image_root_pick(RUNTIME_SCOPE_SYSTEM, class, runtime, &image_root); + if (r < 0) + return log_error_errno(r, "Failed to pick image root: %m"); _cleanup_(sd_json_variant_unrefp) sd_json_variant *j = NULL; r = sd_json_buildo( @@ -220,7 +224,7 @@ static int parse_pull_expression(const char *v) { .type = type, .local = TAKE_PTR(local), .remote = TAKE_PTR(remote), - .image_root = image_root, + .image_root = TAKE_PTR(image_root), .json = TAKE_PTR(j), .blockdev = blockdev, }; diff --git a/src/import/import.c b/src/import/import.c index f41a42f7e6b..531c0820770 100644 --- a/src/import/import.c +++ b/src/import/import.c @@ -26,12 +26,14 @@ #include "string-util.h" #include "verbs.h" -static const char *arg_image_root = NULL; +static char *arg_image_root = NULL; static ImportFlags arg_import_flags = IMPORT_BTRFS_SUBVOL | IMPORT_BTRFS_QUOTA | IMPORT_CONVERT_QCOW2 | IMPORT_SYNC; static uint64_t arg_offset = UINT64_MAX, arg_size_max = UINT64_MAX; static ImageClass arg_class = IMAGE_MACHINE; static RuntimeScope arg_runtime_scope = _RUNTIME_SCOPE_INVALID; +STATIC_DESTRUCTOR_REGISTER(arg_image_root, freep); + static int normalize_local(const char *local, char **ret) { _cleanup_free_ char *ll = NULL; int r; @@ -360,7 +362,10 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_IMAGE_ROOT: - arg_image_root = optarg; + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image_root); + if (r < 0) + return r; + break; case ARG_READ_ONLY: @@ -460,8 +465,11 @@ static int parse_argv(int argc, char *argv[]) { if (arg_offset != UINT64_MAX && !FLAGS_SET(arg_import_flags, IMPORT_DIRECT)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "File offset only supported in --direct mode."); - if (!arg_image_root) - arg_image_root = image_root_to_string(arg_class); + if (!arg_image_root) { + r = image_root_pick(arg_runtime_scope < 0 ? RUNTIME_SCOPE_SYSTEM : arg_runtime_scope, arg_class, /* runtime= */ false, &arg_image_root); + if (r < 0) + return log_error_errno(r, "Failed to pick image root: %m"); + } return 1; } diff --git a/src/import/importctl.c b/src/import/importctl.c index 731c311ffe7..d131943413d 100644 --- a/src/import/importctl.c +++ b/src/import/importctl.c @@ -50,15 +50,23 @@ static RuntimeScope arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; #define PROGRESS_PREFIX "Total:" static int settle_image_class(void) { + int r; if (arg_image_class < 0) { _cleanup_free_ char *j = NULL; - for (ImageClass class = 0; class < _IMAGE_CLASS_MAX; class++) + for (ImageClass class = 0; class < _IMAGE_CLASS_MAX; class++) { + _cleanup_free_ char *root = NULL; + + r = image_root_pick(arg_runtime_scope, class, /* runtime= */ false, &root); + if (r < 0) + return log_error_errno(r, "Failed to pick image root: %m"); + if (strextendf_with_separator(&j, ", ", "%s (downloads to %s/)", image_class_to_string(class), - image_root_to_string(class)) < 0) + root) < 0) return log_oom(); + } return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No image class specified, retry with --class= set to one of: %s.", j); diff --git a/src/import/pull.c b/src/import/pull.c index 9c915536d8a..a41b4b01555 100644 --- a/src/import/pull.c +++ b/src/import/pull.c @@ -28,7 +28,7 @@ #include "verbs.h" #include "web-util.h" -static const char *arg_image_root = NULL; +static char *arg_image_root = NULL; static ImportVerify arg_verify = IMPORT_VERIFY_SIGNATURE; static ImportFlags arg_import_flags = IMPORT_PULL_SETTINGS | IMPORT_PULL_ROOTHASH | IMPORT_PULL_ROOTHASH_SIGNATURE | IMPORT_PULL_VERITY | IMPORT_BTRFS_SUBVOL | IMPORT_BTRFS_QUOTA | IMPORT_CONVERT_QCOW2 | IMPORT_SYNC; static uint64_t arg_offset = UINT64_MAX, arg_size_max = UINT64_MAX; @@ -37,6 +37,7 @@ static ImageClass arg_class = IMAGE_MACHINE; static RuntimeScope arg_runtime_scope = _RUNTIME_SCOPE_INVALID; STATIC_DESTRUCTOR_REGISTER(arg_checksum, freep); +STATIC_DESTRUCTOR_REGISTER(arg_image_root, freep); static int normalize_local(const char *local, const char *url, char **ret) { _cleanup_free_ char *ll = NULL; @@ -358,7 +359,10 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_IMAGE_ROOT: - arg_image_root = optarg; + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image_root); + if (r < 0) + return r; + break; case ARG_VERIFY: { @@ -541,8 +545,11 @@ static int parse_argv(int argc, char *argv[]) { if (arg_checksum && (arg_import_flags & (IMPORT_PULL_SETTINGS|IMPORT_PULL_ROOTHASH|IMPORT_PULL_ROOTHASH_SIGNATURE|IMPORT_PULL_VERITY)) != 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Literal checksum verification only supported if no associated files are downloaded."); - if (!arg_image_root) - arg_image_root = image_root_to_string(arg_class); + if (!arg_image_root) { + r = image_root_pick(arg_runtime_scope < 0 ? RUNTIME_SCOPE_SYSTEM : arg_runtime_scope, arg_class, /* runtime= */ false, &arg_image_root); + if (r < 0) + return log_error_errno(r, "Failed to pick image root: %m"); + } /* .nspawn settings files only really make sense for machine images, not for sysext/confext/portable */ if (auto_settings && arg_class != IMAGE_MACHINE) diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index 15875968b78..53938a12567 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -110,7 +110,7 @@ static const char *const image_root_table[_IMAGE_CLASS_MAX] = { [IMAGE_CONFEXT] = "/var/lib/confexts", }; -DEFINE_STRING_TABLE_LOOKUP_TO_STRING(image_root, ImageClass); +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(image_root, ImageClass); static const char *const image_root_runtime_table[_IMAGE_CLASS_MAX] = { [IMAGE_MACHINE] = "/run/machines", @@ -119,7 +119,7 @@ static const char *const image_root_runtime_table[_IMAGE_CLASS_MAX] = { [IMAGE_CONFEXT] = "/run/confexts", }; -DEFINE_STRING_TABLE_LOOKUP_TO_STRING(image_root_runtime, ImageClass); +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(image_root_runtime, ImageClass); static const char *const image_dirname_table[_IMAGE_CLASS_MAX] = { [IMAGE_MACHINE] = "machines", @@ -2022,3 +2022,45 @@ static const char* const image_type_table[_IMAGE_TYPE_MAX] = { }; DEFINE_STRING_TABLE_LOOKUP(image_type, ImageType); + +int image_root_pick( + RuntimeScope scope, + ImageClass c, + bool runtime, + char **ret) { + + int r; + + assert(scope >= 0); + assert(scope < _RUNTIME_SCOPE_MAX); + assert(c >= 0); + assert(c < _IMAGE_CLASS_MAX); + assert(ret); + + /* Picks the primary target directory for downloads, depending on invocation contexts */ + + _cleanup_free_ char *s = NULL; + switch (scope) { + + case RUNTIME_SCOPE_SYSTEM: { + s = strdup(runtime ? image_root_runtime_to_string(c) : image_root_to_string(c)); + if (!s) + return -ENOMEM; + + break; + } + + case RUNTIME_SCOPE_USER: + r = sd_path_lookup(runtime ? SD_PATH_USER_RUNTIME : SD_PATH_USER_STATE_PRIVATE, "machines", &s); + if (r < 0) + return r; + + break; + + default: + return -EOPNOTSUPP; + } + + *ret = TAKE_PTR(s); + return 0; +} diff --git a/src/shared/discover-image.h b/src/shared/discover-image.h index 7ba44bfb79d..a7370450a83 100644 --- a/src/shared/discover-image.h +++ b/src/shared/discover-image.h @@ -95,8 +95,7 @@ bool image_is_host(const struct Image *i); int image_to_json(const struct Image *i, sd_json_variant **ret); -const char* image_root_to_string(ImageClass c) _const_; -const char* image_root_runtime_to_string(ImageClass c) _const_; +int image_root_pick(RuntimeScope scope, ImageClass c, bool runtime, char **ret); extern const struct hash_ops image_hash_ops;