Use the same ordering of the relevant fields as we do for the format
layer -blockdev so that later they can be refactored without test
fallout.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the return value type to 'virDomainDiskGetDetectZeroes'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
At this point only a single code path (for formatting -drive for legacy
SD cards) uses the 'legacy' output and that code path doesn't populate
the node name. Thus we can unify the code block and simplify the JSON
formatters.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'systemd-analyze security' command looks at the unit file
configuration and reports on any settings which increase the
attack surface for the daemon. Since most systemd units are
fairly minimalist, this is generally informing us about settings
that we never put any thought into using before.
In its current configuration it reports
# systemd-analyze security virtlogd.service
...snip...
→ Overall exposure level for virtlogd.service: 9.6 UNSAFE 😨
which is pretty terrible as a score.
If we apply all of the recommendations that appear possible
without (knowingly) breaking functionality it reports:
# systemd-analyze security virtlogd.service
...snip...
→ Overall exposure level for virtlogd.service: 2.2 OK 🙂
which is a pretty decent improvement.
Some of the settings we would like to enable require a systemd
version that is newer than that available in our oldest distro
target - RHEL-8 at v239.
NB, RestrictSUIDSGID is technically newer than 239, but RHEL-8
backported it, and other distros we target have it by default.
Remaining recommendations are
✗ CapabilityBoundingSet=~CAP_(DAC_*|FOWNER|IPC_OWNER)
We block FOWNER/IPC_OWNER, but can't block the two DAC
capabilities. Historically apps/users might point QEMU
to log files in $HOME, pre-created with their own user
ID.
✗ IPAddressDeny=
Not required since RestrictAddressFamilies blocks IP
usage. Ignoring this avoids the overhead of creating
a traffic filter than will never be used.
✗ NoNewPrivileges=
Highly desirable, but cannot enable it yet, because it
will block the ability to transition to the virtlogd_t
SELinux domain during execve. The SELinux policy needs
fixing to permit this transition under NNP first.
✗ PrivateTmp=
There is a decent chance people have VMs configured
with a serial port logfile pointing at /tmp. We would
cause a regression to use private /tmp for logging
✗ PrivateUsers=
This would put virtlogd inside a user namespace where
its root is in fact unprivileged. Same problem as the
User= setting below
✗ ProcSubset=
Libraries we link to might read certain non-PID related
files from /proc
✗ ProtectClock=
Requires v245
✗ ProtectHome=
Same problem as PrivateTmp=. There's a decent chance
that someone has a VM configured to write a logfile
to /home
✗ ProtectHostname=
Requires v241
✗ ProtectKernelLogs
Requires v244
✗ ProtectProc
Requires v247
✗ ProtectSystem=
We only set it to 'full', as 'strict' is not viable for
our required usage
✗ RootDirectory=/RootImage=
We are not capable of running inside a custom chroot
given needs to write log files to arbitrary places
✗ RestrictAddressFamilies=~AF_UNIX
We need AF_UNIX to communicate with other libvirt daemons
✗ SystemCallFilter=~@resources
We link to libvirt.so which links to libnuma.so which has
a constructor that calls set_mempolicy. This is highly
undesirable todo during a constructor.
✗ User=/DynamicUser=
This is highly desirable, but we currently read/write
logs as root, and directories we're told to write into
could be anywhere. So using a non-root user would have
a major risk of regressions for applications and also
have upgrade implications
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The hyperkitty search facility does a massively better job
than google docs for mailing lists.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Setup the VDPA bits of the appropriate part of the image chain for block
copy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code which opens the VDPA device and prepares it for FD passing was
not called in the hotplug code path, preventing hotplug of VDPA disks
with:
error: internal error: argument key 'path' must not have null value
Use the new helper qemuProcessPrepareHostStorageDisk to setup the VDPA
definition.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/539
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently the code sets up only VDPA backends but will be used later in
hotplug code too.
This patch also uses normal forward iteration in the loop in
qemuProcessPrepareHostStorage as we don't need to remove disks from the
disk list at that point.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the docs for the <acpi><table> element under a common section as
it's not specific for direct kernel boot. In fact the original use was
for Windows activation.
Fixes: 72f652da63
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Legacy mode used to be needed for use with -drive, which was almost
completely deleted. We now have qemuxml2argvtest test cases checking a
few cases and the rest uses the modern mode only. Thus we don't need to
test the legacy mode any more.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Condition handling failure of the first virTestRun was lacking the 'ret
= -1' line thus the subsequent line was taken as it's body rendering the
first invocation useless.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
QEMU deprecated and removed support for those protocols, but due to a
logic bug in the tests it was not caught. Remove the test cases first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Update to v8.1.0-2161-ga95260486a
Notable changes:
- new CPU topology parameters: 'books', 'drawers'
- new migration parameter 'avail-switchover-bandwidth'
- new s390x CPU properties: 'dedicated', 'entitlement'
- new 'hostmem' parameter for 'virtio-gpu'
- new device 'piix4-isa'
- new property 'dynamic-memslots' of 'virtio-mem' device
- new 'gsi*' parameters of 'ICH9-LPC'
- new experimental 'x-south-bridge' machine option
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Given that this variable now controls not just whether C tests
are built, but also whether any test at all is executed, the new
name is more appropriate.
Update the description for the corresponding meson option
accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Currently, passing -Dtests=disabled only disables a subset of
tests: those that are written in C and thus require compilation.
Other tests, such as the syntax-check ones and those that are
implemented as scripts, are always enabled.
There's a potentially dangerous consequence of this behavior:
when tests are disabled, 'meson test' will succeed as if they
had been enabled. No indication of this will be shown, so the
user will likely make the reasonable assumption that everything
is fine when in fact the significantly reduced coverage might
be hiding failures.
To solve this issues, disable *all* tests when asked to do so,
and inject an intentionally failing test to ensure that 'meson
test' doesn't succeed.
Best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It only makes sense to enable expensive tests when tests are
enabled. Disallow invalid configurations.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There are some cases in which we automatically disable tests when
using Clang as the compiler. If the user has explicitly asked for
tests to be enabled, however, we should error out instead of
silently disabling things.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This will make future patches nicer.
Note that we need to handle these somewhat late because of the
dependency on information about the compiler and the flags it
supports.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
As explained in the comment, the syntax-check machinery uses git
to figure out the list of files it should operate on, so we can
only enable it when building from git.
Despite only registering the various tests with meson in that
case, however, we unconditionally perform a bunch of preparation
that is only useful for the purpose of registering and running
the tests. If we're not going to do that, we can skip a few steps
and save a bit of time.
Best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Clang can be too aggressive at optimizations, which can end up
breaking our test suite. See f9f5ab5718 for details.
As a result of this, since 7944700b40 we are automatically
disabling tests when Clang is used unless it supports the
-fsemantic-interposition compiler flag.
Since the version of Clang included in macOS doesn't support that
compiler flag, we end up always disabling the test suite on that
platform.
This is already far from ideal, considering that it was just last
year when we finally managed to get the test suite to successfully
pass on macOS, and it would be a real shame if the situation
regressed again.
With the upcoming changes, which will turn running 'meson test'
into a hard failure if tests are disabled, this behavior will
result in every single pipeline failing.
Work around the problem the only way we can: disabling
optimizations entirely for the macOS CI jobs.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Test cases that depend on duplicating fds are using fairly big
values as targets.
This works fine on Linux, where RLIMIT_NOFILE is 1024 by
default, but fails on macOS which uses 256 as the default.
Decrease the values so that they're valid across all platforms.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Both tests pass a disk source definition which didn't go through the
preparation steps and thus contains only the target information that
were originally present, thus we should be using the
QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_TARGET_ONLY flag.
For the same reason QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_AUTO_READONLY
used in 'testJSONtoJSON' doesn't make sense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Return whether a relevant cachemode was presented rather than returning
an error, so that callers can be simplified. Use the proper enum type as
argument rather than typecasting in the switch statement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The aforementioned fields in virStorageSource struct are copies of the
disk properties, but were not converted to the proper type yet.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Certain disk config fields are mirrored between the disk and storage
source definitions, but the proper types are not available for use in
the virStorageSource definition. Move them so they can be used properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Formatting of the 'nbdkit' driven backend breaks out of the switch
statement so we don't need to have an unnecessary block and indentation
level for the case when nbdkit is not in use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuBuildDriveSourceStr' used to build the legacy -drive commandline
for SD cards is the only user of qemuDiskSourceGetProps. Move the helper
directly inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'auto-read-only' blockdev option is available in all supported qemu
versions so we can remove the migration hack which disabled it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a few examples of SD cards backed with network storage to capture
the current state as the formatter code is about to be refactored.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The situation is the same as Linux: since glibc no
longer includes the RPC functionality, libtirpc must
be used to complement it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We want at least one file to always be present, so that it can
serve as a pointer for users. Ensure that this is the case by
unconditionally using the value of the respective keys.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It's somewhat confusing that some of the services have a
corresponding foo.service.extra.in and foo.socket.extra.in, some
have just one of the two, and some have neither.
In order to make things more approachable, make sure that both
files exists for each service.
In most cases the extra units are currently unused, so they will
just contain a comment briefly explaining their purpose and
pointing users to meson.build, where they can find more
information. The same comment is also added to the top of
extra units that already have some contents in them for
consistency.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently the script will reject any type of contents outside
of a section, but we want to be able to have some useful
comments at the top of each file to help users understand how
they are processed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that the underlying script is able to merge an arbitrary
number of units into the base template, expose this possibility
in the build system.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In order to further deduplicate the contents of the various unit
files, we need to be able to merge multiple additional units
into the initial one.
Luckily the merge logic is in no way constrained to working with
just two units, so achieving this is pretty much just a matter
of lifting the existing limitation on the number of arguments
that the script accepts.
As a special case, it's now also possible to call the script
with just the base unit as argument. No merging will be performed
in that case, obviously, but we'll still go through the basic
validation and cleanup steps.
This also fixes a bug in the check for the number of arguments:
sys.argv also contains the name of the script, so we should have
checked that its size was at least 3. The check is now written in
a way that's less prone to misunderstandings.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It uses custom templates which already hardcode the correct
value.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
