Commit Graph

40027 Commits

Author SHA1 Message Date
Ján Tomko
9a20d8ac07 qemu: introduce qemuBuildFDSet
An alternative to qemuVirCommandGetFDSet that takes the index
into the passed FD set as an argument and does not try to look it up.

Use it as well ass virCommandPassFDIndex in qemuBuildChrChardevFileStr
and qemuBuildInterfaceCommandLine.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-12-01 17:24:20 +01:00
Ján Tomko
49c66026cf util: introduce virCommandPassFDIndex
Just like virCommandPassFD, but it also returns an index of
the passed FD in the FD set.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2020-12-01 17:24:20 +01:00
Ján Tomko
366891533f udevConnectListAllInterfaces: delete pointless cleanup code
We only jump to cleanup before allocating any lists.

Drop the dead code.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-12-01 17:22:09 +01:00
Ján Tomko
fd5df67dce udevConnectListAllInterfaces: initialize ret
Currently, ret is only used in the 'cleanup' section
and initialized right before the jump.

Switch to the customary initialization to -1 and only
leave in the 'ret = 0' statement on an empty list.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-12-01 17:22:09 +01:00
Michal Privoznik
6e91453bb6 qemu: Use virJSONValueObjectGetStringArray() more
In a few commit back (v6.10.0-5-gb3dad96972) a new helper for
obtaining string arrays from a virJSONObject was introduced:
virJSONValueObjectGetStringArray(). I've identified three places
where it can be used instead of open coding it:
qemuAgentSSHGetAuthorizedKeys(),
qemuMonitorJSONGetStringListProperty() and
qemuMonitorJSONGetCPUDefinitions().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-12-01 17:21:14 +01:00
Michal Privoznik
043b50b948 virJSONValueObjectGetStringArray: Report error if @key is not an array
The virJSONValueObjectGetStringArray() function is given a @key
which is supposed to be an array inside given @object. Well, if
it's not then an error state is returned (NULL), but no error
message is set.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-12-01 17:21:14 +01:00
Andrea Bolognani
ebc241fcff docs: Update language bindings spotlight
We should highlight the language bindings that are actively
maintained, keep up with the core library's development pace,
have good API coverage and are relevant to people looking to
integrate libvirt into their projects today: based on these
criteria, it makes sense to highlight the Go binding instead
of the Java one.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-12-01 17:06:21 +01:00
Tuguoyi
c4f4e195a1 qemu_conf: Fix double free problem for cfg->firmwares
cfg->firmwares still points to the original memory address after being
freed by virFirmwareFreeList(). As cfg get freed, it will be freed again
even if cfg->nfirmwares=0 which eventually lead to crash.

The patch fix it by setting cfg->firmwares to NULL explicitly after
virFirmwareFreeList() returns

Signed-off-by: Guoyi Tu<tu.guoyi@h3c.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-12-01 14:26:33 +01:00
Pavel Hrdina
0cbcd21b1f vircgroupv2: fix virCgroupV2DenyDevice
The original logic is incorrect. We would delete the device entry
from eBPF map only if the newval would be same as current val in the
map. In case that the device was allowed only as read-only but later
we remove all permissions for that device it would remain in the table
with empty values.

The old code would still deny the device but it's not working as
intended. Instead we will update the value in advance. If the updated
value is 0 it means that we are removing all permissions so it should
be removed from the map, otherwise we will update the value in map.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1810356

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-12-01 12:46:55 +01:00
Pavel Hrdina
ed1ba69f5a vircgroup: fix cpu quota maximum limit
Kernel commit <d505b8af58912ae1e1a211fabc9995b19bd40828> added proper
check for cpu quota maximum limit to prevent internal overflow.

Even though this change is not present in all kernels it makes sense
to enforce the same limit in libvirt.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1750315

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-12-01 12:41:36 +01:00
Pavel Hrdina
98a09ca48e vircgroupv2: use defines for cpu period and quota limits
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-12-01 12:41:35 +01:00
Pavel Hrdina
bc760f4d7c vircgroupv1: use defines for cpu period and quota limits
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-12-01 12:41:33 +01:00
Pavel Hrdina
a818e3f6f0 qemu: move cgroup cpu period and quota defines to vircgroup.h
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-12-01 12:41:24 +01:00
Marc-André Lureau
172b830435 virsh: add --disk informations to guestinfo command
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:52 +01:00
Marc-André Lureau
0cb2d9f05d qemu_driver: report guest disk informations
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:50 +01:00
Marc-André Lureau
05a75ca2ce domain: add disk informations to virDomainGetGuestInfo
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:46 +01:00
Marc-André Lureau
8401a586a2 qemu_agent: add qemuAgentGetDisks
guest-get-disks is available since QEMU 5.2:
https://wiki.qemu.org/ChangeLog/5.2#Guest_agent

Note that the test response was manually edited based on a reply on my
bare-metal computer. It shows partial results due to pcieport driver not
being currently supported by QGA.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:41 +01:00
Marc-André Lureau
3169db81f6 qemu: use virJSONValueObjectGetStringArray
There might be more potential users around, I haven't looked thoroughly.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:39 +01:00
Marc-André Lureau
b3dad96972 util: json: add virJSONValueObjectGetStringArray convenience
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:37 +01:00
Marc-André Lureau
c6fcb75f77 qemu_agent: factor out qemuAgentGetDiskAddress
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:35 +01:00
Marc-André Lureau
f534eae275 qemu_agent: export qemuAgentDiskAddressFree & add g_auto
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:32 +01:00
Marc-André Lureau
7b1bebdf3d qemu_agent: rename qemuAgentDiskInfo->qemuAgentDiskAddress
To match the QGA schema name (we are introducing a qemuAgentDiskInfo
struct again for different purpose).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
2020-12-01 11:23:21 +01:00
Jiri Denemark
38dfd3e80e Post-release version bump to 7.0.0
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2020-12-01 09:54:20 +01:00
Jiri Denemark
e27603980a Release of libvirt-6.10.0
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2020-12-01 09:51:03 +01:00
Peter Krempa
08ae9e5f40 NEWS: Mention network disk support in 'virsh attach-disk'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2020-11-30 08:48:32 +01:00
Daniel P. Berrangé
5d789c7b37 NEWS: add note about virt-ssh-helper perf improvements
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-27 16:40:55 +00:00
Roman Bolshakov
78999beabc qemucapsprobemock: Fix lookup of qemu functions
qemucapsprobemock can't find real versions of qemuMonitorSend() and
qemuMonitorJSONIOProcessLine() on macOS. That breaks qemucapsprobe.

The failure can be explained by documented behaviour of dlsym(3) on
macOS:

  If dlsym() is called with the special handle RTLD_NEXT, then dyld
  searches for the symbol in the dylibs the calling image linked against
  when built.

  [...] For flat linked images, the search starts in the load ordered
  list of all images, in the image right after the caller's image.

That means qemucapsprobemock must be linked against qemu test driver to
find symbols there with RTLD_NEXT.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-27 16:31:50 +01:00
Roman Bolshakov
11e67a2910 tests: Delay mock creation
There might be mocks that need to reference qemu test driver and link
with it. It's not possible now because qemu test driver is defined after
mocks.

While at it, add 'link_with' parameter to mock definition that allows to
specify a set of libraries the mock has to be linked with.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-27 16:31:36 +01:00
Roman Bolshakov
82256eaa4a ci: Run test suite on macOS
There's no need to have different CI process between macOS and FreeBSD
as test suite has been fixed on macOS.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-27 16:31:30 +01:00
Roman Bolshakov
94146c9d2b qemuxml2argvtest: Increase timeout
The test takes 40+ seconds on MBP 2012, MBA 2015. Cirrus completes the
test within default timeout, just above 29 seconds but the error margin
is narrow, under a second.

It'd be good to provide reasonable default timeout to avoid test suite
failure if "meson test" is invoked without arguments.

Closes https://gitlab.com/libvirt/libvirt/-/issues/58
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-27 16:31:07 +01:00
Roman Bolshakov
dbe922c945 tests: Fix mock chaining on macOS
Some tests in qemuxml2argvtest need opendir() from virpcimock, others
need opendir() from virfilewrapper.

But as of now, only opendir() from virpcimock has an effect.
real_opendir in virpcimock has a pointer to opendir$INODE64 in
libsystem_kernel.dylib instead of pointing to opendir$INODE64 in
qemuxml2argvtest (from virfilewrapper). And because the second one is
never used, tests that rely on prefixes added by virFileWrapperAddPrefix
fail.

That can be fixed if dlsym(3) is asked explicitly to search symbols in
main executable with RTLD_MAIN_ONLY before going to other dylibs.
Existing RTLD_NEXT handle results into libsystem_kernel.dylib being
searched before main executable.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-11-27 16:28:31 +01:00
Roman Bolshakov
c2745d009f tests: Fix opendir mocks on macOS
opendir() mocks need to search for decorated function with $INODE64
suffix, like stat mocks.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
2020-11-27 15:59:26 +01:00
Göran Uddeborg
f1436fc76e Translated using Weblate (Swedish)
Currently translated at 10.5% (1100 of 10440 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 10.2% (1070 of 10440 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Co-authored-by: Göran Uddeborg <goeran@uddeborg.se>
Signed-off-by: Göran Uddeborg <goeran@uddeborg.se>
2020-11-27 15:16:10 +01:00
Yuri Chornoivan
323624f461 Translated using Weblate (Ukrainian)
Currently translated at 100.0% (10440 of 10440 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/uk/

Translated using Weblate (Ukrainian)

Currently translated at 99.3% (10370 of 10440 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/uk/

Co-authored-by: Yuri Chornoivan <yurchor@ukr.net>
Signed-off-by: Yuri Chornoivan <yurchor@ukr.net>
2020-11-27 15:16:09 +01:00
Weblate
f8846320e2 Update translation files
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>
2020-11-27 15:16:07 +01:00
Göran Uddeborg
e7b4609272 Translated using Weblate (Swedish)
Currently translated at 10.0% (1041 of 10399 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Translated using Weblate (Swedish)

Currently translated at 9.4% (981 of 10399 strings)

Translation: libvirt/libvirt
Translate-URL: https://translate.fedoraproject.org/projects/libvirt/libvirt/sv/

Co-authored-by: Göran Uddeborg <goeran@uddeborg.se>
Signed-off-by: Göran Uddeborg <goeran@uddeborg.se>
2020-11-27 15:16:03 +01:00
Erik Skultety
1a0af38ae7 ci: Switch to meson build system
Add meson required bits to the ci logic in the repo to be able to run
a meson build in a container.
This patch also drops several environment variables we don't need with
meson anymore.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2020-11-27 15:14:35 +01:00
Daniel P. Berrangé
6d69afe451 util: avoid glib event loop workaround where possible
I previously did a workaround for a glib event loop race
that causes crashes:

  commit 0db4743645
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Tue Jul 28 16:52:47 2020 +0100

    util: avoid crash due to race in glib event loop code

it turns out that the workaround has a significant performance
penalty on I/O intensive workloads. We thus need to avoid the
workaround if we know we have a new enough glib to avoid the
race condition.

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-26 13:30:35 +00:00
Daniel P. Berrangé
829142699e remote: make ssh-helper massively faster
It was reported that the performance of tunnelled migration and
volume upload/download regressed in 6.9.0, when the virt-ssh-helper
is used for remote SSH tunnelling instead of netcat.

When seeing data available to read from stdin, or the socket,
the current code will allocate at most 1k of extra space in
the buffer it has.

After writing data to the socket, or stdout, if more than 1k
of extra space is in the buffer, it will reallocate to free
up that space.

This results in a huge number of mallocs when doing I/O, as
well as a huge number of syscalls since at most 1k of data
will be read/written at a time.

Also if writing blocks for some reason, it will continue to
read data with no memory bound which is bad.

This changes the code to use a 1 MB fixed size buffer in each
direction. If that buffer becomes full, it will update the
watches to stop reading more data. It will never reallocate
the buffer at runtime.

This increases the performance by orders of magnitude.

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-11-26 10:14:18 +00:00
Jiri Denemark
259b43673f po: Refresh potfile for v6.10.0
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-25 08:47:39 +01:00
Martin Kletzander
511013b57b qemu: Tweak debug message for qemuMigrationSrcPerformPeer2Peer3
Commit 49186372db forgot to add the new parameter.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2020-11-25 00:21:49 +01:00
Martin Kletzander
9e93d87c00 docs: Document SELinux caveats when migrating over UNIX sockets
The information about sockets having different label than the one on the file
and the way it needs to be set is very difficult to find for those who did not
come across it before.  Let's describe what needs to happen in order for the
migration to go through rather than rely on general knowledge of others.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-25 00:10:52 +01:00
Martin Kletzander
3430a77182 qemu: Disable NBD TLS migration over UNIX socket
Even though it is technically possible, when running the migrations QEMU's
nbd-server-start errors out with:

  "TLS is only supported with IPv4/IPv6"

We can always enable it when QEMU adds this feature, but for now it is safer to
show our error message rather than rely on QEMU to error out properly.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-25 00:10:52 +01:00
Collin Walling
d1e00f84c0 qemu: allow hypervisor-cpu-baseline with single cpu
When executing the hypervisor-cpu-baseline command and if there is
only a single CPU definition present in the XML file, then the
baseline handler will exit early and libvirt will print an unhelpful
message:

"error: An error occurred, but the cause is unknown"

This is due to no CPU definition ever being "baselined", since the
handler expects at least two CPU models.

Let's fix this by performing a CPU model expansion on the single CPU
definition and returning the result to the caller. This will also
ensure the CPU model's feature set is sane if any were provided in
the file.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-24 21:04:07 +01:00
Collin Walling
c5ed1fdee2 qemu: check if cpu model is supported before baselining
Check the provided CPU models against the CPU models
known by the hypervisor before baselining and print
an error if an unrecognized model is found.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-24 21:04:07 +01:00
Collin Walling
e2df0b488a qemu: report error if missing model name when baselining
When executing the hypervisor-cpu-baseline command and the
XML file contains a CPU definition without a model name, or
an invalid CPU definition, then the commands will fail and
return an error message from the QMP response.

Let's clean this up by checking for a valid definition and
presence of a model name.

This code is copied from virCPUBaseline.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-24 21:04:07 +01:00
Collin Walling
60bb33293b qemu: fix one instance of rc check styling in baseline
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-24 21:04:07 +01:00
Collin Walling
c003041034 qemu: check for model-expansion cap before baselining
Hypervisor-cpu-baseline requires the cpu-model-expansion
capability when expanding CPU model features if the
--features flag is provided.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2020-11-24 21:04:07 +01:00
Laine Stump
b19863640d util: call iptables directly rather than via firewalld
When libvirt added support for firewalld, we were unable to use
firewalld's higher level rules, because they weren't detailed enough
and could not be applied to the iptables FORWARD or OUTPUT chains
(only to the INPUT chain). Instead we changed our code so that rather
than running the iptables/ip6tables/ebtables binaries ourselves, we
would send these commands to firewalld as "passthrough commands", and
firewalld would run the appropriate program on our behalf.

This was done under the assumption that firewalld was somehow tracking
all these rules, and that this tracking was benefitting proper
operation of firewalld and the system in general.

Several years later this came up in a discussion on IRC, and we
learned from the firewalld developers that, in fact, adding iptables
and ebtables rules with firewalld's passthrough commands actually has
*no* advantage; firewalld doesn't keep track of these rules in any
way, and doesn't use them to tailor the construction of its own rules.

Meanwhile, users have been complaining for some time that whenever
firewalld is restarted on a system with libvirt virtual networks
and/or nwfilter rules active, the system logs would be flooded with
warning messages whining that [lots of different rules] could not be
deleted because they didn't exist. For example:

firewalld[3536040]: WARNING: COMMAND_FAILED:
  '/usr/sbin/iptables -w10 -w --table filter --delete LIBVIRT_OUT
  --out-interface virbr4 --protocol udp --destination-port 68
  --jump ACCEPT' failed: iptables: Bad rule
  (does a matching rule exist in that chain?).

(See https://bugzilla.redhat.com/1790837 for many more examples and a
discussion)

Note that these messages are created by iptables, but are logged by
firewalld - when an iptables/ebtables command fails, firewalld grabs
whatever is in stderr of the program, and spits it out to the system
log as a warning. We've requested that firewalld not do this (and
instead leave it up to the calling application to do the appropriate
logging), but this request has been respectfully denied.

But combining the two problems above ( 1) firewalld doesn't do
anything useful when you use it as a proxy to add/remove iptables
rules, 2) firewalld often insists on logging lots of
annoying/misleading/useless "error" messages when you use it as a
proxy to remove iptables rules that don't already exist), leads to a
solution - simply stop using firewalld to add and remove iptables
rules. Instead, exec iptables/ip6tables/ebtables directly in the same
way we do when firewalld isn't active.

We still need to keep track of whether or not firewalld is active, as
there are some things that must be done, e.g. we need to add some
actual firewalld rules in the firewalld "libvirt" zone, and we need to
take notice when firewalld restarts, so that we can reload all our
rules.

This patch doesn't remove the infrastructure that allows having
different firewall backends that perform their functions in different
ways, as that will very possibly come in handy in the future when we
want to have an nftables direct backend, and possibly a "pure"
firewalld backend (now that firewalld supports more complex rules, and
can add those rules to the FORWARD and OUTPUT chains). Instead, it
just changes the action when the selected backend is "firewalld" so
that it adds rules directly rather than through firewalld, while
leaving as much of the existing code intact as possible.

In order for tests to still pass, virfirewalltest also had to be
modified to behave in a different way (i.e. by capturing the generated
commandline as it does for the DIRECT backend, rather than capturing
dbus messages using a mocked dbus API).

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-24 14:22:06 -05:00
Laine Stump
070690538a util: synchronize with firewalld before we start calling iptables directly
When it is starting up, firewalld will delete all existing iptables
rules and chains before adding its own rules. If libvirtd were to try
to directly add iptables rules during the time before firewalld has
finished initializing, firewalld would end up deleting the rules that
libvirtd has just added.

Currently this isn't a problem, since libvirtd only adds iptables
rules via the firewalld "passthrough command" API, and so firewalld is
able to properly serialize everything. However, we will soon be
changing libvirtd to add its iptables and ebtables rules by directly
calling iptables/ebtables rather than via firewalld, thus removing the
serialization of libvirtd adding rules vs. firewalld deleting rules.

This will especially apparent (if we don't fix it in advance, as this
patch does) when libvirtd is responding to the dbus NameOwnerChanged
event, which is used to learn when firewalld has been restarted. In
that case, dbus sends the event before firewalld has been able to
complete its initialization, so when libvirt responds to the event by
adding back its iptables rules (with direct calls to
/usr/bin/iptables), some of those rules are added before firewalld has
a chance to do its "remove everything" startup protocol. The usual
result of this is that libvirt will successfully add its private
chains (e.g. LIBVIRT_INP, etc), but then fail when it tries to add a
rule jumping to one of those chains (because in the interim, firewalld
has deleted the new chains).

The solution is for libvirt to preface it's direct calling to iptables
with a iptables command sent via firewalld's passthrough command
API. Since commands sent to firewalld are completed synchronously, and
since firewalld won't service them until it has completed its own
initialization, this will assure that by the time libvirt starts
calling iptables to add rules, that firewalld will not be following up
by deleting any of those rules.

To minimize the amount of extra overhead, we request the simplest
iptables command possible: "iptables -V" (and aside from logging a
debug message, we ignore the result, for good measure).

(This patch is being done *before* the patch that switches to calling
iptables directly, so that everything will function properly with any
fractional part of the series applied).

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
2020-11-24 14:21:58 -05:00