Andrea Bolognani [Tue, 17 Sep 2024 12:28:30 +0000 (14:28 +0200)]
rpm: Add riscv64 to arches_qemu_kvm
The riscv64 architecture is not yet fully integrated into
Fedora, but KVM support is already implemented across the stack
and the Fedora package for QEMU is already set up to generate
the qemu-kvm binary package when targeting it.
Thanks: David Abdurachmanov <davidlt@rivosinc.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Laine Stump [Wed, 4 Sep 2024 19:17:25 +0000 (15:17 -0400)]
network: *un*set the firewalld zone while shutting down a network
When a bridge device for a virtual network had been placed in a
firewalld zone while starting the network, then even after the network
is shut down and the bridge device is deleted, its name will still
show up in the list of interfaces for whichever zone it had been in,
and this setting will persist through the next time a device with the
same name is created (until a zone is once again explicitly set, or
the device is removed via a firewalld API call).
Usually this isn't a problem, but in the case of forward mode='open',
someone might start the network once with a zone specified, then
shut down the network, remove the zone from its config, and start it
again; in this case the bridge device would come up using the zone
from the previous time it was started.
The solution to this is to remove the interface from whatever zone it
is in as the network is being shut down. There is no downside to doing
this, since the device is going to be deleted anyway. Note that
forward mode='bridge' uses a bridge device that was created outside of
libvirt, and libvirt won't be deleting that bridge, so we take care to
not unset the zone in that case.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Laine Stump [Thu, 5 Sep 2024 15:56:10 +0000 (11:56 -0400)]
network: remove firewalld version check from networkSetBridgeZone()
At the time the version check in this function was written, there were
still several supported versions of some distros that were using a
version of firewalld too old to support the "rich rule priorities"
used by the 'libvirt' zone that we installed for firewalld. Today the
newest distro that has a version of firewalld < 0.7.0 is
RHEL7/CentOS7, so we can remove the complexity and if the libvirt zone
is missing simply say "the libvirt zone is missing".
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Laine Stump [Tue, 3 Sep 2024 01:38:50 +0000 (21:38 -0400)]
network: support setting firewalld zone for bridge device of open networks
The bit of code that sets the firewalld zone was previously a part of
the function networkAddFirewallRules(), which is not called for
networks with <forward mode='open'/>.
Setting the 'libvirt' zone for the bridge device of virtual networks
that also add firewall rules is usually necessary in order to get the
expected traffic through without modifying firewalld's default zone
(which would be a bad idea, because that would affect all the other
host interfaces set to the default zone), but in general we would
*not* want the bridge device for a mode='open' virtual network to be
automatically placed in the "libvirt" zone. However, a user might want
to *explicitly* set some other firewalld zone for mode='open'
networks, and libvirt's network config is a convenient place to do
that.
We enable this by moving the code that sets the firewalld zone into a
separate function that is called for all forward modes that use a
bridge device created/managed by libvirt (nat, route, isolated,
open). If no zone is specified, then the bridge device will be in
whatever zone interfaces are put in by default, but if the <bridge>
element has a "zone" attribute, then the new bridge device will be
placed in the specified zone.
NB: This function is only called when the network is started, and
*not* when the firewall rules of an active network are reloaded at
virtnetworkd restart time, because the firewalld zone of an interface
isn't something that gets inadvertantly changed as a part of some
other unrelated action. For example all iptables rules are cleared by a
firewalld restart, including those rules added by libvirt, but there
is no blanket action that changes the zone of all interfaces, so it's
useful for libvirt to reload its rules when restarting virtnetworkd,
but pointless to re-add the interface to its preferred zone.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/215 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Laine Stump [Mon, 2 Sep 2024 20:13:08 +0000 (16:13 -0400)]
network: permit <forward mode='open'/> when a network has no IP address
The whole point of <forward mode='open'/> is to supress libvirt from
adding any firewall rules for a network, and someone might want to
create a network with no IP address (i.e. they don't want the guests
to have connectivity to the host via this interface) and no firewall
rules (they don't want any, or they want to add their own). So there's
no reason to fail when a network has <forward mode='open'/> and also
has no IP address.
Kind-of-Resolves: https://gitlab.com/libvirt/libvirt/-/issues/588 Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
network: Try to read dnsmasq PIDs for inactive networks too
Just in case one needs a clean up.
Resolves: https://issues.redhat.com/browse/RHEL-50968 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
network: Clean up after inactive objects during start
Once networkUpdateState() identifies a dead network it should clean up
after it as well.
Resolves: https://issues.redhat.com/browse/RHEL-50968 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
network: Do not update network ports for inactive networks
The semantic does not change since inside networkUpdatePort() (well,
networkNotifyPort, for which the former is a wrapper) exits for inactive
networks, but with an error we can easily avoid with this patch.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
Andrea Bolognani [Mon, 16 Sep 2024 14:39:11 +0000 (16:39 +0200)]
apparmor: Don't check for existence of templates upfront
Currently, if either template is missing AppArmor support is
completely disabled. This means that uninstalling the LXC
driver from a system results in QEMU domains being started
without AppArmor confinement, which obviously doesn't make any
sense.
The problematic scenario was impossible to hit in Debian until
very recently, because all AppArmor files were shipped as part
of the same package; now that the Debian package is much closer
to the Fedora one, and specifically ships the AppArmor files
together with the corresponding driver, it becomes trivial to
trigger it.
Drop the checks entirely. virt-aa-helper, which is responsible
for creating the per-domain profiles starting from the
driver-specific template, already fails if the latter is not
present, so they were always redundant.
resctrl: Do not rewrite default MB values for new allocations
The code did it "just in case" the allocation was not reset for new
subdirectories. That might've happened in the past with CAT settings,
but checking it now it is properly reset to its maximum values for each
new CLOSID (Class of Service ID).
The advantage of this is that we do not rewrite the value with itself
which causes an issue with the current linux kernel and mba_MBps option
where the default is UINT_MAX (or (uint32_t) -1), but gets rounded up to
bandwidth granularity (10), overflows and small number (4) is set
instead.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Mon, 16 Sep 2024 08:29:40 +0000 (10:29 +0200)]
Revert "vircommand: Parse /dev/fd on *BSD-like systems when looking for opened FDs"
Unfortunately, devfs on FreeBSD (accessible via /dev/fd) exposes
only those FDs which can be represented as a file. To cite
manpage [1]:
The files /dev/fd/0 through /dev/fd/# refer to file descriptors
which can be accessed through the file system.
This means FDs representing pipes and/or unnamed sockets are not
visible by default. To expose all FDs a slightly different
filesystem must be mounted [2]:
mount -t fdescfs none /dev/fd
Apparently, on my test machine fdescfs is mounted by default and
thus I haven't seen any problem. Only after aforementioned patch
was merged our CI started reporting problems. While we could try
to figure out whether correct FS is mounted, it's a needless
micro optimization. Just revert the code to the state it was
before I touched it.
Michal Privoznik [Tue, 29 Aug 2023 06:49:27 +0000 (08:49 +0200)]
vircommand: Parse /dev/fd on *BSD-like systems when looking for opened FDs
On BSD-like systems "/dev/fd" serves the same purpose as
"/proc/self/fd". And since procfs is usually not mounted, on such
systems we can use "/dev/fd" instead.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/518 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Tue, 29 Aug 2023 07:22:09 +0000 (09:22 +0200)]
vircommand: Make sysconf(_SC_OPEN_MAX) failure non-fatal
The point of calling sysconf(_SC_OPEN_MAX) is to allocate big
enough bitmap so that subsequent call to
virCommandMassCloseGetFDsDir() can just set the bit instead of
expanding memory (this code runs in a forked off child and thus
using async-signal-unsafe functions like malloc() is a bit
tricky).
But on some systems the limit for opened FDs is virtually
non-existent (typically macOS Ventura started reporting EINVAL).
But with both glibc and musl using malloc() after fork() is safe.
And with sufficiently new glib too, as it's using malloc() with
newer releases instead of their own allocator.
Therefore, pick a sufficiently large value (glibc falls back to
256, [1], Darwin to 10240 [2] so 10240 should be good enough) to
fall back to and make the error non-fatal.
Michal Privoznik [Tue, 29 Aug 2023 06:48:56 +0000 (08:48 +0200)]
vircommand: Isolate FD dir parsing into a separate function
So far, virCommandMassCloseGetFDsLinux() opens "/proc/self/fd",
iterates over it marking opened FDs in @fds bitmap. Well, we can
do the same on other systems (with altered path), like MacOS or
FreeBSD. Therefore, isolate dir iteration into a separate
function that accepts dir path as an argument.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
resctrl: Use cache IDs instead of max_id/max_cache_id
It is not guaranteed for the cache IDs to be continuous, especially for
L3 caches. Hence do not assume so and instead record the individual IDs
in a virBitmap.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
resctrl: Don't assume MBA availability in virResctrlAllocNewFromInfo
Weirdly, the existence of /sys/fs/resctrl/info/MB does not always mean
that MBA is available and used on the system. Instead of assuming that
copy the values from the default (root) allocation. This also makes it
nicer to use the proper values in case the system does not use
percentages or when the root allocation already limits the bandwidth.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
resctrl: Relax the limit of maximum memory bandwidth allocation
The value 100 represented the percentage as it was originally done from
Intel in the Linux kernel and on their CPUs. Since then the situation
changed and there is no error-prone way of figuring out the meaning of
the value in the current configuration, let alone its possible maximum.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
docs: Document memory bandwidth allocation limits more clearly
The meaning of the values as well as their maximums are hard to predict
and accounting for all the possibilities (which by the way might change
during daemon's execution) is borderline hallucinatory. There is
already a way we represent them, which is the same as the Linux kernel.
We do not interpret them at all, just blindly use them. In order to
make this more apparent for the users change the documentation for the
<memorytune/> (not <memtune/>) element more boldly.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
resctrl: Account for memory bandwidth of 0 being valid
In some scenarios the memory bandwidth in the schemata file might be 0
and so can the minimum allocation in other ones. Remove checks which
were added for extra cautiousness.
Resolves: https://issues.redhat.com/browse/RHEL-54235 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Tue, 10 Sep 2024 21:40:13 +0000 (15:40 -0600)]
docs: Clarify hypervisor support for nwfilter profiles
Enhance the 'since' annotation of <filterref> documentation to note
it's only supported by the QEMU, LXC, and ch hypervisor drivers.
Suggested-by: Demi Marie Obenour <demi@invisiblethingslab.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Laine Stump <laine@redhat.com>
documentation: Remove untrue statement in GetVersion() description
The description of virConnectGetVersion() says the function might only
work with a privileged access to the hypervisor, not with a read-only
connection. However that is not true since commit a2e2e4652f29 and can
be safely removed.
Signed-off-by: Stepan Zobal <szobal@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Jakub Palacky [Wed, 11 Sep 2024 12:36:40 +0000 (14:36 +0200)]
util/virutil: Use readpassphrase when libbsd is available
When libbsd is available, use the preferred readpassphrase() function isntead of getpass()
as the getpass() function has been marked as obsolete and shouldnt be used
Signed-off-by: Jakub Palacky <jpalacky@redhat.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When connecting to a VMware server (eg using vpx://) we download and
try to parse the VMware metadata '*.vmx' file of a guest. In this
case a VMX file was found which contained this key:
pciPassthru*.present = "False"
The '*' character was not previously allowed in keys so this failed to
parse with the error:
Resolves: https://issues.redhat.com/browse/RHEL-58446
Thanks: Daniel Berrange Signed-off-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Tom [Tue, 20 Aug 2024 23:30:59 +0000 (23:30 +0000)]
Allow apparmor parser to be executed in /usr/bin
This commit modifies the AppArmor profile for virt-aa-helper to
accommodate an observed behavior in certain Linux distributions,
such as ArchLinux.
In these distributions, /usr/sbin symlinks to /usr/bin. To ensure
that virt-aa-helper can execute apparmor_parser when it resides
in /usr/bin, the profile has been updated accordingly.
Signed-off-by: Tom <libvirt-patch@douile.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Mon, 9 Sep 2024 14:46:09 +0000 (16:46 +0200)]
virDiskNameParse: Fix integer overflow in disk name parsing
The conversion to index entails multiplication and accumulation by user
provided data which can easily overflow, use VIR_MULTIPLY_ADD_IS_OVERFLOW
to check if the string is valid.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/674 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In case when the user specifies the '<hyperv/>' feature multiple times
we could overwrite already parsed data. Clear it beforehand.
As before this isn't trying to address the case of features being
specified multiple times not making much sense.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/675 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 9 Sep 2024 14:46:03 +0000 (16:46 +0200)]
virBitmapShrink: Do not attempt to clear bits beyond end of buffer
'virBitmapShrink' clears the bits beyond the end of the bitmap when
shrinking and then reallocates to match the new size. As it uses the
address of the first bit beyond the bitmap to do the clearing it can
overrun the allocated buffer if we're not actually going to shrink it
and the last bit's address is on the chunk boundary.
Fix it by returning in that corner case and add few more tests to be
sure.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/673 Fixes: d6e582da80d Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 9 Sep 2024 14:46:02 +0000 (16:46 +0200)]
virDomainDefParseBootInitOptions: Don't leak 'name' on failure
One of the failure paths skips code which would assign the string from
the temporary variable to the parsed struct, thus leaking it on failure.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/672 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
In one of recent commits new CPU model was introduced. But
corresponding change in meson.build is missing which results in
the XML file not being installed.
Fixes: 3afbb1644c4f9d5237459bd544d0f511ff99eb80 Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 6 Sep 2024 12:42:25 +0000 (14:42 +0200)]
qemuBackupDiskDataCleanupOne: Don't skip rest of cleanup if we can't enter monitor
Recent fix to use the proper 'async' monitor function would cause
libvirt to leak some of the objects it's supposed to clean up in other
places besides qemu.
Don't skip the whole function on failure to enter the job but just the
monitor section.
Fixes: 9b22c25548aa658acdeac2269ddae32584df32d8 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 5 Sep 2024 12:55:59 +0000 (14:55 +0200)]
qemu: backup: Use 'async' monitor in 'qemuBackupDiskDataCleanupOne'
'qemuBackupDiskDataCleanupOne()' is entering the monitor while we're in
the async backup job inside 'qemuBackupBegin()' which is semantically
wrong and per upstream report causes crashes if some monitoring commands
are run in parallel.
Use qemuDomainObjEnterMonitorAsync() instead.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/668 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Thu, 5 Sep 2024 12:17:38 +0000 (14:17 +0200)]
virsh: cmdList: Revert to script-friendly output for 'virsh list --uuid'
Commit 271940223c2914bf63cbec00930ce46d6eef30ba which strived to add
support to use '--uuid' in the table output of 'virsh list' went too far
and also allowed the default table view to be enabled when just '--uuid'
is specified.
This broke the script-friendly output which previously had this format:
Peter Krempa [Tue, 3 Sep 2024 08:03:04 +0000 (10:03 +0200)]
qemuProcessSetupRawIO: Refactor return value and remove useless #ifdef
The function can return directly rather than setting 'ret' as there's no
cleanup.
It also doesn't make sense to conditionally compile out the 'break'
statement when checking whether a disk has rawio enabled if
'CAP_SYS_RAWIO' is _not_ defined as the function will still behave the
same.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Mon, 2 Sep 2024 12:28:52 +0000 (14:28 +0200)]
spec: Demote 'nfs-utils' as a weak dependency of 'daemon-driver-storage-core'
The 'nfs-utils' package provides 'showmount' used to detect NFS-based
storage pool sources. As the lookup of storage pool sources can fail
gracefully and does so e.g. if the gluster backend is not installed we
can do the same for NFS.
Apart from allowing a tighter footprint when installing libvirt, this
also allows installation of the storage driver core in cases when a
security policy prohibits use of NFS.
Resolves: https://issues.redhat.com/browse/RHEL-56611 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
virnetdevtap: Add better error message for a possible common user error
When users pre-create a tap device to use with multiqueue interface that
has `managed="no"`, change the error so that it does not indicate we are
trying to create the device, and on top of that hint at the most
probable error cause.
Resolves: https://issues.redhat.com/browse/RHEL-55749 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function generates *ifname from the get go and most functions do not
wrap the string in a NULLSTR as it is not necessary. The few leftovers
are outliers that are changed to fit the theme better.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Tue, 27 Aug 2024 14:19:53 +0000 (16:19 +0200)]
qemu: Use pvpanic by default on aarch64
pvpanic-pci is the only reasonable implementation of a panic
device for aarch64/virt guests. Right now we're asking users to
provide the model name manually, but we can be more helpful and
fill it in automatically instead.
With this change, the aarch64-panic-no-model test no longer
fails and so it's no longer useful to us. Instead, we can amend
the aarch64-virt-default-models test case to include panic
coverage, something that until now wasn't possible.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Tue, 27 Aug 2024 14:44:31 +0000 (16:44 +0200)]
qemu: Sometimes the default panic model doesn't exist
Right now the fallback behavior is to use MODEL_ISA if we
haven't been able to find a better match, but that's not very
useful as we're still going to hit an error later, when
QEMU_CAPS_DEVICE_PANIC is not found at Validate time.
Instead of doing that, allow MODEL_DEFAULT to get all the
way to Validate and report an error upon encountering it.
The reported error changes slightly, but other than that the
set of configurations that are allowed and blocked remains
the same.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Tue, 27 Aug 2024 13:03:31 +0000 (15:03 +0200)]
qemu: Refactor default panic model
Perform decisions based on the architecture and machine type
in a single place instead of duplicating them.
This technically adds new behavior for MODEL_ISA in
qemuDomainDefAddDefaultDevices(), but it doesn't make any
difference functionally since we don't set addPanicDevice
outside of ppc64(le) and s390(x). If we did, the lack of
handling for that value would be a latent bug.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Wed, 21 Aug 2024 13:18:31 +0000 (15:18 +0200)]
udevListInterfaces: Honour array length for zero-length NULL arrays (CVE-2024-8235)
The refactor of 'udevListInterfacesByStatus()' which attempted to make
it usable as backend for 'udevNumOfInterfacesByStatus()' neglected to
consider the corner case of 'g_new0(..., 0)' returning NULL if the user
actually requests 0 elements.
As the code was modified to report the full number of interfaces in the
system when the list of names is NULL, the RPC code would be asked to
serialize a NULL-list of interface names with declared lenth of 1+
causing a crash.
To fix this corner case we make callers pass '-1' as @names_len (it's
conveniently an 'int' due to RPC type usage) if they don't wish to fetch
the actual list and convert all decisions to be done on @names_len being
non-negative instead of @names being non-NULL.
CVE-2024-8235
Fixes: bc596f275129bc11b2c4bcf737d380c9e8aeb72d
Resolves: https://issues.redhat.com/browse/RHEL-55373 Reported-by: Yanqiu Zhang <yanqzhan@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
NEWS: Add an entry for network support in ch driver.
Signed-off-by: Praveen K Paladugu <praveenkpaladugu@gmail.com> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Praveen K Paladugu <praveenkpaladugu@gmail.com> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>