The basic idea is that --chain should stop option parsing. But
previously this didn't work, so --chain could be specified anywhere
in the command line. To maintain with compatibility with that,
allow --chain to be specified anywhere until the first positional
arg or option in the command string. This allows options to be passed
in the expected fashion:
userdbctl --chain ssh-authorized-keys user cmd --opt1 --opt2
userdbctl --chain ssh-authorized-keys user -- cmd --opt1 --opt2
but also allows the invocations which worked previously:
userdbctl ssh-authorized-keys user --chain cmd
userdbctl ssh-authorized-keys user cmd --chain
Fixes 8072a7e6a9.
The error messages are extended a bit. "binary path" is misleading:
we support all kinds of executables, not only compiled programs.
The situation with --chain is complicated. The old code tried to use "+…"
in getopt_long() to stop option parsing. But it didn't actually work.
This logic was originally added in 8072a7e6a9.
ef9c12b157 added an comment about 'optind=0'
which explains why the code doesn't work, but the code wasn't changed.
To wit:
$ userdbctl.old --no-pager --chain ssh-authorized-keys zbyszek -- /bin/echo --asdf
--asdf
$ userdbctl.old --no-pager --chain ssh-authorized-keys zbyszek /bin/echo -- --asdf
--asdf
$ userdbctl.old --no-pager --chain ssh-authorized-keys zbyszek /bin/echo --asdf
userdbctl.old: unrecognized option '--asdf'
(Basically, if "--" is used, it can be anywhere, since getopt_long() doesn't do
anything special after --chain and looks for the next option. There were some
tests of --chain, but they all used the username as the positional argument, so
it wasn't misinterpreted as an option.)
This behaviour is preserved in the conversion.
--help is generally the same except for expected formatting changes.
--json= is moved above between --output= and -j. For some reason it was
further down.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
parse_from_file doesn't set arg_from_file itself, but returns a
sd_json_variant ref to the caller. I think the change of arg_from_file
is more readable with this structure.
The helper would create a new ref, even though we had one handy
and didn't need to create a new ref. So change the helper to
take an existing reference.
We have a number of *_unref_and_replace macros. One could think that
they are like the various free_and_replace variants, but they actually
create a new ref to the passed object. The free_and_replace variants
take ownership of the argument. This inconsistency is surprising. Rename
all those functions to have "_new_ref" at the end to make the difference
clear.
Use the "resolvconf" namespace to keep these options separate from the
resolvectl/systemd-resolve sets.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Use OPTION_NAMESPACE() to keep the resolvectl and systemd-resolve
option sets separate. The resolvconf-compat path (resolvconf
invocation) keeps its own getopt-based parsing.
--help output has the expected changes to formatting. Synopis
for [status] is now shows that the verb is optional.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
The test used `timeout 30 bash -c "journalctl -b -u $unit -f | grep -m1 ..."`
to wait for systemd-networkd-wait-online to log that no DNS server is
accessible. The expected message is actually emitted ~1s after the unit
starts, but `grep -m1` exiting doesn't tear down `journalctl -f`: journalctl
only notices the closed pipe on its next write, which may never happen for
an otherwise idle unit. The pipeline therefore hangs until the 30s timeout
fires, eventually causing the test to fail.
Replace the follow+pipe with a polling `journalctl --grep` loop, which
exits cleanly as soon as the message lands in the journal.
Logs from the failing run:
[ 2650.871441] systemd-networkd-wait-online[2190]: dns0: No DNS configuration yet
[ 2651.723180] systemd-networkd-wait-online[2190]: dns0: No DNS server is accessible.
[ 2680.909048] systemd-networkd-wait-online[2190]: json-stream: Got POLLHUP from socket.
[ 2680.909092] systemd-networkd-wait-online[2190]: DNS configuration monitor disconnected, reconnecting...
[ 2680.914368] systemd-networkd-wait-online[2190]: Failed to connect to io.systemd.Resolve.Monitor: Connection refused
[ 2681.966674] systemd-networkd-wait-online[2190]: dns0: No DNS server is accessible.
[ 2681.969527] systemd-networkd-wait-online[2190]: Failed to connect to io.systemd.Resolve.Monitor: Connection refused
[ 2682.077032] systemd[1]: Stopping wait-online-dns-0f9e4f6d-8b34-4cff-b2da-03612ca731e8.service - [systemd-run] /usr/lib/systemd/systemd-networkd-wait-online --timeout=0 --dns --interface=dns0...
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Previously, we'd show a partial synopsis for systemd-mount
in --help for systemd-umount. I don't think it makes sense to do that.
So now the --help for systemd-umount is separate, with just its syntax
and a new blurb.
"transiently" is dropped from the description. Mount points generally
are transient, so no need to say that. (E.g. the man page for mount just
says "attach" and "detach".)
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
The order of options is changed (to what was present in parse_argv).
I don't the order in --help was mostly random, as is the new one,
so I didn't try to preserve the old order. Some help strings are
reworded/adjusted.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
In the man page and in the actual code, the first arg is
optional. But the arg limit in the verbs table did not allow
only one arg to be specified.
Fixes: 61d0578b07
get_firmware_search_dirs() previously hardcoded /usr/share/qemu/firmware
as the only system-wide search path. That assumption breaks on
distributions that deliberately do not populate /usr/share, making
vmspawn fail: "Failed to find OVMF config: No such file or directory".
NixOS exposes those firmware locations through XDG_DATA_DIRS.
Extend the search list with XDG_DATA_HOME/XDG_DATA_DIRS. This is the
standard XDG mechanism and is already what QEMU itself uses for the same
descriptors, so behavior matches user expectations across tooling.
To avoid regressing setups where user has set XDG_DATA_DIRS to a custom
value that omits /usr/share, keep /usr/share/qemu/firmware as an
unconditional fallback.
Precedence is unchanged: XDG_CONFIG_HOME/qemu/firmware still wins
over /etc/qemu/firmware, which still wins over any shared-data dir.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paul Meyer <katexochen0@gmail.com>
Let's limit the number of environment variables when creating a
transient unit via StartTransientUnit as well, since validating the
environment variable names/assignments is expensive.
Follow-up for 49c1e1bcf2.
bind_volume_parse() does not look at peers, so passing the same
PROVIDER:VOLUME twice on the command line silently produces two parsed
entries in arg_bind_volumes. vmspawn_bind_volume_acquire() then builds
two DriveInfo with identical d->id ("<provider>:<volume>"). At boot,
bridge_register_drive() puts d->id into the b->block_devices hashmap;
the second insert returns -EEXIST and the user sees a bare "File exists"
with no context for which volume is responsible.
Reject the collision at the parse site with a linear scan over the
existing array — n_items is small (one entry per --bind-volume on the
command line), and a clear error message naming the offending volume is
much more useful than the late EEXIST from the QMP setup loop.
bind_volume_parse() does not look at peers, so passing the same
PROVIDER:VOLUME twice on the command line silently produces two parsed
entries in arg_bind_volumes. vmspawn_bind_volume_acquire() then builds
two DriveInfo with identical d->id ("<provider>:<volume>"). At boot,
bridge_register_drive() puts d->id into the b->block_devices hashmap;
the second insert returns -EEXIST and the user sees a bare "File
exists" with no context for which volume is responsible.
Reject the collision at the parse site with a linear scan over the
existing array — n_items is small (one entry per --bind-volume on the
command line), and a clear error message naming the offending volume
is much more useful than the late EEXIST from the QMP setup loop.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
The more(1) pager (part of util-linux) now supports secure mode,
which can be enabled with the MORESECURE environment variable.
Adding support for more(1) in secure mode serves as an alternative
for systems that do not have less installed or for users who prefer
it.
Commit d2fce960f9cac740 introduced secure mode in more(1) and the
new feature is available in version 2.42 of util-linux.
s|attach/detach from the local system|in the local system|, because
"attach from" doesn't work.
The synopses for 'list' and 'set-limit' are fixed.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
The order in --help is changed to move have 'list', 'inspect' (query
operations), 'attach'/'detach'/'reattach' (main ops), and then the
other more specialized verbs.
The functions are then reordered to match this.
PR #41776 introduced the io.systemd.StorageProvider Varlink interface
and
two backends ('block' exposes host block devices, 'fs' exposes regular
files / dirs / subvolumes under /var/lib/storage), plus the
storagectl(1)
CLI to enumerate them. The only consumer so far was mount.storage. This
series wires up the first of the three integrations called out in
TODO.md:
systemd-vmspawn --bind-volume=PROVIDER:VOLUME[:CONFIG][:K=V,...]
Boot-time attach. Drives added this way are immutable at runtime.
io.systemd.MachineInstance.AddStorage / .RemoveStorage
Two new generic methods on the per-machine control socket. vmspawn
implements them (this series); systemd-nspawn will reuse the same
methods later.
machinectl bind-volume MACHINE PROVIDER:VOLUME[:CONFIG][:K=V,...]
machinectl unbind-volume MACHINE PROVIDER:VOLUME
Runtime hotplug front-end: machinectl Acquire()s the fd locally and
pushes it across to the target machine's MachineInstance socket.
Volumes are identified by a user-visible name "<provider>:<volume>"
(e.g.
"block:/dev/sda"). The 3rd 'config' field is opaque to the shared layer
and interpreted per backend — vmspawn maps it to a DiskType from
disk_type_table[] (virtio-blk default, virtio-scsi, nvme, scsi-cd; same
vocabulary as --extra-drive); future nspawn will read it as a mount
path.
- Document the new --bind-volume= option in systemd-vmspawn(1) and
the new bind-volume / unbind-volume verbs in machinectl(1).
- Add an integration test
(TEST-87-AUX-UTILS-VM.bind-volume.sh) covering boot-time attach
via --bind-volume, runtime attach via 'machinectl bind-volume',
runtime detach via 'machinectl unbind-volume', the StorageImmutable
rejection of attempts to detach boot-time volumes, and the
NoSuchStorage rejection of detach on unknown names.
- Strike "hook-up in systemd-vmspawn" from TODO.md; the nspawn and
service-manager hookups remain.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
machinectl bind-volume MACHINE PROVIDER:VOLUME[:CONFIG][:K=V,...]
machinectl unbind-volume MACHINE PROVIDER:VOLUME
For bind-volume, machinectl parses the SPEC with the shared
bind_volume_parse(), Acquires the storage volume from the named
provider on the machinectl side, locates the target machine's
io.systemd.MachineInstance control socket via
machine_get_control_address(), pushes the fd across, and calls
io.systemd.MachineInstance.AddStorage with name='<provider>:<volume>'
and the user-supplied config string.
For unbind-volume, machinectl just forwards the name string to
io.systemd.MachineInstance.RemoveStorage.
Volumes attached at machine startup (e.g. via systemd-vmspawn's
--bind-volume=) are rejected with StorageImmutable when the user
attempts to unbind them at runtime.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Wire up the runtime hotplug Varlink methods on the per-VM control
socket:
AddStorage → take fd from the link, look up the DiskType from the
'config' field, build a DriveInfo flagged
QMP_DRIVE_REMOVABLE, dispatch to
vmspawn_qmp_add_block_device(). Reply delivered async
by on_add_device_add_complete() once the guest sees
the device.
RemoveStorage → forward the user-visible name to
vmspawn_qmp_remove_block_device(); the existing
device_del / DEVICE_DELETED / blockdev-del chain
replies on the link.
Add SD_VARLINK_SERVER_ALLOW_FD_PASSING_INPUT to the server flags so
clients can push storage fds across via sd_varlink_push_fd().
Maps -EEXIST → StorageExists and -EOPNOTSUPP/-EINVAL →
ConfigNotSupported in the AddStorage handler so callers see the
specific MachineInstance errors.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
systemd-vmspawn --bind-volume=PROVIDER:VOLUME[:CONFIG][:K=V,...]
For each --bind-volume passed at startup, vmspawn calls Acquire() on
the named StorageProvider and attaches the resulting fd to the VM as
an additional drive. The drive is identified by the user-visible name
'<provider>:<volume>' on the bridge — that is also the handle used
later when machinectl unbind-volume detaches drives at runtime
(though boot-time drives like these are NOT removable; that is the
StorageImmutable behaviour added earlier).
The colon grammar is parsed by the shared bind_volume_parse() helper.
The 3rd 'config' field selects the guest device type from the
disk_type_table[] vocabulary (virtio-blk, virtio-scsi, nvme, scsi-cd);
empty defaults to virtio-blk per the TASK grammar.
Wiring lives next to the existing --extra-drive setup: parse_argv()
appends a parsed BindVolume to arg_bind_volumes, and prepare_device_info()
hands the array to vmspawn_bind_volume_prepare_boot() which Acquires
each volume and pushes a DriveInfo onto the existing drives array.
PCIe port assignment (assign_pcie_ports()) and the QMP setup loop pick
them up automatically.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
This is vmspawn's per-backend code for the StorageProvider integration.
Other backends (future systemd-nspawn, future service-manager
BindVolume=) consume the same shared parser and Acquire helper but
each provides its own attach/detach glue; this is vmspawn's.
- disk_type_from_bind_volume_config() turns the opaque BindVolume
'config' field (e.g. "scsi-cd") into a DiskType. Empty defaults to
virtio-blk to match the --bind-volume CLI grammar.
- vmspawn_bind_volume_acquire() takes a parsed BindVolume, calls
storage_acquire_volume() for the fd, and builds a DriveInfo ready
for vmspawn_qmp_setup_drives() (boot) or vmspawn_qmp_add_block_device()
(hotplug). Rejects directory-typed volumes (vmspawn block devices
need a regular file or a host block device).
- vmspawn_bind_volume_attach_fd() is the runtime path: takes a fd
that was already pushed across by an AddStorage caller plus the
name+config it specified, builds the DriveInfo with
QMP_DRIVE_REMOVABLE set and a varlink link, and dispatches to
vmspawn_qmp_add_block_device(). Reply is delivered asynchronously
by the existing on_add_device_add_complete() callback.
- vmspawn_bind_volume_prepare_boot() is a thin loop the boot-time
path uses to populate DriveInfos.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Drives attached at boot via the existing CLI options (--image,
--extra-drive) must not be detachable at runtime via the upcoming
RemoveStorage Varlink method, while drives added at runtime via
AddStorage must be. Track this distinction with a new QMP_DRIVE_REMOVABLE
property flag — placed alongside QMP_DRIVE_BLOCK_DEVICE, not in the
transient BlockDeviceStateFlags state-machine, since "may be removed"
is a permanent property of the drive.
vmspawn_qmp_remove_block_device() now early-rejects unknown ids with
io.systemd.MachineInstance.NoSuchStorage and immutable drives with
io.systemd.MachineInstance.StorageImmutable.
vmspawn_qmp_add_block_device() loses its 'static' qualifier and gets a
declaration in the header, so the runtime hotplug path
(vmspawn-bind-volume.c, next) can dispatch into it directly.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Define two new methods on the generic 'MachineInstance' Varlink
interface that systemd-vmspawn (this series) and (future)
systemd-nspawn implement on their per-machine control sockets:
AddStorage(fileDescriptorIndex, name, config?) -> ()
Attach a storage volume — the caller passes an fd previously
acquired from a StorageProvider, plus a unique name of the form
'<provider>:<volume>' that identifies this binding for later
removal, plus a backend-specific 'config' field (vmspawn: guest
device type; future nspawn: mount path).
RemoveStorage(name) -> ()
Detach a previously-added storage volume.
Plus errors NoSuchStorage, StorageExists, StorageImmutable (the volume
was attached at boot and cannot be removed), BadConfig, and
ConfigNotSupported. Names follow the io.systemd.StorageProvider
vocabulary (NoSuchVolume, BadTemplate, TypeNotSupported, etc.) so the
two interfaces are visually consistent.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Drop the inline socket-build + sd_varlink_callbo() + reply-dispatch
+ take_fd block from run_as_mount_helper() in favour of the shared
helper. Preserves the type-fallback retry (TypeNotSupported / WrongType
re-tries with requestAs="blk") and the per-error-id message mapping;
the helper just reports the io.systemd.StorageProvider.* error name
back to the caller.
Net effect: ~50 lines of dedup, no functional change.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
storagectl's mount.storage helper bundles "open StorageProvider socket
+ Acquire() + dispatch reply + take fd" inline. Future consumers
(systemd-vmspawn boot-time --bind-volume, machinectl bind-volume) need
the same dance.
Factor it into a single libshared helper that takes the Acquire()
parameters by value and returns the fd plus the actual type/read-only
flags. Library code, so no logging — varlink errors are surfaced via
sd_varlink_error_to_errno() and the StorageProvider error_id is
returned to the caller via reterr_error_id (caller decides how to
format messages).
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Add a universal parser for the colon-separated grammar
'PROVIDER:VOLUME[:CONFIG][:K=V,K=V,…]' that backs --bind-volume on
systemd-vmspawn (next), machinectl bind-volume, and the future nspawn
+ service-manager BindVolume= integrations.
The 'config' field is opaque to shared code and interpreted per
backend (vmspawn: a DiskType name, future nspawn: a mount path). The
trailing key=value list is parsed into the io.systemd.StorageProvider
.Acquire() parameters (template, create, read-only/ro, size/create-size
and request-as), with values validated against the existing
storage-util enums and validators. Provider/volume names are checked
with storage_provider_name_is_valid() and storage_volume_name_is_valid();
the combined "<provider>:<volume>" string is also validated as
string_is_safe so it is safe to use as a QEMU device id.
Add a test-machine-util unit test covering the happy paths plus a
handful of malformed inputs.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
The storage backend providers (block, fs) and storagectl currently each
extract storage-util.c into their target. Several upcoming consumers
(machine-util's BindVolume parser, vmspawn's hotplug glue, machinectl's
new bind-volume verbs) need the StorageProvider type/string-table
helpers and a future shared Acquire client helper.
Move storage-util.{c,h} to src/shared so libshared exports the symbols
once and every consumer (storage providers, storagectl, libshared
itself) picks them up by linking libshared. Drop the now-redundant
'extract'/'objects' wiring in src/storage/meson.build.
No code changes; this is purely a relocation.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
The mount.storage helper open-codes the conventional 64K UID/GID
delegation block size as 0x10000 / 0x10000U in four places. Several
other places in the tree do the same (nspawn's arg_uid_range default,
homed's mount setup, …), but with no shared name.
Add USERNS_RANGE_SIZE in user-util.h alongside UID_NOBODY and friends,
and switch storagectl over to it. Other call sites can adopt it
incrementally.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
In https://gcc.gnu.org/PR121911 `gcc` started enforcing the type of
`__stack_chk_guard` to `uintptr_t` and broke `systemd` build as:
```
../src/boot/efi-log.c:136:17: error: conflicting types for '__stack_chk_guard'; have 'intptr_t' {aka 'long int'}
136 | _used_ intptr_t __stack_chk_guard = (intptr_t) 0x70f6967de78acae3;
| ^~~~~~~~~~~~~~~~~
cc1: note: previous declaration of '__stack_chk_guard' with type 'long unsigned int'
../src/boot/efi-log.c:136:17: error: declaration of '__stack_chk_guard' shadows a global declaration [-Werror=shadow]
136 | _used_ intptr_t __stack_chk_guard = (intptr_t) 0x70f6967de78acae3;
| ^~~~~~~~~~~~~~~~~
```
Let's match the declaration to unsigned type as suggested by upstream in
https://gcc.gnu.org/PR121911#c6.
