Boris Fiuczynski [Tue, 23 Apr 2024 18:08:50 +0000 (20:08 +0200)]
nodedev: reset active config data on udev remove event
When a mdev device is destroyed or stopped the udev remove event
handling needs to reset the active config data of the node object
representing a persisted mdev.
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Boris Fiuczynski [Tue, 23 Apr 2024 18:08:49 +0000 (20:08 +0200)]
nodedev: immediate update of active config on udev events
When an udev add, change or remove event occurs the mdev active config data
requires an update via mdevctl as the udev does not contain all config data.
This update needs to occur immediately and to be finished before the libvirt
nodedev event is issued to keep the API usage reliable.
After this change, scheduleMdevctlUpdate call is already called in
`udevAddOneDevice` and can therefore be removed in `udevHandleOneDevice`.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Marc Hartmayer [Tue, 23 Apr 2024 18:08:48 +0000 (20:08 +0200)]
node_device_udev: Set @def to NULL
@def is owned by @obj after adding it the node device object list. As soon as
the @obj lock has been released, another thread could free @obj and therefore
@def. If now someone accesses @def this would lead to a heap-use-after-free and
therefore most likely to a segmentation fault, therefore set @def to NULL after
the ownership has moved.
While at it, add comments to other code places why @def is set to NULL.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Boris Fiuczynski [Tue, 23 Apr 2024 18:08:47 +0000 (20:08 +0200)]
nodedev: fix mdev add udev event data handling
Two situations will trigger an udev add event:
1) the mdev is created when started (transient) or
2) the mdev was defined and is started
In case 1 there is no node object existing and no config data is copied.
In case 2 copying the active config data of an existing node object will
only copy invalid data. Instead copying the defined config data will
store valid data into the newly added node object.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Adam Julis [Tue, 18 Jun 2024 09:56:05 +0000 (11:56 +0200)]
qemu: implement iommu coldplug/unplug
Resolves: https://issues.redhat.com/browse/RHEL-23833 Signed-off-by: Adam Julis <ajulis@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Adam Julis [Mon, 17 Jun 2024 13:51:53 +0000 (15:51 +0200)]
qemu_driver: add validation of potential dependencies on cold plug
Although virDomainDeviceDefValidate() is called as a part of
parsing device XML routine, it validates only that single device.
The virDomainDefValidate() function performs a more comprehensive
check. It should detect errors resulting from dependencies
between devices, or a device and some other part of XML config.
Therefore, a call to virDomainDefValidate() is added at the end
of qemuDomainAttachDeviceConfig().
Signed-off-by: Adam Julis <ajulis@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There are two scenarios identified after the recent firewall backend
selection was introduced, which result in libvirtd failing to startup
due to an inability to find either iptables/nftables
- On Linux if running unprivileged with $PATH lacking the dir
containing iptables/nftables
- On non-Linux where iptables/nftables never existed
In the former case, it is preferrable to restore the behaviour whereby
the driver starts successfully. Users will get an error reported when
attempting to start any virtual network, due to the lack of permissions
needed to create bridge devices. This makes the missing firewall backend
irrelevant.
In the latter case, the network driver calls the 'nop' platform
implementation which does not attempt to implement any firewall logic,
just allowing the network to start without firewall rules.
To solve this are number of changes are required
* Introduce VIR_FIREWALL_BACKEND_NONE, which does nothing except
report a fatal error from virFirewallApply(). This code path
is unreachable, since we'll never create a virFirewall
object with with VIR_FIREWALL_BACKEND_NONE, so the error reporting
is just a sanity check.
* Ignore the compile time backend defaults and assume use of
the 'none' backend if running unprivileged.
This fixes the first regression, avoiding the failure to start
libvirtd on Linux in unprivileged context, instead allowing use
of the driver and expecting a permission denied when creating a
bridge.
* Reject the use of compile time backend defaults no non-Linux
and hardcode the 'none' backend. The non-Linux platforms have
no firewall implementation at all currently, so there's no
reason to permit the use of 'firewall_backend_priority'
meson option.
This fixes the second regression, avoiding the failure to start
libvirtd on non-Linux hosts due to non-existant Linux binaries.
* Change the Linux platform backend to raise an error if the
firewall backend is 'none'. Again this code path is unreachable
by default since we'll fail to create the bridge before getting
here, but if someone modified network.conf to request the 'none'
backend, this will stop further progress.
* Change the nop platform backend to raise an error if the
firewall backend is 'iptables' or 'nftables'. Again this code
path is unreachable, since we should already have failed to
find the iptables/nftables binaries on non-Linux hosts, so
this is just a sanity check.
* 'none' is not permited as a value in 'firewall_backend_priority'
meson option, since it is conceptually meaningless to ask for
that on Linux.
NB, 'firewall_backend_priority' allows repeated options temporarily,
which we don't want. Meson intends to turn this into a hard error
DEPRECATION: Duplicated values in array option is deprecated. This will become a hard error in the future.
and we can live with the reduced error checking until that happens.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
These projects are not strictly part of libvirt, but are closely related
with many of the same developers and we manage them with 'lcitool
manifest' so it is useful to have them on the dashboard.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Thu, 16 May 2024 11:19:32 +0000 (13:19 +0200)]
virprocess: Debug affinity map in virProcessSetAffinity()
The aim of virProcessSetAffinity() is to set affinity of given
process to given CPUs. While we currently print the PID into
logs, the CPU map is not printed. It may help when debugging
weird scenarios.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 16 May 2024 11:09:02 +0000 (13:09 +0200)]
qemu_process: Issue an info message when subtracting isolcpus
In one of my previous commits I've made us substract isolcpus
from all online CPUs when setting affinity on QEMU threads. See
commit below for more info on that. Nevertheless, this is
something that surely deserves an entry in log. I've chosen INFO
priority for now. We can promote that to a regular WARN if users
complain.
Fixes: da95bcb6b2d9b04958e0f2603202801dd29debb8 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
We currently hardcode the systemd sysusersdir, but it is desirable to be
able to choose a different location in some cases. For example, Fedora
flatpak builds change the RPM %_sysusersdir macro, but we can't currently
honour that.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reported-by: Yaakov Selkowitz <yselkowi@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 11 Jun 2024 14:14:02 +0000 (16:14 +0200)]
qemucapabilitiestest: Update test data for qemu 9.1 dev cycle
Update to v9.0.0-1388-g80e8f06021 plus a patch from upstream fixing a
crash when probing, which has no impact on the data.
Notable changes:
- 'MEM_UNPLUG_ERROR' event removed
- 'discard-source' argument for 'blockdev-backup' added
- 'sev-snp-guest' QOM object added
- 'query-sev' now returns variants of the return object based on sev
type
- removed deprecated 'vcpu' field from trace-event infrastructure
- 'scsi' option of 'virtio-blk-pci' removed
(a variant of 'virtio-lun' qemuxmlconftest case was pinned to the
previous version to continue testing the positive use case)
- new cpu features:
'fred', 'succor', 'vmx-nested-exception', 'lkgs', 'overflow-recov',
'wrmsrns'
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virt-pki-validate command can validate the system certificate
directories. The remote driver, however, also supports a standard
per-user certs location, as well as a runtime custom path. This
extends the validation tool to be able to cope with these alternate
locations too.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virt-pki-validate tool is currently a shell script. We have a
general goal of eliminating use of shell in the project. By doing a
new implementation in C, we can also make use of our more thorough
sanity checking code to validate the certificate setup.
This new implementation the same output format as the host validation
tool for a more consistent user experiance.
It also eliminates the requirement to have certtool installed on
libvirt hosts, which has been an issue for Fedora flatpak packages
since certtool isn't in the default platform runtime.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The /etc/sysconfig/libvirtd file is a Fedora/RHEL specific concept.
Since those distros switched to systemd socket activation, the
existance of --listen parameter in /etc/sysconfig/libvirtd is no
longer a reliable check. This was further degraded with the switch
to modular daemons where virtproxyd takes over the role.
The /etc/sysconfig/iptables file is a Fedora/RHEL specific concept.
Since those distros switched to firewalld, this file is no longer
a reliable check.
Rather than complicating these checks, just remove them, so that
the virt-pki-validate tool focuses exclusively on TLS configuration
validation.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
* docs/libvir.html docs/remote.html: update the remote page,
add an index
* docs/pki_check.sh: shell script to check the PKI and client/server
environment.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Georgia Garcia [Tue, 4 Jun 2024 17:34:56 +0000 (14:34 -0300)]
virt-aa-helper: use 'include if exists' on .files
Change the 'include' in the AppArmor policy to use 'include if exists'
when including <uuid>.files. Note that 'if exists' is only available
after AppArmor 3.0, therefore a #ifdef check must be added.
When the <uuid>.files is not present, there are some failures in the
AppArmor tools like the following, since they expect the file to exist
when using 'include':
ERROR: Include file /etc/apparmor.d/libvirt/libvirt-8534a409-a460-4fab-a2dd-0e1dce4ff273.files not found
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We currently hardcode the systemd unitdir, but it is desirable to be
able to choose a different location in some cases. For examples, Fedora
flatpak builds change the RPM %_unitdir macro, but we can't currently
honour that.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We link to libsasl2.so, so get a dep on cyrus-sasl-libs automatically.
The dep on cyrus-sasl-gssapi gets us the mechanism that matches our
default config.
The 'cyrus-sasl' package merely contains some man pages and the
saslauthd daemon, which is not required by libvirt. This dep appears
to have been redundant since we first added in
Andrea Bolognani [Mon, 27 May 2024 16:38:52 +0000 (18:38 +0200)]
qemu: Reject TPM 1.2 in most scenarios
Everywhere we use TPM 2.0 as our default, the chances of TPM
1.2 being supported by the guest OS are very slim. Just reject
such configurations outright.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Andrea Bolognani [Mon, 27 May 2024 16:37:58 +0000 (18:37 +0200)]
tests: Add TPM coverage to default-models tests
We have a non-trivial amount of architecture-specific logic
dealing with TPM, so it's good to have coverage for it.
Note that two architectures currently don't have support for
TPM devices enabled by default in QEMU: loongarch64 and s390x.
The situation might change for the former, but that's unlikely
to happen for the latter.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
rpm: ensure -Werror is disabled for mingw builds on Fedora
This copies the behaviour of the native builds that disable -Werror
on Fedora, since frequently updating toolchains and deps often
introduce new warnings.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemumonitortestutils: Fix G_GNUC_PRINTF annotation of qemuMonitorTestAddErrorResponse()
The qemuMonitorTestAddErrorResponse() function is a printf-like
function. But the annotation was mistakenly done in .c file
instead of corresponding .h file rendering the annotation
ineffective. Move the annotation to the header file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
While __attribute((sentinel)) (exposed by glib under
G_GNUC_NULL_TERMINATED macro) is a gcc extension, it's supported
by clang too. It's already being used throughout our code but
some functions that take variadic arguments and expect NULL at
the end were lacking such annotation. Fill them in.
After this, there are still some functions left untouched because
they expect a different sentinel than NULL. Unfortunately, glib
does not provide macro for different sentinels. We may come up
with our own, but let's save that for future work.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
interface: fix udev reference leak with invalid flags
The udevInterfaceGetXMLDesc method takes a reference on the udev
driver as its first action. If the virCheckFlags() condition
fails, however, this reference is never released.
Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
rpc: avoid leak of GSource in use for interrupting main loop
We never release the reference on the GSource created for
interrupting the main loop, nor do we remove it from the
main context if our thread is woken up prior to the wakeup
callback firing.
This can result in a leak of GSource objects, along with an
ever growing list of GSources attached to the main context,
which will gradually slow down execution of the loop, as
several operations are O(N) for the number of attached GSource
objects.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Refusing to change selinux context of file './src/libvirtd' outside build directory
which is obviously wrong. The problem is 'being inside of build
directory' is detected by simple progpath.startswith(builddir).
While builddir is an absolute path, progpath isn't necessarily.
And while looking into the code, I've noticed chcon() function
accessing variable outside its scope when printing out the path
it's working on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Fri, 31 May 2024 13:06:45 +0000 (15:06 +0200)]
ci: Always run 'pages' job
When the 'pages' job is configured to run 'on_success' it's skipped if
any other pipeline fails. This is bad in cases such as if an external
service runs out of CI minutes as the web stops being updated.
Since the 'artifacts' of the 'website_job' are generated only if that
phase succeeds this will update the web when the web part is buildable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Thu, 30 May 2024 14:30:47 +0000 (16:30 +0200)]
qemu: validate: Fix check for unsupported FS-device bootindex use on un-assigned addresses
When hot-plugging a FS device with un-assigned address with a bootindex
the recently-added validation check would fail as validation on hotplug
is done prior to address assignment.
To fix this problem we can simply relax the check to also pass on _NONE
addresses. Unsupported configurations will still be caught as previous
commit re-checks the definition after address assignment prior to
hotplug.
Resolves: https://issues.redhat.com/browse/RHEL-39271 Fixes: 4690058b6d3dab672bd18ff69c83392245253024 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 29 May 2024 07:24:44 +0000 (09:24 +0200)]
log_cleaner: Detect rotated filenames properly
When removing rotated log files, their name is matched against a
regex (@log_regex) and if they contain '.N' suffix the 'N' is
then parsed into an integer. Well, due to a bug in
virLogCleanerParseFilename() this is not how the code works. If
the suffix isn't found then g_match_info_fetch() returns an empty
string instead of NULL which then makes str2int parsing fail.
Just check for this case before parsing the string.
Based on the original patch sent by David.
Reported-by: David Negreira <david.negreira@canonical.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Michal Privoznik [Thu, 30 May 2024 09:51:07 +0000 (11:51 +0200)]
qemu_hotplug: Clear QoS if required in qemuDomainChangeNet()
In one of my recent commits, I've introduced
virDomainInterfaceClearQoS() which is a helper that either calls
virNetDevBandwidthClear() ('tc' implementation) or
virNetDevOpenvswitchInterfaceClearQos() (for ovs ifaces). But I
made a micro optimization which leads to a bug: the function
checks whether passed iface has any QoS set and returns early if
it has none. In majority of cases this is right thing to do, but
when removing QoS on virDomainUpdateDeviceFlags() this is
problematic. The new definition (passed as argument to
virDomainInterfaceClearQoS()) contains no QoS (because user
requested its removal) and thus instead of removing the old QoS
setting nothing is done.
Fortunately, the fix is simple - pass olddev which contains the
old QoS setting.
Fixes: 812a146dfe784315edece43d09f8d9e432f8230e Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In addition to being more verbose than one would hope, there
are several things that could go wrong.
First of all, meson performs no validation on the provided
values, so mistakes will only be caught by the compiler.
Additionally, it's entirely possible to provide nonsensical
combinations, such as repeating the same value twice.
Change things so that the preference can now be configured
as such:
-Dfirewall_backend_priority=iptables,nftables
Checks have been added to prevent invalid values from being
accepted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Feature additions:
- 9.1 machine type added
- 'SierraForest' cpu type added
- 'SapphireRapids-v3-x86_64-cpu' added
- 'VFIO_MIGRATION' event added (and corresponding 'migration-events'
bool for the device
- 'exit-on-error' argument for 'migrate-incoming' added
- 'sev-guest' gained 'legacy-vm-type' boolean
- cpu topology added 'module' fields
- 'compat-props' argument 'query-machines' added
- 'deprecated-props' argument for 'query-cpu-model-expansion' added
Deprecated removals:
- legacy non-shared-storage migration fully removed (config/stats)
- legacy migration compression fully removed
- RDMA support removed
- dropped 'nios2' field type from 'query-cpus-fast' return data
Note that this dump was done on a newer kernel version which resulted in
the 'pcommit' feature being removed from the few test cases which depend
on the real CPU flag dump.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Michal Privoznik [Mon, 27 May 2024 09:06:41 +0000 (11:06 +0200)]
vsh: Don't crash when @text is NULL in vshCompleterFilter()
This can happen only for cmdComplete() in interactive mode (which
I'm still not convinced is any useful for users and whether we
should support it). Anyway, running plain 'complete' command with
no additional arguments boils down to @text being NULL in
vshReadlineParse() which handles the case just right but is then
subsequently passed to vshCompleterFilter() which isn't prepared
for this case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Mon, 27 May 2024 08:39:21 +0000 (10:39 +0200)]
vsh: Restore original rl_line_buffer after completion
Problem with readline is its API. It's basically a bunch of
global variables with no clear dependencies between them. In this
specific case that I'm seeing: in interactive mode the
cmdComplete() causes instant crash of virsh/virt-admin:
==27999== Invalid write of size 1
==27999== at 0x516EF71: _rl_init_line_state (readline.c:742)
==27999== by 0x5170054: rl_initialize (readline.c:1192)
==27999== by 0x516E5E4: readline (readline.c:379)
==27999== by 0x1B7024: vshReadline (vsh.c:3048)
==27999== by 0x140DCF: main (virsh.c:905)
==27999== Address 0x0 is not stack'd, malloc'd or (recently) free'd
This is because readline keeps a copy of pointer to
rl_line_buffer and the moment cmdComplete() returns and readline
takes over, it accesses the copy which is now a dangling pointer.
To fix this, just keep the original state of rl_line_buffer and
restore it.
Fixes: 41400ac1dda55b817388a4050aa823051bda2e05 Fixes: a0e1ada63c0afdc2af3b9405cbf637d8bd28700c Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Thu, 23 May 2024 13:49:05 +0000 (15:49 +0200)]
vsh: Close stderr among with stdin in cmdComplete
Our completer callbacks must refrain from printing anything onto
stderr, but unfortunately that's not how service code around
behaves. It may call vshError() and what not. Rather trying to
fix all possible paths (just consider opening a connection), just
close the stderr. We're already closing stdin.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Michal Privoznik [Thu, 23 May 2024 13:41:16 +0000 (15:41 +0200)]
bash-completion: Run virsh/virt-admin in quiet mode
In some cases (e.g. when virt-admin connects to the default URI)
some info message is printed onto stdout (using vshPrintExtra()).
This hurts user experience, just consider:
Laine Stump [Thu, 23 May 2024 03:13:33 +0000 (23:13 -0400)]
network: use iif/oif instead of iifname/oifname in nftables rules
iifname/oifname need to lookup the string that contains the name of
the interface each time a packet is checked, while iif/oif compare the
ifindex of the interface, which is included directly in the
packet. Conveniently, the rule is created using the *name* of the
interface (which gets converted to ifindex as the rule is added), so
no extra work is required other than changing the commandline option.
If it was the case that the interface could be deleted and re-added
during the life of the rule, we would have to use Xifname (since
deleting and re-adding the interface would result in ifindex
changing), but for our uses this never happens, so Xif works for us,
and undoubtedly improves performance by at least 0.0000001%.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Unfortunately in 'virFileOpenForked' the return value was used to report
the error back to the main process from the fork'd child. As process
return codes are positive only, the code negates the value of 'ret' and
reports it. This resulted in the parent thinking the process exited with
failure:
# virsh save avocado-vt-vm1 /mnt/save
error: Failed to save domain 'avocado-vt-vm1' to /mnt/save
error: Error from child process creating '/mnt/save': Unknown error 255
This error reproduces on NFS mounts with 'root_squash' enabled. I've
also observed it in one specific migration case when root_squash NFS is
used with following error:
Failed to open file '/var/lib/libvirt/images/alpine.qcow2': Unknown error 255'
To fix the issue the code is refactored so that it doesn't actually
touch the 'ret' variable needlessly and assigns to it only on failure
cases, which prevents the '1' to be propagated to the parent process as
'255' after negating and storing in the process return code.
Peter Krempa [Mon, 20 May 2024 12:56:12 +0000 (14:56 +0200)]
virt-host-validate: Improve translatability of messages printed by 'virHostMsgCheck()'
Move the word 'Checking' into the appropriate formatting strings and
mark all outstanding ones for translation.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/637 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Mon, 29 Apr 2024 18:47:05 +0000 (14:47 -0400)]
network: eliminate pointless host input/output rules from nftables backend
The iptables backend (which was used as the model for the nftables
backend) used the same "filter" and "nat" tables used by other
services on the system (e.g. firewalld or any other host firewall
management application), so it was possible that one of those other
services would be blocking DNS, DHCP, or TFTP from guests to the host;
we added our own rules at the beginning of the chain to allow this
traffic no matter if someone else rejected it later.
But with nftables, each service uses their own table, and all traffic
must be acepted by all tables no matter what - it's not possible for
us to just insert a higher priority/earlier rule that will override
some reject rule put in by, e.g., firewalld. Instead the firewalld (or
other) table must be setup by that service to allow the traffic. That,
along with the fact that our table is already "accept by default",
makes it possible to eliminate the individual accept rules for DHCP,
DNS, and TFTP. And once those rules are eliminated, there is no longer
any need for the guest_to_host or host_to_guest tables.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Mon, 29 Apr 2024 18:21:14 +0000 (14:21 -0400)]
network: rename chains used by network driver nftables backend
Because the chains added by the network driver nftables backend will
go into a table used only by libvirt, we don't need to have "libvirt"
in the chain names. Instead, we can make them more descriptive and
less abrasive (by using lower case, and using full words rather than
abbreviations).
Also (again because nobody else is using the private "libvirt_network"
table) we can directly put our rules into the input ("guest_to_host"),
output ("host_to_guest"), and postrouting ("guest_nat") chains rather
than creating a subordinate chain as done in the iptables backend.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Mon, 29 Apr 2024 18:06:55 +0000 (14:06 -0400)]
network: name the nftables table "libvirt_network" rather than "libvirt"
This way when we implement nftables for the nwfilter driver, we can
create a separate table called "libvirt_nwfilter" and everything will
look all symmetrical and stuff.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Sat, 20 Apr 2024 03:25:34 +0000 (23:25 -0400)]
spec: require nftables rather than iptables for newer RHEL/Fedora
It will still be possible to install iptables and use the iptables
backend, but we'll be showing a greater preference for nftables, which
is the proper thing to be doing.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Sun, 21 Apr 2024 18:10:37 +0000 (14:10 -0400)]
network: prefer the nftables backend over iptables
The patch that added the nftables backend for virtual networks left
iptables as the default backend when both nftables and iptables are
installed.
The only functional difference between the two backends is that the
nftables backend doesn't add any rules to fix up the checksum of DHCP
packets, which will cause failures on guests with very old OSes
(e.g. RHEL5) that have a virtio-net network interface using vhost
packet processing (the default), connected to a libvirt virtual
network, and configured to acquire the interface IP using DHCP. Since
RHEL5 has been out of support for several years already, we might as
well start off nftables support right by making it the default.
Distros that aren't quite ready to default to nftables (e.g. maybe
they're rebasing libvirt within a release and don't want to surprise
anyone with an automatic switch from iptables to nftables) can simply
run meson with "-Dfirewall_backend=iptables" during their official
package build.
In the extremely unlikely case that this causes a problem for a user,
they can work around the failure by adding "<driver name='qemu'/> to
the guest <interface> element.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Sat, 20 Apr 2024 02:19:43 +0000 (22:19 -0400)]
tests: test cases for nftables backend
Run all the networkxml2firewall tests twice - once with iptables
backend, and once with the nftables backend.
The results files for the existing iptables tests were previously
named *.args. That has been changed to *.iptables, and the results
files for the new nftables tests are named *.nftables.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Sat, 20 Apr 2024 02:19:43 +0000 (22:19 -0400)]
network: add an nftables backend for network driver's firewall construction
Support using nftables to setup the firewall for each virtual network,
rather than iptables. The initial implementation of the nftables
backend creates (almost) exactly the same ruleset as the iptables
backend, determined by running the following commands on a host that
has an active virtual network:
(and the similar ip6tables-save/ip6tables-restore-translate for an
IPv6 network). Correctness of the new backend was checked by comparing
the output of:
nft list ruleset
when the backend is set to iptables and when it is set to nftables.
The only differences between the rules created by the nftables backed
vs. the iptables backend (aside from a few inconsequential changes in
display order of some chains/options) are:
1) When we add nftables rules, rather than adding them in the
system-created "filter" and "nat" tables, we add them in a private
table (ie only we should be using it) created by us called "libvirt"
(the system-created "filter" and "nat" tables can't be used because
adding any rules to those tables directly with nft will cause failure
of any legacy application attempting to use iptables when it tries to
list the iptables rules (e.g. "iptables -S").
(NB: in nftables only a single table is required for both nat and
filter rules - the chains for each are differentiated by specifying
different "hook" locations for the toplevel chain of each)
2) Since the rules that were added to allow tftp/dns/dhcp traffic from
the guests to the host are unnecessary in the context of nftables,
those rules aren't added.
(Longer explanation: In the case of iptables, all rules were in a
single table, and it was always assumed that there would be some
"catch-all" REJECT rule added by "someone else" in the case that a
packet didn't match any specific rules, so libvirt added these
specific rules to ensure that, no matter what other rules were added
by any other subsystem, the guests would still have functional
tftp/dns/dhcp. For nftables though, the rules added by each subsystem
are in a separate table, and in order for traffic to be accepted, it
must be accepted by *all* tables, so just adding the specific rules to
libvirt's table doesn't help anything (as the default for the libvirt
table is ACCEPT anyway) and it just isn't practical/possible for
libvirt to find *all* other tables and add rules in all of them to
make sure the traffic is accepted. libvirt does this for firewalld (it
creates a "libvirt" zone that allows tftp/dns/dhcp, and adds all
virtual network bridges to that zone), however, so in that case no
extra work is required of the sysadmin.)
3) nftables doesn't support the "checksum mangle" rule (or any
equivalent functionality) that we have historically added to our
iptables rules, so the nftables rules we add have nothing related to
checksum mangling.
(NB: The result of (3) is that if you a) have a very old guest (RHEL5
era or earlier) and b) that guest is using a virtio-net network
device, and c) the virtio-net device is using vhost packet processing
(the default) then DHCP on the guest will fail. You can work around
this by adding <driver name='qemu'/> to the <interface> XML for the
guest).
There are certainly much better nftables rulesets that could be used
instead of those implemented here, and everything is in place to make
future changes to the rules that are used simple and free of surprises
(e.g. the rules that are added have coresponding "removal" commands
added to the network status so that we will always remove exactly the
rules that were previously added rather than trying to remove the
rules that "the current build of libvirt would have added" (which will
be incorrect the first time we run a libvirt with a newly modified
ruleset). For this initial implementation though, I wanted the
nftables rules to be as identical to the iptables rules as possible,
just to make it easier to verify that everything is working.
The backend can be manually chosen using the firewall_backend setting
in /etc/libvirt/network.conf. libvirtd/virtnetworkd will read this
setting when it starts; if there is no explicit setting, it will check
for availability of FIREWALL_BACKEND_DEFAULT_1 and then
FIREWALL_BACKEND_DEFAULT_2 (which are set at build time in
meson_options.txt or by adding -Dfirewall_backend_default_n=blah to
the meson commandline), and use the first backend that is available
(ie, that has the necessary programs installed). The standard
meson_options.txt is set to check for nftables first, and then
iptables.
Although it should be very safe to change the default backend from
iptables to nftables, that change is left for a later patch, to show
how the change in default can be undone if someone really needs to do
that.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Tested-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Sat, 20 Apr 2024 02:19:43 +0000 (22:19 -0400)]
meson: stop looking for iptables/ip6tables/ebtables at build time
This was the only reason we required the iptables and ebtables
packages at build time, and many other external commands already have
their binaries found at runtime by looking through $PATH (virCommand
automatically does this), so we may as well do it for these commands
as well.
Since we no longer need iptables or iptables at build time, we can
also drop the BuildRequires for them from the rpm specfile.
Inspired-by: 6aa2fa38b04b802f137e51ebbeb4ca9b67487575 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Sat, 20 Apr 2024 02:19:42 +0000 (22:19 -0400)]
network: save network status when firewall rules are reloaded
In the case that a new version of libvirt is started that uses
different rules to build the network firewall, we need to re-save the
status so that when the network is destroyed (or the *next* time
libvirt is restarted and wants to remove/re-add the firewall), it will
have the proper information to perform the firewall removal.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
projects / libvirt.git / log