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>
Move HostdevNeedsVFIO method to hypervisor to be reused between qemu
and ch drivers.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case when a management application will require to store the nvram in
a block device instead of a file libvirt needs to be able to set up the
block device.
This patch introduces support for setting up the block device by using
'qemu-img convert' to produce a qcow2-formatted block device.
The use of 'qcow2' is made mandatory as the UEFI firmware requires that
the NVRAM image has the exact expected size, which is almost impossible
with block devices. 'qcow2' also allows libvirt to detect wheher the
block device is formatted allowing file-like semantics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The setup of nvram will later be extended to also support block-device
backed nvram, so extract the file-backed nvram setup steps from
'qemuPrepareNVRAM' into 'qemuPrepareNVRAMFile'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
Basing the selection on the format of the actual NVRAM image makes no
sense as user may format the image themselves.
Additionally it doesn't make much sense to even limit the firmware
selection based on the nvram template itself. As format of the template
is given and firmware images don't really provide any choice.
Remove the limitation so that autoselection can pick a template
regardless of the selected format or template format.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refuse situations where the user configures a different format for a
file-backed nvram than the template file has.
At this point it's still required that the NVRAM and firmware share
format, but that is going to be relaxed, thus we need to refuse
configurations that the code can't handle.
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>
Restructure the code to assign first (as this is simpler to refactor in
the future) and avoid mixing implicit value checks with explicit ones by
checking for _NONE.
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>
'virFileRewrite()' which is used to setup the NVRAM image if it doesn't
exist or when it is requested by the user forcibly replaces the
destination file by the file it creates. For block devices this
overwrites the device node file or the symlink pointing to the device
node by a regular file instead of formatting it.
As this not only makes the VM fail to start but also breaks user's /dev/
filesystem forbid it for now.
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 qemuDomainDeleteDevice() gets "DeviceNotFound" error it is a
special case as we're trying to remove a device which does not exists
any more. Such occasion is indicated by the return value -2.
Callers of the aforementioned function ought to base their behaviour on
the return value. However not all callers take as much care for the
return value as one could realistically anticipate.
Follow the usual direction of removing possible backend object (in case
of character devices), remove the device from its XML without waiting
for the device removal from QEMU (since it is already not there) and
basically follow the same algorithm as there is when the device was
removed, skipping over the wait for the device removal.
The overall return value also needs to be adjusted since
qemuDomainDeleteDevice() does not set an error on the -2 return value
and would otherwise trigger an unknown error being reported to the user
or management application.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
The function the comment is referring to is
virNetServerClientPrivNew() not virNetServerClintPrivNew(). The
latter doesn't even exist.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When removing a socket in virCHMonitorClose() fails, a warning is
printed. But it doesn't contain errno nor g_strerror() which may
shed more light into why removing of the socket failed.
Oh, and since virCHMonitorClose() is registered as autoptr
cleanup for virCHMonitor() it may happen that virCHMonitorClose()
is called with mon->socketpath allocated but file not existing
yet (see virCHMonitorNew()). Thus ignore ENOENT and do not print
warning in that case - the file doesn't exist anyways.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virCHMonitorClose() is meant to be called when monitor to
cloud-hypervisor process closes. It removes the socket and frees
string containing path to the socket.
In general, there is a problem with the following pattern:
if (var) {
do_something();
g_free(var);
}
because if the pattern executes twice the variable is freed
twice. That's why we have VIR_FREE() macro. Well, replace plain
g_free() with g_clear_pointer(). Mind you, this is NOT a
destructor where clearing pointers is needless.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: John Levon <john.levon@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add some basic plumbing, based on the qemu driver.
Signed-off-by: John Levon <john.levon@nutanix.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This should be better than the current for both hotplug:
error: internal error: Invalid target model for serial device
and hot-unplug:
error: An error occurred, but the cause is unknown
which should not be reached at all.
Resolves: https://issues.redhat.com/browse/RHEL-66222
Resolves: https://issues.redhat.com/browse/RHEL-66223
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If QEMU supports multi boot device make use of it instead of using the
single boot device machine parameter.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Add capability QEMU_CAPS_VIRTIO_CCW_DEVICE_LOADPARM to detect multi boot
device support in QEMU by checking the virtio-blk-ccw device property
existence of loadparm.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Let me preface this with stating the obvious: documentation on
QoS in OVS is very sparse. This is all based on my observation
and OVS codebase analysis.
For the following QoS setting:
<bandwidth>
<inbound average="512" peak="1024" burst="32"/>
</bandwidth>
the following QoS setting is generated into OVS (NB, our XML
values are in KiB/s, OVS has them in bits/s):
# ovs-vsctl list qos
_uuid : a087226b-2da6-4575-ad4c-bf570cb812a9
external_ids : {ifname=vnet1, vm-id="7714e6b5-4885-4140-bc59-2f77cc99b3b5"}
other_config : {burst="262144", max-rate="8192000", min-rate="4096000"}
queues : {0=655bf3a7-e530-4516-9caf-ec9555dfbd4c}
type : linux-htb
from which the following topology is generated:
# for i in qdisc class; do tc -s -d -g $i show dev vnet1; done
qdisc htb 1: root refcnt 2 r2q 10 default 0x1 direct_packets_stat 0 ver 3.17 direct_qlen 1000
Sent 2186 bytes 16 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
+---(1:fffe) htb rate 8192Kbit ceil 8192Kbit linklayer ethernet burst 1499b/1mpu 60b cburst 1499b/1mpu 60b level 7
| Sent 2186 bytes 16 pkt (dropped 0, overlimits 0 requeues 0)
| backlog 0b 0p requeues 0
|
+---(1:1) htb prio 0 quantum 51200 rate 4096Kbit ceil 8192Kbit linklayer ethernet burst 32Kb/1mpu 60b cburst 32Kb/1mpu 60b level 0
Sent 2186 bytes 16 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
Long story short, the default class (1:) for an OVS interface has
average and peak set exactly as requested. But since it's nested
under another class (1:fffe), it can borrow unused bandwidth. And
the parent is set to have rate = ceil = peak from our XML. From
[1]: htb_tc_install() calls htb_parse_qdisc_details__() which
sets: 'hc->min_rate = hc->max_rate;' and then calls
htb_setup_class_(..., tc_make_handle(1, 0xfffe), tc_make_handle(1, 0), &hc);
to set up the top parent class.
In other words - the interface is set up to so that it can always
consume 'peak' bandwidth and there is no way for us to set it up
differently. It's too late to deny setting 'peak' different to
'average' at XML validation phase so do the next best thing -
throw a warning, just like we do in case <bandwidth/> is set for
an unsupported <interface/> type.
1: https://github.com/openvswitch/ovs/blob/main/lib/netdev-linux.c#L5039
Resolves: https://issues.redhat.com/browse/RHEL-53963
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
If a Q35 domain has huge number of vCPUS (over 255, currently), then
it needs IOMMU with Extended Interrupt Mode enabled (see check in
qemuValidateDomainVCpuTopology()).
Well, we already add some devices and to other tricks when
parsing new domain XML. Might as well add IOMMU device if above
condition is met.
Resolves: https://issues.redhat.com/browse/RHEL-65844
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If a Q35 domain has huge number of vCPUS (over 255, currently), then
it needs IOMMU with Extended Interrupt Mode enabled (see check in
qemuValidateDomainVCpuTopology()).
Well, we already add some devices and to other tricks when
parsing new domain XML. Might as well turn the EIM on for IOMMU
device.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It only errors out when presented with a non-array, but we do check
it everywhere else.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In virJSONValueFromJsonC, the return value of virJSONValueFromJsonC
was not checked in one case.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In the rare case where int and long long are not the same size,
the multiplication of an int variable and an int constant might
overflow. Cast the constant to long long to avoid this.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: baa4edfb79d5ee861a08b5ec11416c5c156d8cd2
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With upcoming v0.10 swtpm (commit
aa483aeb6d),
file locking with "lock" option is now supported and reflected in
"tpmstate-opt-lock" capability.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
When swtpm reports "nvram-backend-dir", it can accepts a single file or
block device where TPM state will be stored. --tpmstate must be
backend-uri=file://<path>.
Teach the storage to use custom directory or file source location.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Learn to parse a directory for the TPM state.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Learn to parse a file path for the TPM state.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Mechanically replace existing 'storagepath' with 'source_path', as the
following patches introduce <source path='..'> configuration.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Domain capabilities include information about support for various
devices and models.
Panic devices are not included in the output which means that management
applications need to include the logic for choosing the right device
model or request a default model and try defining such a domain.
Add reporting of panic device models into the domain capabilities based
on the logic in qemuValidateDomainDefPanic() and also report whether
panic devices are supported based on whether at least one model is
supported. That way consumers of the domain capability XML can
differentiate between libvirt not reporting the panic device models or
no model being supported.
Resolves: https://issues.redhat.com/browse/RHEL-65187
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add an error message for the rare case if json_tokener_new
fails (allocation failure) and guard any use of json_tokener_free
where tok might be NULL (this was possible in libvirt-nss
when the json file could not be opened).
https://gitlab.com/libvirt/libvirt/-/issues/581
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Simon Pilkington
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Completely remove use of g_malloc (without zeroing of the allocated
memory) and forbid further use.
Replace use of g_malloc0 in cases where the variable holding the pointer
has proper type.
In all of the above cases we can use g_new0 instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The error message from 'json-c' was passed along without any libvirt
string which makes it hard to find in the source and isn't exactly clear
when present in logs:
libvirtd[843]: internal error : invalid utf-8 string
Prefix the message with 'failed to parse JSON'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We should include maximum physical address size in the CPU definition
created by virConnectBaselineHypervisorCPU only if we know the value for
all input CPUs. Otherwise we would create a CPU definition that is not
usable on all hosts from which we gathered the CPU info.
https://issues.redhat.com/browse/RHEL-24850
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This reverts commit 42ab0148dd11727f7e3fd31dce4485469af290d5.
This patch was supposed to fix the checksum of dhcp response packets
by setting it to 0 (because having a non-0 but incorrect checksum was
causing the packets to be droppe on FreeBSD guests).
Early testing was positive, but after the patch was pushed upstream
and more people could test it, it turned out that while it fixed the
dhcp checksum problem for virtio-net interfaces on FreeBSD and
OpenBSD, it also *broke* dhcp checksums for the e1000 emulated NIC on
*all* guests (but not e1000e).
So we're reverting this fix and looking for something more universal
to be included in the next release.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Many years ago (April 2010), soon after "vhost" in-kernel packet
processing was added to the virtio-net driver, people running RHEL5
virtual machines with a virtio-net interface connected via a libvirt
virtual network noticed that when vhost packet processing was enabled,
their VMs could no longer get an IP address via DHCP - the guest was
ignoring the DHCP response packets sent by the host.
(I've been informed by danpb that the same issue had been encountered,
and "fixed" even earlier than that, in 2006, with Xen as the
hypervisor.)
The "gory details" of the 2010 discussion are chronicled here:
https://lists.isc.org/pipermail/dhcp-hackers/2010-April/001835.html
but basically it was because packet checksums weren't being fully
computed on the host side (because QEMU on the host and the NIC driver
in the guest had agreed between themselves to turn off checksums
because they were unnecessary due to the "link" between the two being
entirely in local memory rather than an error-prone physical cable),
but
1) a partial checksum was being put into the packets at some point by
"someone"
2) the "don't use checksums" info was known by the guest kernel, which
would properly ignore the "bad" checksum), and
3) the packets were being read by the dhclient application on the
guest side with a "raw" socket (thus bypassing the guest kernel UDP
processing that would have known the checksum was irrelevant and
ignore it)),
The "fix" for this ended up being two-tiered:
1) The ISC DHCP package (which contains the aforementioned dhclient
program) made a fix to their dhclient code which caused it to accept
packets anyway even if they didn't have a proper checksum (NB: that's
not a full explanation, and possibly not accurate). This remedied the
problem for guests with an updated dhclient. Here is the code with the
fix to ISC DHCP:
https://github.com/isc-projects/dhcp/blob/master/common/packet.c#L365
This eliminated the issue for any new/updated guests that had the
fixed dhclient, but it didn't solve the problem for existing/old guest
images that didn't/couldn't get their dhclient updated. This brings us
to:
2) iptables added a new "CHECKSUM" target and "--checksum-fill"
action:
http://patchwork.ozlabs.org/patch/58525/
and libvirt added an iptables rule for each virtual network to match
DHCP response packets and perform --checksum-fill. This way by the
time dhclient on the guest read the raw packet, the checksum would be
corrected, and the packet would be accepted. This was pushed upstream
in libvirt commit v0.8.2-142-gfd5b15ff1a.
The word at the time from those more knowledgeable than me was that
the bad checksum problem was really specific to ISC's dhclient running
on Linux, and so once their fix was in use everywhere dhclient was
used, bad checksums would be a thing of the past and the
--checksum-fill iptables rules would no longer be needed (but would
otherwise be harmless if they were still there).
(Plot twist: the dhclient code in fix (1) above apparently is on a
Linux-only code path - this is very important later!)
Based on this information (and also due to the opinion that fixing it
by having iptables modify the packet checksum was really the wrong way
to permanently fix things, i.e. an "ugly hack"), the nftables
developers made the decision to not implement an equivalent to
--checksum-fill in nftables. As a result, when I wrote the nftables
firewall backend for libvirt virtual networks earlier this year, it
didn't add in any rule to "fix" broken UDP checksums (since there was
apparently no equivalent in nftables and, after all, that was fixed
somewhere else 14 years ago, right???)
But last week, when Rich Jones was doing routine testing using a Fedora
40 host (the first Fedora release to use the nftables backend of libvirt's
network driver by default) and a FreeBSD guest, for "some strange
reason", the FreeBSD guest was unable to get an IP address from DHCP!!
https://www.spinics.net/linux/fedora/libvirt-users/msg14356.html
A few quick tests proved that it was the same old "bad checksum"
problem from 2010 come back to haunt us - it wasn't a Linux-only issue
after all.
Phil Sutter and Eric Garver (nftables people) pointed out that, while
nftables doesn't have an action that will *compute* the checksum of a
packet, it *does* have an action that will set the checksum to 0, and
suggested we try adding a "zero the checksum" rule for dhcp response
packets to our nftables ruleset. (Why? Because a checksum value of 0
in a IPv4 UDP packet is defined by RFC768 to mean "no checksum
generated", implying "checksum not needed"). It turns out that this
works - dhclient properly recognizes that a 0 checksum means "don't
bother with the checksum", and accepts the packet as valid.
So to once again fix this timeless bug, this patch adds such a
checksum zeroing rule to the nftables rules setup for each virtual
network.
This has been verified (on a Fedora 40 host) to fix DHCP with FreeBSD
and OpenBSD guests, while not breaking it for Fedora or Windows (10)
guests.
Fixes: b89c4991daa0ee9371f10937fab3b03c5ffdabc6
Reported-by: Rich Jones <rjones@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: Daniel P. Berrangé <berrange@redhat.com>
Any time the firewalld zone for an interface is set, by definition
that removes it from any previous zone that it was in, so there is
really no point in unsetting the zone if it's just going to be
immediately set again.
This is useful because when firewalld reloads its rules, 3 things happen:
1) firewalld flushes *all* firewall rules (including those added by libvirt)
2) firewalld unsets the zones for all interfaces (including those set
by libvirt)
3) firewalld re-adds its own rules, and sets the zone for all the
interfaces it manages
4) firewalld sends a dbus message that libvirt is watching for, and
when libvirt receives that message, it reloads all of the
libvirt-generated rules, and also re-sets the firewalld zone for
the bridge interfaces managed by libvirt.
libvirt accomplishes step 4 by a) calling
networkRemoveFirewallRules(), and then b) calling
networkAddFirewallRules(). But (because it is useful in other
contexts) networkRemoveFirewallRules() will attempt to *unset* the
zone for each bridge interface, and when firewalld receives this
request, it sees that the bridge interface *has no zone* (because it
was unset by firewalld in step (2) above), and thus logs an error
message.
There is no way for libvirt to suppress an error message that is
logged by firewalld when a request to firewalld fails. But what
libvirt *can* do is realize that in these cases, the firewalld zone is
about to be set again anyway, and so we don't need to unset the zone.
This patch handles that by adding a bool unsetZone to the arguments of
networkRemoveFirewallRules(); most calls to networkRemoveFirewallRules()
have unsetZone=true, but in two cases where the zone is about to be
reset, networkRemoveFirewallRules() is called with unsetZone=false,
which prevents the call to virFirewallDInterfaceUnsetZone() and thus
avoids the unnecessary (and confusing to users!) error message that
would have been logged by firewalld.
Signed-off-by: Laine Stump <laine@redat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The most common "error" when trying to unset the firewalld zone of an
interface is for firewalld to tell us that the interface already isn't
in any zone. Since this is what we want, no need to alarm the user by
logging it as an error.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When a CPU model is reported as usable='no' an additional
<blockers model='...'> element is added for that CPU model to show which
features are missing for the CPU model to become usable.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
