Due to a bug in the optimization to avoid testing symlinked tests
multiple times all tests were skipped.
In commit f997fcca71 I made an attempt to optimize the
tests by avoiding testing symlinks. This optimization was buggy as I've
passed the 'd_name' field of 'struct dirent' which is just the filename
to 'g_lstat()'. 'g_lstat()' obviously always failed with ENOENT. As the
logic checked only for successful return of 'g_lstat()' the optimizatio
was a dud.
Now in 4d8ebbfee8 the 'g_lstat()' call was replaced by
'virFileIsLink()' checking all non-zero values. This meant that if
'virFileIsLink()' failed the test was skipped. Now since a bad argument
was passed this failed always and thus was always skipped making
'virschematest' useless.
Fix it by passing the full path of the test and also explicitly check
for '1' return value instead of any non-zero.
Fixes: f997fcca71
Fixes: 4d8ebbfee8
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Please see the commit log for commit v10.9.0-rc1-1-g42ab0148dd for the
history and explanation of the problem that this patch is fixing.
A shorter explanation is that when a guest is connected to a libvirt
virtual network using a virtio-net adapter with in-kernel "vhost-net"
packet processing enabled, it will fail to acquire an IP address from
a DHCP seever running on the host.
In commit v10.9.0-rc1-1-g42ab0148dd we tried fixing this by *zeroing
out* the checksums of these packets with an nftables rule (nftables
can't recompute the checksum, but it can set it to 0) . This
*appeared* to work initially, but it turned out that zeroing the
checksum ends up breaking dhcp packets on *non* virtio/vhost-net guest
interfaces. That attempt was reverted in commit v10.9.0-rc2.
Fortunately, there is an existing way to recompute the checksum of a
packet as it leaves an interface - the "tc" (traffic control) utility
that libvirt already uses for bandwidth management. This patch uses a
tc filter rule to match dhcp response packets on the bridge and
recompute their checksum.
The filter rule must be attached to a tc qdisc, which may also have a
filter attached for bandwidth management (in the <bandwidth> element
of the network config). Not only must we add the qdisc only once
(which was already handled by the patch two prior to this one), but
also the filter rule for checksum fixing and the filter rule for
bandwidth management must be different priorities so they don't clash;
this is solved by adding the checksum-fix filter with "priority 2",
while the bandwidth management filter remains "priority 1" (both will
always be evaluated anyway, it's just a matter of which is evaluated
first).
So far this method has worked with every different guest we could
throw at it, including several that failed with the previous method.
Fixes: b89c4991da
Reported-by: Rich Jones <rjones@redhat.com>
Reported-by: Andrea Bolognani <abologna@redhat.com>
Fix-Suggested-by: Eric Garver <egarver@redhat.com>
Fix-Suggested-by: Phil Sutter <psutter@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There will soon be two separate users of tc on virtual networks, and
both will use the "qdisc root handle 1: htb" to add tx filters. One or the
other could get the first chance to add the qdisc, and then if at a
later time the other decides to use it, we need to prevent the 2nd
user from attempting to re-add the qdisc (because that just generates
an error).
We do this by running "tc qdisc show dev $bridge handle 1:" then
checking if the output of that command contains both "qdisc" and " 1:
".[*] If it does then the qdisc has already been added. If not then we
need to add it now.
[*]As of this writing, the output more exactly starts with "qdisc
htb 1: root", but our comparison is made purposefully generous to
increase the chances that it will continue to work properly if tc
modifies the format of its output.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virNetDevBandwidthSet() always clears all existing qdiscs and their
subordinate filters before adding all the new qdiscs/filters. This is
normally exactly what we want, but there is one case (the network
driver) where the Qdisc added by virNetDevBandwidthSet() may already
be in use by the nftables backend (which will add a rule to fix the
checksum of dhcp packets); in that case, we *don't* want
virNetDevBandwidthSet() to clear out the qdisc that was already added
for nftables, and none of the bandwidth filters have been added yet,
so there already aren't any "old" filters that need to be removed
either - it is safe to just skip virNetDevBandwidthClear() in this
case.
To allow the network driver to set bandwidth without first clearing
it, this patch adds the flag VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL to the
virNetDevBandwidthSetFlags enum, and recognizes it in
virNetDevBandwidthSet() - if the flag is set, then
virNetDevBandwidth() will call virNetDevBandwidthClear() just as it
always has. But if the flag isn't set it *won't* call
virNetDevBandwidthClear().
As suggested above, VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL is set for all
calls to virNetdevBandwidthSet() except for two places in the network
driver.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Having two bools in the arg list is on the borderline of being
confusing to anyone trying to read the code, but we're about to add a
3rd. This patch replaces the two bools with a single flags argument
which will instead have one or more bits from virNetDevBandwidthFlags
set.
Signed-off-by: Laine Stump <laine@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some models are just aliases to other models. Make this relation
available to users via domain capabilities.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Add two test images showing the use of 'data_file' and 'data_file_raw'
(although the latter is not detected by libvirt) so that we can see that
the qcow2 metadata parser and backing chain populators work correctly.
The example files were created by:
qemu-img create -f qcow2 -o data_file=raw,data_file_raw=true,preallocation=off datafile.qcow2 1k
qemu-img create -f qcow2 -o data_file=rawpreallocation=off -F qcow2 -b datafile.qcow2 qcow2datafile-datafile.qcow2
Note that 'data_file_raw' is mutually exclusive with backing images.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Update to v9.2.0-rc0-42-g3428a3894c
Apart from the changes below there are changes to CPU features reported
by qemu, some of which were reported multiple times previously which no
longer happens.
Notable changes:
- 'reconnect-ms' added and 'reconnect' deprecated for 'stream' variant
of 'netdev-add' backend
- 'BLOCK_IO_ERROR' event removed 'qom-path' parameter
- 'GraniteRapids-v2-x86_64-cpu' added
- 'sm3' hashing algorithm for 'luks' added
- 'acpi-generic-port' object added
- deprecated field 'loaded' of 'secret'/'secret_keyring'/'tls-creds*'
removed
- 'sh4eb' target added
- 'query-migrationthreads' command deprecated
- 'busnr' and 'x-pcie-ext-tag' attributes added for
'ICH9-LPC'/'PIIX4_PM'/'VGA'/'mch'/'pcie-root-port'/'qxl'/'vfio-pci'/
'virtio-*'/'vmware-svga'
devices
- 'stale-tm' property added for 'intel-iommu' device
Experimental features:
- 'device-sync-config' command added
As the addition of the 'reconnect-ms' property of the 'stream' network
backend happened along with deprecation of the 'reconnect' field which
was already in use by libvirt this patch also captures the change to the
new format.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'stream' type for 'netdev-add' recently added support for
'reconnect-ms' which supersedes 'reconnect' (now deprecated). Add a
capability which will allow us to switch to the new property.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some VMware guests have a boolean uefi.secureBoot.enabled. If found,
and it's set to "TRUE", and if it's a UEFI guest, then add this clause
into the domain XML:
<os firmware='efi'>
<firmware>
<feature enabled='yes' name='enrolled-keys'/>
<feature enabled='yes' name='secure-boot'/>
</firmware>
</os>
This approximates the meaning of this VMware flag.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Fixes: https://issues.redhat.com/browse/RHEL-67836
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Refactor the parts of qemuBlockGetNamedNodeData which fetch the names of
internal snapshots present in the on-disk state of QCOW2 images to also
extract the presence of the 'vmstate' section.
This requires conversion of the snapshot list to a hash table as we
always know the name of the snapshot that we're looking for, and the
hash table allows also storing of additional data which we'll use to
store the presence of the 'vmstate'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Get the JSON profile that the swtpm instance was created with from the
output of 'swtpm socket --tpm2 --print-info 0x20 --tpmstate ...'. Get the
name of the profile from the JSON and set it in the current and persistent
emulator descriptions as 'name' attribute and have the persistent
description stored with this update. The user should avoid setting this
'name' attribute since it is meant to be read-only. The following is
an example of how the XML could look like:
<profile source='local:restricted' name='custom:restricted'/>
If the user provided no profile node, and therefore swtpm_setup picked its
default profile, the XML may now shows the 'name' attribute with the name
of the profile. This makes the 'source' attribute now optional.
<profile name='default-v1'/>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Extend the parser and XML builder with support for the profile parameter
and its remove_disabled attribute.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move HostdevHostSupportsPassthroughVFIO method to hypervisor to be
shared between qemu and ch drivers.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The nvram image can have any supported format and there's no technical
requirement of them having the same format. In fact the actual nvram
image doesn't necessarily need to have the same format as the template
if the user is willing to format it themselves (as libvirt is not going
to convert it).
Remove the nonsensical check and adjust tests. The test case required
swapping around the format in order to work properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code historically skipped the 'format' field for 'raw' images as we
didn't output it when no format support was present. Stop misleading and
output the format also for 'raw' images.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As the 'format' field is meant to carry the format of the nvram image we
should output it even when the image is 'raw'.
Currently this is not a problem but later patches will allow mismatch
between the nvram format and loader format (as nothing really
technically requires them to be the same and this then could become
problem).
Modify the condition and update tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently the qemu firmware code weirdly depends on the 'format' field
of the nvram image itself to do the auto-selection process as well as
then uses it to declare the actual type to qemu.
As it's not technically required that the template and the on disk image
share the type introduce a 'templateFormat' field which will split off
from the shared purpose of the type and will be used for the selection
and instantiation process, while 'format' will be left for the actual
type of the on disk image.
This patch introduces the field, adds XML infrastructure as well as
plumbs it to the firmware bits.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemu driver does support qcow2 images for the firmware and nvram
pflash devices, but we do not do the full backing chain setup for them
as we don't expect that those images would actually have a backing
store. We don't tell that to qemu though which theoretically can lead to
qemu probing the backing store from the image itself. We don't want that
for now.
Deny qemu probing the backing store by installing a "terminator" empty
virStorageSource as 'backingStore' for pflash and nvram.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuFirmwareEnsureNVRAM' which fills the NVRAM configuration bits which
may be missing was basing its decision to do something based on whether
the 'path' field was set. This is insufficient if remote storage is to
be considered.
Use 'virStorageSourceIsEmpty()' instead as that properly considers
remote filesystems and explain why the source is unref'd when the
function decides to rewrite the config.
The 'firmware-auto-efi-format-nvram-qcow2-network-nbd' is modified to
omit filling the 'path' field, which without this fix would result in
the nvram to be reset to a local file.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Problem with qemu_domain.c is that it's constantly growing. But
there are few options for improvement. For instance, validation
functions were moved out and now live in qemu_validate.c. We can
do the same for PostParse functions, though since PostParse may
modify domain definition, some functions need to be exported from
qemu_domain.c.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When moving function and/or renaming them sometimes corresponding
change to corresponding header file is not done. This leaves us
with functions that are declared in header files, but nowhere
implemented. Drop such declarations.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
