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>
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>
The 'open' forward type probably hadn't yet been added when this
message was written.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
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>
If a network disappeared the daemon should not only remove it from the
list of networks, but also do a proper cleanup.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The new function (networkCleanupInactive) can be called from an iterator
over the list of networks without the risk of deadlock.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
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>
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>
It skips the cleanup from networkStartNetwork and the only other path
already checks if the network is active or not.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
It will be more useful in there when calling from new places.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
The function networkShutdownNetwork already does that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
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>
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.
https://bugs.debian.org/1081396
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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>
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.
1: https://man.freebsd.org/cgi/man.cgi?query=fd&sektion=4&manpath=freebsd-release-ports
2: https://man.freebsd.org/cgi/man.cgi?query=fdescfs&sektion=5&n=1
This reverts commit 308ec0fb2c.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
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>
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.
1: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getdtsz.c;h=4c5a6208067d2f9eaaac6dba652702fb4af9b7e3;hb=HEAD
2 https://github.com/apple/darwin-xnu/blob/main/bsd/sys/syslimits.h#L104
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
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>
Both virCommandMassCloseGetFDsLinux() and
virCommandMassCloseGetFDsGeneric() take @cmd argument only to
mark it as unused. Drop it from both.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Add tests for two new system dumps which show various configurations
that were fixed in the previous commits.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
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>
Since some systems support control for L2 caches as well as L3 caches it
would be useful to report their configuration in capabilities.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It will be easier to add more dynamic data later on.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It will be easier to add more dynamic data later on
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This way it can be used later in virResctrlAllocGetUnused().
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
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>
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>
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>
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 a2e2e4652f and can
be safely removed.
Signed-off-by: Stepan Zobal <szobal@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
xmlBuffer->content was deprecated in libxml2 v2.13.0-33-gb34dc1e4
xmlBufferDetach(xmlBuffer) should be used instead
Signed-off-by: Jakub Palacky <jpalacky@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
xmlParserCtxt->lastError was deprecated in libxml2 v2.13.0-103-g1228b4e0
xmlCtxtGetLastError(xmlParserCtxt) should be used instead
Signed-off-by: Jakub Palacky <jpalacky@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
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:
VIR_ERR_CONF_SYNTAX: VIR_FROM_CONF: configuration file syntax error:
memory conf:74: expecting an assignment
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>
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>
Add a test case that the numeric overflow when parsing disk target is
detected.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
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>
The previous fix didn't check the overflow in addition. Use the new
macro to check both multiplication and addition overflows.
Fixes: 8666523b7d
Closes: https://gitlab.com/libvirt/libvirt/-/issues/671
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The macro does the two checks together so that it's obvious what we're
checking as doing it in place is really unpleasant.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Few of the handlers didn't take that possibility into account. Warn
others.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Similarly to other cases users may specify the feature flag multiple
times.
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>
'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: d6e582da80
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
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: 3afbb1644c
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
