So here's something we should always keep in mind:
systemd-udevd actually does *two* things with BSD file locks on block
devices:
1. While it probes a device it takes a LOCK_SH lock. Thus everyone else
taking a LOCK_EX lock will temporarily block udev from probing
devices, which is good when making changes to it.
2. Whenever a device is closed after write (detected via inotify), udevd
will issue BLKRRPART (requesting the kernel to reread the partition
table). It does this while holding a LOCK_EX lock on the block
device. Thus anyone else taking LOCK_SH or LOCK_EX will temporarily
block udevd from issuing that ioctl. And that's quite relevant, since
the kernel will temporarily flush out all partitions while re-reading
the partition table and then create them anew. Thus it is smart to
take LOCK_SH when dissecting a block device to ensure that no
BLKRRPART is issued in the background, until we mounted the devices.
A reattach might go from img.raw to img_0.1.raw or viceversa, but this is
not allowed right now as we try to match the full name.
Also take into account that running strcspn(a, '/') on an image name, without
leading path, will return the length of the full string, but the versions
might be different so they won't match, eg:
img_0.1.raw -> 12
img_0.1.1.raw -> 14
So adjust the check to take that into account, and skip it if we are not
dealing with directories
Same idea as 03677889f0.
No functional change intended. The type of the iterator is generally changed to
be 'const char*' instead of 'char*'. Despite the type commonly used, modifying
the string was not allowed.
I adjusted the naming of some short variables for clarity and reduced the scope
of some variable declarations in code that was being touched anyway.
The complaint was that the output array was used for two kinds of data, and the
input flag decided whether this extra data should be included. The flag is
removed, and instead the old method is changed to include the data always as
a separate parameter.
This breaks backward compatibility, but the old method is effectively broken
and does not appear to be used yet, at least in open source code, by
searching on codesearch.debian.net and github.com.
Fixes#22404.
Co-authored-by: Luca Boccassi <bluca@debian.org>
The approach to use '''…'''.split() instead of a list of strings was initially
used when converting from automake because it allowed identical blocks of lines
to be used for both, making the conversion easier.
But over the years we have been using normal lists more and more, especially
when there were just a few filenames listed. This converts the rest.
No functional change.
This should make things a bit more robust since it ensures system
extension can only applied to the right environments. Right now three
different "scopes" are defined:
1. "system" (for regular OS systems, after the initrd transition)
2. "initrd" (for sysext images that apply to the initrd environment)
3. "portable" (for sysext images that apply to portable images)
If not specified we imply a default of "system portable", i.e. any image
where the field is not specified is implicitly OK for application to OS
images and for portable services – but not for initrds.
(Or when -H is used, since -H and -M are incompatible.)
Note that the slightly unusual form with separate boolean variables (hint_vars,
hint_addr) instead of e.g. a const char* variable to hold the message, because this
way we don't trigger the warning about non-literal format.
Most sd_notify() calls are like log_info() — the result is only informative
and if they fail, it's best ignore this. But if a call with READY=1 fails,
the unit may enter a failed state, so we should warn about this. Similarly
for FSTOREREMOVE=1: the manager may be left with a stale fd, at least wasting
resources.
Let's define two helpers strdupa_safe() + strndupa_safe() which do the
same as their non-safe counterparts, except that they abort if called
with allocations larger than ALLOCA_MAX.
This should ensure that all our alloca() based allocations are subject
to this limit.
afaics glibc offers three alloca() based APIs: alloca() itself,
strndupa() + strdupa(). With this we have now replacements for all of
them, that take the limit into account.
Units are copied out via sendmsg datafd from images, but that means
the SELinux labels get lost in transit. Extract them and copy them over.
Given recvmsg cannot use multiple IOV transparently when the sizes are
variable, use a '\0' as a separator between the filename and the label.
When attaching a portable service with extensions, immediately validate
that the os-release and extension-release metadata values match, rather
than letting it fail when the units are started
In general we almost never hit those asserts in production code, so users see
them very rarely, if ever. But either way, we just need something that users
can pass to the developers.
We have quite a few of those asserts, and some have fairly nice messages, but
many are like "WTF?" or "???" or "unexpected something". The error that is
printed includes the file location, and function name. In almost all functions
there's at most one assert, so the function name alone is enough to identify
the failure for a developer. So we don't get much extra from the message, and
we might just as well drop them.
Dropping them makes our code a tiny bit smaller, and most importantly, improves
development experience by making it easy to insert such an assert in the code
without thinking how to phrase the argument.
DISKSEQ is a reliable way to find out if we missed a uevent or not, as
it's monotonically increasing. If we parse an event with a smaller or
no sequence number, we know we need to wait longer. If we parse an
event with a greater sequence number, we know we missed it and the
device was reused.
fd_duplicate_data_fd() is renamed to copy_data_fd(). This makes
the two functions have nicely similar names.
Now fd-util.[ch] is again about low-level file descriptor manipulations.
copy_data_fd() is a complex function that internally wraps the other
functions in copy.c. I want to move copy.c and the whole cluster of
related code from basic/ to shared/ later on, and this is a preparatory
step for that.
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.)
This tries to shorten the race of device reuse a bit more: let's ignore
udev database entries that are older than the time where we started to
use a loopback device.
This doesn't fix the whole loopback device raciness mess, but it makes
the race window a bit shorter.
Let's drop all monitor uevent that were enqueued before we actually
started setting up the device.
This doesn't fix the race, but it makes the race window smaller: since
we cannot determine the uevent seqnum and the loopback attachment
atomically, there's a tiny window where uevents might be generated by
the device which we mistake for being associated with out use of the
loopback device.
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.
