From 11eebc2357d09786709247937c4b5ed5534ce7f1 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 14 Nov 2025 16:44:49 +0800 Subject: [PATCH 1/2] nspawn: Fix broken host links for container journals Commit 88252ca changed nspawn to always run from a temporary mount directory (e.g., /tmp/nspawn-root-XXXXXX). This was a good simplification for mount logic, but it unintentionally broke the --link-journal feature. The setup_journal() helper was subsequently passed this ephemeral path instead of the persistent machine path (from --directory= or --image=). This caused the host to create broken symlinks pointing to a temporary directory that would soon be gone. Fix this by storing the original path and plumbing it through to setup_journal(). All other mount-related logic in outer_child() continues to use the temporary `directory` variable. Fixes: #39472 --- src/nspawn/nspawn.c | 103 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 84 insertions(+), 19 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 1252a73187..c3c9d75a50 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2551,12 +2551,14 @@ static int setup_hostname(void) { return 0; } -static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range) { +static int setup_journal(const char *root_dir, const char *persistent_path, uid_t chown_uid, uid_t chown_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; @@ -2566,6 +2568,17 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range 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"); @@ -2579,7 +2592,7 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range } FOREACH_STRING(dirname, "/var", "/var/log", "/var/log/journal") { - r = userns_mkdir(directory, dirname, 0755, 0, 0); + r = userns_mkdir(root_dir, dirname, 0755, 0, 0); if (r < 0) { bool ignore = r == -EROFS && try; log_full_errno(ignore ? LOG_DEBUG : LOG_ERR, r, @@ -2592,10 +2605,14 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range if (!p) return log_oom(); - _cleanup_free_ char *q = path_join(directory, p); + _cleanup_free_ char *q = path_join(persistent_path, 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; @@ -2604,12 +2621,14 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range "%s: already a mount point, refusing to use for journal", p); } - if (path_is_mount_point(q) > 0) { + if (path_is_mount_point(container_path) > 0) { if (try) return 0; - return log_error_errno(SYNTHETIC_ERRNO(EEXIST), - "%s: already a mount point, refusing to use for journal", q); + return log_error_errno( + SYNTHETIC_ERRNO(EEXIST), + "%s: already a mount point, refusing to use for journal", + container_path); } r = readlink_and_make_absolute(p, &d); @@ -2617,9 +2636,9 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range if (IN_SET(arg_link_journal, LINK_GUEST, LINK_AUTO) && path_equal(d, q)) { - r = userns_mkdir(directory, p, 0755, 0, 0); + r = userns_mkdir(root_dir, p, 0755, 0, 0); if (r < 0) - log_warning_errno(r, "Failed to create directory %s: %m", q); + log_warning_errno(r, "Failed to create directory %s: %m", container_path); return 0; } @@ -2649,9 +2668,9 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range return log_error_errno(errno, "Failed to symlink %s to %s: %m", q, p); } - r = userns_mkdir(directory, p, 0755, 0, 0); + r = userns_mkdir(root_dir, p, 0755, 0, 0); if (r < 0) - log_warning_errno(r, "Failed to create directory %s: %m", q); + log_warning_errno(r, "Failed to create directory %s: %m", container_path); return 0; } @@ -2671,25 +2690,25 @@ static int setup_journal(const char *directory, uid_t uid_shift, uid_t uid_range } else if (access(p, F_OK) < 0) return 0; - if (dir_is_empty(q, /* ignore_hidden_or_backup= */ false) == 0) - log_warning("%s is not empty, proceeding anyway.", q); + if (dir_is_empty(container_path, /* ignore_hidden_or_backup= */ false) == 0) + log_warning("%s is not empty, proceeding anyway.", container_path); - r = userns_mkdir(directory, p, 0755, 0, 0); + r = userns_mkdir(root_dir, p, 0755, 0, 0); if (r < 0) - return log_error_errno(r, "Failed to create %s: %m", q); + return log_error_errno(r, "Failed to create %s: %m", container_path); return mount_custom( - directory, + root_dir, &(CustomMount) { .type = CUSTOM_MOUNT_BIND, - .options = (char*) (uid_is_valid(uid_shift) ? "rootidmap" : NULL), + .options = (char*) (uid_is_valid(chown_uid) ? "rootidmap" : NULL), .source = p, .destination = p, .destination_uid = UID_INVALID, }, /* n = */ 1, - uid_shift, - uid_range, + chown_uid, + chown_range, arg_selinux_apifs_context, MOUNT_NON_ROOT_ONLY); } @@ -3861,6 +3880,7 @@ 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, @@ -4267,7 +4287,7 @@ static int outer_child( if (r < 0) return r; - r = setup_journal(directory, chown_uid, chown_range); + r = setup_journal(directory, persistent_path, chown_uid, chown_range); if (r < 0) return r; @@ -5065,6 +5085,7 @@ 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, @@ -5215,6 +5236,7 @@ static int run_container( r = outer_child(&barrier, directory, + persistent_path, mount_fd, dissected_image, fd_outer_socket_pair[1], @@ -5927,6 +5949,7 @@ 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(); @@ -6388,6 +6411,47 @@ 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) { @@ -6434,6 +6498,7 @@ static int run(int argc, char *argv[]) { for (;;) { r = run_container( rootdir, + persistent_path, mount_fd, dissected_image, userns_fd, From 887f54adceeef49562bc3710530ffaa04606d0d8 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 14 Nov 2025 16:46:08 +0800 Subject: [PATCH 2/2] nspawn: Add integration test for --link-journal --- test/units/TEST-13-NSPAWN.nspawn.sh | 126 ++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/test/units/TEST-13-NSPAWN.nspawn.sh b/test/units/TEST-13-NSPAWN.nspawn.sh index ae5dfdce38..266ac91bba 100755 --- a/test/units/TEST-13-NSPAWN.nspawn.sh +++ b/test/units/TEST-13-NSPAWN.nspawn.sh @@ -1533,4 +1533,130 @@ 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