From: Lennart Poettering Date: Mon, 24 Nov 2025 15:34:00 +0000 (+0100) Subject: Revert "nspawn: Fix broken host links for container journals" X-Git-Tag: v259-rc2~28^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F39879%2Fhead;p=thirdparty%2Fsystemd.git Revert "nspawn: Fix broken host links for container journals" --- diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index caef138f1ad..ae081f88f25 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2553,14 +2553,12 @@ static int setup_hostname(void) { return 0; } -static int setup_journal(const char *root_dir, const char *persistent_path, uid_t chown_uid, uid_t chown_range) { +static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range) { _cleanup_free_ char *d = NULL; sd_id128_t this_id; bool try; int r; - assert(root_dir); - /* Don't link journals in ephemeral mode */ if (arg_ephemeral) return 0; @@ -2570,17 +2568,6 @@ static int setup_journal(const char *root_dir, const char *persistent_path, uid_ try = arg_link_journal_try || arg_link_journal == LINK_AUTO; - if (!persistent_path) { - if (try) { - log_debug("No persistent path available, skipping journal linking."); - return 0; - } else - return log_error_errno( - SYNTHETIC_ERRNO(ENOENT), - "Journal linking requested but no persistent directory available. " - "Use --directory, or --link-journal=auto/try-guest/try-host for optional linking."); - } - r = sd_id128_get_machine(&this_id); if (r < 0) return log_error_errno(r, "Failed to retrieve machine ID: %m"); @@ -2594,7 +2581,7 @@ static int setup_journal(const char *root_dir, const char *persistent_path, uid_ } FOREACH_STRING(dirname, "/var", "/var/log", "/var/log/journal") { - r = userns_mkdir(root_dir, dirname, 0755, 0, 0); + r = userns_mkdir(directory, dirname, 0755, 0, 0); if (r < 0) { bool ignore = r == -EROFS && try; log_full_errno(ignore ? LOG_DEBUG : LOG_ERR, r, @@ -2607,14 +2594,10 @@ static int setup_journal(const char *root_dir, const char *persistent_path, uid_ if (!p) return log_oom(); - _cleanup_free_ char *q = path_join(persistent_path, p); + _cleanup_free_ char *q = path_join(directory, p); if (!q) return log_oom(); - _cleanup_free_ char *container_path = path_join(root_dir, p); - if (!container_path) - return log_oom(); - if (path_is_mount_point(p) > 0) { if (try) return 0; @@ -2623,14 +2606,12 @@ static int setup_journal(const char *root_dir, const char *persistent_path, uid_ "%s: already a mount point, refusing to use for journal", p); } - if (path_is_mount_point(container_path) > 0) { + if (path_is_mount_point(q) > 0) { if (try) return 0; - return log_error_errno( - SYNTHETIC_ERRNO(EEXIST), - "%s: already a mount point, refusing to use for journal", - container_path); + return log_error_errno(SYNTHETIC_ERRNO(EEXIST), + "%s: already a mount point, refusing to use for journal", q); } r = readlink_and_make_absolute(p, &d); @@ -2638,9 +2619,9 @@ static int setup_journal(const char *root_dir, const char *persistent_path, uid_ if (IN_SET(arg_link_journal, LINK_GUEST, LINK_AUTO) && path_equal(d, q)) { - r = userns_mkdir(root_dir, p, 0755, 0, 0); + r = userns_mkdir(directory, p, 0755, 0, 0); if (r < 0) - log_warning_errno(r, "Failed to create directory %s: %m", container_path); + log_warning_errno(r, "Failed to create directory %s: %m", q); return 0; } @@ -2670,9 +2651,9 @@ static int setup_journal(const char *root_dir, const char *persistent_path, uid_ return log_error_errno(errno, "Failed to symlink %s to %s: %m", q, p); } - r = userns_mkdir(root_dir, p, 0755, 0, 0); + r = userns_mkdir(directory, p, 0755, 0, 0); if (r < 0) - log_warning_errno(r, "Failed to create directory %s: %m", container_path); + log_warning_errno(r, "Failed to create directory %s: %m", q); return 0; } @@ -2692,25 +2673,25 @@ static int setup_journal(const char *root_dir, const char *persistent_path, uid_ } else if (access(p, F_OK) < 0) return 0; - if (dir_is_empty(container_path, /* ignore_hidden_or_backup= */ false) == 0) - log_warning("%s is not empty, proceeding anyway.", container_path); + if (dir_is_empty(q, /* ignore_hidden_or_backup= */ false) == 0) + log_warning("%s is not empty, proceeding anyway.", q); - r = userns_mkdir(root_dir, p, 0755, 0, 0); + r = userns_mkdir(directory, p, 0755, 0, 0); if (r < 0) - return log_error_errno(r, "Failed to create %s: %m", container_path); + return log_error_errno(r, "Failed to create %s: %m", q); return mount_custom( - root_dir, + directory, &(CustomMount) { .type = CUSTOM_MOUNT_BIND, - .options = (char*) (uid_is_valid(chown_uid) ? "rootidmap" : NULL), + .options = (char*) (uid_is_valid(uid_shift) ? "rootidmap" : NULL), .source = p, .destination = p, .destination_uid = UID_INVALID, }, /* n = */ 1, - chown_uid, - chown_range, + uid_shift, + uid_range, arg_selinux_apifs_context, MOUNT_NON_ROOT_ONLY); } @@ -3882,7 +3863,6 @@ static DissectImageFlags determine_dissect_image_flags(void) { static int outer_child( Barrier *barrier, const char *directory, - const char *persistent_path, int mount_fd, DissectedImage *dissected_image, int fd_outer_socket, @@ -4289,7 +4269,7 @@ static int outer_child( if (r < 0) return r; - r = setup_journal(directory, persistent_path, chown_uid, chown_range); + r = setup_journal(directory, chown_uid, chown_range); if (r < 0) return r; @@ -5090,7 +5070,6 @@ static int load_oci_bundle(void) { static int run_container( const char *directory, - const char *persistent_path, int mount_fd, DissectedImage *dissected_image, int userns_fd, @@ -5241,7 +5220,6 @@ static int run_container( r = outer_child(&barrier, directory, - persistent_path, mount_fd, dissected_image, fd_outer_socket_pair[1], @@ -5954,7 +5932,6 @@ static int run(int argc, char *argv[]) { _cleanup_(dissected_image_unrefp) DissectedImage *dissected_image = NULL; _cleanup_(sd_netlink_unrefp) sd_netlink *nfnl = NULL; _cleanup_(pidref_done) PidRef pid = PIDREF_NULL; - _cleanup_free_ char *persistent_path_storage = NULL; log_setup(); @@ -6420,47 +6397,6 @@ static int run(int argc, char *argv[]) { arg_architecture = dissected_image_architecture(dissected_image); } - /* Journal linking creates symlinks from the host's /var/log/journal/ to a persistent directory - * that survives container restarts, allowing journalctl -M to read container logs. But where that - * persistent directory is can be quite different depending on the mode: - * - * For --directory: The directory itself is the persistent location. - * For --image: Use /var/lib/machines/ if it exists as a directory. - * - * Otherwise, we skip journal linking. - */ - const char *persistent_path; - if (arg_directory) - persistent_path = arg_directory; - else if (arg_image) { - persistent_path_storage = path_join("/var/lib/machines", arg_machine); - if (!persistent_path_storage) { - r = log_oom(); - goto finish; - } - - /* Validate that the persistent path exists and is actually a directory. We cannot create it - * ourselves as that would conflict with image management. */ - struct stat st; - if (stat(persistent_path_storage, &st) < 0) { - if (errno == ENOENT) - log_debug("Persistent directory %s does not exist, skipping journal linking.", - persistent_path_storage); - else - log_warning_errno( - errno, - "Failed to access persistent directory %s, skipping journal linking: %m", - persistent_path_storage); - persistent_path = NULL; - } else if (!S_ISDIR(st.st_mode)) { - log_warning("%s exists but is not a directory, skipping journal linking.", - persistent_path_storage); - persistent_path = NULL; - } else - persistent_path = persistent_path_storage; - } else - assert_not_reached(); - /* Create a temporary place to mount stuff. */ r = mkdtemp_malloc("/tmp/nspawn-root-XXXXXX", &rootdir); if (r < 0) { @@ -6507,7 +6443,6 @@ static int run(int argc, char *argv[]) { for (;;) { r = run_container( rootdir, - persistent_path, mount_fd, dissected_image, userns_fd, diff --git a/test/units/TEST-13-NSPAWN.nspawn.sh b/test/units/TEST-13-NSPAWN.nspawn.sh index 266ac91bbab..ae5dfdce383 100755 --- a/test/units/TEST-13-NSPAWN.nspawn.sh +++ b/test/units/TEST-13-NSPAWN.nspawn.sh @@ -1533,130 +1533,4 @@ testcase_cap_net_bind_service() { rm -fr "$root" } -testcase_link_journal_directory() { - # Test that journal symlinks point to the persistent path - # for --directory. - local root machine_id journal_path symlink_target - - root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-persist.XXX)" - - create_dummy_container "$root" - machine_id="$(systemd-id128 new)" - echo "$machine_id" >"$root/etc/machine-id" - journal_path="/var/log/journal/$machine_id" - - mkdir -p "$journal_path" - - # Run container with journal linking - systemd-nspawn --register=no \ - --directory="$root" \ - --link-journal=try-guest \ - bash -c "exit 0" - - # Verify the symlink was created and is not broken - test -L "$journal_path" - test -e "$journal_path" - - # Verify the symlink points to the persistent path (e.g., /var/lib/machines/...) - # NOT to a temporary /tmp/nspawn-root-* path - symlink_target="$(readlink -f "$journal_path")" - [[ "$symlink_target" == "$root"* ]] - [[ "$symlink_target" != /tmp/nspawn-root-* ]] - - rm -f "$journal_path" - rm -fr "$root" -} - -testcase_link_journal_image() { - # Test that journal symlinks point to the persistent path - # for --image. - local root machine_id journal_path symlink_target image mnt_dir - local machine_name="test-journal-image" - local persistent_dir="/var/lib/machines/$machine_name" - - root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-image-root.XXX)" - image="$(mktemp /var/lib/machines/TEST-13-NSPAWN.journal-image.XXX.img)" - mnt_dir="$(mktemp -d)" - - mkdir -p "$persistent_dir" - - create_dummy_container "$root" - dd if=/dev/zero of="$image" bs=1M count=256 status=none - mkfs.ext4 -q "$image" - mount -o loop "$image" "$mnt_dir" - cp -r "$root"/* "$mnt_dir/" - machine_id="$(systemd-id128 new)" - echo "$machine_id" >"$mnt_dir/etc/machine-id" - umount "$mnt_dir" - journal_path="/var/log/journal/$machine_id" - - mkdir -p "$journal_path" - - # Run container with journal linking - systemd-nspawn --register=no \ - --image="$image" \ - --machine="$machine_name" \ - --link-journal=try-guest \ - bash -c "exit 0" - - # Verify the symlink was created and points to the persistent directory. - # Note: We don't check that the target exists, as journald hasn't run in this test - # to create the directory structure - that's journald's responsibility, not nspawn's. - test -L "$journal_path" - symlink_target="$(readlink "$journal_path")" - [[ "$symlink_target" == "$persistent_dir"* ]] - - rm -f "$journal_path" - rm -f "$image" - rm -fr "$root" - rm -fr "$mnt_dir" - rm -fr "$persistent_dir" -} - -testcase_link_journal_ephemeral() { - # Test that journal symlinks are NOT created for --ephemeral. - local root machine_id journal_path - - root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-ephemeral.XXX)" - - create_dummy_container "$root" - machine_id="$(systemd-id128 new)" - echo "$machine_id" >"$root/etc/machine-id" - journal_path="/var/log/journal/$machine_id" - - # Run ephemeral container with journal linking - systemd-nspawn --register=no \ - --directory="$root" \ - --ephemeral \ - --link-journal=try-guest \ - bash -c "exit 0" - - # Verify the symlink was NOT created, as per existing logic - test ! -e "$journal_path" - - rm -fr "$root" -} - -testcase_link_journal_negative() { - # Test that no symlink is created when --link-journal is not used. - local root machine_id journal_path - - root="$(mktemp -d /var/lib/machines/TEST-13-NSPAWN.journal-no-link.XXX)" - - create_dummy_container "$root" - machine_id="$(systemd-id128 new)" - echo "$machine_id" >"$root/etc/machine-id" - journal_path="/var/log/journal/$machine_id" - - # Run container WITHOUT journal linking - systemd-nspawn --register=no \ - --directory="$root" \ - bash -c "exit 0" - - # Verify the symlink was NOT created - test ! -e "$journal_path" - - rm -fr "$root" -} - run_testcases