From: dongshengyuan <545258830@qq.com> Date: Sat, 4 Jul 2026 09:05:33 +0000 (+0800) Subject: portable: detect drop-in-only attachments X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a977bf4d06d57652f44b1e8e3d5de700bb977434;p=thirdparty%2Fsystemd.git portable: detect drop-in-only attachments Commit edea370222 (portable: remove drop-in configs even if the main unit file does not exist) taught detach to handle leftover .service.d directories after the main unit symlink was removed. portable_get_state_internal() still had the same blind spot: it only considered regular unit entries, so portablectl is-attached could report detached while a portable drop-in directory was still present. Handle those drop-in-only entries like detach does, preserve the existing unit-file based enabled checks when the main unit file is still present, and add TEST-29 coverage. Signed-off-by: dongshengyuan --- diff --git a/src/portable/portable.c b/src/portable/portable.c index f0bdb40fd22..c1abe16985d 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -2295,6 +2295,42 @@ static int test_chroot_dropin( return r; } +static int portable_attached_dirent_name( + const struct dirent *de, + char **ret_unit_name, + bool *ret_dropin) { + + _cleanup_free_ char *unit_name = NULL; + const char *dropin_suffix; + + assert(de); + assert(ret_unit_name); + + /* When a portable service is enabled with "portablectl --copy=symlink --enable --now attach", + * and is disabled with "portablectl --enable --now detach", which calls DisableUnitFilesWithFlags + * DBus method, the main unit file is removed, but its drop-ins are not. Hence, we need to list both + * main unit files and drop-in directories (without the main unit files). */ + + dropin_suffix = endswith(de->d_name, ".d"); + if (dropin_suffix) + unit_name = strndup(de->d_name, dropin_suffix - de->d_name); + else + unit_name = strdup(de->d_name); + if (!unit_name) + return -ENOMEM; + + if (!unit_name_is_valid(unit_name, UNIT_NAME_ANY)) + return 0; + + if (dropin_suffix ? !IN_SET(de->d_type, DT_LNK, DT_DIR) : !IN_SET(de->d_type, DT_LNK, DT_REG)) + return 0; + + *ret_unit_name = TAKE_PTR(unit_name); + if (ret_dropin) + *ret_dropin = dropin_suffix != NULL; + return 1; +} + int portable_detach( RuntimeScope scope, sd_bus *bus, @@ -2331,31 +2367,17 @@ int portable_detach( FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to enumerate '%s' directory: %m", where)) { _cleanup_free_ char *marker = NULL, *unit_name = NULL; - const char *dot; - - /* When a portable service is enabled with "portablectl --copy=symlink --enable --now attach", - * and is disabled with "portablectl --enable --now detach", which calls DisableUnitFilesWithFlags - * DBus method, the main unit file is removed, but its drop-ins are not. Hence, here we need - * to list both main unit files and drop-in directories (without the main unit files). */ - dot = endswith(de->d_name, ".d"); - if (dot) - unit_name = strndup(de->d_name, dot - de->d_name); - else - unit_name = strdup(de->d_name); - if (!unit_name) - return -ENOMEM; - - if (!unit_name_is_valid(unit_name, UNIT_NAME_ANY)) + r = portable_attached_dirent_name(de, &unit_name, /* ret_dropin= */ NULL); + if (r < 0) + return r; + if (r == 0) continue; /* Filter out duplicates */ if (set_contains(unit_files, unit_name)) continue; - if (dot ? !IN_SET(de->d_type, DT_LNK, DT_DIR) : !IN_SET(de->d_type, DT_LNK, DT_REG)) - continue; - r = test_chroot_dropin(d, where, unit_name, name_or_path, extension_image_paths, &marker); if (r < 0) return r; @@ -2521,39 +2543,54 @@ static int portable_get_state_internal( } FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to enumerate '%s' directory: %m", where)) { - UnitFileState state; + _cleanup_free_ char *unit_name = NULL; + bool dropin; - if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY)) + r = portable_attached_dirent_name(de, &unit_name, &dropin); + if (r < 0) + return r; + if (r == 0) continue; /* Filter out duplicates */ - if (set_contains(unit_files, de->d_name)) + if (set_contains(unit_files, unit_name)) continue; - if (!IN_SET(de->d_type, DT_LNK, DT_REG)) - continue; + if (dropin) { + /* If the main unit file still exists, let the regular entry handle it so that + * enabled/running state is determined from the unit file as before. */ + r = RET_NERRNO(faccessat(dirfd(d), unit_name, F_OK, AT_SYMLINK_NOFOLLOW)); + if (r >= 0) + continue; + if (r != -ENOENT) + return log_debug_errno(r, "Failed to check if '%s/%s' exists: %m", where, unit_name); + } - r = test_chroot_dropin(d, where, de->d_name, name_or_path, extension_image_paths, NULL); + r = test_chroot_dropin(d, where, unit_name, name_or_path, extension_image_paths, NULL); if (r < 0) return r; if (r == 0) continue; - r = unit_file_lookup_state(scope, &paths, de->d_name, &state); - if (r < 0) - return log_debug_errno(r, "Failed to determine unit file state of '%s': %m", de->d_name); - if (!IN_SET(state, UNIT_FILE_STATIC, UNIT_FILE_DISABLED, UNIT_FILE_LINKED, UNIT_FILE_LINKED_RUNTIME)) - found_enabled = true; + if (!dropin) { + UnitFileState state; - r = unit_file_is_active(bus, de->d_name, error); + r = unit_file_lookup_state(scope, &paths, unit_name, &state); + if (r < 0) + return log_debug_errno(r, "Failed to determine unit file state of '%s': %m", unit_name); + if (!IN_SET(state, UNIT_FILE_STATIC, UNIT_FILE_DISABLED, UNIT_FILE_LINKED, UNIT_FILE_LINKED_RUNTIME)) + found_enabled = true; + } + + r = unit_file_is_active(bus, unit_name, error); if (r < 0) return r; if (r > 0) found_running = true; - r = set_put_strdup(&unit_files, de->d_name); + r = set_ensure_consume(&unit_files, &string_hash_ops_free, TAKE_PTR(unit_name)); if (r < 0) - return log_debug_errno(r, "Failed to add unit name '%s' to set: %m", de->d_name); + return log_oom_debug(); } *ret = found_running ? (!set_isempty(unit_files) && (flags & PORTABLE_RUNTIME) ? PORTABLE_RUNNING_RUNTIME : PORTABLE_RUNNING) : diff --git a/test/units/TEST-29-PORTABLE.directory.sh b/test/units/TEST-29-PORTABLE.directory.sh index 8f35c9ee083..42f280b78ad 100755 --- a/test/units/TEST-29-PORTABLE.directory.sh +++ b/test/units/TEST-29-PORTABLE.directory.sh @@ -144,9 +144,17 @@ portablectl detach --now --runtime --extension /tmp/app0 /tmp/rootdir app0 # Provides coverage for https://github.com/systemd/systemd/issues/23481 portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/rootdir minimal-app0 portablectl detach --now --runtime --enable /tmp/rootdir minimal-app0 -# attach and detach again to check if all drop-in configs are removed even if the main unit files are removed +# Attach and detach again to check if drop-in-only leftovers are still recognized and removed. portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/rootdir minimal-app0 +rm /run/systemd/system.attached/minimal-app0*.service +status="$(portablectl is-attached --runtime /tmp/rootdir)" +[[ "${status}" == "running-runtime" ]] +systemctl stop minimal-app0.service minimal-app0-foo.service +status="$(portablectl is-attached --runtime /tmp/rootdir)" +[[ "${status}" == "attached-runtime" ]] portablectl detach --now --runtime --enable /tmp/rootdir minimal-app0 +[[ ! -d /run/systemd/system.attached/minimal-app0.service.d ]] +[[ ! -d /run/systemd/system.attached/minimal-app0-foo.service.d ]] # The wrong file should be ignored, given the right one has the xattr set trap 'rm -rf /var/cache/wrongext' EXIT