Longer-term, I think we should just make BindMount= automatically "upgrade"
(or "downgrade", depending on how you look at this), any InaccessiblePath=
mountpoints to "tmpfs". I don't see much point in forcing users to remember
this interaction. But let's at least document the status quo, we can always
update the docs if the code changes.
Jan Klötzke [Wed, 7 Mar 2018 13:16:49 +0000 (14:16 +0100)]
core: immediately trigger watchdog action on WATCHDOG=trigger
A service might be able to detect errors by itself that may require the
system to take the same action as if the service locked up. Add a
WATCHDOG=trigger state change notification to sd_notify() to let the
service manager know about the self-detected misery and instantly
trigger the configured watchdog behaviour.
The interface provided by those two functions is huge, so this text could
probably be made two or three times as long if all details were described.
But I think it's a good start.
This wraps the call to org.freedesktop.DBus.Introspectable.Introspect.
Using "busctl call" directly is inconvenient because busctl escapes the
string before printing.
test-bus-{vtable,introspect}: share data and test introspect_path()
test-bus-introspect is also applied to the tables from test-bus-vtable.c.
test-bus-vtable.c is also used as C++ sources to produce test-bus-vtable-cc,
and our hashmap headers are not C++ compatible. So let's do the introspection
part only in the C version.
udev: net_id: introduce predictable names for netdevsim
In order to properly and predictably name netdevsim netdevices,
introduce a separate implementation, as the netdevices reside on a
specific netdevsim bus. Note that this applies only to netdevsim devices
created using sysfs, because those expose phys_port_name attribute.
sd-bus: split introspection into the content creation and reply creation parts
Just moving code around, in preparation to allow the content creation
part to be used in other places.
On the surface of things, introspect_path() should be in bus-introspect.c, but
introspect_path() uses many static helper functions in bus-objects.c, so moving
it would require all of them to be exposed, which is too much trouble.
test-bus-introspect is updated to actually write the closing bracket.
sd-bus: allow vtable format structure to grow in the future
We would check the size of sd_bus_vtable entries, requring one of the two known
sizes. But we should be able to extend the structure in the future, by adding
new fields, without breaking backwards compatiblity.
Incidentally, this check was what caused -EINVAL failures before, when programs
were compiled with systemd-242 and run with older libsystemd.
sd-bus: add symbol to tell linker that new vtable functions are used
In 856ad2a86bd9b3e264a090fcf4b0d05bfaa91030 sd_bus_add_object_vtable() and
sd_bus_add_fallback_vtable() were changed to take an updated sd_bus_vtable[]
array with additional 'features' and 'names' fields in the union.
The commit tried to check whether the old or the new table format is used, by
looking at the vtable[0].x.start.element_size field, on the assumption that the
added fields caused the structure size to grow. Unfortunately, this assumption
was false, and on arm32 (at least), the structure size is unchanged.
In libsystemd we use symbol versioning and a major.minor.patch semantic
versioning of the library name (major equals the number in the so-name). When
systemd-242 was released, the minor number was (correctly) bumped, but this is
not enough, because no new symbols were added or symbol versions changed. This
means that programs compiled with the new systemd headers and library could be
successfully linked to older versions of the library. For example rpm only
looks at the so-name and the list of versioned symbols, completely ignoring the
major.minor numbers in the library name. But the older library does not
understand the new vtable format, and would return -EINVAL after failing the
size check (on those architectures where the structure size did change, i.e.
all 64 bit architectures).
To force new libsystemd (with the functions that take the updated
sd_bus_vtable[] format) to be used, let's pull in a dummy symbol from the table
definition. This is a bit wasteful, because a dummy pointer has to be stored,
but the effect is negligible. In particular, the pointer doesn't even change
the size of the structure because if fits in an unused area in the union.
The number stored in the new unsigned integer is not checked anywhere. If the
symbol exists, we already know we have the new version of the library, so an
additional check would not tell us anything.
An alternative would be to make sd_bus_add_{object,fallback}_vtable() versioned
symbols, using .symver linker annotations. We would provide
sd_bus_add_{object,fallback}_vtable@LIBSYSTEMD_221 (for backwards
compatibility) and e.g. sd_bus_add_{object,fallback}_vtable@@LIBSYSTEMD_242
(the default) with the new implementation. This would work too, but is more
work. We would have to version at least those two functions. And it turns out
that the .symver linker instructions have to located in the same compilation
unit as the function being annotated. We first compile libsystemd.a, and then
link it into libsystemd.so and various other targets, including
libsystemd-shared.so, and the nss modules. If the .symver annotations were
placed next to the function definitions (in bus-object.c), they would influence
all targets that link libsystemd.a, and cause problems, because those functions
should not be exported there. To export them only in libsystemd.so, compilation
would have to be rearranged, so that the functions exported in libsystemd.so
would not be present in libsystemd.a, but a separate compilation unit containg
them and the .symver annotations would be linked solely into libsystemd.so.
This is certainly possible, but more work than the approach in this patch.
856ad2a86bd9b3e264a090fcf4b0d05bfaa91030 has one more issue: it relies on the
undefined fields in sd_bus_vtable[] array to be zeros. But the structure
contains a union, and fields of the union do not have to be zero-initalized by
the compiler. This means that potentially, we could have garbarge values there,
for example when reading the old vtable format definition from the new function
implementation. In practice this should not be an issue at all, because vtable
definitions are static data and are placed in the ro-data section, which is
fully initalized, so we know that those undefined areas will be zero. Things
would be different if somebody defined the vtable array on the heap or on the
stack. Let's just document that they should zero-intialize the unused areas
in this case.
The symbol checking code had to be updated because otherwise gcc warns about a
cast from unsigned to a pointer.
socket-util: make sure flush_accept() doesn't hang on unexpected EOPNOTSUPP
So apparently there are two reasons why accept() can return EOPNOTSUPP:
because the socket is not a listening stream socket (or similar), or
because the incoming TCP connection for some reason wasn't acceptable to
the host. THe latter should be a transient error, as suggested on
accept(2). The former however should be considered fatal for
flush_accept(). Let's fix this by explicitly checking whether the socket
is a listening socket beforehand.
test: don't timeout while waiting for other test units
The main testsuite service timeouts sporadically when waiting for
other testsuite-* units. As the test timeout is handled by
the "test executor" (test.sh), let's disable it for the service.
The L2TP_ATTR_UDP_ZERO_CSUM6_{TX,RX} attributes are introduced by 6b649feafe10b293f4bd5a74aca95faf625ae525, which is included in
kernel-3.16. To support older kernel, let's import the header.
linux: also import linux/in.h and in6.h from kernel-5.0
Now linux/in.h has better conflict detection with glibc's
netinet/in.h. So, let's import the headers.
Note that our code already have many workarounds for the conflict,
but in this commit does not drop them. Let's do that in the later
commits if this really helps.
tmp.conf was dealing with 2 different kind of paths: one dealing with general
temporary paths such as /var/tmp and /tmp and the other one dealing with
temporary directories owned by systemd.
If for example a user wants to adjust the age argument of the general paths
only, he had to overload the whole file which is cumbersome and error prone
since any future changes in tmp.conf shipped by systemd will be lost.
So this patch splits out tmp.conf so the systemd directories are dealt
separately in a dedicated conf file. It's named "systemd-tmp.conf" based on the
naming recommendation made in tmpfiles.d man page.
In practice it shouldn't cause any regression since it's very unlikely that
users override paths owned by systemd.
With gcc-9.0.1-0.10.fc30.x86_64:
../src/network/netdev/macsec.c: In function ‘config_parse_macsec_port’:
../src/network/netdev/macsec.c:584:24: warning: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Waddress-of-packed-member]
584 | dest = &c->sci.port;
| ^~~~~~~~~~~~
../src/network/netdev/macsec.c:592:24: warning: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Waddress-of-packed-member]
592 | dest = &b->sci.port;
| ^~~~~~~~~~~~
(The alignment was probably OK, but it's nicer to avoid the warning anyway.)
Jan Klötzke [Wed, 29 Nov 2017 06:43:44 +0000 (07:43 +0100)]
service: handle abort stops with dedicated timeout
When shooting down a service with SIGABRT the user might want to have a
much longer stop timeout than on regular stops/shutdowns. Especially in
the face of short stop timeouts the time might not be sufficient to
write huge core dumps before the service is killed.
This commit adds a dedicated (Default)TimeoutAbortSec= timer that is
used when stopping a service via SIGABRT. In all other cases the
existing TimeoutStopSec= is used. The timer value is unset by default
to skip the special handling and use TimeoutStopSec= for state
'stop-watchdog' to keep the old behaviour.
If the service is in state 'stop-watchdog' and the service should be
stopped explicitly we still go to 'stop-sigterm' and re-apply the usual
TimeoutStopSec= timeout.
Chris Down [Thu, 28 Mar 2019 12:50:50 +0000 (12:50 +0000)]
cgroup: Implement default propagation of MemoryLow with DefaultMemoryLow
In cgroup v2 we have protection tunables -- currently MemoryLow and
MemoryMin (there will be more in future for other resources, too). The
design of these protection tunables requires not only intermediate
cgroups to propagate protections, but also the units at the leaf of that
resource's operation to accept it (by setting MemoryLow or MemoryMin).
This makes sense from an low-level API design perspective, but it's a
good idea to also have a higher-level abstraction that can, by default,
propagate these resources to children recursively. In this patch, this
happens by having descendants set memory.low to N if their ancestor has
DefaultMemoryLow=N -- assuming they don't set a separate MemoryLow
value.
Any affected unit can opt out of this propagation by manually setting
`MemoryLow` to some value in its unit configuration. A unit can also
stop further propagation by setting `DefaultMemoryLow=` with no
argument. This removes further propagation in the subtree, but has no
effect on the unit itself (for that, use `MemoryLow=0`).
Our use case in production is simplifying the configuration of machines
which heavily rely on memory protection tunables, but currently require
tweaking a huge number of unit files to make that a reality. This
directive makes that significantly less fragile, and decreases the risk
of misconfiguration.
After this patch is merged, I will implement DefaultMemoryMin= using the
same principles.
json: be more careful when iterating through a JSON object/array
Let's exit the loop early in case the variant is not actually an object
or array. This is safer since otherwise we might end up iterating
through these variants and access fields that aren't of the type we
expect them to be and then bad things happen.
Of course, this doesn't absolve uses of these macros to check the type
of the variant explicitly beforehand, but it makes it less bad if they
forget to do so.
json: simplify JSON_VARIANT_OBJECT_FOREACH() macro a bit
There's no point in returning the "key" within each loop iteration as
JsonVariant object. Let's simplify things and return it as string. That
simplifies usage (since the caller doesn't have to convert the object to
the string anymore) and is safe since we already validate that keys are
strings when an object JsonVariant is allocated.