The IO*Max=, IODeviceWeight= and IODeviceLatencyTargetSec= handlers
passed the property name and the path in the wrong order, resulting in
messages such as "Path 'IOReadBandwidthMax' specified in /var/= is not
normalized."
Treat any negative OCI CPU quota as unlimited, with or without a
period. Keep requiring quota and period together for finite quotas.
Cover negative quotas, missing quota or period, zero quota, and the
first quota value that would overflow the conversion.
This is similar to get_user_unit_name, but instead of returning just the
unit name it returns the full cgroup path up to the user unit.
This also refactors the existing get_unit_path helper, so that the bulk
of the logic can be shared. Additionally, the refactor means that we
only duplicate exactly as many bytes as we need, rather than duplicating
the entire cgroup path (which may be quite long!) and putting a
null-terminator in the middle of the memory allocation.
Split out from https://github.com/systemd/systemd/pull/43885
The Claude review agents navigate the systemd and kernel sources with
grep
and by reading whole files. semcode
(https://github.com/masoncl/semcode-devel)
indexes C code with tree-sitter and answers where a function is defined,
who
calls it and what a type looks like from the index, in one call, through
an
MCP server. This gives the agents semcode for the checkout and for the
kernel
tree, and makes them use it.
- A new semcode workflow builds semcode at a pinned commit of
semcode-devel
and indexes mainline once a week, both into the actions cache. A cold
build
takes about twenty minutes and the kernel index is about a gigabyte,
neither fits into a review. systemd uses no actions cache so far, so the
whole quota is available.
- The review job restores both, indexes the pull request (the head plus
the
range from the merge base, so lookups work before and after the change
and
per commit; about a minute on a runner) and gives every lens a systemd
and
a linux server. When there is a kernel index, the kernel is fetched at
the
indexed commit instead of the latest mainline.
- Everything semcode runs in CI runs under bubblewrap: the indexers with
only
the index writable, the servers read-only and without network, and no
server starts if bwrap doesn't work. semcode-mcp runs outside the Claude
Code sandbox, so this gives it one.
- A PreToolUse hook denies recursive grep over src/ and over the kernel
tree
and names the semcode tools instead. Single files, the rest of the
checkout, Documentation/, the other trees and "define" lookups stay
grep-able, since object-like macros aren't indexed.
- Each lens reports how many semcode calls succeeded and how many grep
commands it ran, so we can see whether the agents actually use it.
Nothing changes for reviews until the semcode workflow has run on main
once
and filled the cache. Until then the review runs as before.
We shouldn't tell users to fight with their distro. If there's a global
config in the distro that is not useful, it needs to be fixed there.
Partially reverts cec6bc4ea6.
This adds UAPI.16 support to systemd-sysupdate, systemd-importd,
systemd-repart. It also adds a new tool for generating and verifying
manifests.
It also adds support for additional matches in sysupdate transfer files,
against xattrs on files. This is then also replicated in vpick.
Fixes: #44448Fixes: #38772Fixes: #32819Fixes: #35571Fixes: #38774Fixes: #37931
With the indexes in place, a recursive grep over src/ or the kernel
tree is the expensive way to answer what the semcode tools answer in
one call, and left to themselves the agents keep reaching for grep.
So let's deny these searches in a PreToolUse hook that names the tools
to use instead: a Grep call on the checkout or its src/, or on the
kernel tree outside Documentation/, and a Bash command that runs grep,
rg, ag, ack or git grep there, judged by the operands of that command
alone, relative to a cd before it or the directory of git -C. A
recursive search without operands counts for its directory unless it
reads a pipe. Only the trees whose server runs count. Single files
stay searchable, and so do the parts of the checkout outside src/, the
other trees, $INPUT, searches restricted to other file types by
--include, --glob or the glob and type parameters, and anything with
define in it, since object-like macros are not indexed.
The kernel tree is there so the agents can check kernel behavior
instead of relying on memory, but a question like which error codes an
ioctl can return still means grepping through ninety thousand files.
Indexing the kernel takes too long to do per review, so let's do it in
the semcode workflow once a week and cache the result next to the
binaries. Two analysis threads keep the indexer below nine gigabytes,
which a runner has; with four it peaks above eleven. The index is about
a gigabyte and tied to the semcode that wrote it, so it is looked up
under the commit of the restored binaries.
The index only answers for the commit it was built from. So when there
is one, the review fetches the kernel at that commit, which mainline
keeps, instead of cloning the latest. The fetch has depth two: at
startup semcode-mcp compares HEAD with its parent to see that the index
covers HEAD, and without the parent it checks every file instead. If
the fetch fails, the latest mainline is cloned as before and the index
stays unused. The agents get a second server, linux, next to systemd.
Index the pull request before the review and hand the index to the
agents through semcode's MCP server. The first indexing run covers the
files at the head. The second covers the range from the merge base,
which adds the versions of the changed files at every commit of the
pull request and at the merge base, and the commits themselves. The
agents can then look at a function before and after the pull request
with git_sha, and at the commits with find_commit. On a runner this
takes about a minute. A review without the index is still a review, so
a failed indexing run only logs.
semcode-index parses the pull request and semcode-mcp runs outside the
Claude Code sandbox, so both get a bubblewrap sandbox of their own:
every file read-only but the index, nothing under /run, no network,
none of the job's environment, gone with the agent. A read-only bind
does not stop a connect, and the docker socket lives in /run. Each
agent gets a server of its own, and no server runs when the sandbox
does not work or the server does not answer a probe, so the agents are
never pointed at tools that do not exist. The index is complete for
the commit the checkout is at, so the indexing pass semcode-mcp runs
at startup has nothing to write. The server files live in
$RUNNER_TEMP, which the agent sandbox denies writes to, so a sandboxed
shell cannot swap the server.
Each call to a server has a time limit of two minutes. Without one,
Claude Code waits half an hour for a stdio server, longer than the
agent has, and a single runaway query would cost the whole lens. The
server is not told and finishes the query anyway, but it answers
requests concurrently, so the next call does not queue behind it.
A lens whose every semcode call failed is treated like one whose every
Bash call failed: its result is dropped. Calls a hook denied and calls
that ran into the time limit are left out of that. The numbers of
semcode calls, timeouts, grep calls and hook denials are printed for
each lens, so we can see how much searching still bypasses the index.
The review agents find their way through the sources with grep and by
reading whole files. Each question about the code, where a function is
defined, who calls it, what a type looks like, costs them several tool
calls and the tokens of everything grep turns up on the way. semcode
indexes C code with tree-sitter and answers exactly these questions
from the index, in one call, through an MCP server.
A cold build of semcode takes about twenty minutes on a runner, far
more than a review may spend on it. So let's build it in a workflow of
its own and keep the stripped binaries in the actions cache, from where
the review can restore them in seconds. A commit of semcode-devel is
pinned and is part of the cache key, so bumping it rebuilds. For now
that is upstream main plus the pull request that makes semcode-mcp
answer requests concurrently, from the fork the pull request comes
from. The build fetches that commit directly, so it does not depend on
a branch of the fork still pointing at it. A cache nobody restored for
a week is evicted, so the workflow also runs weekly to cover quiet
weeks.
This was tested on OVHCloud. Unfortunately, Nova provides most of the
information (such as IP v6) as a json file with a specific API, so we
rely on the EC2 compat API.
On OVH, the public IP v4 is not in public-ipv4, but in local-ipv4, and
there is no /1.0/meta-data/public-ipv4 URL. From what I know, this is
openstack installation dependant, but public-ipv4 is the documented
place where it should be.
Let's add a boolean MatchXAttr= setting to the [Source] and [Target]
sections of transfer files that allows restricting individual instances
to specific systems, via extended attributes attached to them. If
enabled, the following extended attributes are checked:
user.uapi.match_hostname → the hostname
user.uapi.match_machine_id → the machine ID
user.uapi.match_machine_tags → the machine tags (TAGS= in machine-info)
user.uapi.match_image_id → IMAGE_ID= from os-release
user.uapi.match_image_version → IMAGE_VERSION= from os-release
user.uapi.match_os_id → ID= from os-release
user.uapi.match_os_version → VERSION_ID= from os-release
In the [Source] section it is enabled by default, so that whoever
publishes instances can restrict them to specific systems without every
transfer file having to opt in. In the [Target] section it is off by
default, i.e. installed instances are only filtered if requested.
The checks are applied to the extended attributes listed in UAPI.16
manifests (relevant for sources) as well as to the extended attributes
of files and directories found on disk (relevant for targets and local
sources). Instances that do not match are entirely ignored, i.e. they are
neither installed nor vacuumed. Since the extended attributes listed in
UAPI.16 manifests are applied to the installed instances, the
restrictions are retained on the target.
The local system properties (hostname, machine ID, machine tags and the
relevant os-release fields) are only read when they are needed for the
first time, i.e. when a candidate actually carries one of these
attributes. If --root=/--image= is used they are read from there.
The setting is also exposed as boolean matchXAttr field of transfers
defined via the io.systemd.SysUpdate.Provider Varlink interface.
UAPI.16 file objects may carry an "extendedAttributes" field. When
installing a file or directory from a source with a UAPI.16 manifest,
let's apply the extended attributes in the "user" namespace listed there
to the inode we create, so that they are retained on the target.
This is done on a best-effort basis: if the file system does not support
extended attributes, or setting them fails for other reasons, this is
logged but not fatal, the instance is fully usable without them.
UAPI.16 manifests carry their signatures inline, hence systemd-pull is
invoked with --verify=no for them, but so far nothing else verified the
signature either, i.e. with the default of Verify=yes unauthenticated
manifests were trusted.
Pass the X.509 certificates found in the VOA hierarchy (UAPI.11, role
"manifest", context "sysupdate") to uapi16_manifest_read(), so that the
manifest signature is verified while the manifest is parsed. If
verification is enabled and no certificates are found, the transfer is
refused.
So far we ignored the "contents" arrays of UAPI.16 file objects, and
always downloaded a file by its name relative to the source URL. Let's
make use of them: each entry is an alternative location of the file's
data, and we now try them in the listed order until one works out.
All three kinds of sources are supported: "file" entries name a file
relative to Path= (also if ManifestPath= is used, which only redirects
the acquisition of the manifest itself), "url" entries carry an absolute
URL, and "literal" entries carry the data inline. Sources may embed the
file in more data, in which case the range at the specified offset is
used and checked against the file's digest. Inline data is cut in memory
and handed to systemd-import via a memory fd as standard input.
Downloads are passed to systemd-pull with --source-offset= and
--source-size=, so that for unencoded sources only the range making up
the file is downloaded. systemd-pull supports ranges for raw images
only, and we cannot cut into encoded inline data, hence such sources
must consist of the file and nothing else; sources that do not qualify
are ignored with a warning.
Everything is still run through systemd-pull or systemd-import, so that
decompression, sparse file handling and sandboxing work the same for
all sources. The encoding declared in the manifest is passed down to
the tools via --compression= (or "none" if none is declared), so that
they strictly decode with that algorithm rather than guessing from the
data.
If a file object carries no "contents" array, it is acquired by its
name as before. For UAPI.16 manifests it is taken as is (i.e. with
--compression=none), since the manifest declares encodings explicitly
via "contents"; for SHA256SUMS manifests the tools keep detecting the
compression on their own.
Fixes: #38772Fixes: #32819Fixes: #35571Fixes: #38774Fixes: #37931
(Of course, these issues are only fixed if people use UAPI.16 instead of
SHA256SUMS, but I think this is an OK requirement to make)
The "literal" field of a contents object may carry empty data, which
encodes an empty file. iovec_is_set() returns false for that though, so
such a field was considered absent, and the check that "file", "url"
and "literal" are mutually exclusive did not refuse an empty "literal"
next to one of the others.
Add uapi16_contents_has_literal(), which checks whether the field was
set instead of whether it carries data, and use it for the check.
This places literal data in a sealed memfd and returns a read-only fd to
it, positioned at the beginning, so that it can be passed to other
processes, for example as standard input. It's a thin wrapper around
memfd_new_and_seal() plus a read-only reopen.
The final "seal and reopen read-only" step of memfd_clone_fd() is split
out into a helper, and copy_data_fd_full() now uses it too, so that its
memfd case returns an O_RDONLY fd like its temporary file cases already
do, instead of a sealed O_RDWR one.
This allows configuring a bond in 802.3ad mode to use active or passive
LACP. During LACP discovery, active LACP routinely transmits LACPDUs, while
passive LACP only sends LACPDUs in response to a partner's LACPDU.
This adds a few typed assertion helpers to src/shared/tests.h that print
where things differ on failure, instead of just aborting:
- ASSERT_MEMEQ() / ASSERT_MEMEQ_NN() for memory regions
- ASSERT_MEMEQ_BYTE() / ASSERT_MEMEQ_ZERO()
- ASSERT_IOVEC_EQ() / ASSERT_IOVW_EQ()
On mismatch they show a hexdump-style, side-by-side diff of the
operands. To back this, hexdump() is moved from basic/hexdecoct to a new
shared/hexdump (the EFI hexdump() in src/boot is renamed to hexdump16()
to avoid the name clash), and the dump helpers live next to it.
The rest is mechanical: existing ASSERT_TRUE(memcmp(...) == 0) and
similar patterns are switched to the new macros, and assert_se() in the
fuzzers (plus a couple of test files) is converted to the typed ASSERT_*
family.
No functional change to non-test code.
I'm tired of reviews that constantly point out to add `/*
parameter_name= */` comments to functions calls where `NULL`, `0`, or
`true`, or `false` are passed. Let's add automated checking for this so
people have a reliable way to test this and the CI warns about it.
This is #43725 again. GitHub closed it when its base branch was deleted
after the tree-wide comments in #44532 were merged.
Since #44532 went in 23 pull requests added 232 new bare literals. The
checker also got stricter in the meantime. It now checks calls through
the shims in `src/include/override`, literals that have a comment after
them and `/* name */` comments that lost their `=`. That's 342 arguments
on current main and they're all fixed up here, one commit per top-level
source directory like in #44532, before the CI step is switched on. The
fixups are comments only. The `fsmount()` shim now uses the parameter
names from fsmount(2), so its last argument is commented as `attr_flags`
instead of glibc's misleading `ms_flags`.
The checker needs libclang 19 or newer. On four CPUs the whole suite
takes about a minute and a half.
This makes them all PID-race-free (on new enough kernels), so we can
stop claiming in the man page that these are unsuitable for use in
security-sensitive contexts
This functionality is generally useful to external clients too: given a
pid/pidfd/peer, it's useful to find the cgroup path of the unit.
Especially since we're going to start tagging information about the unit
onto the cgroup xattrs
This is similar to get_user_unit_name, but instead of returning just the
unit name it returns the full cgroup path up to the user unit.
This also refactors the existing get_unit_path helper, so that the bulk
of the logic can be shared. Additionally, the refactor means that we
only duplicate exactly as many bytes as we need, rather than duplicating
the entire cgroup path (which may be quite long!) and putting a
null-terminator in the middle of the memory allocation.
The argument name in the .c file didn't match the name in the .h file,
which made me accidentally use the wrong name in the comment, which
caused lint failure and wasted CI resources. We should be consistent to
avoid this issue
The tree is clean now, keep it that way. Run the whole argument-comments
suite like the clang-tidy one. Checking only the lines a pull request
changed misses comments that go stale when a parameter is renamed,
since no caller's line changes then. On four CPUs the suite takes about
a minute and a half.
meson test reports a test that exits with 77 as skipped, so make sure
the libclang bindings are there first. Otherwise a tools tree without
them would pass the step without checking anything.
While at it, add the script to the mypy and tab checks.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
tools/check-argument-comments.py only takes the /* name= */ form that
docs/CODING_STYLE.md shows as naming a parameter, as clang-tidy's
bugprone-argument-comment does. Add the '=' these comments are missing.
Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
While at it, add the '=' that some of the existing comments are missing.
By hand, drop the /* uncompressed */ in dns_packet_read_rr() which only
repeats the new comment, and wrap the get_mempolicy() call in
numa-util.c that grew to 141 columns.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
tools/check-argument-comments.py only takes the /* name= */ form that
docs/CODING_STYLE.md shows as naming a parameter, as clang-tidy's
bugprone-argument-comment does. Add the '=' these comments are missing.
Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
While at it, add the '=' that some of the existing comments are missing.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
docs/CODING_STYLE.md asks for a /* name= */ comment whenever NULL or
another value meaning "unset" is passed to a function. Add it wherever
a bare NULL, 0, false or true is passed for a parameter with a telling
name. Mechanical, generated by tools/check-argument-comments.py --fix.
The fsmount() shim took its parameter names from glibc which calls the
last one ms_flags. That's misleading as it takes MOUNT_ATTR_* flags and
not MS_* flags. The kernel and fsmount(2) call it attr_flags. Use the
names from fsmount(2) and fix up the two comments in namespace.c that
called it mount_attrs and flags.
Lennart Poettering