We recently started making more use of malloc_usable_size() and rely on
it (see the string_erase() story). Given that we don't really support
sytems where malloc_usable_size() cannot be trusted beyond statistics
anyway, let's go fully in and rework GREEDY_REALLOC() on top of it:
instead of passing around and maintaining the currenly allocated size
everywhere, let's just derive it automatically from
malloc_usable_size().
I am mostly after this for the simplicity this brings. It also brings
minor efficiency improvements I guess, but things become so much nicer
to look at if we can avoid these allocation size variables everywhere.
Note that the malloc_usable_size() man page says relying on it wasn't
"good programming practice", but I think it does this for reasons that
don't apply here: the greedy realloc logic specifically doesn't rely on
the returned extra size, beyond the fact that it is equal or larger than
what was requested.
(This commit was supposed to be a quick patch btw, but apparently we use
the greedy realloc stuff quite a bit across the codebase, so this ends
up touching *a*lot* of code.)
The naming of variables is very inconsistent. I tried to use more
modern style naming (UNDERSCORED_TITLE_CASE), but I didn't change existing
names too much. Only SYSTEM_DATA_UNIT_PATH is renamed to SYSTEM_DATA_UNIT_DIR
to match SYSTEM_CONFIG_UNIT_DIR.
We usually call specifier_printf() and then check the validity of
the result. In many cases, validity checkers, e.g. path_is_valid(),
refuse too long strings. This makes specifier_printf() refuse such
long results earlier.
Moreover, unit_full_string() and description field in sysuser now
refuse results longer than LONG_LINE_MAX. config_parse() already
refuses the line longer than LONG_LINE_MAX. Hence, it should be ok
to set the same value as the maximum length of the resolved string.
The function returns non-negative UnitNameFlags on success, and negative
errno on error. In the past we kept the return type as int because of those
negative return values. But nowadays _UNIT_NAME_INVALID == -EINVAL. And if
we tried to actually return something that doesn't fit in the return type,
the compiler would throw an error. By changing to the "real" return type,
we allow the debugger to use symbolic representation for the variables.
Currently, if [Install] section contains WantedBy=target that doesn't exist,
systemd creates the symlinks anyway. That is just user-unfriendly.
Let's be nice and warn about installing non-existent targets.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1835351.
Replaces: #15834
With some versions of the compiler, the _cleanup_ attr makes it think
the variable might be freed/closed when uninitialized, even though it
cannot happen. The added cost is small enough to be worth the benefit,
and optimized builds will help reduce it even further.
After all we are only interested in symlinks either in top-level config
directory or in .wants and .requires sub-directories.
As a bonus this should speed up ListUnitFiles() roughly 3-4x on systems
with a lot of units that use drop-ins (e.g. SSH jump hosts with a lot of
user session scopes).
We almost never use the named enum type, in almost all cases we use
"int" instead, since we overload it with negative errnos. To simplify
things, let's use "int" really everywhere.
Moreover, let's rename the fields for this enum to "type_or_errno", to
make the overloading clear. And let's ad some assertions that things are
in the right range.
I started working on this because I wanted to change how
DEFINE_TRIVIAL_CLEANUP_FUNC is defined. Even independently of that change, it's
nice to make make things more consistent and predictable.
The new methods work as the unflavoured ones, but takes flags as a
single uint64_t DBUS parameters instead of different booleans, so
that it can be extended without breaking backward compatibility.
Add new flag to allow adding/removing symlinks in
[/etc|/run]/systemd/system.attached so that portable services
configuration files can be self-contained in those directories, without
affecting the system services directories.
Use the new methods and flags from portablectl --enable.
Useful in case /etc is read-only, with only the portable services
directories being mounted read-write.
This partially undoes the parent commit. We follow the symlink and
if it appears to be a symlink to /dev/null, even if /dev/null is not
present, we treat it as such. The addition of creation of /dev/null
in the test is reverted.
There's some inconsistency in the what is considered a masked unit:
some places (i.e. load-fragment.c) use `null_or_empty()` while others
check if the file path is symlinked to "/dev/null". Since the latter
doesn't account for things like non-absolute symlinks to "/dev/null",
this commit switches the check for "/dev/null" to use `null_or_empty_path()`
Six years ago we declared it obsolete and removed it from the docs
(c073a0c4a5) and added a note about it in
NEWS. Two years ago we add warning messages about it, indicating the
feature will be removed (41b283d0f1) and
mentioned it in NEWS again.
Let's now kill it for good.
This is a follow-up for 9f83091e3c.
Instead of reading the mtime off the configuration files after reading,
let's do so before reading, but with the fd we read the data from. This
is not only cleaner (as it allows us to save one stat()), but also has
the benefit that we'll detect changes that happen while we read the
files.
This also reworks unit file drop-ins to use the common code for
determining drop-in mtime, instead of reading system clock for that.
Units created with "systemctl edit --full" from generator units aren't
removed with "systemctl revert" because units in the generator,
generator_early, and generator_late paths aren't considered vendor
units.
Alter path_is_vendor() to return true if the given path is a
generator unit, which in turn causes "systemctl revert" to remove units
created with "systemctl edit --full" as they now have vendor versions.
Fixes#10053
For units which are aliases of other units, reporting preset status as
"enabled" is rather misleading. For example, dbus.service is an alias of
dbus-broker.service. In list-unit-files we'd show both as "enabled". In
particular, systemctl preset ignores aliases, so showing any preset status at
all is always going to be misleading. Let's introduce a new state "alias" and
use that for all aliases.
I was trying to avoid adding a new state, to keep compatibility with previous
behaviour, but for alias unit files it simply doesn't seem very useful to show
any of the existing states. It seems that the clearly showing that those are
aliases for other units will be easiest to understand for users.
When doing list-unit-files with --root, we would re-read the preset
list for every unit. This uses a cache to only do it once. The time
for list-unit-files goes down by about ~30%.
unit_file_query_preset() is also called from src/core/. This patch does not
touch that path, since the saving there are smaller, since preset status is
only read on demand over dbus, and caching would be more complicated.
If we have a template unit template@.service, it should be allowed to specify a
dependency on a unit without an instance, bar@.service. When the unit is created,
the instance will be propagated into the target, so template@inst.service will
depend on bar@inst.service.
This commit changes unit_dependency_name_compatible(), which makes the manager
accept links like that, and unit_file_verify_alias(), so that the installation
function will agree to create a symlink like that, and finally the tests are
adjusted to pass.
It might be needed to follow symlinks more deeply when we're looking for
enablement symlinks pointing to the removed service.
Let's consider the case where service 'old' is being renamed 'new' (will happen
most likely during package upgrade). Before the service is going to be renamed,
there's the following enablement symlink:
/etc/systemd/system/multi-user.target.wants/old.service -> /usr/lib/systemd/system/old.service
In order to rename 'old' into 'new' and transparently restart the service, the
old name is still provided as a 'static' alias for the new service. This should
also help keeping backward compatibilities since the old name might still be
embedded in unit files, scripts, generators and such.
Hence after the package is upgraded, the following symlinks including the
enablement symlink are present:
/usr/lib/systemd/system/old.service -> new.service
/etc/systemd/system/multi-user.target.wants/old.service -> /usr/lib/systemd/system/old.service
If later the user decides to disable the service, we should figure out that the
enablement symlink (which still has the old name) is actually referring to 'new'
(indirectly) even if it points to the alias.
This mostly reuses existing checkers used by pid1, so handling of aliases
should be consistent. Hopefully, with the test it'll be clearer what it
happening.
Support for .wants/.requires "aliases" is restored. Those are still used in the
wild quite a bit, so we need to support them.
See https://github.com/systemd/systemd/pull/13119 for a discussion of aliases
with an instance that point to a different template: this is allowed.
It turns out that this is not necessary. When we try to resolve alias@inst, we
first check alias@inst, and if that is not found, fall back to alias@. Since we
already created a symlink for alias@, we will find that and the result will be
the same.
Ideally, we would want to report this over back over dbus. But that is pretty hard,
because the unitfile parsing logic doesn't provide any feedback.
systemd-analyze verify also doesn't notice the issue, because it doesn't look
at the [Install] section at all. Let's print a message in the logs at least.
As in the previous commit, ignoring unkown sections means users may be confused
easily. It is better to warn about misspellt section names.
In this case, we are using a separate item table, so we'd ignore all those
sections anyway, so we could list them with out the minus prefixes and the
effect would be the same. But I think it's clearer to prefix them.
It is currently possible to override the DefaultInstance via drop-ins but
not remove it completely. Allow to do that by specifying an empty
DefaultInstance=
