TEST-15: add test for transient units with drop-ins
We want to test four things:
- that the transient units are successfully started when drop-ins exist
- that the transient setings override the defaults
- the drop-ins override the transient settings (the same as for a normal unit)
- that things are the same before and after a reload
To make things more fun, we start and stop units in two different ways: via
systemctl and via a direct busctl invocation. This gives us a bit more coverage
of different code paths.
TEST-15: also test hierarchical drop-ins for slices
Slices are worth testing too, because they don't need a fragment path so they
behave slightly differently than service units. I'm making this a separate
patch from the actual tests that I wanted to add later because it's complex
enough on its own.
TEST-15: allow helper functions to accept other unit types
clear_services() is renamed to clear_units() and now takes a full
unit name including the suffix as an argument.
_clear_service() is renamed to clear_unit() and changed likewise.
create_service() didn't have the same underscore prefix, and I don't think
it's useful or needed for a local function, so it is removed.
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 ;)
Yu Watanabe [Thu, 13 Oct 2022 08:34:08 +0000 (17:34 +0900)]
test: drop unused modules
Suggested by CodeQL#167 (https://github.com/systemd/systemd/security/code-scanning/167)
and CodeQL#168 (https://github.com/systemd/systemd/security/code-scanning/168).
analyze: extend the dump command to accept patterns
The new function DumpPatterns() can be used to limit (drastically) the size of
the data returned by PID1. Hence the optimization of serializing data into a
file descriptor should be less relevant than having the possibility to limit
the data when communicating with the service manager remotely.
NB: when passing patterns, the dump command omits the version of the manager as
well as the features and the timestamps.
dissect-image: open dissected or decrypted partitions and mount through the file descriptor
If multiple services with the same encrypted image are simultaneously
starting, one may deactivate the dm device while others using it.
Or, similary, after (regular) partitions are dissected, another process
may try to remove them before we mount them.
To prevent such situations, let's keep the dissected and decrypted
partitions opened. Then, use the file descriptors when we mount the
partitions.
Frantisek Sumsal [Wed, 12 Oct 2022 15:28:47 +0000 (17:28 +0200)]
test: disable LSan in the ASan env wrapper
This wrapper is used in situations where we don't care about *San reports,
we just want to make things work. However, with enabled LSan we might
trigger some bogus reports we're definitely not interested in, causing
unexpected test fails.
Michael Biebl [Wed, 12 Oct 2022 09:07:57 +0000 (11:07 +0200)]
logind: fix getting property OnExternalPower via D-Bus
The BUS_DEFINE_PROPERTY_GET_GLOBAL macro requires a value as third
argument, so we need to call manager_is_on_external_power(). Otherwise
the function pointer is interpreted as a boolean and always returns
true:
```
$ busctl get-property org.freedesktop.login1 /org/freedesktop/login1 org.freedesktop.login1.Manager OnExternalPower
b true
$ /lib/systemd/systemd-ac-power --verbose
no
```
Thanks: Helmut Grohne <helmut@subdivi.de>
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1021644
PID 1 is not allowed to do nss lookups because this may take a long time or
even deadlock.
While at it, the comparisons are reordered to do the "easy" comparisons which
only require a string comparison first. Delay parsing of the UID until it is
really necessary. The result is the same, because we know that "root" and
"nobody" parse as valid.
basic/chase-symlinks: add note that CHASE_WARN is not for PID 1
This should be enough to make us remember that constraint. A more heavy-weight
approach like renaming the option to CHASE_WARN_NSS_LOOKUP would be possible,
but it'd make the callers quite verbose and doesn't seem worth it.
Those may go via the PC speaker, which is annoying and unexpected.
Most people have it off, so this doesn't work reliably anyway, so we can
disable it without much loss.
Let's nudge people towards the use of an anonymous buffer like we
do internally.
"errno" → "errnum", to match the man page for strerror, and also to avoid
confusion with the global variable. In general, I think that errno is a
terrible interface and we shouldn't encourage people to use it. Those functions
use errno-style error numbers, which are a different thing.
The need to set errno is very very ugly, but at least it is thread-safe and
works correctly. Using strerror() is likely to be wrong, so let's not recommend
that. People who do a lot of logging would provide use some wrapper that sets
errno like we do, so nudge people towards %m.
I tested that all the separate .c files compile cleanly.
sd-bus: make bus_error_message() a thread-safe macro
strerror_r() is used instead of strerror(). The usual trick is employed: we
allocate a buffer that lives until the end of the surrounding block to provide
the scratch space. This change is particularly important forn sd-bus and the
pam modules, which may be called from threaded code.
I checked the codebase, and we only use bus_error_message() in log statements,
so the returned pointer is not used beyond its valid lifetime.
shared/journal-importer: use %m instead of strerror()
Here SYNTHETIC_ERRNO() was used based on the general rule that logging
functions should do that when the error value is generated at the call
site. But here we're really propagating a memory allocation error, which
wasn't reported using errno, but the meaning is the same. And it's better
to bend the rule a bit like this than to use strerror().
core,logind,systemctl,journald: replace calls to strerror() with setting errno + %m
strerror() is not thread safe and calling it just isn't worth the effort
required to justify why it would be safe in those cases. It's easier to just
use %m which is thread-safe out of the box. I don't think that any of the
changes in the patch cause any functional difference. This is just about
getting rid of calls to strerror() in general.
When we print an error message and fail to format the string, using something
like "(null)" is good enough. This is very very unlikely to happen anyway.
Error handling in acquire_user_record() was checking the wrong
condition (PAM errors are always >= 0, so r < 0 cannot match).
Apart from the fix for error handling, no change in behaviour is intended.
I did some minor adjustements to formatting and added _cleanup_ in one more
place.
This is a primitive helper that wraps calls to pam_syslog() replacing
@PAMERR@ with pam_strerror() output in the format string. This allows for
a bunch of boilerplate to be removed.
@PAMERR@ is only supported at the end of the string. Similarly to %m,
realistically that's the only place where it is useful.
Note that unlike in logging functions in log.[ch], here the error value is
only used for the message and is not saved anywhere, so we don't need to
care about SYNTHETIC_ERRNO.
Daan De Meyer [Tue, 11 Oct 2022 11:26:41 +0000 (13:26 +0200)]
logs-show: Always retrieve the boot ID from the entry
If _SOURCE_MONOTONIC_TIMESTAMP was set in the entry, we wouldn't
query the boot ID, leading to every kernel entry in the export mode
to have BOOT_ID=000000000000000. Let's fix this by always querying
the boot ID.
Yu Watanabe [Tue, 11 Oct 2022 07:19:12 +0000 (16:19 +0900)]
sd-ndisc: ignore failure in sending solicitation
Even if a bonding master interface has carrier, the underlying slave
interfaces may not. In such a case, sending solicitation fails with
-ENOBUS. Here, let's unconditionally ignore errors, as anyway we will
send a solicitation later.
NEWS: make clear we talk about *system* credentials here
The new conditoins are placed inside of services, but they cannot be
used to test service creds, but only system creds. This deserves
explicit mention, since it might be confusing otherwise.
projects / thirdparty / systemd.git / log
