## Summary
`updatectl features` failed because it expected the old feature JSON
fields,
while `systemd-sysupdate` now returns `id`, `isEnabled`, and
`documentation`
as an array.
This change:
- Aligns `updatectl` with the current feature JSON schema.
- Unifies JSON output between `systemd-sysupdate`, D-Bus, and Varlink.
- Adds support for multiple `Documentation=` entries for Features and
Components.
- Exposes and parses `appstream` and `suggested` metadata.
- Propagates condition-evaluation errors through `condition_test_list()`
and
`feature_is_suggested()`.
- Reports suggestion-evaluation failures instead of silently treating
them as
`false`.
- Improves JSON dump error handling.
- Expands `TEST-72-SYSUPDATE` coverage and fixes its unknown-field
assertion.
- Documents the `DescribeFeature()` JSON field transition in `NEWS`.
- Keeps the existing Component name list for `sysupdated` compatibility.
The `name`/`enabled` to `id`/`isEnabled` transition is intentional and
documented as a v262 payload change.
Fixes#43603
I thought that this would require some bigger changes, but it turns out
that the existing functionality is good enough with some minor
adjustments if used appropriately.
The behaviour of help_section() is changed to simplify all callers.
If a unit has FileDescriptorStorePreserve=yes we'll keep its FDs around
in case it starts again. But if there are no reverse dependencies
referencing it, we'll also GC it and lose all the FDs, which defeats the
point of the setting (which is opt-in).
Do not GC units that have FDs stored to avoid this.
Follow-up for b9c1883a9c
> read_dnsmasq_log_file() will raise FileNotFoundError if dnsmasq hasn’t created the
> log file yet (or if the file was just removed by stop_dnsmasq() before the restart).
> This would error the test instead of retrying.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
As in the previous commit, checking for both requirements and ordering seems
unnecessary. In practical cases, the mount will be pulled in by the rest
of the transaction, so ordering is the part that matters. (The setup is
racy without the ordering.) If we drop the second check, the admin can
just use After=tmp.mount to achieve the desired behaviour, without needing
to explicitly pull in the unit. This is easier to configure and more robust.
This changes the implementation introduced in
6156bec7a4.
Also actually describe the implemented behaviour in the man page.
In https://github.com/systemd/systemd/issues/28515, multiple people report that
services that have PrivateTmp=yes and DefaultDependencies=no fail to create the
temporary directories under /tmp, when /tmp is e.g. a bind mount or some other
kind of mount that takes more time.
Before PrivateTmp=disconnected was added, we didn't have a nice solution:
DefaultDependencies=no is used to start services very early, so we wouldn't
want to add a dependency on /tmp automatically. With PrivateTmp=disconnected we
have a fairly nice solution. Let's "upgrade" to this mode automatically.
Strictly speaking, it is a small compat break, but in practice it's unlikely to
matter for early-boot services whether their /tmp is private or disconnected.
The dependency on /tmp that is checked is After. I think this is enough,
since any tmp.mount would be pulled in by local-fs.target and the rest of
the transaction anyway, so we don't need to check more than After.
The asserts are relaxed, because now the two settings can now diverge
in either way.
Resolves https://github.com/systemd/systemd/issues/28515.
[yhndnzj: fix unit_add_exec_dependencies() to handle the new
combination; add a comment in exec_needs_sys_admin()]
The two options are were not documented or ever used in the codebase.
Additionally, the parser expected an argumentless option, while the option
table declared a required argument. So I think this was added for debugging
and never excercised properly. Since there was no public documentation for
those, it's as if they never existed, so it should be fine to drop them.
The line to set opterr=0 was added in the initial commit in
3d090cc6f3. But afaict, this never worked as
intended, because ':' must be the first char in optstring given to
getopt_long() for it to return ':' for a missing option value. Since
this wasn't set, getopt_long() would return '?', and the missing value
would be handled as an unknown option:
$ build/systemd-journal-upload --key
Unknown option --key.
$ build/systemd-journal-upload --asdf
Unknown option --asdf.
Let's just do the standard thing:
$ build/systemd-journal-upload --key
build/systemd-journal-upload: option '--key' requires an argument
$ build/systemd-journal-upload --asdf
build/systemd-journal-upload: unrecognized option '--asdf'
When we say '-n --iterations=N' in --help, this means that the program
can be invoked with '-n N' or '--iterations=N' or '--iterations N'.
(The short option is specified without the argument.)
Here we tried to use '-p --order=path' to mean that the program can be
invoked with '-p' or '--order=path', but that is incompatible with the
established convention.
Also, indicate that the arg to --cpu is optional.
I was looking at some logs on a console, and because of the lengthy message,
the actually interesting part, i.e. what condition failed, didn't even fit
on the screen. Let's make the sentence legible but brief.
See the [design
doc](https://github.com/systemd/systemd/issues/38023#issue-3192567450)
by @ikruglov
This PR introduces the metrics API framework, adds some basic system
wide/per unit/service metrics, and a basic CLI. The PR is broken into
two commits as described below.
### Deviations from the original design
- Introduced top level field "object" for ease of filtering. Instead of
having `fields: { unit: "foo", unit_type: "service" }`, we now have
`object: foo.service` as the top level field.
### First commit
The first commit includes:
- Metrics API definitions
- Code to set up the varlink server
- The describe method which shows all the metrics families
- The list method which lists all the metrics
- Type definitions related to MetricFamily
- Common code to build json objects
### Second commit
The second commit adds some basic metrics, a basic CLI (systemd-report)
which
lists the metrics, and integration tests.
**System wide metrics:**
- units_by_type_total
- units_by_state_total
**Two per unit metrics:**
- unit_active_state
- unit_load_state
**A service state metric:**
- nrestarts
### Sample outputs
**units_by_type_total**:
```
{
"name" : "io.systemd.Manager.units_by_type_total",
"value" : 52,
"fields" : {
"type" : "target"
}
}
{
"name" : "io.systemd.Manager.units_by_type_total",
"value" : 82,
"fields" : {
"type" : "device"
}
}
{
"name" : "io.systemd.Manager.units_by_type_total",
"value" : 2,
"fields" : {
"type" : "automount"
}
}
```
**units_by_state_total**:
```
{
"name" : "io.systemd.Manager.units_by_state_total",
"value" : 216,
"fields" : {
"state" : "active"
}
}
{
"name" : "io.systemd.Manager.units_by_state_total",
"value" : 0,
"fields" : {
"state" : "reloading"
}
}
{
"name" : "io.systemd.Manager.units_by_state_total",
"value" : 120,
"fields" : {
"state" : "inactive"
}
}
```
**unit_active_state**:
```
{
"name" : "io.systemd.Manager.unit_active_state",
"object" : "multi-user.target",
"value" : "active"
}
{
"name" : "io.systemd.Manager.unit_active_state",
"object" : "systemd-sysusers.service",
"value" : "inactive"
}
```
**unit_load_state**:
```
{
"name" : "io.systemd.Manager.unit_load_state",
"object" : "multi-user.target",
"value" : "loaded"
}
```
**nrestarts**:
```
{
"name" : "io.systemd.Manager.nrestarts",
"object" : "user@0.service",
"value" : 0
}
{
"name" : "io.systemd.Manager.nrestarts",
"object" : "user-runtime-dir@0.service",
"value" : 0
}
```
As reported in https://yeswehack.com/vulnerability-center/reports/705270,
YesWeHack issue #YWH-PGM9780-66, if a newline is injected into the message
field, it is passed through unmolested and lands in the .ini file. This
obviously would cause the parser to be confused.
Let's forbid any control characters or quotes in those fields. If we want
something more complicated in the future, we can add a custom validation
function and allow some escaping mechanism.
This partially reverts commit e58ba80a40. Those
aliases were removed as part of the removal of sysvinit compat, but the removal
was immediately reported as a regression by users. In particular, users can
have one of those set as the default target, or use those names in Wants= or
Requires= dependencies. At least in Fedora, the removal was immediately
reverted.
When looking at our "portability and stability promise", I noticed that those
units are explicitly listed in the promise. Let's restore them upstream. They
are conditionalized under a new -Dcompat-sysv-interfaces=bool option, so that
downstreams like Arch that wasnt to strictly avoid legacy interfaces can easily
disable them. They are dropped from the docs, like we do for all deprecated
interfaces.
The kernel has a "don't break userspace" policy [1] which is very easy
to understand and like (even if the details are complicated). We had a
complicated policy statement, but the actual practice is that we'll
generally avoid changes that cause problems for users. So the whole
detailed table is more of a guideline than something that we can rely
on.
To improve public perception, and to align the docs with practice, let's
make a general promise to keep stability, and change the existing table
to a "guideline" that tells people which interfaces to use and which not
to use so that use and development of systemd don't conflict or create
pain.
The existing document was very long, and I think it'll have a bigger
impact if it shorter. In particular, the chart is dropped, because it
was of limited use and severly outdated, because we have added many new
interfaces and components without updating it. Adding the missing
elements (and then keeping it updated) would be a lot of effort. The
chart would be most useful to somebody trying to build a systemd
reimplementation, but there haven't been many attempts like this, so
let's save the work and drop it and make the document shorter.
I also dropped the mentions of Debian. Back in the day, we did indeed
adopt many Debian conventions for config files and such, but over the
years the scope of the project has grown quite a bit and that part
of systemd is a small fraction now. And we've adopted conventions
and ideas from many other places too, so we don't mention Debian
specifically.
[1] https://docs.kernel.org/admin-guide/reporting-regressions.html
The immediate impulse for this change is the fedora scriptlet which called:
/usr/lib/systemd/systemd-update-helper install-system-units cryptsetup-pre.target cryptsetup.target getty@.service ... system-systemd\x2dcryptsetup.slice system-systemd\x2dveritysetup.slice ...
which called
systemctl preset cryptsetup-pre.target cryptsetup.target getty@.service ... system-systemd\x2dcryptsetup.slice system-systemd\x2dveritysetup.slice ...
which threw an error that system-systemdx2dcryptsetup.slice does not exist
and did nothing at all. (The backslash is consumed by the shell.)
The obvious fix here is to figure out more levels of escaping… But we should
do something more robust in such cases.
If we fail in processing of a single unit, let preset all continue processing
units, report the failure through 'changes'. At the end, return failure. In
general, for operations which operate on a list of units specified by the user,
fail the whole operation if any of the individual operations failed. The only
operation where we don't do this is 'preset-all'.
$ SYSTEMD_LOG_LEVEL=debug build/systemctl --root=/ preset asdf1.servie asdf2.path; echo $?
Cannot find unit asdf1.servie.service.
Cannot find unit asdf2.path.
Failed to preset unit: Unit asdf1.servie.service does not exist
Failed to preset unit: Unit asdf2.path does not exist
1
While at it, fix double logging in the manager: dump_unit_changes() already
logs about errors, so the manager should only log on success.
This partially reverts a4f0e0da35. The
intent was good, we gather the errors, but we have no mechanism to
propagate the result, so the gathered result was ignored. In 'changes'
we can only report errors for specific units. If reading of the
directory fails, we might just as well report the error immediately.
This isn't great, but it's better then ignoring the errors. In practice,
failing halfway in this manner is unlikely, since it'd mean that the fs
is corrupted or something like that. We might as well return immediately
on such catastrophic errors.
In the event that we can't receive all of the fds from the message
(which can happen for a number of reasons including LSM denials or
hitting the fd limit of the process) the kernel will set the MSG_CTRUNC
flag. Through our use of recvmsg_safe() we've been treating this as a
fatal error, which will result in dropping the connection.
Let's dial that back a bit: we can receive the message, but when the
user attempts to access the missing fds via sd_bus_message_read_basic()
we can return the (existing) error code of -EBADMSG to indicate that the
fd is missing.
We can do this by using recvmsg() directly, and relaxing some of the
checks on message creation: when (and only when) we have received
MSG_CTRUNC we allow a smaller than expected (per the header) number of
fds to be present. The error check in sd_bus_message_read_basic() was
already there so we don't need to do anything about that.
This puts the receiver of the message into a difficult situation: you
can call sd_bus_message_read_basic() as often as you want but as long as
it keeps returning -EBADMSG it won't progress through the message and
you won't be able to close whatever container you're in. That means
that the user will probably need to abandon processing the message
anyway. So why not just drop the message up front? This approach is
more likely to yield a useful error message, which will be invaluable
for people trying to track down problems caused by LSM denials.
Fixes#34688
In Fedora, kmsconvt@.service is starting to be used instead of getty@.service
to have nicer font handling. This means that we need the autovt@.service alias
point to the new unit. So far the alias was done through a static symlink
because there was little reason to change it. Let's use [Install] instead so
the decision which implementation to use can be made after installation.
https://bugzilla.redhat.com/show_bug.cgi?id=2429626https://fedoraproject.org/wiki/Changes/UseKmsconVTConsole
This is in preparation for later changes. The 1500+ line switch statement was very
hard to work with and had quite a bit of duplicated code. This is a rework that
splits outs parts into functions. I split this up into a bunch of commits so that it
can be reasonably reviewed. When viewed as one giant commit, diff just shows
a giant block of removed code and a giant block of added code.
In some cases I wasn't sure why one or the other of the identity arrays is used…
I preserved existing behaviour in all cases.
We parse RLIMIT_FOO, FOO, and LimitFOO all the same. But the keys in the
JSON dict are supposed to be "RLIMIT_FOO". We were creating such keys,
but then filtering them using "FOO", i.e. the filtering was not effective.
Using strjoina since both strings are under our control.
The idea is to split out helpers to make parse_argv() itself more
manageable. If possible, the helpers will be reused in many places. But
even if not, as in this case, it seems good to split out the code for a
specific option. Always, the sd_json_variant** pointer or pointers that
are operated on are selected in the caller. This way it is easier to see
which of the identity variables is used and if two different ones are
used.
In recent Fedora, preset-all fails:
[ 155s] Failed to preset unit: File '/buildroot/etc/systemd/user/dbus.service'
already exists and is a symlink to /usr/lib/systemd/user/dbus-broker.service
[ 155s] ‣ "systemctl --root=/buildroot --global preset-all" returned non-zero exit code 1.
Strictly speaking, this is an error in configuration. The presets specify that
both dbus-broker.service and dbus-daemon.service shall be enabled and they both
claim the 'dbus.service' alias. But this kind of error is very easy to make.
Failing the preset operation is too harsh, since in most cases the system will
work fine without an alias and changes in unrelated components can cause the
conflict.
Let's reuse the same logic that was added in
ad5fdd3912: when enabling the unit through
'preset' or 'preset-all', print the message, but suppress the error. When
enabling through 'enable', fail the operation.
The test passes but for two minutes the cleanup function just logs:
Failed to inspect home: No home for user homedsshtest known
Failed to inspect home: No home for user homedsshtest known
Failed to inspect home: No home for user homedsshtest known
...
Do not wait for the user to become inactive or remove the user if 'homectl
inspect' doesn't find the user. This brings a successfull run from 176 s
to 58 s here.
Previously, if execution failed, we'd log at error level both from the
child and the parent, and we were using a bogus variable for the argument
name:
$ build/systemd-inhibit list
Failed to execute : No such file or directory
list failed with exit status 1.
In general, we can and should assume that the program the user is calling
is well behaved, so it'll log the error on its own if appropriate. So we
shouldn't log on "normal errors", but only if the child is terminated by
a signal.
And since the program name is controlled by the user, use quotes everywhere
to avoid ambiguity.
Now:
$ build/systemd-inhibit false
(nothing)
$ build/systemd-inhibit bash -c 'kill -SEGV $$'
src/basic/process-util.c:895: 'bash' terminated by signal SEGV.
menu-force and menu-hidden were added in 97f077df05,
menu-disable was added in 6efdd7fec5, a year later.
So we can assume that if the feature flag is set, the other string values are
supported too. The comment that there's no way check that was added later in
5b45fad4fc, but it was incorrect even at that
time.
Fixes https://github.com/systemd/systemd/issues/39167. As described in the
issue, we documented various string values in the BLI, but bootctl didn't use
the string values. At the time menu-force and menu-hidden were added, using
numerical values for compatibility made sense. But that stopped being needed
when a string value that didn't have a strictly equivalent numerical value and
a feature flag were added.
When converting a large number to menu-force, message is downgraded to debug,
since the severity of the issue is very minor. Debug messages are added in
other places when the requested setting is modified too.
So far we compiled the normal and standalone versions completely
independently. Let's use the 'extract' template pattern to avoid any
additional compilation and only require an single link to produce the
.standalone variants.
Unfortunately, as designed, the 'extract' framework only allows one set of
object files to be extracted. Since we need all the files for the
.standalone version, we cannot use 'extract' for other purposes. Thus, in
the two cases where 'extract' was used for the test binaries, this is now
changed to compile the files a second time. But the number of files in that
list is small, so this seems like a better option.
(If we weren't using the template system, we could easily extract just the
objects we need. But with the current system, at the point of the
definition, the binaries are not defined yet. We'd need to handle all of
this through sets of dictionaries, and that just seems like too much
trouble to avoid double compilation of a few small files.)
If we ever want to add it back, it should be with -DSTANDALONE=0|1, so
that #if instead of #ifdef can be used. We generally converted our internal
defines to that form.
This effectively reverts 3537577c37. Originally,
the #ifdefs were added because we didn't want to pull in the whole tree of
libmount and other dependencies in standalone versions. But dependencies are
now loaded through dlopen(), so this is not needed anymore. (And doesn't even
make much of a difference.)
Allow .standalone version to be built on-demand, even if -Dstandalone=false
is configured. In other words, this changes the meson option from a hard
disablement to a soft "build is on/off by default".
The meson config was originally written in this way but we lost this feature
after the transition to templates. It is nice to build additional targets on
demand during development, so add this back.
The indirection through variables doesn't seem that useful here:
OTOH, the lists are short, and OTOH, there is a bunch of different
programs with similar names. Overall, it's all easier to follow if
the lists are inline.
This allows us to query the window size without moving the cursor. We have
various reports about the cursor being in an unexpected position and/or state.
Closes https://github.com/systemd/systemd/issues/37526.
In aad0d11e7c we stopped supporting XBOOTLDR
with a different fs driver. This was the primary example that comes to mind
when we talk about loading filesystem drivers in the firmware. Since we don't
want people to do load such drivers, use a different example.
We said in the header that "all EFI variables use the vendor UUID
4a67b082-0a4c-41cf-b6c7-440b29bb8c4f", but people not familiar with
UEFI might not know that this is concatenated with the variable name.
Let's use the full form once — when introducing the variable — to
make it easier to grep and search for.
While at it, use sembreaks in the document. This makes subsequent
changes much easier to review. (It also shows that some sentences
are rather long and thus hard to understand.)
This adds minimal definitions provided by libacl to make tar-util built
without libacl support.
Fixes the following compile error:
```
../src/shared/tar-util.c:62:9: error: unknown type name 'acl_t'
62 | acl_t acl_access, acl_default;
| ^~~~~
```
Follow-up for 4ded7f7a43.
Fixes#39928.
Replaces #39954.
manager_varlink_init_resolve_hook and manager_varlink_init_userdb are very
similar, but one didn't set a description and the other one had an error
message which didn't print the offending path.
Define constants for the paths, and also change other similar constants
to have slightly shorter names. (It's an "address" and "path", but we don't
need to have both parts in the name, esp. that it makes the name very long.)
Previously, we looked for scripts in the tools/ directory ad hoc,
wherever they were needed. Let's do those checks in one place.
The main meson.build file is shrunk somewhat, which is always nice.
Quite often we need to specify include_directories('.'). Normally, meson
does this automatically, but we specify implicit_include_directories : false,
see 30d20907bd for an explanation.
Passing the include_directories argument through the template layers was rather
cumbersome. Let's simplify things by always including the directory of the
first source file. This makes the definitions simpler, at the cost of having an
unnecessary include directory in the list in some cases.
(Tests are excluded from this change, because they happen to not need this, but
also because some tests have source[0] which is a custom target, and we'd need
newer meson to support that.)
… and also when it fails, in the few cases where this wasn't done already.
When I was debugging https://github.com/systemd/systemd/issues/39048, it
was quite confusing that we print copious information about D-Bus messages
and all other steps we're undertaking, but nothing about the sd-notify
messages which are much more important. Add some debug messages for the
cases where we didn't print anything. (The case where $NOTIFY_SOCKET is
not set in left unchanged. The variable is often left unset and we don't
need to spam logs in that trivial case.)
--lines=1 means that we show the last message about the unit, whatever it is:
$ SYSTEMD_LOG_LEVEL=debug systemctl --verbose start demo2
...
Invoking 'journalctl -q --follow --no-pager --lines=1 --synchronize-on-exit=yes --unit=demo2.service' as child.
...
Directory /var/log/journal/7d16833bfa924410851e2a193bcfd4ba added.
Journal effective settings seal=no keyed_hash=yes compress=ZSTD compress_threshold_bytes=8B
...
Reiterating files to get inotify watches established.
Considering root directory '/run/log/journal'.
Considering root directory '/var/log/journal'.
Considering directory '/var/log/journal/7d16833bfa924410851e2a193bcfd4ba'.
Nov 26 17:24:35 rawhide systemd[1]: Finished demo2.service.
...
Executing dbus call org.freedesktop.systemd1.Manager StartUnit(demo2.service, replace)
...
Job for demo2.service finished.
Got result done/Success for job demo2.service.
journal: Sending message: {"method":"io.systemd.Journal.Synchronize","parameters":{"offline":false}}
Nov 27 13:05:30 rawhide systemd[1]: Starting demo2.service...
Nov 27 13:05:30 rawhide systemd[1]: demo2.service: Deactivated successfully.
Nov 27 13:05:30 rawhide systemd[1]: Finished demo2.service.
...
We obviously should only show _new_ messages, hence change to --lines=0.
This works properly after the fix in the previous commit.
Fixes#39048.
If --lines=0 is given, we'd skip the setup and not invoke sd_notify,
potentially blocking the caller. Change the condition for the callback
to also include that case. Since then the callback would always be
set, the 'if' statement is not necessary anymore.
Latest glibc uses _Generic to have strstr() and other functions return
const char* or char* based on whether the input is a const char* or a
char*. This causes build failures as we previously always expected a
char*.
Let's fix the compilation failures and add our own macros similar to
glibc's to have string functions that return a mutable or const pointer
depending on the input.
