Let's make umount_verbose() more like mount_verbose_xyz(), i.e. take log
level and flags param. In particular the latter matters, since we
typically don't actually want to follow symlinks when unmounting.
In 4c2ef32767 we enabled propagating
triggered unit state to the triggering unit for service units in more
load states, so that we don't accidentally stop tracking state
correctly.
Do the same for our other triggering unit states: automounts, paths, and
timers.
Also, make this an assertion rather than a simple test. After all it
should never happen that we get called for half-loaded units or units of
the wrong type. The load routines should already have made this
impossible.
Because this was left unset, the unit_write_setting() function was
refusing to write out the automount-specific TimeoutIdleSec= and
DirectoryMode= settings when creating transient automount units.
Set it to the proper value in line with other unit types.
Patch contains a coccinelle script, but it only works in some cases. Many
parts were converted by hand.
Note: I did not fix errors in return value handing. This will be done separate
to keep the patch comprehensible. No functional change is intended in this
patch.
In all the other cases, I think the code was clearer with the static table.
Here, not so much. And because of the existing dump code, the vtables cannot
be made static and need to remain exported. I still think it's worth to do the
change to have the cmdline introspection, but I'm disappointed with how this
came out.
First After=local-fs-pre.target wasn't described in the man page although it's
part of the default dependencies automatically set by pid1.
Secondly, Before=local-fs.target was only set if the automount unit was
generated from the fstab-generator because the dep was explicitly
generated. It was also not documented as a default dependency.
Fix it by managing the dep from pid1 instead.
Similar, refuse triggering deps on units that cannot trigger.
And rework how we ignore After= dependencies on device units, to work
the same way.
See: #14142
From the bug:
> According to the documentation of systemd.automount if the automoint point is
> automagically created if it doesn't exist yet. This ofcourse means the
> filesystem underneath has to be writable, which for / means not only does
> -.mount need to be started but also systemd-remount-fs.service has to be run,
> which isn't guaranteed by the default automount dependencies.
>
> For .mount units there is an automatic default After= dependency on
> local-fs-pre.target, would probably make sense to do the same for automount
> units to avoid it failing on the corner-case where it has to create directory.
Fixes#13306.
No functional change, just adjusting code to follow the same pattern
everywhere. In particular, never call _verify() on an already loaded unit,
but return early from the caller instead. This makes the code a bit easier
to follow.
unit_load_fragment_and_dropin() and unit_load_fragment_and_dropin_optional()
are really the same, with one minor difference in behaviour. Let's drop
the second function.
"_optional" in the name suggests that it's the "dropin" part that is optional.
(Which it is, but in this case, we mean the fragment to be optional.)
I think the new version with a flag is easier to understand.
mkdir -p is called both when setting up the autofs mount, as well
as after being notified that the real mount unit should be called.
However the first mkdir -p is hardcoded with 0555, while the second
uses the value specified to DirectoryMode in the automount unit; the
second mkdir -p is only needed when called from coldplug, so under
normal operation the dirs are incorrectly created with mode 0555.
This replaces the hardcoded 0555 mode with the value of DirectoryMode.
Closes#13683.
Just some renaming, no change in behaviour.
Background: I'd like to add more functions unit_test_xyz() that test
various things, hence let's streamline the naming a bit.
Creating a pipe with O_NONBLOCK causes both the read and the write end to
be marked as non-blocking.
The "write" end is passed to the kernel autofs module, and it does not
expect a non-blocking pipe. If it gets -EAGAIN when trying to write
(which is unlikely, but not completely impossible), it will close the
write end of the pipe, which leads to unexpected errors.
So change the code to only set O_NONBLOCK on the "read" end of the
pipe. This is the only end that systemd interacts with, so the only end
it should be configuring.
This allows clients to follow our internal state changes safely.
Previously, quick state changes (for example, when we restart a unit due
to Restart= after it quickly transitioned through DEAD/FAILED states)
would be coalesced into one bus signal event, with this change there's
the guarantee that all state changes after the unit was announced ones
are reflected on th bus.
Note we only do this kind of guaranteed flushing only for unit state
changes, not for other unit property changes, where clients still have
to expect coalescing. This is because the unit state is a very
important, high-level concept.
Fixes: #10185
Let's be more careful with what we serialize: let's ensure we never
serialize strings that are longer than LONG_LINE_MAX, so that we know we
can read them back with read_line(…, LONG_LINE_MAX, …) safely.
In order to implement this all serialization functions are move to
serialize.[ch], and internally will do line size checks. We'd rather
skip a serialization line (with a loud warning) than write an overly
long line out. Of course, this is just a second level protection, after
all the data we serialize shouldn't be this long in the first place.
While we are at it also clean up logging: while serializing make sure to
always log about errors immediately. Also, (void)ify all calls we don't
expect errors in (or catch errors as part of the general
fflush_and_check() at the end.
"ExitCode" is a bit of a misnomer in two ways: it suggests this was
about the "exit code" concept that exit()/waitid() deal with, but really
isn't. Moreover, it's not event just about exiting either, but more
often about reloading/reexecing or rebooting. Let's hence pick a new
name for this that is a bit more correct.
I initially thought about naming this the "state", but that'd be a
misnomer too, as the value really encodes a "goal" more than a current
state. Also we already have the externally visible ManagerState.
No actual changes in behaviour, just the rename.
These lines are generally out-of-date, incomplete and unnecessary. With
SPDX and git repository much more accurate and fine grained information
about licensing and authorship is available, hence let's drop the
per-file copyright notice. Of course, removing copyright lines of others
is problematic, hence this commit only removes my own lines and leaves
all others untouched. It might be nicer if sooner or later those could
go away too, making git the only and accurate source of authorship
information.
This part of the copyright blurb stems from the GPL use recommendations:
https://www.gnu.org/licenses/gpl-howto.en.html
The concept appears to originate in times where version control was per
file, instead of per tree, and was a way to glue the files together.
Ultimately, we nowadays don't live in that world anymore, and this
information is entirely useless anyway, as people are very welcome to
copy these files into any projects they like, and they shouldn't have to
change bits that are part of our copyright header for that.
hence, let's just get rid of this old cruft, and shorten our codebase a
bit.
This is mostly fall-out from d1a1f0aaf0,
however some cases are older bugs.
There might be more issues lurking, this was a simple grep for "%m"
across the tree, with all lines removed that mention "errno" at all.
The function is similar to path_kill_slashes() but also removes
initial './', trailing '/.', and '/./' in the path.
When the second argument of path_simplify() is false, then it
behaves as the same as path_kill_slashes(). Hence, this also
replaces path_kill_slashes() with path_simplify().
This adds a flags parameter to unit_notify() which can be used to pass
additional notification information to the function. We the make the old
reload_failure boolean parameter one of these flags, and then add a new
flag that let's unit_notify() if we are configured to restart the
service.
Note that this adjusts behaviour of systemd to match what the docs say.
Fixes: #8398
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
This reworks the SELinux and SMACK label fixing calls in a number of
ways:
1. The two separate boolean arguments of these functions are converted
into a flags type LabelFixFlags.
2. The operations are now implemented based on O_PATH. This should
resolve TTOCTTOU races between determining the label for the file
system object and applying it, as it it allows to pin the object
while we are operating on it.
3. When changing a label fails we'll query the label previously set, and
if matches what we want to set anyway we'll suppress the error.
Also, all calls to label_fix() are now (void)ified, when we ignore the
return values.
Fixes: #8566
"check" is unclear: what is true, what is false? Let's rename to "can_gc" and
revert the return value ("positive" values are easier to grok).
v2:
- rename from unit_can_gc to unit_may_gc
It was forbidden to create mount units for a symlink. But the reason is
that the mount unit needs to know the real path that will appear in
/proc/self/mountinfo. The kernel dereferences *all* the symlinks in the
path at mount time (I checked this with `mount -c` running under `strace`).
This will have no effect on most systems. As recommended by docs, most
systems use /etc/fstab, as opposed to native mount unit files.
fstab-generator dereferences symlinks for backwards compatibility.
A relatively minor issue regarding Time Of Check / Time Of Use also exists
here. I can't see how to get rid of it entirely. If we pass an absolute
path to mount, the racing process can replace it with a symlink. If we
chdir() to the mount point and pass ".", the racing process can move the
directory. The latter might potentially be nicer, except that it breaks
WorkingDirectory=.
I'm not saying the race is relevant to security - I just want to consider
how bad the effect is. Currently, it can make the mount unit active (and
hence the job return success), despite there never being a matching entry
in /proc/self/mountinfo. This wart will be removed in the next commit;
i.e. it will make the mount unit fail instead.
It is not necessary to label as loaded to automount unit when its unit
file does not exist. So, let's make automount_load() to fail when the
unit file does not exist.
Partially fixes#7370.
