The --disk-password argument was present in early impls of the patch but
replaced by the more generic --inject-secret argument.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virNumaNodeIsAvailable function is stubbed out when building
without libnuma, such that it just returns a constant value. When
CLang is optimizing, it does inter-procedural analysis across
function calls. When it sees that the call to virNumaNodeIsAvailable
returns a fixed constant, it elides the conditional check for errors
in the callers such as virNumaNodesetIsAvailable.
This is a valid optimization as the C standard declares that there
must only be one implementation of each function in a binary. This
is normally the case, but ELF allows for function overrides when
linking or at runtime with LD_PRELOAD, which is technically outside
the mandated C language behaviour.
So while CLang's optimization works fine at runtime, it breaks in our
test suite which aims to mock the virNumaNodeIsAvailable function so
that it has specific semantics regardless of whether libnuma is built
or not. The return value check optimization though means our mock
override won't have the right effect. The mock will be invoked, but
its return value is not used.
Potentially the same problem could be exhibited with GCC if certain
combinations of optimizations are enabled, though thus far we've
not seen it.
To be robust on both CLang and GCC we need to make it more explicit
that we want to be able to replace functions and thus optimization
of calls must be limited. Currently we rely on 'noinline' which
does successfully prevent inlining of the function, but it cannot
stop the eliding of checks based on the constant return value.
Thus we need a bigger hammer.
There are a couple of options to disable this optimization:
* Annotate a symbol as 'weak'. This is tells the compiler
that the symbol is intended to be overridable at linktime
or runtime, and thus it will avoid doing inter-procedural
analysis for optimizations. This was tried previously but
have to be reverted as it had unintended consequences
when linking .a files into our final .so, resulting in all
the weak symbol impls being lost. See commit
407a281a8e
* Annotate a symbol with 'noipa'. This tells the compiler
to avoid inter-procedural analysis for calls to just this
function. This would be ideal match for our scenario, but
unfortunately it is only implemented for GCC currently:
https://reviews.llvm.org/D101011
* The '-fsemantic-interposition' argument tells the optimizer
that any functions may be replaced with alternative
implementations that have different semantics. It thus
blocks any optimizations across function calls. This is
quite a harsh block on the optimizer, but it appears to be
the only one that is viable with CLang.
Out of those choices option (3) is the only viable option for
CLang. We don't want todo it for GCC though as it is such a
big hammer. Probably we should apply (2) for GCC, should we
experiance a problem in future.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The offline validation example needs to include the firmware path,
and is also missing line continuation markers.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The RPM automatic deps generator for python does not pick these up
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The --loader syntax was left over from an earlier version of the code
before it was renamed to --firmware.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The CURLOPT_PUT constant causes a deprecation warning when compiling on
Alpine Edge. The docs indicate it is deprecated since 7.2.1
https://curl.se/libcurl/c/CURLOPT_PUT.html
Since 7.87 the deprecation is now exposed at build time via a compiler
warning.
We already use CURLOPT_UPLOAD in the ESX driver, so this brings the CH
driver into line.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This fixes a bug in
commit fda53ab3a5
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Thu Dec 22 10:29:32 2022 -0500
remote: use VIR_LOCK_GUARD in client code
Reviewed-by: Erik Skultety <eskultet@redhat>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Using VIR_LOCK_GUARD enables the 'done' goto label to be
eliminated.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Using VIR_LOCK_GUARD enables the 'done' goto label to be
eliminated.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Using VIR_LOCK_GUARD helps to simplify the control flow
logic.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Every enum/struct/union implicitly includes a typedef in the
emitted C code. Furthermore, the syntax used to declare the
redundant typedef is not compliant with the XDR spec.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The RFC spec for XDR does not allow enums to omit their
values, they must be explicitly given. Don't rely on this
rpcgen language extension.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Every member of the args variable will be initialized
explicitly. A few methods had a redundant call to memset
the args which can be removed.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remote_*_args methods will generally borrow pointers
passed in the caller, so should not be freed.
On failure of the virTypedParamsSerialize method, however,
xdr_free was being called. This is presumably because it
was thought that the params may have been partially
serialized and need cleaning up. This is incorrect, as
virTypedParamsSerialize takes care to cleanup partially
serialized data. This xdr_free call would lead to free'ing
the borrowed cookie pointers, which would be a double free.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
A few admin client methods had the xdr_free call the wrong
side of the cleanup label, so typed parameters would not
be freed on error.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since the takeover of the bird site, the bulk of tech people who want
a more friendly and inclusive media site have jumped over to Mastodon.
With its decentralized nature, there's no one replacement that captures
everything, but the fosstodon.org site is a topic relevant choice.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
While only a couple of the message types include sensitive data,
the overhead of calling secure erase is not noticable enough
to worry about making the erasure selective per type. Thus it is
simplest to unconditionally securely erase the buffer.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The buffer length refers to the allocated buffer memory size,
while the offset refers to have much of the buffer we have
read/written. After reading the message payload we must thus
update the latter.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is available on at least FreeBSD and GLibc >= 2.25.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We currently list the locale file paths via a wildcard in %files, but the
normal pattern for mingw RPMs in Fedora is to use %mingw_find_lang.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The BPF_CGROUP_DEVICE constant was introduced to Linux in
commit ebc614f687369f9df99828572b1d85a7c2de3d92
Author: Roman Gushchin <roman.gushchin@linux.dev>
Date: Sun Nov 5 08:15:32 2017 -0500
bpf, cgroup: implement eBPF-based device controller for cgroup v2
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The BPF_PROG_QUERY constant was introduced to Linux in
commit defd9c476fa6b01b4eb5450452bfd202138decb7
Author: Alexei Starovoitov <ast@kernel.org>
Date: Mon Oct 2 22:50:26 2017 -0700
libbpf: sync bpf.h
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The VHOST_VSOCK_SET_GUEST_CID constant was introduced to Linux in
commit 433fc58e6bf2c8bd97e57153ed28e64fd78207b8
Author: Asias He <asias@redhat.com>
Date: Thu Jul 28 15:36:34 2016 +0100
VSOCK: Introduce vhost_vsock.ko
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The linux/magic.h header has existed since
commit e18fa700c9a31360bc8f193aa543b7ef7b39a06b
Author: Jeff Garzik <jeff@garzik.org>
Date: Sun Sep 24 11:13:19 2006 -0400
Move several *_SUPER_MAGIC symbols to include/linux/magic.h.
This is old enough that all our supported platforms can be assumed
to have this header.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The DEVLINK_CMD_ESWITCH_GET constant was introduced to Linux in
commit adf200f31c000d707e4afe238ed1d1199e0cce7c
Author: Jiri Pirko <jiri@mellanox.com>
Date: Thu Feb 9 15:54:33 2017 +0100
devlink: fix the name of eswitch commands
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The headers required by virnetdevbridge.c have all exited since
before Linux moved to git. It is sufficient to check for just
one of them in order to give an error message about needing
kernel headers installed.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The GET_VLAN_VID_CMD constant has existed since before Linux moved
to git.
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETHTOOL_GCOALESCE constant has existed since before Linux moved
to git.
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETHTOOL_GFEATURES constant was introduced to Linux in
commit 5455c6998d34dc983a8693500e4dffefc3682dc5
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Tue Feb 15 16:59:17 2011 +0000
net: Introduce new feature setting ops
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETH_FLAG_RXHASH constant was introduced to Linux in
commit b00fabb4020d17bda4bea59507e09fadf573088d
Author: stephen hemminger <shemminger@vyatta.com>
Date: Mon Mar 29 14:47:27 2010 +0000
netdev: ethtool RXHASH flag
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETH_FLAG_NTUPLE constant was introduced to Linux in
commit 15682bc488d4af8c9bb998844a94281025e0a333
Author: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Date: Wed Feb 10 20:03:05 2010 -0800
ethtool: Introduce n-tuple filter programming support
This is old enough that all our supported platforms can be assumed
to have this feature.
A typo in the existing condition "NTUBLE" instead of "NTUPLE" meant the
code was never enabled in the first place, which is an illustration of
why it is worth eliminating redundant conditional checks.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETH_FLAG_TXVLAN/RXVLAN constants were introduced to Linux in
commit d5dbda23804156ae6f35025ade5307a49d1db6d7
Author: Jesse Gross <jesse@nicira.com>
Date: Wed Oct 20 13:56:07 2010 +0000
ethtool: Add support for vlan accleration.
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETH_FLAG_LRO constant was introduced to Linux in
commit 3ae7c0b2e3747b50c3a6c63ebb67469e0a6b3203
Author: Jeff Garzik <jeff@garzik.org>
Date: Wed Aug 15 16:00:51 2007 -0700
[ETHTOOL]: Add ETHTOOL_[GS]FLAGS sub-ioctls
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETHTOOL_GFLAGS constant was introduced to Linux in
commit 3ae7c0b2e3747b50c3a6c63ebb67469e0a6b3203
Author: Jeff Garzik <jeff@garzik.org>
Date: Wed Aug 15 16:00:51 2007 -0700
[ETHTOOL]: Add ETHTOOL_[GS]FLAGS sub-ioctls
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETHTOOL_GGRO constant was introduced to Linux in
commit b240a0e5644eb817c4a397098a40e1ad42a615bc
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon Dec 15 23:44:31 2008 -0800
ethtool: Add GGRO and SGRO ops
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The ETHTOOL_GGSO constant was introduced to Linux in
commit 37c3185a02d4b85fbe134bf5204535405dd2c957
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu Jun 22 03:07:29 2006 -0700
[NET]: Added GSO toggle
This is old enough that all our supported platforms can be assumed
to have this feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
ethtool is a Linux specific feature that has existed since before Linux
moved to git. Checking against SIOCETHTOOL + WITH_STRUCT_IFREQ is
overkill for our needs.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The unshare() syscall was introduced to Linux in
commit 2da436e00f9a5fdd0fb6b31e4b2b2ba82e8f5ab8
Author: JANAK DESAI <janak@us.ibm.com>
Date: Tue Feb 7 12:59:03 2006 -0800
[PATCH] unshare system call -v5: system call registration for i386
This is old enough that all our supported platforms can be assumed
to have this feature. Furthermore, the virprocess.c file was already
using unshare() with nothing more than a #ifdef __linux__ check.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The LO_FLAGS_AUTOCLEAR constant was introduced to Linux in
commit 96c5865559cee0f9cbc5173f3c949f6ce3525581
Author: David Woodhouse <dwmw2@infradead.org>
Date: Wed Feb 6 01:36:27 2008 -0800
Allow auto-destruction of loop devices
This is old enough that all our supported platforms can be assumed
to have this feature. For added fun this whole meson check was
semantically insane because EPOLL_CLOEXEC is not a valid arg
to unshare().
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The EPOLL_CLOEXEC constant was introduced to Linux in
commit a0998b50c3f0b8fdd265c63e0032f86ebe377dbf
Author: Ulrich Drepper <drepper@redhat.com>
Date: Wed Jul 23 21:29:27 2008 -0700
flag parameters: epoll_create
This is old enough that all our supported platforms can be assumed
to have this feature. For added fun this whole meson check was
semantically insane because EPOLL_CLOEXEC is not a valid arg
to unshare().
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The LOOP_CTL_GET_FREE constant was introduced to Linux in
commit 770fe30a46a12b6fb6b63fbe1737654d28e84844
Author: Kay Sievers <kay.sievers@vrfy.org>
Date: Sun Jul 31 22:08:04 2011 +0200
loop: add management interface for on-demand device allocation
This is old enough that all our supported platforms can be assumed
to have this feature. As a plus point, this meson check is going
to start failing with future GCC. It fails to set _GNU_SOURCE, thus
'unshare' is not defined by the header, and its relying on an
implicit function decl. For added fun this whole meson check was
semantically insane because LOOP_CTL_GET_FREE is not a valid arg
to unshare().
Fixes https://fedoraproject.org/wiki/Toolchain/PortingToModernC
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Despite efforts to make the virt-qemu-sev-validate tool friendly, it is
a certainty that almost everyone who tries it will hit false negative
results, getting a failure despite the VM being trustworthy.
Diagnosing these problems is no easy matter, especially for those not
familiar with SEV/SEV-ES in general. This extra docs text attempts to
set out a checklist of items to look at to identify what went wrong.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In general we expect to be able to construct a SEV-ES VMSA
blob from knowledge about the AMD achitectural CPU register
defaults, KVM setup and QEMU setup. If any of this unexpectedly
changes, figuring out what's wrong could be horrible. This
systemtap script demonstrates how to capture the real VMSA
that is used for a SEV-ES as it is booted. The captured data
can be fed into the 'sevctl vmsa show' command in order to
produce formatted info with named registers, allowing a
'diff' to be performed.
This script will need updating for any kernel version that is
not 6.0, to set the correct line numbers.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Expand the SEV guest kbase guide with information about how to configure
a SEV/SEV-ES guest when attestation is required, and mention the use of
virt-qemu-sev-validate as a way to confirm it.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
It is possible to build OVMF for SEV with an embedded Grub that can
fetch LUKS disk secrets. This adds support for injecting secrets in
the required format.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When validating a SEV-ES guest, we need to know the CPU count and VMSA
state. We can get the CPU count directly from libvirt's guest info. The
VMSA state can be constructed automatically if we query the CPU SKU from
host capabilities XML. Neither of these is secure, however, so this
behaviour is restricted.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The VMSA files contain the expected CPU register state for the VM. Their
content varies based on a few pieces of the stack
- AMD CPU architectural initial state
- KVM hypervisor VM CPU initialization
- QEMU userspace VM CPU initialization
- AMD CPU SKU (family/model/stepping)
The first three pieces of information we can obtain through code
inspection. The last piece of information we can take on the command
line. This allows a user to validate a SEV-ES guest merely by providing
the CPU SKU information, using --cpu-family, --cpu-model,
--cpu-stepping. This avoids the need to obtain or construct VMSA files
directly.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With the SEV-ES policy the VMSA state of each vCPU must be included in
the measured data. The VMSA state can be generated using the 'sevctl'
tool, by telling it a QEMU VMSA is required, and passing the hypevisor's
CPU SKU (family, model, stepping).
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When connected to libvirt we can validate that the guest configuration
has the kernel hashes property enabled, otherwise including the kernel
GUID table in our expected measurements is not likely to match the
actual measurement.
When running locally we can also automatically detect the kernel/initrd
paths, along with the cmdline string from the XML.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When doing direct kernel boot we need to include the kernel, initrd and
cmdline in the measurement.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Accept information about a connection to libvirt and a guest on the
command line. Talk to libvirt to obtain the running guest state and
automatically detect as much configuration as possible.
It will refuse to use a libvirt connection that is thought to be local
to the current machine, as running this tool on the hypervisor itself is
not considered secure. This can be overridden using the --insecure flag.
When querying the guest, it will also analyse the XML configuration in
an attempt to detect any options that are liable to be mistakes. For
example the NVRAM being measured should not have a persistent varstore.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virt-qemu-sev-validate program will compare a reported SEV/SEV-ES
domain launch measurement, to a computed launch measurement. This
determines whether the domain has been tampered with during launch.
This initial implementation requires all inputs to be provided
explicitly, and as such can run completely offline, without any
connection to libvirt.
The tool is placed in the libvirt-client-qemu sub-RPM since it is
specific to the QEMU driver.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
On aarch64 the 'id' file is not present for CPU cache information in
sysfs. This causes the local stateful hypervisor drivers to fail to
initialize capabilities:
virStateInitialize:657 : Initialisation of cloud-hypervisor state driver failed: no error
The 'no error' is because the 'virFileReadValueNNN' methods return
ret==-2, with no error raised, when the requeted file does not exist.
None of the callers were checking for this scenario when populating
capabilities. The most graceful way to handle this is to skip the
cache bank in question. This fixes failure to launch libvirt drivers
on certain aarch64 hardware.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/389
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This lets us simplify the cleanup paths when populating the host cache
bank information in capabilities XML.
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Two notable changes:
* the macOS platform has switched from x86_64 to aarch64
* if a new pipeline starts before a previous one finishes,
jobs marked 'interruptible: true' will be auto-cancelled
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since they are simply normal RPC messages, the keep alive packets are
subject to the "max_client_requests" limit just like any API calls.
Thus, if a client hits the 'max_client_requests' limit and all the
pending API calls take a long time to complete, it may result in
keep-alives firing and dropping the client connection.
This has been seen by a number of users with the default value of
max_client_requests=5, by issuing 5 concurrent live migration
operations.
By printing a warning message when this happens, admins will be alerted
to the fact that their active clients are exceeding the default client
requests limit.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Libvirt provides QMP passthrough APIs for the QEMU driver and these are
exposed in virsh. It is not especially pleasant, however, using the raw
QMP JSON syntax. QEMU has a tool 'qmp-shell' which can speak QMP and
exposes a human friendly interactive shell. It is not possible to use
this with libvirt managed guest, however, since only one client can
attach to the QMP socket at any point in time. While it would be
possible to configure a second QMP socket for a VM, it may not be
an known requirement at the time the guest is provisioned.
The virt-qmp-proxy tool aims to solve this problem. It opens a UNIX
socket and listens for incoming client connections, speaking QMP on
the connected socket. It will forward any QMP commands received onto
the running libvirt QEMU guest, and forward any replies back to the
QMP client. It will also forward back events.
$ virsh start demo
$ virt-qmp-proxy demo demo.qmp &
$ qmp-shell demo.qmp
Welcome to the QMP low-level shell!
Connected to QEMU 6.2.0
(QEMU) query-kvm
{
"return": {
"enabled": true,
"present": true
}
}
Note this tool of course has the same risks as the raw libvirt
QMP passthrough. It is safe to run query commands to fetch information
but commands which change the QEMU state risk disrupting libvirt's
management of QEMU, potentially resulting in data loss/corruption in
the worst case. Any use of this tool will cause the guest to be marked
as tainted as an warning that it could be in an unexpected state.
Since this tool introduces a python dependency it is not desirable
to include it in any of the existing RPMs in libvirt. This tool is
also QEMU specific, so isn't appropriate to bundle with the generic
tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
contain additional QEMU specific tools, with extra external deps.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This refresh switches the CI for contributors to be triggered by merge
requests. Pushing to a branch in a fork will no longer run CI pipelines,
in order to avoid consuming CI minutes. To regain the original behaviour
contributors can opt-in to a pipeline on push
git push <remote> -o ci.variable=RUN_PIPELINE=1
This variable can also be set globally on the repository, through the
web UI options Settings -> CI/CD -> Variables, though this is not
recommended. Upstream repo pushes to branches will run CI.
The use of containers has changed in this update, with only the upstream
repo creating containers, in order to avoid consuming contributors'
limited storage quotas. A fork with existing container images may delete
them. Containers will be rebuilt upstream when pushing commits with CI
changes to the default branch. Any other scenario with CI changes will
simply install build pre-requisite packages in a throaway environment,
using the ci/buildenv/ scripts. These scripts may also be used on a
contributor's local machines.
With pipelines triggered by merge requests, it is also now possible to
workaround the inability of contributors to run pipelines if they have
run out of CI quota. A project member can trigger a pipeline from the
merge request, which will run in context of upstream, however, note
this should only be done after reviewing the code for any malicious
CI changes.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Updated example covers:
* UUID
* CPU model, vendor, microcode, signature, counters,
topology, maxphysaddr, features,
* Power management
* NUMA page size info, multiple nodes, CPU topology IDs, distances
* CPU cache bank info
* Multiple secmodels
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This updates the FreeBSD 13 image to 13.1 which should fix the
symbol lookup errors seen in CI recently.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
One specfile containing both native and mingw builds is the
new best practice for Fedora. This reduces the maint burden
and ensures the mingw packages don't fall behind.
Note this adds many more BuildRequires for anyone building
on Fedora, which will now need installing.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When the <loader stateless='yes'/> attribute is set, the QEMU driver
needs to do three things
- Avoid looking for an NVRAM template
- Avoid auto-populating an <nvram/> path
- Find firmware descriptors with mode=stateless instead of mode=split
Note, the first thing happens automatically when we solve the second
thing.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Normally when an UEFI firmware is marked as read-only, an associated
NVRAM file will be created. Some builds of UEFI firmware, however, wish
to remain stateless and so will be read-only, but never have any NVRAM
file. To represent this concept a 'stateless' tristate bool attribute
is introduced on the <loader/> element.
There are rather a large number of permutations to consider.
With default firmware selection
* <os/>
=> Historic default, no change
* <os>
<loader stateless='yes'/>
</os>
=> Explicit version of historic default, no change
* <os>
<loader stateless='no'/>
</os>
=> Invalid, bios is always stateless
With manual legacy BIOS selection
* <os>
<loader>/path/to/seabios</loader>
...
</os>
=> Historic default, no change
* <os>
<loader stateless='yes'>/path/to/seabios</loader>
...
</os>
=> Explicit version of historic default, no change
* <os>
<loader stateless='no'>/path/to/seabios</loader>
...
</os>
=> Invalid, bios is always stateless
With manual UEFI selection
* <os>
<loader type='pflash'>/path/to/edk2</loader>
...
</os>
=> Historic default, no change
* <os>
<loader type='pflash' stateless='yes'>/path/to/edk2</loader>
...
</os>
=> Skip auto-filling NVRAM / template
* <os>
<loader type='pflash' stateless='no'>/path/to/edk2</loader>
...
</os>
=> Explicit version of historic default, no change
With automatic firmware selection
* <os firmware='bios'/>
=> Historic default, no change
* <os firmware='bios'>
<loader stateless='yes'/>
</os>
=> Explicit version of historic default, no change
* <os firmware='bios'>
<loader stateless='no'/>
</os>
=> Invalid, bios is always stateless
* <os firmware='uefi'/>
=> Historic default, no change
* <os firmware='uefi'>
<loader stateless='yes'/>
</os>
=> Skip auto-filling NVRAM / template
* <os firmware='uefi'>
<loader stateless='no'/>
</os>
=> Explicit version of historic default, no change
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Fix docs illustrating call with an obsolete macro.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
While libvirt solves most of the problem of ensuring compatibility, when
there is incompatibility it can be hard for users to track down the
cause. Everything knows to check the physical CPU model, but there are a
surprisingly large number of other factors influencing compatibility.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When firewalld is restarted or has its rules reloaded, we trigger a
reload of the nwfilter driver. This is done directly in the main
event loop thread which is a bad idea.
In a previous commit we fixed a actual deadlock problem with the
virStateReload API, when triggered from SIGHUP:
commit 33c6eb9689
Author: Jim Fehlig <jfehlig@suse.com>
Date: Thu Mar 8 15:04:48 2018 -0700
libvirtd: fix potential deadlock when reloading
The same deadlock problem previously existed with the firewalld reload
trigger, however, today it is not quite so series. The QEMU driver uses
a private event thread for each VM, so the particular deadlock would
not occur. None the less during the time the filters are reloading all
use of the event loop is blocked, which prevents APIs being serviced.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
While you can chain the virsh output up to a later 'xmllint' or 'xpath'
command, integrating it into virsh avoids needs for installing extra
binaries which we've often found to be missing on production installs
of libvirt. It also gives better response if the initial virsh command
hits an error, as you don't get an aborted pipeline.
$ virsh pool-dumpxml --xpath //permissions default
<permissions>
<mode>0711</mode>
<owner>1000</owner>
<group>1000</group>
<label>unconfined_u:object_r:svirt_home_t:s0</label>
</permissions>
If multiple nodes match, they are emitted individually:
$ virsh dumpxml --xpath '//devices/*/address[@type="pci"]' --wrap demo
<address type="pci" domain="0x0000" bus="0x05" slot="0x00" function="0x0"/>
<address type="pci" domain="0x0000" bus="0x03" slot="0x00" function="0x0"/>
...snip...
<address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0" multifunction="on"/>
<address type="pci" domain="0x0000" bus="0x07" slot="0x00" function="0x0"/>
but if intending to post-process the output further, the results
can be wrapped in a parent node
$ virsh dumpxml --xpath '//devices/*/address[@type="pci"]' --wrap demo
<nodes>
<address type="pci" domain="0x0000" bus="0x05" slot="0x00" function="0x0"/>
<address type="pci" domain="0x0000" bus="0x03" slot="0x00" function="0x0"/>
...snip...
<address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0" multifunction="on"/>
<address type="pci" domain="0x0000" bus="0x07" slot="0x00" function="0x0"/>
</nodes>
Fixes https://gitlab.com/libvirt/libvirt/-/issues/244
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The trivial case of fully printing an XML document is boring, but
this helper does more by allowing an XPath expression to be given.
It will then print just the subset of nodes which match the
expression. It either print each match as a standalone XML doc
or can put them into one new XML doc wrapped woith <nodes>...</nodes>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We currently build cross-arch containers for all three Debian
releases (10, 11, Sid), and do libvirt builds covering each
arch. This is overkill in terms of the number of problems it
identifies. The most important aspect of cross arch builds is
to find problems with 32-bit builds and problems with big
endian builds.
With this in mind the cross arch jobs are altered as follows
- Debian 10
- build the containers by default
- build armv7 (32-bit) & s390x (big endian)
- other arch builds manual
- Debian 11 / Sid
- container builds all optional
- arch builds all optional
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We currently build on Ubuntu 22.04 twice, for GCC and CLang
with santizers turned on. Moving the GCC santizers build
to 20.04 lets us cull one of the 22.04 jobs
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Currently we do native builds on all distros that are covered
by the support matrix. This reduces that such that we mostly
only run builds on the newest (ie bleeding edge non-released)
version and the oldest version. The effect is that cut out
builds on the newest release version. This is acceptable,
because that version is sandwiched between two versions we
do still test, so unlikely to have failures not already
identified by other jobs.
This has the effect of disabling:
- AlmaLinux 8 GCC - still has a CLang build
and CentOS 8 Stream also gives coverage
- Debian 11 - still has a Debian 10 and Sid
build
- Alpine 3.15 - still has a Alpine 3.14 and Edge
build
Ideally Fedora 35 would be disabled too, but we rely on that
for the integration tests.
The Ubuntu jobs will be handled in the next patch.
The containers are still built since this is cheap-ish.
The build jobs can also be triggered manually if desired.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This refreshes the containers bringing in new behaviour when
builds/containers are disabled.
Instead of deleting the job entirely, the job still exists
but is set to be a manual job. It won't affect the pipeline
result, but can be triggered by the developer if they wish
to test a specific scenario.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The container jobs are all optional in upstream now, only
running if dockerfile changes were made. Thus any build
jobs which reference the container jobs need to mark their
dependency as optional too.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
These checks made sense when we were in process of converting code.
Since the definition of the macros has been entirely removed now,
the compiler will already thrown an error. There aren't likely to
be any in-flight patches that would hit this anyone either.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
On macOS when BROKEN_POLL is set in GLib, our tests will periodically
trigger a warning:
(process:50880): GLib-WARNING **: 02:54:15.272: poll(2) failed due to: Bad file descriptor.
Our code is inherantly racy, calling g_source_destroy which
removes the FD from the event thread poll asynchronously but
we close the FD immediately after g_source_destroy returns.
With poll() this results in POLLNVAL which we're ignoring, but
with select() it generates the BADF error on macOS.
We need to ignore the warnings on macOS to avoid abort()ing
our test programs.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/303
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Mostly the tests should fail cleanly, but sometimes the test might crash
or abort. In this case we'll need to know the stack traces in order to
debug the problem. Fortunately on the Cirrus CI macOS instances, the
crash reporting service is active and saving crashes to the directory
~/Library/Logs/DiagnosticReports/
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't have access to the 'testlog.txt' file, so we need meson to
print the failures for any broken tests directly.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The updateLock is a R/W lock held by anything which needs to read or
modify the rules associated with an NWFilter.
APIs for defining/undefining NW filters rules hold a write lock on
updateLock.
APIs for creating/deleting NW filter bindings hold a read lock on
updateLock, which prevents define/undefine taking place concurrently.
The problems arise when we attempt to creating two NW filter bindings in
parallel.
Thread 1 can acquire the mutex for filter A
Thread 2 can acquire the mutex for filter B
Consider if filters A and B both reference filters C and D, but in
different orders:
Filter A
-> filter C
-> filter D
Filter B
-> filter D
-> filter C
Thread 1 will try to acquire locks in order A, C, D while thread 1 will
try to acquire in order A, D, C. Deadlock can still occur.
Think we can sort the list of filters before acquiring locks on all of
them ? Nope, we allow arbitrary recursion:
Filter A
-> filter C
-> filter E
-> filter F
-> filter H
-> filter K
-> filter D
-> filter G
-> filter I
So we can't tell from looking at 'A' which filters we're going to
need to lock. We can only see the first level of filters references
and we need to lock those before we can see the second level of
filters, etc.
We could probably come up with some cleverness to address this but
it isn't worth the time investment. It is simpler to just keep the
process of creating NW filter bindings totally serialized.
Using two separate locks for this serialization though is pointless.
Every code path which gets a read(updateLock) will go on to hold
updateMutex. It is simpler to just hold write(updateLock) and
get rid of updateMutex. At that point we don't need updateLock
to be a R/W lock, it can be a plain mutex.
Thus this patch gets rid of the current updateLock and updateMutex
and introduces a new top level updateMutex.
This has a secondary benefit of introducing fairness into the
locking. With a POSIX R/W lock, you get writer starvation if
you have lots of readers. IOW, if we call virNWFilterBIndingCreate
and virNWFilterBindingDelete in a tight loop from a couple of
threads, we can prevent virNWFilterDefine from ever acquiring
a write lock.
Getting rid of the R/W lock gives us FIFO lock acquisition
preventing starvation of any API call servicing.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The code parsing thue query-cpu-definitions response will short-circuit
the for loop in the case where usable=yes, resulting in us failing to
parse the CPU deprecation flag.
IOW, we only reported deprecations in domain capabilities for CPU models
which were not runnable on the host.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Right now the jobs have no rules so they will always be created in
a pipeline. If the user's fork has no runner configured, then the
jobs will never be able to execute and the pipeline will not finish.
Even on upstream, there might be times the runner has to be taken
offline for maint work, or unexpectedly fail. We need a quick way
to disable the integration tests if we decide we don't want to
have pipelines queued until the runner comes back online.
Both these problems can be addressed by requiring a environment
variable to be set
LIBVIRT_CI_INTEGRATION=1
This can be done in the GitLab repo CI settings for permanent
enablement. Alternatively it can be set for individual
scheduled jobs, or using a push option
git push -o ci.variable=LIBVIRT_CI_INTEGRATION=1
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Although we split out jobs across many files, the template / job
namespace is global, so we should use something more specific
than '.tests' as the template name.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
virHashTableForEach unhelpfully has payload/key args in
its callback reversed compared to g_hash_table_foreach.
When converting from one to the other the semantics
change but you don't get a compile error
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The current use of an array for nwfilter objects requires
the caller to iterate over all elements to find a filter,
and also requires locking each filter.
Switching to a pair of hash tables enables O(1) lookups
both by name and uuid, with no locking required.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The comment against the 'updateMutex' refers to a problem with
lock ordering when looking up filters in the virNWFilterObjList
which uses an array. That problem does indeed exist.
Unfortunately it claims that switching to a hash table would
solve the lock ordering problems during instantiation. That
is not correct because there is a second lock ordering
problem related to how we traverse related filters when
instantiating filters. Consider a set of filters:
Filter A:
Reference Filter C
Reference Filter D
Filter B:
Reference Filter D
Reference Filter C
In one example, we lock A, C, D, in the other example
we lock A, D, C.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virNWFilterObjListNumOfNWFilters method iterates over the
driver->nwfilters, accessing virNWFilterObj instances. As such
it needs to be protected against concurrent modification of
the driver->nwfilters object.
This API allows unprivileged users to connect, so users with
read-only access to libvirt can cause a denial of service
crash if they are able to race with a call of virNWFilterUndefine.
Since network filters are usually statically defined, this is
considered a low severity problem.
This is assigned CVE-2022-0897.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When using thue 'run' script to launch a daemon, it is intended to
temporarily stop the systemd units and re-start them again after.
When using this script over an SSH connection, it will get SIGHUP
if the connection goes away, and in this case it fails to re-start
the systemd units. We need to catch SIGHUP and turn it into a
normal python exception. For good measure we do the same for
SIGQUIT and SIGTERM too. SIGINT already gets turned into an
exception by default which we handle.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the 'run' script modifies $PATH to add the 'tools'
directly to pick up client programs. It fails to add the 'src'
directory to pick up the daemons.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
For the various structs storing lists of objects, the access
to the hash tables is not lockless. The mutex on the object
owning the hash table must be held.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Long ago we adapted to Linux kernel changes which inverted the
behaviour of the conntrack --ctdir setting:
commit a6a04ea47a
Author: Stefan Berger <stefanb@us.ibm.com>
Date: Wed May 15 21:02:11 2013 -0400
nwfilter: check for inverted ctdir
Linux netfilter at some point (Linux 2.6.39) inverted the meaning of the
'--ctdir reply' and newer netfilter implementations now expect
'--ctdir original' instead and vice-versa.
We check for the kernel version and assume that all Linux kernels with version
2.6.39 have the newer inverted logic.
Any distro backporting the Linux kernel patch that inverts the --ctdir logic
(Linux commit 96120d86f) must also backport this patch for Linux and
adapt the kernel version being tested for.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Given our supported platform targets, we no longer need to
consider a version of Linux before 2.6.39, so can drop
support for the old direction behaviour.
The test suite updates are triggered because that never
probed for the ctdir direction, and so the iptables syntax
generator unconditionally dropped the ctdir args.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Long ago we adapted to iptables changes by introducing support
for '-m conntrack':
commit 06844ccbaa
Author: Stefan Berger <stefanb@us.ibm.com>
Date: Tue Aug 6 20:30:46 2013 -0400
nwfilter: Use -m conntrack rather than -m state
Since iptables version 1.4.16 '-m state --state NEW' is converted to
'-m conntrack --ctstate NEW'. Therefore, when encountering this or later
versions of iptables use '-m conntrack --ctstate'.
Given our supported platform targets, we no longer need to
consider a version of iptables before 1.4.16, so can drop
support for the old syntax.
The test suite updates are triggered because that never
probed for the new syntax, and so unconditionally
generated the old syntax.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Daniel P. Berrangé