Peter Krempa [Tue, 10 Nov 2020 07:55:03 +0000 (08:55 +0100)]
qemu: backup: Install bitmap for incremental backup to appropriate node only
Libvirt's backup code has two modes:
1) push - where qemu actively writes the difference since the checkpoint
into the output file
2) pull - where we instruct qemu to expose a frozen disk state along
with a bitmap of blocks which changed since the checkpoint
For push mode qemu needs the temporary bitmap we use where we calculate
the actual changes to be present on the block node backing the disk.
For pull mode where we expose the bitmap via NBD qemu actually wants the
bitmap to be present for the exported block node which is the scratch
file.
Until now we've calculated the bitmap twice and installed it both to the
scratch file and to the disk node, but we don't need to since we know
when it's needed.
Pass in the 'pull' flag and decide where to install the bitmap according
to it and also when to register the bitmap name with the blockjob.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Fri, 13 Nov 2020 14:20:58 +0000 (15:20 +0100)]
qemu: conf: Enable 'backup_tls_x509_verify' by default
The NBD server used to export pull-mode backups doesn't have any other
form of client authentication on top of the TLS transport, so the only
way to authenticate clients is to verify their certificate.
Enable this option by defauilt when both 'backup_tls_x509_verify' and
'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Fri, 13 Nov 2020 14:20:58 +0000 (15:20 +0100)]
qemu: conf: Enable 'migrate_tls_x509_verify' by default
The migration stream connection and also the NBD server for non-shared
storage migration don't have any other form of client authentication on
top of the TLS transport, so the only way to authenticate clients is to
verify their certificate.
Enable this option by defauilt when both 'migrate_tls_x509_verify' and
'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Fri, 13 Nov 2020 14:20:58 +0000 (15:20 +0100)]
qemu: conf: Enable 'chardev_tls_x509_verify' by default
Chardevs don't have any other form of client authentication on top of
the TLS transport, so the only way to authenticate clients is to verify
their certificate.
Enable this option by defauilt when both 'chardev_tls_x509_verify' and
'default_tls_x509_verify' were not configured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1879477 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 13 Nov 2020 14:18:37 +0000 (15:18 +0100)]
qemu: conf: Clarify default of "vnc_tls_x509_verify"
If both "vnc_tls_x509_verify" and "default_tls_x509_verify" are missing
from the config file the client certificate validation is disabled. VNC
provides a layer of authentication so client certificate validation is
not strictly required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 13 Nov 2020 14:13:29 +0000 (15:13 +0100)]
qemu: conf: Allow individual control of default value for *_tls_x509_verify
Store whether "default_tls_x509_verify" was provided and enhance the
SET_TLS_VERIFY_DEFAULT macro so that indiviual users can provide their
own default if "default_tls_x509_verify" config option was not provided.
For now we keep setting it to 'false'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Typecast the controller type variable to the appropriate type and add
the missing controller types for future extension.
Note that we currently allow only unplug of
VIR_DOMAIN_CONTROLLER_TYPE_SCSI thus the other controller types which
are not implemented return false now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 13 Nov 2020 13:07:40 +0000 (14:07 +0100)]
qemuDomainDiskControllerIsBusy: Fix logic of matching disk bus to controller type
The tests which match the disk bus to the controller type were backwards
in this function. This meant that any disk bus type (such as
VIR_DOMAIN_DISK_BUS_SATA) would not skip the controller index comparison
even if the removed controller was of a different type.
Switch the internals to a switch statement with selects the controller
type in the first place and a proper type so that new controller types
are added in the future.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870072 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 4 Nov 2020 16:31:57 +0000 (17:31 +0100)]
docs: kbase: Reorder some articles in the 'Usage' section
Historically we've added them in chronological order, but certain
articles are more likely to be needed and thus are easier to find when
placed earlier.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Wed, 4 Nov 2020 15:27:32 +0000 (16:27 +0100)]
docs: xsl: Unify stylability of main container element
page.xsl was adding '<div id="content">' wrapper for the content picked
up from the <body> element from the original input file. Optionally
class="$DOCNAME" was added for some documents taken from <body>.
Since docs generated from RST by docutils have a '<div class='document'
id='$DOCNAME>' we actually don't need an extra wrapper for them.
Additionally if we standardize on one of them we can use the same styles
for both. I've picked the latter because it makes more sense to use the
document name as 'id'.
This patch:
1) Modifies the XSL trasformation to add the wrapper only if it's not
present.
2) Modifies the XSL transformation to use 'id' for document name and
class='document' for the wrapper element.
3) Changes docs.html/index.html/hvsupport.html to use 'id' instead of
'class' for document name.
4) Modifies the main stylesheet to keep styling the elements properly
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Wed, 4 Nov 2020 14:51:17 +0000 (15:51 +0100)]
docs: kbase: Split articles into sections
Split the existing list of kbase articles into a 'Usage' category and
into 'Internals/Debugging'. This will later represent the two columns on
the web page.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Wed, 4 Nov 2020 14:37:46 +0000 (15:37 +0100)]
docs: kbase: Remove extra container from index page
The container was used to apply CSS classes to the content, so the looks
are degraded. The idea is to have a similar layout to the 'docs.html'
page with multiple columns, which will be added later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Wed, 4 Nov 2020 14:18:29 +0000 (15:18 +0100)]
docs: kbase: Move index page to docs/kbase
Move docs/kbase.rst to docs/kbase/index.rst so that the directory itself
shows our index page rather than the autogenerated list of files by the
webserver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Thu, 12 Nov 2020 18:42:00 +0000 (19:42 +0100)]
util: Make virFileClose() quiet on success
While it's certainly good to log events like "failed to close fd"
and "tried to close invalid fd", which are likely to be the
consequence of some bug in libvirt, logging a message every single
time a file descriptor is closed successfully is perhaps excessive
and can lead to useful information being missed among the noise.
Log filters don't help in this situation, because filtering out all
of util.file is too big a hammer and would cause important messages
to be left out as well.
To give an idea of just how much noise this single debug statement
can cause, here's a real life example from a quite large libvirtd
log I had to look at recently:
Laine Stump [Fri, 13 Nov 2020 17:13:59 +0000 (12:13 -0500)]
util: remove ATTRIBUTE_NONNULL from virDirClose declaration
Before commit 24d8968c, virDirClose took a DIR**, and that was never
NULL, so its declaration included ATTRIBUTE_NONNULL(1). Since that
commit, virDirClose takes a DIR*, and it may be NULL (e.g. if the DIR*
is initialized to NULL and was never closed).
Even though virDirClose() is currently only called implicitly (as the
cleanup for a g_autoptr(DIR)), and (as I've just newly learned) the
autocleanup function g_autoptr will only be called if the pointer in
question is non-null (see the definition of
_GLIB_AUTOPTR_CLEAR_FUNC_NAME in
/usr/include/glib-2.0/glib/gmacros.h), it does still cause Coverity to
complain that it *could* be called with a NULL, and it's also possible
that in the future someone might add code that explicitly calls
virDirClose.
To eliminate the Coverity complaints, and protect against the
hypothetical future where someone both explicitly calls virDirClose()
with a potentially NULL value, *and* re-enables the nonnull directive
when not building with Coverity (disabled by commit eefb881) this
patch removes the ATTRIBUTE_NONNULL(1) from the declaration of
virDirClose().
Fixes: 24d8968cd0a718af4badbbc858b1b449fea7205a Reported-by: John Ferlan <jferlan@redhat.com> Details-Research-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Laine Stump <laine@redhat.com>
Andrea Bolognani [Fri, 13 Nov 2020 15:27:24 +0000 (16:27 +0100)]
kbase: Shorten "less verbose QEMU logging" example
Rationale for the changes:
* access can be filtered out entirely, as nothing very
interesting is produced by the only other component in the
same package (access.accessdriverpolkit);
* util.udev doesn't exist.
Related filters are also more consistently grouped together.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Jonathan Watt [Fri, 13 Nov 2020 13:30:45 +0000 (13:30 +0000)]
docs: compiling.html: pass -d to xz to decompress
tar on macOS recognizes XZ compression automatically, but that is
not the case for GNU tar (1.32 at least). On Fedora 33 the current
instructions result in the following error:
$ xz -c libvirt-6.9.0.tar.xz | tar xvf -
tar: Archive is compressed. Use -J option
tar: Error is not recoverable: exiting now
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Jonathan Watt <jwatt@jwatt.org>
qemu: qemuDomainDefineXMLFlags: move cleanup logic to cleanup section
Let's move objlist restoring to cleanup section so that we can handle failure
of actions between virDomainObjListAdd and virDomainDefSave. We are going
to add such actions in next patch.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu: rename: support renaming checkpoints directory
This is basically just saves checkpoints metadata on disk after name is changed
in memory as path to domain checkpoints directory depends on name. After that
old checkpoint directory is deleted with checkpoint metadata files.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu: rename: support renaming snapshots directory
This is basically just saves snapshots metadata on disk after name is changed
in memory as path to domain snapshot directory depends on name. After that
old snapshot directory is deleted with snapshot metadata files.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemu: remove duplicate code for removing remnant files
This patch also changes functionality a bit.
First if unlinking of old config file is failed we rollback and return error
previously and now we return success. I don't think this makes much difference.
I guess in both cases on libvirtd restart we have to deal with both new and old
config existing on disk with different names but same uuid.
Second if unlinking of old autolink is failed we rollback previously which
was not right as at this point we already unlink old config file. So this
is fixed now.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Peter Krempa [Thu, 12 Nov 2020 13:31:57 +0000 (14:31 +0100)]
tools: virsh: Reset error when keepalive registration fails
We try to enable keepalive oportunistically. If it's not supported by
the connection driver and it was not explicitly requested we keep the
error object set and can report it in some cases accidentally:
--- stdout ---
TEST: /home/pipo/libvirt/tests/virsh-self-test
! 1 FAILED
--- stderr ---
error: parameter 'target' of command 'attach-disk' must be listed before optional parameters
error: this function is not supported by the connection driver: virConnectSetKeepAlive
-------
Clear the stored libvirt error.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The help formatter called vshCmddefOptParse just for validation
purposes. Since vshCmddefOptParse no longer validates the command itself
and we don't need the bitmaps returned by it we can drop the call
entirely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 12 Nov 2020 12:30:29 +0000 (13:30 +0100)]
tools: vshCmddefCheckInternals: Port mandatory options check from vshCmddefOptParse
'vshCmddefCheckInternals' is the go-to place for all checks related to
the definition of parameters for commands, but the check that all
mandatory parameters must be ordered before optional parameters was
still only in vshCmddefOptParse.
Adding a non-compliant option would not be caught by our test suite as
'virsh self-test' doesn't call vshCmddefOptParse.
Re-implement the check in vshCmddefCheckInternals.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Mon, 25 May 2020 09:35:12 +0000 (11:35 +0200)]
qemu: Do not require TSC frequency to strictly match host
Some CPUs provide a way to read exact TSC frequency, while measuring it
is required on other CPUs. However, measuring is never exact and the
result may slightly differ across reboots. For this reason both Linux
kernel and QEMU recently started allowing for guests TSC frequency to
fall into +/- 250 ppm tolerance interval around the host TSC frequency.
Let's do the same to avoid unnecessary failures (esp. during migration)
in case the host frequency does not exactly match the frequency
configured in a domain XML.
Every time we create new virCommand of OVS_VSCTL it must be
followed by virNetDevOpenvswitchAddTimeout() call which adds the
--timeout=X argument to freshly created cmd. Instead of having
this as two separate function calls it can be just one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
virnetdevopenvswitch: Get names for dpdkvhostuserclient too
There are two types of vhostuser ports:
dpdkvhostuser - OVS creates the socket and QEMU connects to it
dpdkvhostuserclient - QEMU creates the socket and OVS connects to it
But of course ovs-vsctl syntax for fetching ifname is different.
So far, we've implemented the former. The lack of implementation
for the latter means that we are not detecting the interface name
and thus not reporting it in domain XML, or failing to get
interface statistics.
Jiri Denemark [Wed, 11 Nov 2020 15:50:16 +0000 (16:50 +0100)]
conf: Use unsigned long long for timer frequency
Although the code in qemuProcessStartValidateTSC works as if the
timer frequency was already unsigned long long (by using an appropriate
temporary variable), the virDomainTimerDef structure actually defines
frequency as unsigned long, which is not guaranteed to be 64b.
Fixes support for frequencies higher than 2^32 - 1 on 32b systems.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Masayoshi Mizuma [Wed, 11 Nov 2020 13:35:24 +0000 (08:35 -0500)]
qemu: Move qemuExtDevicesStop() before removing the pidfiles
A qemu guest which has virtiofs config fails to start if the previous
starting failed because of invalid option or something.
That's because the virtiofsd isn't killed by virPidFileForceCleanupPath()
on the former failure because the pidfile was already removed by
virFileDeleteTree(priv->libDir) in qemuProcessStop(), so
virPidFileForceCleanupPath() just returned.
Move qemuExtDevicesStop() before virFileDeleteTree(priv->libDir) so that
virPidFileForceCleanupPath() can kill virtiofsd correctly.
For example of the reproduction:
# virsh start guest
error: Failed to start domain guest
error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -foo: invalid option
... fix the option ...
# virsh start guest
error: Failed to start domain guest
error: Cannot open log file: '/var/log/libvirt/qemu/guest-fs0-virtiofsd.log': Device or resource busy
#
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>