Libxml2 has awful error reporting behaviour when reading files. When
we fail to load a file from the test driver we see:
$ virsh -c test:///wibble.xml
I/O warning : failed to load external entity "/wibble.xml"
error: failed to connect to the hypervisor
error: XML error: failed to parse xml document '/wibble.xml'
where the I/O warning line is something printed by libxml2 itself,
which also lacks any useful detail.
Switching to our own file reading code we can massively improve
things:
$ ./build/tools/virsh -c test:///wibble.xml
error: failed to connect to the hypervisor
error: Failed to open file '/wibble.xml': No such file or directory
Using 10 MB as an upper limit on XML file size ought to be sufficient
for any XML files libvirt is reading.
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently given an input of '<dom\n' we emit an error:
error: Failed to define domain from tests/qemuxmlconfdata/broken-xml-invalid.xml
error: at line 2: Couldn't find end of Start Tag dom line 1
(null)
^
With this fix we emit:
error: Failed to define domain from tests/qemuxmlconfdata/broken-xml-invalid.xml
error: at line 2: Couldn't find end of Start Tag dom line 1
<dom
----^
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The exit-on-error argument (added in QEMU 9.1.0) can be used to tell
QEMU not to exit when incoming migration fails so that the error can be
retrieved via QMP. This patch adds a new capability bit indicating
support for the new argument.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In my previous commit v10.10.0-48-g2d222ecf6e I've made us enable
I/O APIC when there is an IOMMU with EIM. This works well. What
does not work is case when there's just an IOMMU without EIM but
with 256+ vCPUS. Problem is that post parsing happens in two
stages: general domain post parse (where
qemuDomainDefEnableDefaultFeatures() is called) and then per
device post parse (where qemuDomainIOMMUDefPostParse() is
called). Now, in aforementioned case it is the device post parse
phase where EIM is enabled but the code that would enable
VIR_DOMAIN_FEATURE_IOAPIC has already run.
To resolve this, make the domain post parse callback "foresee"
the future enabling of EIM so that it can turn on I/O APIC
beforehand.
Resolves: https://issues.redhat.com/browse/RHEL-65844
Fixes: 2d222ecf6e
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add a new a attribute, deprecated_features='on|off' to the <cpu>
element. This is used to toggle features flagged as deprecated on the
CPU model on or off. When this attribute is paired with 'on',
deprecated features will not be filtered. When paired with 'off', any
CPU features that are flagged as deprecated will be listed under the
CPU model with the 'disable' policy.
Example:
<cpu mode='host-model' check='partial' deprecated_features='off'/>
The absence of this attribute is equivalent to the 'on' option.
The deprecated features that will populate the domain XML are the same
features that result in the virsh domcapabilities command with the
--disable-deprecated-features argument present.
It is recommended to define a domain XML with this attribute set to
'off' to ensure migration to machines that may outright drop these
features in the future.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS for detecting
if query-cpu-model-expansion can report deprecated CPU model properties.
QEMU introduced this capability in 9.1 release. Add flag and deprecated
features to the capabilities test data for QEMU 9.1 and 9.2 replies/XML
since it can now be accounted for.
When probing for the host CPU, perform a full CPU model expansion to
retrieve the list of features deprecated across the entire architecture.
The list and count are stored in the host's CPU model info within the
QEMU capabilities. Other info resulting from this query (e.g. model
name, etc) is ignored.
The new capabilities flag is used to fence off the extra query for
architectures/QEMU binaries that do not report deprecated CPU model
features.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
query-cpu-model-expansion may report an array of deprecated properties.
This array is optional, and may not be supported for a particular
architecture or reported for a particular CPU model. If the output is
present, then capture it and store in a qemuMonitorCPUModelInfo struct
for later use.
The deprecated features will be retained in qemuCaps->kvm->hostCPU.info
and will be stored in the capabilities cache file under the <hostCPU>
element using the following format:
<deprecatedFeatures>
<property name='bpb'/>
<property name='csske'/>
<property name='cte'/>
<property name='te'/>
</deprecatedFeatures>
At this time the data is only queried, parsed, and cached. The data
will be utilized in a subsequent patch.
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This is a follow up of my previous commits. If the number of
vCPUs exceeds some arbitrary value (255) then QEMU requires IOMMU
with EIM and intremap enabled. But in turn, intremap IOMMU
requires split I/O APIC (per virDomainDefIOMMUValidate()). Since
after my previous commits (e.g. v10.10.0-rc1~183) IOMMU is added
automagically, the I/O APIC can be also enabled automagically.
Relates to: https://issues.redhat.com/browse/RHEL-65844
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This function return value is invariant since 18f3771, so change
its type and remove all dependent checks.
Found by Linux Verification Center (linuxtesting.org) with Svace.
Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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>
