Yu Watanabe [Fri, 25 Mar 2022 20:01:40 +0000 (05:01 +0900)]
udevadm: introduce new 'wait' command
Prompted by https://github.com/systemd/systemd/pull/22717#issuecomment-
1067348496.
The new command 'udevadm wait' waits for device or device symlink being
created. This may be useful to wait for a device is processed by udevd
after e.g. formatting or partitioning the device.
Yu Watanabe [Mon, 28 Mar 2022 18:57:49 +0000 (03:57 +0900)]
test: add more tests for sd_device_new_from_xxx()
Yu Watanabe [Sun, 27 Mar 2022 14:38:36 +0000 (23:38 +0900)]
sd-device: introduce sd_device_new_from_devname()
and sd_device_new_from_path() which takes devname or syspath.
Lennart Poettering [Wed, 30 Mar 2022 14:11:28 +0000 (16:11 +0200)]
systemctl: show tainted state
Lennart Poettering [Wed, 30 Mar 2022 08:46:16 +0000 (10:46 +0200)]
pid1: check for kernels older than baseline
Let's make this detectable explicitly.
Luca Boccassi [Thu, 31 Mar 2022 21:11:03 +0000 (22:11 +0100)]
Merge pull request #22923 from poettering/userns-check-refactor
virt: minor running_in_userns() modernizations
Lennart Poettering [Thu, 31 Mar 2022 14:50:37 +0000 (16:50 +0200)]
Merge pull request #22919 from poettering/cryptsetup-tweaks
various minor tweaks to cryptsetup/veritysetup/integritysetup
Lennart Poettering [Thu, 31 Mar 2022 11:28:18 +0000 (13:28 +0200)]
virt: use read_virtual_file() for reading /proc/self/setgroups
Lennart Poettering [Thu, 31 Mar 2022 11:27:21 +0000 (13:27 +0200)]
virt: simplify userns_has_mapping() by using fscanf() instead of scanf()
And while we are at it, also fix propagation of an uninitialized errno
error.
Antonio Alvarez Feijoo [Thu, 31 Mar 2022 08:09:29 +0000 (10:09 +0200)]
cryptsetup: fall back to traditional unlocking if any TPM2 operation fails
If any TPM2 operation fails, the boot process should continue and
prompt for a text password (if configured to do so).
Fixes #22870
Lennart Poettering [Thu, 31 Mar 2022 12:27:13 +0000 (14:27 +0200)]
update TODO
Martin Liska [Thu, 31 Mar 2022 08:27:45 +0000 (10:27 +0200)]
Support -D_FORTIFY_SOURCE=3 by using __builtin_dynamic_object_size.
As explained in the issue, -D_FORTIFY_SOURCE=3 requires usage
of __builtin_dynamic_object_size in MALLOC_SIZEOF_SAFE macro.
Fixes: #22801
Franck Bui [Thu, 31 Mar 2022 09:17:10 +0000 (11:17 +0200)]
meson: build kernel-install man page when necessary
Sebastian Pucilowski [Thu, 31 Mar 2022 05:31:28 +0000 (16:31 +1100)]
Fix "link-local" language inconsistencies
"Link-local" and "link local" are used throughout man pages and program
output, with the former used far more than the latter. This commit makes
it consistent throughout the project.
Lennart Poettering [Thu, 31 Mar 2022 09:22:07 +0000 (11:22 +0200)]
integritysetup: also validate volume name
Exactly like for veritysetup/cryptsetup
Lennart Poettering [Thu, 31 Mar 2022 09:21:37 +0000 (11:21 +0200)]
integritysetup: also port to mangle_none()
Let's make the tool work more like veritysetup/cryptsetup in this regard
too.
Lennart Poettering [Thu, 31 Mar 2022 09:20:25 +0000 (11:20 +0200)]
integritysetup: log when attempted to detach already detached volume
To make the tool behave more like cryptsetup/veritysetup
Lennart Poettering [Thu, 31 Mar 2022 09:20:01 +0000 (11:20 +0200)]
integritysetup: rename action → verb, to match other code
Lennart Poettering [Thu, 31 Mar 2022 09:09:48 +0000 (11:09 +0200)]
tree-wide: unify some code that looks for --help in the command line
Lennart Poettering [Thu, 31 Mar 2022 09:03:06 +0000 (11:03 +0200)]
veritysetup: do some superficial checking on volume name
cryptsetup does this too, so let's better be safe here, too.
Lennart Poettering [Thu, 31 Mar 2022 09:01:52 +0000 (11:01 +0200)]
veritysetup: mangle option strings like in cryptsetup
Lennart Poettering [Thu, 31 Mar 2022 08:52:50 +0000 (10:52 +0200)]
veritysetup: give command line parameters proper names
Accessing the various arguments always through argv[] is nasty, since
it's not obvious what we are talking about here. Let's give things nice
names.
We did the same in cryptsetup a while back.
Lennart Poettering [Thu, 31 Mar 2022 08:49:30 +0000 (10:49 +0200)]
cryptsetup: adjust some log levels
Let's upgrade log levels of some noteworthy messages from LOG_DEBUG to
LOG_NOTICE. These messages contain information that previous log
messages in the error path didn't say, namely that we'll now fall back
to traditional unlocking.
Note that this leaves similar log messages for cases where
TPM2/PKCS#11/FIDO2 support is disabled at build at LOG_DEBUG, since in
that case nothing really failed, we just systematically can't do
TPM2/PKCS#11/FIDO2 and hence it is pointless and not actionable for
users to do anything about it...
Lennart Poettering [Thu, 31 Mar 2022 08:48:37 +0000 (10:48 +0200)]
cryptsetup: add helper for mangling "none" option strings
let's unify some code here, and let's do so in cryptsetup-util.h so that
we can later reuse this in integritysetup/veritysetup
Lennart Poettering [Thu, 31 Mar 2022 08:47:24 +0000 (10:47 +0200)]
cryptsetup: rename functions that try to do FIDO2/TPM2/PKCS#11 via cryptsetup plugins to say so
The are so many different flavours of functions that attach volumes,
hence say explicitly that these are about libcryptsetup plugins, and
nothing else.
Just some renaming, no code changes beyond that.
Yu Watanabe [Thu, 31 Mar 2022 02:40:30 +0000 (11:40 +0900)]
Merge pull request #22899 from yuwata/network-ignore-carrier-loss
network: automatically determine timeout of waiting for carrier regain
Yu Watanabe [Tue, 29 Mar 2022 16:04:26 +0000 (01:04 +0900)]
network: shorten code a bit
Currently, there exist only two MTU sources, static and DHCPv4, and they
are exclusive. Hence, it is not necessary to check the existence of the
MTU option in the acquired DHCP lease. Let's unconditionally reset the
MTU. Note that, if the current and original MTU are equivalent, then
link_request_to_set_mtu() handles that gracefully.
Yu Watanabe [Tue, 29 Mar 2022 15:52:09 +0000 (00:52 +0900)]
network: automatically determine timeout of waiting for carrier regain
The commit
6706ce2fd2a13df0ae5e469b72d688eaf643dac4 made
IgnoreCarrierLoss= setting also take timespan, to make users handle
issues like #18738 or #20887. But still users needed to explicitly set
a timespan.
This makes networkd automatically determine the timeout when the
situations #18738 or #19832 is detected. Unfortunately, still users have
issue #20887 need to specify a value.
Closes #19832.
Yu Watanabe [Thu, 31 Mar 2022 00:24:38 +0000 (09:24 +0900)]
Merge pull request #22913 from yuwata/sd-device-cleanups
sd-device,udev: several cleanups
Zbigniew Jędrzejewski-Szmek [Wed, 30 Mar 2022 07:38:33 +0000 (09:38 +0200)]
veritysetup: fix parsing of root-hash-signature= option
The function was named confusingly and we managed to confused ourselves. The
parameter was assigned incorrectly and then reassigned correctly in the caller.
Let's simplify the whole thing by just saving the optarg param.
I considered moving the unhexmemming and/or reading of the file to the parse
function, but decided against it. I think it's nicer to parse all options
before opening external files.
Yu Watanabe [Wed, 30 Mar 2022 19:31:46 +0000 (04:31 +0900)]
udev: rename functions to emphasize whole disk is locked
Yu Watanabe [Wed, 30 Mar 2022 19:26:22 +0000 (04:26 +0900)]
udev: ignore one more error in device_get_block_device()
Yu Watanabe [Wed, 30 Mar 2022 19:14:49 +0000 (04:14 +0900)]
sd-device: do not ignore critical errors in device_new_from_child()
Yu Watanabe [Wed, 30 Mar 2022 19:11:30 +0000 (04:11 +0900)]
sd-device: use path_extract_directory() at one more place
Yu Watanabe [Wed, 30 Mar 2022 18:27:17 +0000 (03:27 +0900)]
sd-device: try to get DISKSEQ from uevent file
Otherwise, if the sd-device object is created from e.g. syspath, then
sd_device_get_diskseq() returns -ENOENT.
Yu Watanabe [Mon, 28 Mar 2022 19:04:54 +0000 (04:04 +0900)]
sd-device: drop /sys/subsystem support
Follow-ups for
37cf83d9bfdd9f6859b6f2654d8ec3bbb17873b2.
Gaël PORTAY [Thu, 25 Feb 2021 18:02:26 +0000 (13:02 -0500)]
man: update root-hash-signature option with value
This documents two possible values expected by the option
root-hash-signature for veritytab and veritysetup-generator.
Yu Watanabe [Wed, 30 Mar 2022 17:04:44 +0000 (02:04 +0900)]
udev: do not use sd_event_source_disable_unref() at more places
Fixes a bug introduced by
9612da361a825d70a9fd392f3ee5a53bf8896887.
Follow-up for
f777e745a7966ea52ef29f9e4edfdd16874cfe86.
Yu Watanabe [Wed, 30 Mar 2022 09:39:50 +0000 (18:39 +0900)]
udev: do not append unknown errno or signal name
Follow-up for
6467bda59d571696b645e8bbdf31926676890956.
Addresses https://github.com/systemd/systemd/pull/22871#discussion_r837705779.
Lennart Poettering [Wed, 30 Mar 2022 08:45:31 +0000 (10:45 +0200)]
update TODO
Frantisek Sumsal [Wed, 30 Mar 2022 09:32:31 +0000 (11:32 +0200)]
ci: drop clang 11 & add clang 14
Yu Watanabe [Wed, 30 Mar 2022 12:10:06 +0000 (21:10 +0900)]
fix typo
Luca Boccassi [Tue, 29 Mar 2022 20:54:01 +0000 (21:54 +0100)]
NEWS: specify that public headers are still C89
Luca Boccassi [Tue, 29 Mar 2022 20:52:21 +0000 (21:52 +0100)]
NEWS: mention that C11 is now used
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 18:28:47 +0000 (20:28 +0200)]
NEWS: add entry for the unit enablement stuff
It should be merged soon.
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 20:39:08 +0000 (22:39 +0200)]
test-systemctl-enable: skip test for %v if kver is not a valid instance
On arm, we'd fail with:
target@v:5.16.8-200.fc35.armv7hl+lpae.socket: not a valid unit name "target@v:5.16.8-200.fc35.armv7hl+lpae.socket": Invalid argument
наб [Tue, 29 Mar 2022 17:43:01 +0000 (19:43 +0200)]
test-copy: use non-0 data block in copy_holes
Some filesystems (e.g. zfs with compression!=off, which is the default
configuration) automatically hole-punch all-zero blocks ‒ write a block
full of ones instead
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 19:10:03 +0000 (21:10 +0200)]
Merge pull request #22649 from keszybz/symlink-enablement-yet-again-punish-me-harder
Fixups to the unit enablement logic
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 19:08:44 +0000 (21:08 +0200)]
Merge pull request #22871 from yuwata/udev-worker-error-code
udev: append error code on failure
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 17:46:47 +0000 (19:46 +0200)]
meson: bump numbers for v251-rc1
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 17:43:54 +0000 (19:43 +0200)]
NEWS: update contributor list
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 17:42:01 +0000 (19:42 +0200)]
tools/git-contrib: list contributions not only from Weblate
It seems that --invert-grep used to affect --author, but now it doesn't (with
git-2.35.1-1.fc36.x86_64), so effectively we would only show the one entry that
was supposed to be filtered out.
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 17:36:29 +0000 (19:36 +0200)]
NEWS: two more small features and some rewordings
наб [Tue, 29 Mar 2022 16:32:07 +0000 (18:32 +0200)]
Fix typos in user-util.c and dbus-unit.c
Yu Watanabe [Tue, 29 Mar 2022 05:58:58 +0000 (14:58 +0900)]
test: add tests for worker error code
Yu Watanabe [Fri, 25 Mar 2022 17:01:42 +0000 (02:01 +0900)]
udev: append error code in broadcasted message
So, libudev listners can easily detect errors.
Addresses https://github.com/systemd/systemd/pull/22717#discussion_r834420878.
Yu Watanabe [Fri, 25 Mar 2022 17:00:02 +0000 (02:00 +0900)]
sd-device: introduce device_add_propertyf()
Luca Boccassi [Tue, 29 Mar 2022 16:22:08 +0000 (17:22 +0100)]
NEWS: mention kernel requirement change 3.13 -> 3.15
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 16:17:32 +0000 (18:17 +0200)]
Merge pull request #22894 from keszybz/hwdb-update
Update hwdb again
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 10:22:21 +0000 (12:22 +0200)]
hwdb: update autosuspend entries
Zbigniew Jędrzejewski-Szmek [Thu, 9 Dec 2021 10:05:15 +0000 (11:05 +0100)]
hwdb: update for v251
As usual, there are mostly additions of new entries, and some spelling
correction and company renames, no big removals.
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 10:17:51 +0000 (12:17 +0200)]
hwdb: fix parser to work with newer pyparsing
The handling of whitespace in pyparsing is a bother. There's some
global state, and per-element state, and it's hard to get a handle on
things. With python3-pyparsing-2.4.7-10.fc36.noarch the grammar would
not match. After handling of tabs was fixed to not accept duplicate tabs,
the grammar passes.
It seems that the entry for usb:v8087p8087*
was generated incorrectly because we treated the interface line
(with two TABs) as a device line (with one TAB).
Zbigniew Jędrzejewski-Szmek [Fri, 25 Mar 2022 09:54:39 +0000 (10:54 +0100)]
cryptsetup: shorten message a bit
If it is reported as missing, we don't need to say that we assume
it is missing.
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 16:11:32 +0000 (18:11 +0200)]
Merge pull request #22843 from poettering/bootspec-json
bootctl: bootspec improvements and clean-ups
Zbigniew Jędrzejewski-Szmek [Tue, 29 Mar 2022 13:55:59 +0000 (15:55 +0200)]
Rename UnitFileScope to LookupScope
As suggested in
https://github.com/systemd/systemd/pull/22649/commits/
8b3ad3983f5440eef812b34e5ed862ca59fdf7f7#r837345892
The define is generalized and moved to path-lookup.h, where it seems to fit
better. This allows a recursive include to be removed and in general makes
things simpler.
Zbigniew Jędrzejewski-Szmek [Mon, 28 Mar 2022 18:20:09 +0000 (20:20 +0200)]
test-systemctl-enable: disable the test for %a for now
Zbigniew Jędrzejewski-Szmek [Mon, 28 Mar 2022 18:03:37 +0000 (20:03 +0200)]
test-systemctl-enable: also use freshly-built systemd-id128
Tests were failing on centos7 because systemd-id128 is not in path.
Zbigniew Jędrzejewski-Szmek [Fri, 25 Mar 2022 14:56:16 +0000 (15:56 +0100)]
test-systemctl-enable: use magic syntax to allow inverted tests
Inspired by
7910ec3bcde2ee0086b3e49f8aaa2a9f13f58d97.
'! true' passes, because it's a conditional expression.
But '( ! true )' fails, because '( … )' creates a subshell, i.e. a separate
program, and '! true' becomes the return value of that program, and the whole
thing apparently is not a conditional expression for the outer shell.
This is shorter, so let's just do this.
Zbigniew Jędrzejewski-Szmek [Fri, 25 Mar 2022 14:43:27 +0000 (15:43 +0100)]
shared/install: when creating symlinks, accept different but equivalent symlinks
We would only accept "identical" links, but having e.g. a symlink
/usr/lib/systemd/system/foo-alias.service → /usr/lib/systemd/system/foo.service
when we're trying to create /usr/lib/systemd/system/foo-alias.service →
./foo.service is OK. This fixes an issue found in ubuntuautopkg package
installation, where we'd fail when enabling systemd-resolved.service, because
the existing alias was absolute, and (with the recent patches) we were trying
to create a relative one.
A test is added.
(For .wants/.requires symlinks we were already doing OK. A test is also
added, to verify.)
Zbigniew Jędrzejewski-Szmek [Thu, 24 Mar 2022 10:52:35 +0000 (11:52 +0100)]
test-systemctl-enable: make shellcheck happy
Quoting is not necessary in many places, but I think it's nicer
to use it consistently.
Zbigniew Jędrzejewski-Szmek [Thu, 17 Mar 2022 15:02:10 +0000 (16:02 +0100)]
shared/install: fix handling of a linked unit file
When we have a symlink that goes outside of our search path, we should just
ignore the target file name. But we were verifying it, and rejecting in
the case where a symlink was created manually.
Zbigniew Jędrzejewski-Szmek [Thu, 17 Mar 2022 14:50:16 +0000 (15:50 +0100)]
shared/install: split UNIT_FILE_SYMLINK into two states
The two states are distinguished, but are treated everywhere identically,
so there is no difference in behaviour except for slighlty different log
output.
Zbigniew Jędrzejewski-Szmek [Thu, 17 Mar 2022 10:46:03 +0000 (11:46 +0100)]
basic/unit-file: reverse negative conditional
Having the reverse condition first makes changes that I want to do
later awkward, so reverse it as a separate step first.
Zbigniew Jędrzejewski-Szmek [Thu, 17 Mar 2022 09:16:30 +0000 (10:16 +0100)]
shared/install: stop passing duplicate root argument to install_name_printf()
All callers were just passing info + info->root, we can simplify this.
Zbigniew Jędrzejewski-Szmek [Wed, 16 Mar 2022 16:37:58 +0000 (17:37 +0100)]
shared/install: when looking for symlinks in .wants/.requires, ignore symlink target
We'd say that file is enabled indirectly if we had a symlink like:
foo@.service ← bar.target.wants/foo@one.service
but not when we had
foo@one.service ← bar.target.wants/foo@one.service
The effect of both link types is the same. In fact we don't care
about the symlink target. (We'll warn if it is mismatched, but we honour
it anyway.)
So let's use the original match logic only for aliases.
For .wants/.requires we instead look for a matching source name,
or a source name that matches after stripping of instance.
Zbigniew Jędrzejewski-Szmek [Wed, 16 Mar 2022 09:17:32 +0000 (10:17 +0100)]
shared/install: create relative symlinks for enablement and aliasing
This is a fairly noticable change, but I think it needs to be done.
So far we'd create an absolute symlink to the target unit file:
.wants/foo.service → /usr/lib/systemd/system/foo.service
or
alias.service → /etc/systemd/system/aliased.service.
This works reasonably well, except in one case: where the unit file
is linked. When we look at a file link, the name of the physical file
isn't used, and we only take the account the symlink source name.
(In fact, the destination filename may not even be a well-formed unit name,
so we couldn't use it, even if we wanted to.) But this means that if
a file is linked, and specifies aliases, we'd create absolute links for
those aliases, and systemd would consider each "alias" to be a separate
unit. This isn't checked by the tests here, because we don't have a running
systemd instance, but it is easy enough to check manually.
The most reasonable way to fix this is to create relative links to the
unit file:
.wants/foo.service → ../foo.service
alias.service → aliased.service.
I opted to use no prefix for aliases, both normal and 'default.target',
and to add "../" for .wants/ and .requires/. Note that the link that is
created doesn't necessarily point to the file. E.g. if we're enabling
a file under /usr/lib/systemd/system, and create a symlink in /etc/systemd/system,
it'll still be "../foo.service", not "../../usr/lib/systemd/system/foo.service".
For our unit loading logic this doesn't matter, and figuring out a path
that actually leads somewhere would be more work. Since the user is allowed
to move the unit file, or add a new unit file in a different location, and
we don't actually follow the symlink, I think it's OK to create a dangling
symlink. The prefix of "../" is useful to give a hint that the link points
to files that are conceptually "one level up" in the directory hierarchy.
With the relative symlinks, systemd knows that those are aliases.
The tests are adjusted to use the new forms. There were a few tests that
weren't really testing something useful: 'test -e x' fails if 'x' is a
a dangling symlink. Absolute links in the chroot would be dangling, even
though the target existed in the expected path, but become non-dangling
when made relative and the test fails.
This should be described in NEWS, but I'm not adding that here, because
it'd likely result in conflicts.
Zbigniew Jędrzejewski-Szmek [Wed, 16 Mar 2022 08:51:24 +0000 (09:51 +0100)]
shared/install: also remove symlinks like .wants/foo@one.service → ../foo@one.service
So far 'systemctl enable' would create absolute links to the target template
name. And we would remove such symlinks just fine. But the user may create
symlinks manually in a different form. In particular, symlinks for instanced
units *must* have the instance in the source name, and then it is natural to
also include it in the target name (.wants/foo@one.service → ../foo@one.service
rather than .wants/foo@one.service → ../foo@.service). We would choke on such
links, or not remove them at all. A test is added:
before:
+ build-rawhide/systemctl --root=/tmp/systemctl-test.001xda disable templ1@.service
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@seven.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@six.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@five.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@four.service".
Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@three.service".
Failed to disable unit, refusing to operate on linked unit file /tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@two.service.
Failed to disable unit, refusing to operate on linked unit file /tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@two.service.
after:
+ build-rawhide/systemctl --root=/tmp/systemctl-test.QVP0ev disable templ1@.service
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@seven.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@six.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@five.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@four.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@three.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@two.service".
Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@one.service".
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@one.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@two.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@three.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@four.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@five.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@six.service
+ test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@seven.service
Zbigniew Jędrzejewski-Szmek [Wed, 16 Mar 2022 08:28:46 +0000 (09:28 +0100)]
shared/install: skip unnecessary chasing of symlinks in disable
We use the symlink source name and destination names to decide whether to remove
the symlink. But if the source name is enough to decide to remove the symlink,
we'd still look up the destination for no good reason. This is a slow operation,
let's skip it.
Zbigniew Jędrzejewski-Szmek [Tue, 15 Mar 2022 16:45:34 +0000 (17:45 +0100)]
test-systemctl-enable: enhance the test for unit file linking
Current behaviour is wrong, but it cannot be shown in this test, because we
don't have a running systemd instance here.
Zbigniew Jędrzejewski-Szmek [Tue, 15 Mar 2022 15:35:47 +0000 (16:35 +0100)]
shared/install: do not try to resolve symlinks outside of root directory
I linked a file as root, so I had a symlink /root/test.service ← /etc/systemd/system/test.service.
To my surpise, when running test-systemctl-enable, it failed with a cryptic EACCES.
The previous commit made the logs a bit better. Strace shows that we
were trying to follow the symlink without taking --root into account.
It seems that this bug was introduced in
66a19d85a533b15ed32f4066ec880b5a8c06babd:
before it, we'd do readlink_malloc(), which returned a path relative to root. But
we only used that path for checking if the path is in remove_symlinks_to set, which
contains relative paths. So if the path was relative, we'd get a false-negative
answer, but we didn't go outside of the root. (We need to canonicalize the symlink
to get a consistent answer.) But after 66a19 we use chase_symlinks(), without taking
root into account which is completely bogus.
Zbigniew Jędrzejewski-Szmek [Wed, 2 Mar 2022 15:53:54 +0000 (16:53 +0100)]
shared/install: when we fail to chase a symlink, show some logs
When chase_symlinks() fails, we'd get the generic error:
Failed to disable: Permission denied.
Let's at least add the failure to changes list, so the user gets
a slightly better message. Ideally, we'd say where exactly the permission
failure occured, but chase_symlinks() is a library level function and I don't
think we should add logging there. The output looks like this now:
Failed to resolve symlink "/tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias2.service": Permission denied
Failed to resolve symlink "/tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias.service": Permission denied
Failed to disable unit, file /tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias2.service: Permission denied.
Failed to disable unit, file /tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias.service: Permission denied.
Zbigniew Jędrzejewski-Szmek [Tue, 15 Mar 2022 09:13:18 +0000 (10:13 +0100)]
test-systemctl-enable: extend the test for repeated WantedBy/RequiredBy
I was considering deduplicating the list of target units in
WantedBy/RequiredBy. But to do this meaningfully, we'd need to do alias
expansion first, i.e. after the initial parsing is done. This seems to be
more trouble than it would be worth.
Instead, I added tests that we're doing the right thing and creating symlinks
as expected. For duplicate links, we create the link, and on the second time we
see that the link is already there, so the output is correct.
Zbigniew Jędrzejewski-Szmek [Tue, 15 Mar 2022 08:44:39 +0000 (09:44 +0100)]
shared/install: fix reenable on linked unit files
Zbigniew Jędrzejewski-Szmek [Mon, 14 Mar 2022 11:09:31 +0000 (12:09 +0100)]
shared/install: split unit_file_{disable,enable}() so _reenable doesn't do setup twice
It was pretty ugly that we were creating LookupPaths twice.
Zbigniew Jędrzejewski-Szmek [Fri, 11 Mar 2022 13:27:46 +0000 (14:27 +0100)]
install: when linking a file, create the link first or abort
We'd create aliases and other symlinks first, and only then try to create
the main link. Since that can fail, let's do things in opposite order, and
abort immediately if we can't link the file itself.
Zbigniew Jędrzejewski-Szmek [Thu, 10 Mar 2022 20:33:25 +0000 (21:33 +0100)]
man: fix invalid description of template handling in WantedBy=
We don't need to talk about Alias=. The approach of using Alias= to enable
units is still supported, but hasn't been advertised as the way to do thing
for many years. Using it as an explanation is just confusing.
Also, the description of templated units did not take DefaultInstance=
into account. It is updated and extended.
Zbigniew Jędrzejewski-Szmek [Thu, 10 Mar 2022 19:26:59 +0000 (20:26 +0100)]
shared/install: also check for self-aliases during installation and ignore them
We had a check that was done in unit_file_resolve_symlink(). Let's move
the check to unit_validate_alias_symlink_or_warn(), which makes it available
to the code in install.c.
With this, unit_file_resolve_symlink() behaves almost the same. The warning
about "suspicious symlink" is done a bit later. I think this should be OK.
Zbigniew Jędrzejewski-Szmek [Thu, 10 Mar 2022 15:47:51 +0000 (16:47 +0100)]
systemctl: fix silent failure when --root is not found
Some calls to lookup_path_init() were not followed by any log emission.
E.g.:
$ SYSTEMD_LOG_LEVEL=debug systemctl --root=/missing enable unit; echo $?
1
Let's add a helper function and use it in various places.
$ SYSTEMD_LOG_LEVEL=debug build/systemctl --root=/missing enable unit; echo $?
Failed to initialize unit search paths for root directory /missing: No such file or directory
1
$ SYSTEMCTL_SKIP_SYSV=1 build/systemctl --root=/missing enable unit; echo $?
Failed to initialize unit search paths for root directory /missing: No such file or directory
Failed to enable: No such file or directory.
1
The repeated error in the second case is not very nice, but this is a niche
case and I don't think it's worth the trouble to trying to avoid it.
Zbigniew Jędrzejewski-Szmek [Thu, 10 Mar 2022 14:47:12 +0000 (15:47 +0100)]
shared/install: return failure when enablement fails, but process as much as possible
So far we'd issue a warning (before this series, just in the logs on the server
side, and before this commit, on stderr on the caller's side), but return
success. It seems that successfull return was introduced by mistake in
aa0f357fd833feecbea6c3e9be80b643e433bced (my fault :( ), which was supposed to
be a refactoring without a functional change. I think it's better to fail,
because if enablement fails, the user will most likely want to diagnose the
issue.
Note that we still do partial enablement, as far as that is possible. So if
e.g. we have [Install] Alias=foo.service foobar, we'll create the symlink
'foo.service', but not 'foobar', since that's not a valid unit name. We'll
print info about the action taken, and about 'foobar' being invalid, and return
failure.
Zbigniew Jędrzejewski-Szmek [Thu, 10 Mar 2022 10:03:41 +0000 (11:03 +0100)]
shared/install: propagate errors about invalid aliases and such too
If an invalid arg appears in [Install] Alias=, WantedBy=, RequiredBy=,
we'd warn in the logs, but not propagate this information to the caller,
and in particular not over dbus. But if we call "systemctl enable" on a
unit, and the config if invalid, this information is quite important.
Zbigniew Jędrzejewski-Szmek [Thu, 10 Mar 2022 08:19:37 +0000 (09:19 +0100)]
shared/install: simplify unit_file_dump_changes()
No functional change.
Zbigniew Jędrzejewski-Szmek [Wed, 9 Mar 2022 21:29:19 +0000 (22:29 +0100)]
shared/specifier: fix %u/%U/%g/%G when called as unprivileged user
We would resolve those specifiers to the calling user/group. This is mostly OK
when done in the manager, because the manager generally operates as root
in system mode, and a non-root in user mode. It would still be wrong if
called with --test though. But in systemctl, this would be generally wrong,
since we can call 'systemctl --system' as a normal user, either for testing
or even for actual operation with '--root=…'.
When operating in --global mode, %u/%U/%g/%G should return an error.
The information whether we're operating in system mode, user mode, or global
mode is passed as the data pointer to specifier_group_name(), specifier_user_name(),
specifier_group_id(), specifier_user_id(). We can't use userdata, because
it's already used for other things.
Zbigniew Jędrzejewski-Szmek [Wed, 9 Mar 2022 16:51:36 +0000 (17:51 +0100)]
shared/install: move scope into InstallContext
This makes it easier to pass it around in preparation for future changes.
While at it, let's rename InstallContext c → ctx, and InstallInfo i → info.
'c' and 'i' are bad names for variables that are passed through multiple layers
of functions calls. It's easier to follow what is happening with a meaningful
variable names.
Zbigniew Jędrzejewski-Szmek [Wed, 9 Mar 2022 15:06:24 +0000 (16:06 +0100)]
shared/install: provide proper error messages when invalid specifiers are used
$ build/systemctl --root=/tmp/systemctl-test.KXY8fu enable some-some-link6@.socket
Failed to enable unit, invalid specifier in "target@C:%C.socket".
Zbigniew Jędrzejewski-Szmek [Tue, 8 Mar 2022 11:08:00 +0000 (12:08 +0100)]
shared/specifier: provide proper error messages when specifiers fail to read files
ENOENT is easily confused with the file that we're working on not being
present, e.g. when the file contains %o or something else that requires
os-release to be present. Let's use -EUNATCH instead to reduce that chances of
confusion if the context of the error is lost.
And once we have pinpointed the reason, let's provide a proper error message:
+ build/systemctl --root=/tmp/systemctl-test.TO7Mcb enable some-some-link6@.socket
/tmp/systemctl-test.TO7Mcb/etc/systemd/system/some-some-link6@.socket: Failed to resolve alias "target@A:%A.socket": Protocol driver not attached
Failed to enable unit, cannot resolve specifiers in "target@A:%A.socket".
Zbigniew Jędrzejewski-Szmek [Tue, 8 Mar 2022 10:38:46 +0000 (11:38 +0100)]
shared/specifier: clarify and add test for missing data
In systemd.unit we document that unset fields resolve to "". But we didn't
directly test this, so let's do that. Also, we return -ENOENT if the file
is missing, which we didn't document or test.
Zbigniew Jędrzejewski-Szmek [Tue, 8 Mar 2022 09:10:12 +0000 (10:10 +0100)]
man/os-release: add a note about repeating entries
We didn't actually say that keys should not be repeated. At least the
examples in docs (both python and shell) would do that, and any simple
parser that builds a dictionary would most likely behave the same way.
But let's document this expectation, but also say how to deal with malformed
files.
Zbigniew Jędrzejewski-Szmek [Tue, 8 Mar 2022 09:08:05 +0000 (10:08 +0100)]
basic/env-file: make load-env-file deduplicate entries with the same key
We generally assume parsing like the shell would do it, so the last value
should win when there are repeats.
Zbigniew Jędrzejewski-Szmek [Mon, 7 Mar 2022 18:22:01 +0000 (19:22 +0100)]
test-os-util: add basic tests for os-release parsing
Zbigniew Jędrzejewski-Szmek [Mon, 7 Mar 2022 17:54:50 +0000 (18:54 +0100)]
basic: add new variable $SYSTEMD_OS_RELEASE to override location of os-release
The test for the variable is added in test-systemctl-enable because there we
can do it almost for free, and the variable is most likely to be used with
'systemctl enable --root' anyway.