This fixes a long-standing issue in packaging scriptlets: daemon-reload
was moved to the end of the transaction, but restarting services was still
straightaway after package installation.
Note that daemon-reload is called twice. This wouldn't be hardly noticable,
except that now a bunch of units (at least in Fedora) generate very verbose
warnings about deprecated features. So we get those warnings twice…
reload-or-restart --needing-restart is also called twice, but the second call
is usually a noop, because the first clears the flag for restarted units. The
second call is necessary for the case where we only uninstall packages, and the
%transfiletriggerpostun trigger fires, but not the %transfiletriggerin
scriptlet.
Also note that this assumes that units are marked only for restart if paths
under @systemunitdir@ or /etc/systemd/system have been touched. I would prefer
make the trigger that does 'restart --needing-restart' fire always, but it
seems rpm doesn't have such functionality. (Except as a %transfiletrigger that
would trigger on "/*" to catch all transactions, but that seems ineffiecient
and ugly.)
rpm: order sysctl/sysusers/tmpfiles execution before package scriptlets
P>1000000 is *before* "normal" scriptlets, P<1000000 is *after*. I think it
makes sense to do stuff like execution of sysctl/sysusers/tmpfiles configuration
before package scriptlets. I think that was the intent, but a single digit got
dropped ;(
Also, let's reorder the scriptlets in the file to match execution order, to
make it easier to see what is going on.
Most of those may happen in any order, but there are some exceptions:
tmpfiles should be after sysusers,
udevadm --reload should be after hwdb.
The trigger was initially written to use %transfiletriggerun instead
of %transfiletriggerpostun because the latter would not fire. It turned
out to a buffer overread in rpm that since has been long fixed:
https://bugzilla.redhat.com/show_bug.cgi?id=1284645
https://github.com/rpm-software-management/rpm/commit/f6521c50f6836374a0f7995f8f393aaf36e178ea
rpm: sync the shell version of triggers.systemd with the lua version
Note that this goes both ways: in particular the lua version had udev
scriptlets in the wrong package, fixed in
https://src.fedoraproject.org/rpms/systemd/c/3c9433d7cf4afc8d76660402f6c3d9d991596b83.
rpm: pull in the alternative trigger implementation in sh
From https://src.fedoraproject.org/rpms/systemd/blob/master/f/triggers.systemd.
In 12dde791d519bc80d5cca4ab6f088763cd481015 scriptlets were converted to lua.
This is not only faster and cleaner, but also avoids a nasty dependency loop:
rpm implements the lua scripting internally, so we don't need a working shell
for the scriplets. This is nice and all, but unfortunately ostree wants to
capture scriptlets and execute them at a later time and does not support lua.
So in Fedora we ended up with a revert back to a shell-based implementation
[1]. At the time I hoped this would only be a temporary workaround, but three
years later I think it's fair to assume that this will not happen any time
soon. But carrying the upstream lua version and the downstream sh version is
error prone. So let's import the other version into our tree too so that they
can be kept in sync.
This is almost equivalent to 'busctl call-method org.freedesktop.systemd1
/org/freedesktop/systemd1 org.freedesktop.systemd1.Manager EnqueueMarkedJobs',
but waits for the jobs to finish.
core: add EnqueueMarkedJobs method to reload/restart marked units
We support two return types for methods that start jobs. EnqueueJob support the
full-monty mode with affected jobs. I didn't do this here, since it seems
unlikely to be used. In the common case there'd be a huge list of jobs and
affected jobs. EnqueueMarkedJobs() just returns a list of jobs that we can wait
upon.
The name of the method is generic in case we decide to add something other than
just reload/restart later on.
When errors occur, resource errors are treated as fatal, but for other error
types we queue up other jobs, and only return an error at the end. The
assumption is that the caller will ignore the result error anyway, so it's
better to try to reload/restart as much as possible.
The property is never set by systemd, only reset after a stop or restart or
reload. It may externally be set to mark the unit for a later restart/reload.
I wasn't sure whether to configure the property only for the types where this
makes sense (Service, Swap, etc). But Restart() method is defined on the unit,
and also having this always under the same property name is more convenient.
We had a lone 'bool job_running_timeout_set:1', which generated a hole. Let's
move things around a bit. The structure is a tiny bit smaller and has less
holes:
/* size: 1192, cachelines: 19, members: 149 */
/* sum members: 1175, holes: 3, sum holes: 11 */
/* sum bitfield members: 27 bits, bit holes: 1, sum bit holes: 7 bits */
/* bit_padding: 14 bits */
/* last cacheline: 40 bytes */
/* size: 1184, cachelines: 19, members: 149 */
/* sum members: 1175, holes: 1, sum holes: 4 */
/* sum bitfield members: 27 bits (3 bytes) */
/* bit_padding: 13 bits */
/* last cacheline: 32 bytes */
A helper function would seem more natural, but there are two reasons why a
macro is needed:
- many bool fields are bitfields, so we can't take a pointer, and using a macro
allows us to avoid taking a pointer.
- we have a few diffent types (bool, uint64_t, FreezerState), and we can have
type safety without specifying the type by using the macro.
This also makes the error messages more informative: they print the exact field
identifier that failed, which is more useful for debugging than a description.
sd-bus: extend sd_bus_message_read_strv() to paths and signatures
It's rather convenient to be able to read all three types with this function.
Strictly speaking this change is not fully compatible, in case someone was
relying on sd_bus_message_read_strv() returning an error for anything except
"as", but I hope nobody was doing that.
Frantisek Sumsal [Wed, 10 Feb 2021 09:16:09 +0000 (10:16 +0100)]
test: count call instructions as well
Binaries on the latest Arch Linux use `call` instructions instead of
`callq`, which breaks the ASan detection and eventually the image
building process (due to insufficient space).
resolved: rework a few functions to return early on error
The implementation is a bit ugly because we set the output variable
twice. But we were already doing this on error, so this is not
significantly worse. Doing this allows us to avoid goto's, and the
compiler should be able to figure this out and only set once.
We'd return -1 (-EPERM), even though we have a general rule to use real errno
values. The particular case that caught my attention was:
$ sudo udevadm control -l asdf
Failed to parse log priority 'asdf': Operation not permitted
... but "git grep 'r =.*_from_string' src/" return 110 hits. Confusingly, some
of the _from_string functions already return a proper errno value, so not all
of those are broken, but probably quite a few.
A hole was/is present after the booleans, so changing them to be one byte each
doesn't change the structure size (122 bits on amd64). If we add more stuff
later, it might make sense to turn some of those into bitfields again. For now,
let's take the easy route. EINVAL fits into type now.
Code size it minimally reduced:
-rwxrwxr-x 1 zbyszek zbyszek 4109792 Feb 10 14:00 build/libsystemd.so.0.30.0
-rwxrwxr-x 1 zbyszek zbyszek 4109712 Feb 10 14:01 build/libsystemd.so.0.30.0
sd-netlink: pahole optimization of sd_netlink_slot
By rearranging the fields, we can avoid one of the holes (on amd64).
By adding more space for .type, -EINVAL can be used as a value later on.
The structure is reduced from 96 to 88 bytes (on amd64).
Text size is also smaller:
-rwxrwxr-x 1 zbyszek zbyszek 4109832 Feb 9 19:50 build/libsystemd.so.0.30.0
-rwxrwxr-x 1 zbyszek zbyszek 4109792 Feb 9 19:51 build/libsystemd.so.0.30.0
Every bitfield may make the object a little smaller, but requires additional
code when accessing the field. So it only makes sense in objects that are
created in many many copies. The Manager is pretty much a singleton.
-rwxrwxr-x 1 zbyszek zbyszek 4443840 Feb 9 16:14 build/systemd
-rwxrwxr-x 1 zbyszek zbyszek 4442552 Feb 9 16:42 build/systemd
We save 1288 bytes of code by "wasting" a few bytes on storage. (The speed
advantages are probably more important, but harder to measure…)
Doing it all in one line and with negation in front seemed like a good
idea at some point, but I think it is vastly easier to understand when
it is split out a bit.
Let's use our table formatter for generating the coredump table. Bring
support up to our current standards, with a bit of color, JSON output
and so on.
Also adds supports for setting a max for the number of lines to
generate. (with the new -n switch)
The existing -1 switch now becomes a synonym for "-n 1 --reverse"
systemd-inhibit when invoked with a command line will put the whole
command line in the "who" field of the inhibitor lock. This can get
extremely long for shell expressions, making the table "systemd-inhibit
--list" shows ridiculously weirdly formatted. Let's put a limit on the
column width: half of the screen, not more.
format-table: don't hit assert if column got less width than it asked for
If one field in a specific column has a maximum size limit, other fields
in the same column might affected by it and get less than they asked
for. Let's make sure we can handle this, and don't assert on this
because surprisingly we got less than what we asked for.
Does what the name suggests. Obviously inspired by sudoers, but note that
our tools are not supposed to be installed suid, so there is no privilege
boundary to cross here.
fuzz-journal-remote: do not assert on resource conditions
We have a number of issues where oss-fuzz reports input-independent crashes of
fuzz-journal-remote. Instead of asserting that stuff that allocated fds and
memory never fails, let's instead just return an error.
I don't see any docs as to whether LLVMFuzzerTestOneInput() is allowed to
return non-zero. Propagating the error code is easiest, so let's just do that.
If it turns out that this causes oss-fuzz to still report a failure, we can
suppress that later.
ci: temporarily pin Arch repositories to glibc 2.32-5
glibc 2.33-3 shipped on 2021-02-06 breaks running Arch containers on
systems with older kernels (like Ubuntu Focal). Until the issue is
resolved, let's pin the Arch repositories to glibc 2.32-5 to mitigate
the annoying CI fails.
network: Delay addition of IPv6 Proxy NDP addresses
Setting of IPv6 Proxy NDP addresses must be done at the same
time as static addresses, static routes, and other link attributes
that must be configured when the link is up. Doing this ensures
that they are reconfigured on the link if the link goes down
and returns to service.
Yu Watanabe [Wed, 3 Feb 2021 18:21:08 +0000 (03:21 +0900)]
fundamental: move several macros and functions into src/fundamental/
sd-boot has a copy of a subset of codes from libbasic. This makes
sd-boot share the code with libbasic, and dedup the code.
Note, startswith_no_case() is dropped from sd-boot, as
- it is not used,
- the previous implementation is not correct,
- gnu-efi does not have StrniCmp() or so.
Yu Watanabe [Fri, 22 Jan 2021 07:38:52 +0000 (16:38 +0900)]
hostname: re-read file later when failed to update file
Previously, even when writing e.g. /etc/hostname fails, the static
hostname in Context is not restored. So, the subsequent call of the same
method succeeds:
```
$ sudo chattr +i /etc/hostname
$ sudo hostnamectl --static set-hostname aaa
Could not set static hostname: Access denied
$ echo $?
1
$ sudo hostnamectl --static set-hostname aaa
$ echo $?
0
```
This makes when updating file is failed, the saved stat is cleared. So,
the static hostname or machine information in the context are always
consistent to the corresponding files.
Yu Watanabe [Fri, 22 Jan 2021 02:20:50 +0000 (11:20 +0900)]
hostnamectl: try to set transient hostname even if updating static or pretty hostname failed
If no target (--pretty, --static, or --transient) is specified, then
let's try to set transient hostname even if setting static or pretty
hostname failed. This may be useful for read-only filesystem.
tree-wide: fix the string concatenation warning with clang-12
e.g.:
./src/shared/dissect-image.c:2218:39: error: suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma? [-Werror,-Wstring-concatenation]
"/usr/lib/os-release\0",
^
../src/shared/dissect-image.c:2217:39: note: place parentheses around the string literal to silence warning
[META_OS_RELEASE] = "/etc/os-release\0"
^
1 error generated.
Antonius Frie [Mon, 8 Feb 2021 08:15:15 +0000 (09:15 +0100)]
Use correct config parser for MountAPIVFS (#18501)
As far as I can see, at some point the parser function for MountAPIVFS
was changed from the generic bool parser to a custom implementation, to
allow the context to keep track of whether MountAPIVFS had been set
explicitly. If not, exec_context_get_effective_mount_apivfs would fall
back to a default value. However, the corresponding entry in the big
parser table wasn't updated, meaning that the old bool parser was still
used, meaning that context->mount_apivfs_set remained at its default
value of false, meaning that the default value was always used and the
config option was effectively ignored.
The last release of Upstart was July 2014 [1], and there have been no new
commits to the repo. We should move on too.
[1] https://lists.ubuntu.com/archives/upstart-devel/2014-July/003313.html
The only real support was in the code that looked for $PREVLEVEL.
https://codesearch.debian.net/search?q=PREVLEVEL&literal=1&perpkg=1 shows this
string in our own code (or the copy in elogind), our own man pages, and init
scripts for two packages (brltty, salt), which shouldn't be used with systemd.
(And both *check* for PREVLEVEL, and don't set it. So most likely nothing at
all sets it.)
When executed in test mode, "OUTDATED" is appropriate. But when executed
to actually update the text, after the tool executes, those pages are the
opposite, not outdated.
It happens too often that what people ask for already is implemented.
Let's help cut the noise a bit, and make people check things first
hopefully, and at least make it either for us to detect such cases.
resolved: suppress ifindex info in varlink JSON responses if zero
If we don't have ifindex info, don't set the field for it.
We already do that for parsed IP address replies, let's do it for all
cases: it's a bit nicer to suppress the ifindex prop if it doesn't apply
than to pass it invalid.
This is the other side of #18482, i.e. fixes things so that the parser
doesn't get tripped up by this.
(This too makes a problem go away we should track down properly, i.e.
figure out how the ifindex got lost in
https://github.com/systemd/systemd/pull/17823#issuecomment-742439422 )
nss-resolve: accept zero ifindex when parsing resolved reply
Sometimes a reply isn't associated to any specific interface, it might
be a general truth (for example served from /etc/hosts or so). In this
case the server might pass ifindex == 0. Accept that.
Since the test suite overhaul, the test units are now under
/usr/lib/systemd/tests/testdata/tetsuite-06.units with
system_u:object_r:lib_t context. This causes an AVC denial, since the
systemd unit files are expected to have the
system_u:object_r:systemd_unit_file_t context. Let's fix this by using a
custom file context definition.