89 lines
4.4 KiB
Diff
89 lines
4.4 KiB
Diff
From fa8ed66b5ff48e0d8e02cfae28e90e65c70e52e3 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
|
Date: Fri, 14 Oct 2022 15:02:20 +0200
|
|
Subject: [PATCH] manager: allow transient units to have drop-ins
|
|
|
|
In https://github.com/containers/podman/issues/16107, starting of a transient
|
|
slice unit fails because there's a "global" drop-in
|
|
/usr/lib/systemd/user/slice.d/10-oomd-per-slice-defaults.conf (provided by
|
|
systemd-oomd-defaults package to install some default oomd policy). This means
|
|
that the unit_is_pristine() check fails and starting of the unit is forbidden.
|
|
|
|
It seems pretty clear to me that dropins at any other level then the unit
|
|
should be ignored in this check: we now have multiple layers of drop-ins
|
|
(for each level of the cgroup path, and also "global" ones for a specific
|
|
unit type). If we install a "global" drop-in, we wouldn't be able to start
|
|
any transient units of that type, which seems undesired.
|
|
|
|
In principle we could reject dropins at the unit level, but I don't think that
|
|
is useful. The whole reason for drop-ins is that they are "add ons", and there
|
|
isn't any particular reason to disallow them for transient units. It would also
|
|
make things harder to implement and describe: one place for drop-ins is good,
|
|
but another is bad. (And as a corner case: for instanciated units, a drop-in
|
|
in the template would be acceptable, but a instance-specific drop-in bad?)
|
|
|
|
Thus, $subject.
|
|
|
|
While at it, adjust the message. All the conditions in unit_is_pristine()
|
|
essentially mean that it wasn't loaded (e.g. it might be in an error state),
|
|
and that it doesn't have a fragment path (now that drop-ins are acceptable).
|
|
If there's a job for it, it necessarilly must have been loaded. If it is
|
|
merged into another unit, it also was loaded and found to be an alias.
|
|
Based on the discussion in the bugs, it seems that the current message
|
|
is far from obvious ;)
|
|
|
|
Fixes https://github.com/containers/podman/issues/16107,
|
|
https://bugzilla.redhat.com/show_bug.cgi?id=2133792.
|
|
|
|
(cherry picked from commit 1f83244641f13a9cb28fdac7e3c17c5446242dfb)
|
|
|
|
Resolves: #2156620
|
|
---
|
|
src/core/dbus-manager.c | 3 ++-
|
|
src/core/unit.c | 14 ++++++++------
|
|
2 files changed, 10 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
|
|
index 8a41eda4a6..ea2f3e7f59 100644
|
|
--- a/src/core/dbus-manager.c
|
|
+++ b/src/core/dbus-manager.c
|
|
@@ -892,7 +892,8 @@ static int transient_unit_from_message(
|
|
return r;
|
|
|
|
if (!unit_is_pristine(u))
|
|
- return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit %s already exists.", name);
|
|
+ return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS,
|
|
+ "Unit %s was already loaded or has a fragment file.", name);
|
|
|
|
/* OK, the unit failed to load and is unreferenced, now let's
|
|
* fill in the transient data instead */
|
|
diff --git a/src/core/unit.c b/src/core/unit.c
|
|
index 78666e73bf..76fb9f8075 100644
|
|
--- a/src/core/unit.c
|
|
+++ b/src/core/unit.c
|
|
@@ -4842,16 +4842,18 @@ int unit_fail_if_noncanonical(Unit *u, const char* where) {
|
|
bool unit_is_pristine(Unit *u) {
|
|
assert(u);
|
|
|
|
- /* Check if the unit already exists or is already around,
|
|
- * in a number of different ways. Note that to cater for unit
|
|
- * types such as slice, we are generally fine with units that
|
|
- * are marked UNIT_LOADED even though nothing was actually
|
|
- * loaded, as those unit types don't require a file on disk. */
|
|
+ /* Check if the unit already exists or is already around, in a number of different ways. Note that to
|
|
+ * cater for unit types such as slice, we are generally fine with units that are marked UNIT_LOADED
|
|
+ * even though nothing was actually loaded, as those unit types don't require a file on disk.
|
|
+ *
|
|
+ * Note that we don't check for drop-ins here, because we allow drop-ins for transient units
|
|
+ * identically to non-transient units, both unit-specific and hierarchical. E.g. for a-b-c.service:
|
|
+ * service.d/….conf, a-.service.d/….conf, a-b-.service.d/….conf, a-b-c.service.d/….conf.
|
|
+ */
|
|
|
|
return IN_SET(u->load_state, UNIT_NOT_FOUND, UNIT_LOADED) &&
|
|
!u->fragment_path &&
|
|
!u->source_path &&
|
|
- strv_isempty(u->dropin_paths) &&
|
|
!u->job &&
|
|
!u->merged_into;
|
|
}
|