Inspired by #23913, let's complain if people use paths with ".."
in Type #1 bootspec entries.
Let's prefix all paths with "/" if it is missing.
Let's simplify all paths.
let's refuse paths/warn with "..".
Fixes: #23913
So this is a bit of a bikeshedding thing. But I think we should do this
nonetheless, before this is released.
Playing around with the glob matches I realized that "=$" is really hard
to grep for, since in shell code it's an often seen construct. Also,
when reading code I often found myself thinking first that the "$"
belongs to the rvalue instead of the operator, in a variable expansion
scheme.
If we move the $ character to the left hand, I think we are on the safer
side, since usually lvalues are much more restricted in character sets
than rvalues (at least most programming languages do enforce limits on
the character set for identifiers).
It makes it much easier to grep for the new operator, and easier to read
too. Example:
before:
ConditionOSRelease=ID=$fedora-*
after:
ConditionOSRelease=ID$=fedora-*
The only reason to do this is to ensure uniformity with the other
options, that work like this, i.e. ConditionOSRelease= or
ConditionSecurity=.
This is a compatibility break, but a minor one, given that string
comparison and version comparison is mostly the same for equality and
inequality.
This ensures that "=" and "!=" are now interpreted as literal string
compares, and "==" and "<>" are for version compares.
This is not a compat break, since the SMBIOS stuff has not been included
in any release yet.
Main reason to do this, is to be systematic with the other conditions
that check for text stuff.
These two operators always indicate ordering comparisons, as opposed to
"=" and "!=" which depending on context mean literal string compares.
This is useful for ConditionOSRelease= for example, as this means
there's now always a way to do version compares.
Let's change the lookup table to contain pairs of operator/strings,
instead of being indexed by operator.
The table isn't dense anymore, and this allows us to add alias strings
sooner or later.
None of our other fnmatch() calls make use of this, and the concept was
new to me at least. Given that this is only used for the recently added
SMBIOS field matches (and is not included in any release) let's disable
"extended" matches for now. We can certainly revisit this, and enable it
later if there is real demand, but if we do, we should probably add that
all over the place, not just for smbios matches.
Let's move the operator enum into its own .c/.h file, so that we can
reuse it elsewhere, in particular systemd-analyze's compare-versions
logic.
Let's rename the concept CompareOperator, since it is nowadays
genericlaly about both order *and* fnmatch comparisons, hence just
naming it "order" is misleading.
This mirrors what we already do during allocation. We lock the control
device first, and then release the block device and then delete it.
This makes things substantially more robust as long all participants do
such locking: we won't attempt to delete a block device somebody else
already is using.
Back when I wrote this code I wasn't aware of BLKPG and what it can do.
Hence I came up with this hack to attach an empty file to delete all
partitions. But today we can do better with BLKPG: let's just explicitly
remove all partitions, and then try again.
Let's rework how we lock loopback block devices in two ways:
1. Lock a separate fd, instead of the main block device fd. We already
did that for our internal locking when allocating loopback block
devices, but do so for the exposed locking (i.e.
loop_device_flock()), too, so that the lock is independent of the
main fd we actually use of IO.
2. Instead of locking the device during allocation of the loopback
device, then unlocking it (which will make udev run), and then
re-locking things if we need, let's instead just keep the lock the
whole time, to make things a bit safer and faster, and not have to
wait for udev at all. This is done by adding a "lock_op" parameter to
loop device allocation functions that declares the initial state of
the lock, and is one of LOCK_UN/LOCK_SH/LOCK_EX. This change also
shortens a lot of code, since we allocate + immediately lock loopback
devices pretty much everywhere.
This reverts a major chunk of 75d7e04eb4
Now that the loopback device code already destroys the partitions we
don't have to do this here anymore.
I am sure the right place to delete the partitions is in the loopback
code, since we really only should do that for loopback devices, see
bug #24431, and not on "real" block devices.
I am also not convinced dropping partitions the dissection logic doesn't
care about is a good idea, after all. The dissection stuff should
probably not consider itself the "owner" of the block devices it
analyzes, but take a more passive role: figure out what is what, but not
modify it.
Fixes: #24431
Whenever we release a loopback device, let's first synchronously delete
all partitions, so that we know that's complete and not done
asynchronously in the background. Take a BSD lock on the device while
doing so, so that udev won't make the devices busy while we do this.
Often I end up debugging a problem on a system, and I
do e.g. `journalctl --grep=failed|error`. The use of the term
"failed" for condition checks adds a *lot* of unnecessary noise into
this.
Now, I know this regexp search isn't precise, but it has proven
to be useful to me.
I think "failed" is too strong of a term as a baseline, and also
just stands out to e.g. humans watching their servers boot or
whatever.
The term "met condition" is fairly widely used, e.g.
https://stackoverflow.com/questions/63751794/what-does-the-condition-is-met-exactly-mean-in-programming-languages
Use that instead.
Lennart Poettering
Johannes Schauer Marin Rodrigues
Luca Boccassi
Yu Watanabe
Antonio Alvarez Feijoo
dependabot[bot]
Colin Walters
Frantisek Sumsal
Jan Janssen