Laine Stump [Fri, 8 May 2020 01:54:39 +0000 (21:54 -0400)]
network: force re-creation of iptables private chains on firewalld restart
When firewalld is stopped, it removes *all* iptables rules and chains,
including those added by libvirt. Since restarting firewalld means
stopping and then starting it, any time it is restarted, libvirt needs
to recreate all the private iptables chains it uses, along with all
the rules it adds.
We already have code in place to call networkReloadFirewallRules() any
time we're notified of a firewalld start, and
networkReloadFirewallRules() will call
networkPreReloadFirewallRules(), which calls
networkSetupPrivateChains(); unfortunately that last call is called
using virOnce(), meaning that it will only be called the first time
through networkPreReloadFirewallRules() after libvirtd starts - so of
course when firewalld is later restarted, the call to
networkSetupPrivateChains() is skipped.
The neat and tidy way to fix this would be if there was a standard way
to reset a pthread_once_t object so that the next time virOnce was
called, it would think the function hadn't been called, and call it
again. Unfortunately, there isn't any official way of doing that (we
*could* just fill it with 0 and hope for the best, but that doesn't
seem very safe.
So instead, this patch just adds a static variable called
chainInitDone, which is set to true after networkSetupPrivateChains()
is called for the first time, and then during calls to
networkPreReloadFirewallRules(), if chainInitDone is set, we call
networkSetupPrivateChains() directly instead of via virOnce().
It may seem unsafe to directly call a function that is meant to be
called only once, but I think in this case we're safe - there's
nothing in the function that is inherently "once only" - it doesn't
initialize anything that can't safely be re-initialized (as long as
two threads don't try to do it at the same time), and it only happens
when responding to a dbus message that firewalld has been started (and
I don't think it's possible for us to be processing two of those at
once), and even then only if the initial call to the function has
already been completed (so we're safe if we receive a firewalld
restart call at a time when we haven't yet called it, or even if
another thread is already in the process of executing it. The only
problematic bit I can think of is if another thread is in the process
of adding an iptable rule at the time we're executing this function,
but 1) none of those threads will be trying to add chains, and 2) if
there was a concurrency problem with other threads adding iptables
rules while firewalld was being restarted, it would still be a problem
even without this change.
This is yet another patch that fixes an occurrence of this error:
COMMAND_FAILED: '/usr/sbin/iptables -w10 -w --table filter --insert LIBVIRT_INP --in-interface virbr0 --protocol tcp --destination-port 67 --jump ACCEPT' failed: iptables: No chain/target/match by that name.
In particular, this resolves: https://bugzilla.redhat.com/1813830
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Fri, 8 May 2020 02:32:59 +0000 (22:32 -0400)]
network: make it safe to call networkSetupPrivateChains() multiple times
networkSetupPrivateChains() is currently called only once per run of
libvirtd, so it can assume that errInitV4 and errInitV6 are empty/null
when it is called. In preparation for potentially calling this
function multiple times during one run, this patch moves the reset of
errInitV[46] to the top of the function, to assure no memory is
leaked.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 24 Apr 2020 11:17:51 +0000 (13:17 +0200)]
virDevMapperGetTargetsImpl: quit early if device is not a devmapper target
As suggested in the linked bug, libvirt should firstly check
whether the major number of the device is device mapper major.
Because if it isn't subsequent DM_DEVICE_DEPS task may not only
fail, but also yield different results. In the bugzilla this is
demonstrated by creating a devmapper target named 'loop0' and
then creating loop target /dev/loop0. When the latter is then
passed to a domain, our virDevMapperGetTargetsImpl() function
blindly asks devmapper to provide target dependencies for
/dev/loop0 and because of the way devmapper APIs work, it will
'sanitize' the input by using the last component only which is
'loop0' and thus return different results than expected.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1823976 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's been more than six months since we adopted GLib and we've
been pretty aggressive at replacing our homegrown APIs with more
standard ones, so by now most of the symbols mentioned in this
document haven't been around for quite a long time already.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
docs: introduce a page showing a overall CI dashboard
With the move to GitLab CI one of the things we miss from Jenkins is a
single page dashboard showing CI status across all projects. This is a
very simple replacement that uses badges for CI pipeline status.
A CSS tweak is needed because RST->HTML adds redundant <p> tags inside
table cells which causes excessive vertical whitespace to appear.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
qemuDomainCleanupRun: Actually run cleanup callbacks in reverse order
We have a framework to register cleanup callbacks that are run
when a domain is shut down. The idea is to run callbacks in
reverse order than they were registered. However, looking at the
code this is not the case. Fortunately, this framework is used to
register a single callback and a single callback only -
qemuMigrationDstPrepareCleanup() - therefore there was no problem
just yet.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Thu, 7 May 2020 23:38:41 +0000 (17:38 -0600)]
tests: Fix the libxl xml2domconfig test
Commit a13b2905f7 missed an adjustment to a test that is only run when
building against xen <= 4.9, where LIBXL_HAVE_BUILDINFO_NESTED_HVM is
not defined. Adjust fullvirt-cpuid-legacy-nest test similar to the others.
Revert "tests: Use qemu:///embed to simplify fakerootdir management"
Turns out that it's not enough to pass the qemu:///embed root to
virQEMUDriverConfigNew(), you also have to make sure the same
string is copied into the virQEMUDriver structure yourself, and
not doing so in our case resulted in the cleanup never happening
and in distcheck failing because of that.
On the other hand, actually setting config->embeddedRoot would
result in different paths being generated for each test run, which
would obviously break qemuxml2argvtest, so that's not an option
either.
tests: Use qemu:///embed to simplify fakerootdir management
Now that the QEMU driver natively supports storing all its runtime
data inside an arbitrary directory, we can avoid having multiple
copies of the same logic in the test suite.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
tests: poison $HOME and $XDG_RUNTIME_DIR env variables
To attempt to catch unit tests which accidentally create files in $HOME,
or $XDG_RUNTIME_DIR, poison these env vars by pointing them to
directories which don't exist. This should give easier to debug test
failures. For example:
$ VIR_TEST_DEBUG=1 ./qemuhotplugtest
Could not initialize HostdevManager - operation failed: Failed to create state dir '/bad-test-used-env-xdg-runtime-dir/libvirt/hostdevmgr'
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We need this for all tests that use virHostdevManager, because
during creation of this object for unprivileged connections
like those used in the test suite we would end up writing inside
the user's home directory.
That's bad manners in general, but when running the test suite
inside a purposefully constrained environment such as the one
exposed by pbuilder, it turns into an outright test failure:
Could not initialize HostdevManager - operation failed: Failed
to create state dir '/nonexistent/.cache/libvirt/hostdevmgr'
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
tests: Update capabilities for QEMU 5.0.0 on ppc64
The churn in the output files is caused primarily by the fact that
replies were generated on a POWER9 machine, which is good because
we didn't have coverage of that before.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
tests: Enable directory override for qemucapsprobe
Currently, qemucapsprobe fails when libvirt is not already installed
on the system:
$ ./tests/qemucapsprobe /path/to/qemu-system-ppc64 >/dev/null
I/O warning : failed to load external entity "/usr/share/libvirt/cpu_map/index.xml"
2020-05-06 09:49:59.136+0000: 269822: info : libvirt version: 6.4.0
2020-05-06 09:49:59.136+0000: 269822: info : hostname: [...]
2020-05-06 09:49:59.136+0000: 269822: warning : virQEMUCapsLogProbeFailure:5127 :
Failed to probe capabilities for /path/to/qemu-system-ppc64: XML error: failed to
parse xml document '/usr/share/libvirt/cpu_map/index.xml'
It would be great if the tool could work entirely out of the build
directory, and this patch achieves just that.
Suggested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jiri Denemark [Tue, 5 May 2020 16:35:26 +0000 (18:35 +0200)]
qemu: Don't use CPU from host capabilities as host-model on ARM
We never supported host-model CPUs on ARM and we don't want to support
them even once patches for direct detection of host CPU are merged. And
since using host CPU definition for host-model CPUs exists only for
backward compatibility, we should not use it for any host-model support
added in the future. Such enhancement should exclusively use the result
of query-cpu-model-expansion. Until proper host-model support is
implemented for ARM (if ever), we need to make sure the detected host
CPU is not accidentally used for host-model CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Laine Stump [Fri, 1 May 2020 04:05:50 +0000 (00:05 -0400)]
systemd: start libvirtd after firewalld/iptables services
When a system has enabled the iptables/ip6tables services rather than
firewalld, there is no explicit ordering of the start of those
services vs. libvirtd. This creates a problem when libvirtd.service is
started before ip[6]tables, as the latter, when it finally is started,
will remove all of the iptables rules that had previously been added
by libvirt, including the custom chains where libvirt's rules are
kept. This results in an error message similar to the following when a
user subsequently tries to start a new libvirt network:
"Error while activating network: Call to virNetworkCreate failed:
internal error: Failed to apply firewall rules
/usr/sbin/ip6tables -w --table filter --insert LIBVIRT_FWO \
--in-interface virbr2 --jump REJECT:
ip6tables: No chain/target/match by that name."
(Prior to logging this error, it also would have caused failure to
forward (or block) traffic in some cases, e.g. for guests on a NATed
network, since libvirt's rules to forward/block had all been deleted
and libvirt didn't know about it, so it couldn't fix the problem)
When this happens, the problem can be remedied by simply restarting
libvirtd.service (which has the side-effect of reloading all
libvirt-generated firewall rules)
Instead, we can just explicitly stating in the libvirtd.service file
that libvirtd.service should start after ip6tables.service and
ip6tables.service, eliminating the race condition that leads to the
error.
There is also nothing (that I can see) in the systemd .service files
to guarantee that firewalld.service will be started (if enabled) prior
to libvirtd.service. The same error scenario given above would occur
if libvirtd.service started before firewalld.service. Even before
that, though libvirtd would have detected that firewalld.service was
disabled, and then turn off all firewalld support. So, for example,
firewalld's libvirt zone wouldn't be used, and most likely traffic
from guests would therefore be blocked (all with no external
indication of the source of the problem other than a debug-level log
when libvirtd was started saying that firewalld wasn't in use); also
libvirtd wouldn't notice when firewalld reloaded its rules (which also
simultaneously deletes all of libvirt's rules).
I'm not aware of any reports that have been traced back to
libvirtd.service starting before firewalld.service, but have seen that
error reported multiple times, and also don't see an existing
dependency that would guarantee firewalld.service starts before
libvirtd.service, so it's possible it's been happening and we just
haven't gotten to the bottom of it.
This patch adds an After= line to the libvirtd.service file for each
of iptables.service, ip6tables.service, and firewalld.servicee, which
should guarantee that libvirtd.service isn't started until systemd has
started whichever of the others is enabled.
This race was diagnosed, and patch proposed, by Jason Montleon in
https://bugzilla.redhat.com/1723698 . At the time (April 2019) danpb
agreed with him that this change to libvirtd.service was a reasonable
thing to do, but I guess everyone thought someone else was going to
post a patch, so in the end nobody did.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Mon, 4 May 2020 21:52:34 +0000 (15:52 -0600)]
docs: Xen improvements
In formatdomain, using 'libxl' and 'xen' is redundant since they now
both refer to the same driver. 'xen' predates 'libxl' and unambiguously
identifies the Xen hypervisor, so drop the use of 'libxl'.
In aclpolkit, the connection URI was erroneously identified as 'libxl'
and the name 'xenlight'. Change the URI to 'xen' and driver name to 'Xen'.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jim Fehlig [Mon, 4 May 2020 21:20:37 +0000 (15:20 -0600)]
libxl: Clarify that 'xenlight' should only be used internally
The libxl driver has suffered an identity crisis since its introduction.
It took on the name 'libxl' since at the time libvirt already contained
a 'xen' driver for the old Xen toolstack implementation. 'libxl' is short
for libxenlight, which is often called xenlight. Unfortunately all forms
of the name are used in the libxl driver.
The only remaining use of the 'xenlight' form is when interacting with
the host device manager, which is difficult to change since it would
cause problems when upgrading the driver.
Rename the #define to make it clear the 'xenlight' form is internal and
add a comment describing why the name exists and that its use should be
discouraged.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jim Fehlig [Mon, 4 May 2020 20:51:32 +0000 (14:51 -0600)]
libxl: Use the name 'Xen' in driver tables
The libxl driver declares its name as 'Xen' through the public
virConnectGetType() API. In the virHypervisorDriver table the name is
set to 'xenlight'. To add more confusion, the name is set to 'LIBXL'
in the virStateDriver. For consistency, use the same name in the driver
tables as reported in the public virConnectGetType() API.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In the referenced commit I removed @ret from
virHostValidateBhyve() thinking it wasn't used when in fact it is
- it's usage is hidden under MODULE_STATUS_WARN(). Reintroduce
the variable back.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are few places where a return variable is introduced (ret
or retval), but then is never changed and is then passed to
return. Well, we can return the value that the variable is
initialized to directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virnetserver: Check for virNetServerClientInitKeepAlive() retval
Since it's introduction in v0.9.7-147-gf4324e3292 the
virNetServerClientInitKeepAlive() function returned nothing than
a negative one. Fortunately, this did not pose any problem
because we ignored the retval happily. Well, it's time to check
for the retval because the function might fail regularly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Mon, 27 Apr 2020 10:48:51 +0000 (12:48 +0200)]
CONTRIBUTING: Include note about build system tools
Debian always runs autoreconf at package build time, which means
that apt-get build-dep will bring in everything that's needed to
build libvirt from a git clone; Fedora and RHEL, however, skip
this step, so we have to install some extra packages manually.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Xu Yandong [Mon, 4 May 2020 08:36:19 +0000 (16:36 +0800)]
virsh: Fix return code for dump
After the commit dc0771c, ret variable no longer
represents the status of the return code, use
data->ret replace it.
Signed-off-by: Xu Yandong <xuyandong2@huawei.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In the past we added 1024 bytes of padding to saved state images so that
users can run "virsh managedsave-edit $GUEST" and make XML changes which
increase the size of the XML document. This padding was accidentally
lost a while back
qemu: Remove unnecessary calculations in qemuDomainSaveMemory
The original 1024 bytes was unreasonably stingy when we consider that
the QEMU state is typically going to be many 100's of MB in size. Thus
this adds 64 KB of padding after the XML which should cope with any
plausible modifications a user will want to make.
Bjoern Walk [Thu, 23 Apr 2020 13:15:09 +0000 (15:15 +0200)]
qemu: command: make qemuBuildVirtioOptionsStr void
Now that qemuBuildVirtioOptionsStr can not fail anymore, remove its
return value and make it void.
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Bjoern Walk [Thu, 23 Apr 2020 13:15:08 +0000 (15:15 +0200)]
qemu: move virtio capability validation
Move capability validation of virtio options from command line
generation to post-parse device validation where it belongs.
Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Bjoern Walk [Thu, 23 Apr 2020 13:15:07 +0000 (15:15 +0200)]
tests: more fine-granular tests for virtio-options
Add separate tests for individual options and devices for virtio-options
to have the ability to do more fine-granular testing of various
combinations.
Also, add negative tests for unavailable capabilities.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Bjoern Walk [Thu, 23 Apr 2020 13:15:06 +0000 (15:15 +0200)]
tests: use latest caps for virtio-options test
Convert the virtio-options test in qemuxml2argv and qemuxml2xml to use
the latest available QEMU capabilities.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This patch adds the implementation of the IBS pSeries feature,
using the QEMU_CAPS_MACHINE_PSERIES_CAP_IBS capability added
in the previous patch.
IBS can have the following values: "broken", "workaround",
"fixed-ibs", "fixed-ccd" and "fixed-na".
This is the XML format for the cap:
<features>
<ibs value='fixed-ibs'/>
</features>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
IBS (Indirect Branch Speculation) is the last capability added
in QEMU 2.12 related to Spectre mitigation for Power. It was
added in commit 4be8d4e7d935.
This patch introduces it as QEMU_CAPS_MACHINE_PSERIES_CAP_IBS.
Like CFPC and SBBC, users might want to tune in IBS based on
their HW and guest OS requirements, and it's better to do it
so in a proper Libvirt feature than to put QEMU arguments
in the middle of the domain XML.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch adds the implementation of the SBBC pSeries feature,
using the QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC capability added
in the previous patch.
Like the previously added CFPC feature, SBBC can have the values
"broken", "workaround" or "fixed". Extra code is required to handle
it since it's not a regular tristate capability.
This is the XML format for the cap:
<features>
<sbbc value='workaround'/>
</features>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
SBBC (Speculation Barrier Bounds Checking) is another capability
related to Spectre mitigation efforts in Power processors. It
was implemented in QEMU 2.12 by commit 09114fd81799.
This patch introduces it as QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC to
be implemented in the next patch. Like the case with the now
implemented CFPC, exposing this feature in the XML allows for
a cleaner way for users to tune the SBBC accordingly, given
that not all hypervisor and guest setups supports this
Spectre mitigation.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch adds the implementation of the CFPC pSeries feature,
using the QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC capability added
in the previous patch.
CPFC can have the values "broken", "workaround" or "fixed". Extra
code is required to handle it since it's not a regular tristate
capability.
This is the XML format for the cap:
<features>
<cfpc value='workaround'/>
</features>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
CFPC (Cache Flush on Privilege Change) is one of the capabilities
added to QEMU to mitigate Spectre vulnerabilities in Power chips.
It was implemented in QEMU 2.12 by commit 6898aed77f46.
This capability is still used today due to differences in how
the host setup (hardware and firmware/kernel) can handle this
mitigation. Its default value also varies with the pseries machine
version of the time. There's also certain OSes, like AIX, that
might not support the default value of the pseries machine the
guest uses.
Exposing this in the Libvirt XML as a feature will allow users to tune
CFPC values in a cleaner way, instead of hacking parameters in
<qemu:commandline> elements.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Julio Faracco [Sat, 25 Apr 2020 16:35:37 +0000 (13:35 -0300)]
network: Remove memory leak caused by wrong initialization
This commit fix a wrong variable initialization. There is a variable
called `new_lease` which is being initialized with the content of
parameter `lease`. To avoid memory leak, the proper way is initialize
with NULL first. This wrong statement was added by commit 97a0aa24.
There are some other improvements also.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 14:57:31 +0000 (16:57 +0200)]
qemumonitortestutils: Enforce consumption of all items in test monitor
To prevent unexpected situations where a change in code would stop
looking at some of the tested commands go unnoticed add a mechanism to
force consumption of all test items.
Since there are a few tests which would be hard to fix add also a
mechanism to opt-out of the check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 14:52:12 +0000 (16:52 +0200)]
qemumonitortestutils: Store a string identifying test monitor entry
For each test monitor entry store an optional string which will allow to
identify it. This will be used later when checking that all registered
monitor commands were used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 09:57:14 +0000 (11:57 +0200)]
qemumonitortestutils: Make test monitor failures more prominent
Until now we've tried to report errors from the test monitor code by
passing them back as failures from the qemu we simulate. This doesn't
work well in cases when the monitor logic does not detect failures or
has fallback code. Additionally there isn't much use for continuing the
test execution after first failure as in most cases the test data will
be misaligned and all other calls will fail as well.
To make the errors more obvious this patch moves away from reporting
them via the simulated monitor to reporting them to stderr and
exit()ing afterwards. While this might be less convenient
when developing tests it actually makes failures in the test suite
really obvious and doesn't require any opt-in from the tests themselves.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 12:31:12 +0000 (14:31 +0200)]
qemuhotplugtest: cpu: x86-modern-individual: Remove invalid test case
One of the test cases attempted to use test data meant for modern qemu
without asserting the 'modern' flag. Since that changes the commands
used to query state it won't work with data meant for the modern case.
Remove the invalid test case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 10:03:21 +0000 (12:03 +0200)]
qemuhotplugtest: detach: Add expected 'object-del' to disk-scsi-multipath case
The test verifies unplug of a disk with the persistent reservations
helper. The 'object-del' used to remove the helper was not mentioned in
the list of expected commands.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 09:50:59 +0000 (11:50 +0200)]
tests: monitor: Rename qemuMonitorReportError to qemuMonitorTestAddErrorResponse
It's a method of the test monitor and it adds a response to the monitor
output. The original qemuMonitorTestAddErrorResponse method is renamed
to qemuMonitorTestAddErrorResponseInternal
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 24 Apr 2020 11:59:21 +0000 (13:59 +0200)]
virStorageSourceParseNBDColonString: Rewrite to match what qemu does
Our implementation wasn't quite able to parse everything that qemu does.
This patch rewrites the parser to a code that semantically resembles the
combination of 'nbd_parse_filename' and 'inet_parse' methods in qemu to
be able to parse the strings in an equivalent manner.
The only thing that libvirt doesn't do is to check the lengths of
various components in the nbd string in places where qemu uses constant
size buffers.
The test cases validate that some of the corner cases involving colons
are parsed properly.
travis: explicitly include gnutls to ensure it is updated
Travis includes gnutls in the default package set, but it is
an outdated version linkng to an incompatible libffi version.
The 'update: true' stanza causes the brew toolchain to be
updated but not the installed formula. It is possible to
run 'brew upgrade' to update installed formula, but this is
very slow adding more than 5 minutes to the build time.
Listing the gnutls package explicitly causes it to be updated
without extending the build time.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Tue, 21 Apr 2020 14:56:58 +0000 (16:56 +0200)]
docs: Document reserved PCI addresses for QEMU
From time to time we are asked which PCI addresses are reserved
in QEMU. Let's document them in one place, it's easier than
reconstructing the list from the code each time.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
docs: Add version info for 'backingStoreInput' in domaincaps
Add 'since 5.10' as commonly used in formatdomain to avoid
misunderstandings if element is not present (Is it not supported
because of my version or because of my environment?)
Signed-off-by: Sebastian Mitterle <smitterl@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
1. Use 'setup' consistently as noun, 'set up' as verb
2. Use path variables like '$IMAGE_PATH' consistently
like in Troubleshooting to improve readability
3. Remove ':' from field names
4. Change phrasing in sentences I stumbled upon several
times to improve readability.
5. Minor grammar/vocab fixes.
Signed-off-by: Sebastian Mitterle <smitterl@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Han Han [Tue, 21 Apr 2020 12:19:35 +0000 (20:19 +0800)]
qemu: Implement the aio mode io_uring
This aio mode was added since Linux 5.1[1], QEMU 5.0.0[2], which will
deliever faster and more efficient I/O operations for the file,
host_device, host_cdrom backends.
Peter Krempa [Fri, 17 Apr 2020 08:58:08 +0000 (10:58 +0200)]
qemu: Move interlocking of blockjobs and checkpoints after liveness check
qemuDomainSupportsCheckpointsBlockjobs checks if the
QEMU_CAPS_INCREMENTAL_BACKUP capability is supported to do the
interlocking. Capabilities are not present when the VM isn't running
though which would create false errors.
Move the checks after the liveness check in block job implementations.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Pavel Mores <pmores@redhat.com>
Peter Krempa [Thu, 16 Apr 2020 09:23:07 +0000 (11:23 +0200)]
backup: Store error message for failed backups
If a backup job fails midway it's hard to figure out what happened as
it's running asynchronous. Use the VIR_DOMAIN_JOB_ERRMSG job statistics
field to pass through the error from the first failed backup-blockjob
so that both the consumer of the virDomainGetJobStats and the
corresponding event can see the error.
event 'job-completed' for domain backup-test:
operation: 9
time_elapsed: 46
disk_total: 104857600
disk_processed: 10158080
disk_remaining: 94699520
success: 0
errmsg: No space left on device
virsh domjobinfo backup-test --completed --anystats
Job type: Failed
Operation: Backup
Time elapsed: 46 ms
File processed: 9.688 MiB
File remaining: 90.312 MiB
File total: 100.000 MiB
Error message: No space left on device
Peter Krempa [Wed, 15 Apr 2020 10:27:53 +0000 (12:27 +0200)]
API: Add VIR_DOMAIN_JOB_ERRMSG domain job statistics field
In some cases it's useful to report the error which caused the domain
job to fail. Add an optional field for holding the error message so that
it can be later retrieved from statistics of a completed job.
Add the field name macro and code for extracting it in virsh.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Thu, 26 Mar 2020 16:55:00 +0000 (17:55 +0100)]
qemu: Add free and copy function for qemuDomainJobInfo and use it
In order to add a string to qemuDomainJobInfo we must ensure that it's
freed and copied properly. Add helpers to copy and free the structure
and adjust the code to use them properly for the new semantics.
Additionally also allocation is changed to g_new0 as it includes the
type and thus it's very easy to grep for all the allocations of a given
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
String typed parameter values were introduced in v0.9.7-30-g40624d32fb.
virDomainGetJobStats was introduced in v1.0.2-239-g4dd00f4238 so all
clients already support typed parameter stings at that time thus we can
enable it unconditionally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 08:36:59 +0000 (10:36 +0200)]
qemuhotplugtest: detach: Remove commands which are not issued
The 'human-monitor-command' equates to the 'drive-del' command issued by
the hotplug code on successful detach of a device. This means that it's
not issued for failed attempts and thus should not be added to the
expected list. Unfortunately our test monitor doesn't ensure that all
expected commands were consumed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We always queue the DEVICE_DELETED events before successful return from
the command so that tests are reliable. This means we can decrease the
unplug timeout as it's guaranteed to be executed in correct order.
According to my testing it shaves off ~450ms of test run:
real 0m0.721s
vs.
real 0m0.259s
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 23 Apr 2020 08:30:13 +0000 (10:30 +0200)]
qemuDomainGetUnplugTimeout: Add G_GNUC_NO_INLINE
The function is mocked in qemuhotplugmock.so. Recent clang versions
decided to inline it so the mock stopped working resulting in
qemuhotplugtest wasting 15 seconds waiting for timeouts.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
projects / libvirt.git / log