When attempting to run:
libvirt.git/_build # ./run --selinux ./src/libvirtd
the following error is thrown:
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>
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>
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: 4690058b6d
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some of the checks make sense only after the address is allocated and
thus we need to re-do the validation after the address is assigned.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
While the function is exported via header, the symbol itself was not.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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>
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.
Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/
Co-authored-by: Weblate <noreply@weblate.org>
Signed-off-by: Fedora Weblate Translation <i18n@lists.fedoraproject.org>
These are either features/bugfixes I've worked on or
participated in.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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: 812a146dfe
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The code cleaning up virStorageSource doesn't free data allocated by
virStorageSourceInit() so we need to call virStorageSourceDeinit()
explicitly.
Fixes: 8e66473781
Resolves: https://issues.redhat.com/browse/RHEL-33044
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The current implementation requires users to configure the
preference as such:
-Dfirewall_backend_default_1=iptables
-Dfirewall_backend_default_2=nftables
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>
Add test data based on qemu commit v9.0.0-995-g60b54b67c6 on x86_64
Comparison to previous release:
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>
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>
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: 41400ac1dd
Fixes: a0e1ada63c
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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>
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:
virt-admin<TAB><TAB>
NOTE\:\ Connecting\ to\ default\ daemon.\ Specify\ daemon\ using\ -c\ \(e.g.\ virtqemud\:///system\)
when no daemon is running. Suppress extra prints by passing '-q'
in the bash-completion script.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We currently require full argument specification:
virt-admin daemon-timeout --timeout X
Well, the '--timeout' feels a bit redundant. Turn the argument
into a positional so that the following works too:
virt-admin daemon-timeout X
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
In a few examples we recommend disabling daemon timeout when
fetching debug logs. While it makes sense the actual syntax used
results in an error:
# virt-admin daemon-timeout 0
error: unexpected data '0'
This is because --timeout is required. Update examples to include
it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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>
Commit 91f4ebbac8 (v10.0.0-185-g91f4ebbac8)
changed the return value of virSocketSendFD() from 0 to 1 on success.
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.
Fixes: 91f4ebbac8
Resolves: https://issues.redhat.com/browse/RHEL-36721
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use contemporary style for declarations and automatic memory clearing
for a helper string.
Since the function can't fail any more, remove any mention of returning
errno and remove error checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Declare one argument per line and one variable per line and use boolean
operators at the end of the line rather than at the beginning.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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>
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>
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>
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>
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>
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>
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>
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:
iptables-save >iptables.txt
iptables-restore-translate -f iptables.txt
(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.
This page was used as a guide:
https://wiki.nftables.org/wiki-nftables/index.php/Moving_from_iptables_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>
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: 6aa2fa38b0
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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>
