Ján Tomko [Mon, 23 Jul 2018 14:57:20 +0000 (16:57 +0200)]
tests: also skip qemuagenttest with old jansson
qemuagenttest also depends on JSON object key ordering:
Invalid value of argument 'vcpus' of command 'guest-set-vcpus':
expected '[{"logical-id":1,"online":false}]' got '[{"online":false,"logical-id":1}]'
Peter Krempa [Thu, 19 Jul 2018 15:07:45 +0000 (17:07 +0200)]
tests: qemuxml2argv: Unify testing of 'disk-network-rbd'
Move the authentication and ipv6 cases into the main test file. To allow
removal of the separate testing of the secure credential passing via the
'secret' object in qemu, use the DO_TEST_CAPS_VER macro with version
2.5.0 when the secret object is not supported by qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Unify most of the tests into a common test named disk-cdrom-network by
adding multiple cdroms. The 'http' test is dropped since there can be
only 4 cdroms.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 19 Jul 2018 11:38:29 +0000 (13:38 +0200)]
tests: qemuxml2argv: Remove tests obsoleted by assuming support for '-device'
Few disk tests were testing support for pure -drive command line
generation for disks now that we assume it for all qemu versions the
cases are obsolete.
Andrea Bolognani [Fri, 20 Jul 2018 07:50:37 +0000 (09:50 +0200)]
src: Make virStr*cpy*() functions return an int
Currently, the functions return a pointer to the
destination buffer on success or NULL on failure.
Not only does this kind of error handling look quite
alien in the context of libvirt, where most functions
return zero on success and a negative int on failure,
but it's also somewhat pointless because unless there's
been a failure the returned pointer will be the same
one passed in by the user, thus offering no additional
value.
Change the functions so that they return an int
instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Fri, 20 Jul 2018 12:15:09 +0000 (14:15 +0200)]
src: Don't rely on strncpy()-like behavior
The strncpy() function has this quirk where it will copy
*up* to the requested number of bytes, that is, it will
stop early if it encounters a NULL byte in the source
string.
This makes it legal to pass the size of the destination
buffer (minus one byte needed for the string terminator)
as the number of bytes to copy and still get something
somewhat reasonable out of the operation; unfortunately,
it also makes the function difficult to reason about
and way too easy to misuse.
We want to move away from the way strncpy() behaves and
towards better defined semantics, where virStrncpy()
will always copy *exactly* the number of bytes it's
been asked to copy; before we can do that, though, we
have to change a few of the callers.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Fri, 20 Jul 2018 07:45:12 +0000 (09:45 +0200)]
src: Use VIR_STRDUP() wherever possible
virStrcpy() and friends are useful when the destination
buffer has already been allocated, eg. as part of a struct;
if we have to allocate it on the spot, VIR_STRDUP() is a
better choice.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Mon, 16 Jul 2018 14:20:39 +0000 (16:20 +0200)]
src: Use virStrcpyStatic() wherever possible
This convenience macro was created for the simple cases
where the length of the source string and the size of the
destination buffer can be figued out with strlen() and
sizeof() respectively, so we should use it wherever
possible instead of open-coding parts of it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Tue, 17 Jul 2018 08:21:50 +0000 (10:21 +0200)]
src: Use virStrcpyStatic() to avoid truncation
The way virStrncpy() is called here will never result in
buffer overflow, but it won't prevent or detect truncation
either, despite what the error message might suggest. Use
virStrcpyStatic(), which does all of the above, instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
When gnutls negotiates TLS 1.3 instead of 1.2, the order of messages
sent by the handshake changes. This exposed a logic bug in the test
suite which caused us to wait for the server to see handshake
completion, but not wait for the client to see completion. The result
was the client didn't receive the certificate for verification and the
test failed.
This is exposed in Fedora 29 rawhide which has just enabled TLS 1.3 in
its GNUTLS builds.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit b57a9aec neglected to VIR_FREE(binding->filtername) as seen
in the following valgrind report
==6423== 17,328 bytes in 1,083 blocks are definitely lost in loss record 2,275 of 2,297
==6423== at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==6423== by 0x83B20C9: strdup (in /usr/lib64/libc-2.17.so)
==6423== by 0x533C144: virStrdup (virstring.c:977)
==6423== by 0x54BDD53: virGetNWFilterBinding (datatypes.c:865)
==6423== by 0x318D633C: nwfilterBindingCreateXML (nwfilter_driver.c:767)
==6423== by 0x54F3FC5: virNWFilterBindingCreateXML (libvirt-nwfilter.c:701)
==6423== by 0x539CE29: virDomainConfNWFilterInstantiate (domain_nwfilter.c:116)
==6423== by 0x31E516C2: qemuInterfaceBridgeConnect (qemu_interface.c:589)
==6423== by 0x31D98B56: qemuBuildInterfaceCommandLine (qemu_command.c:8418)
==6423== by 0x31D9F783: qemuBuildNetCommandLine (qemu_command.c:8673)
==6423== by 0x31D9F783: qemuBuildCommandLine (qemu_command.c:10354)
==6423== by 0x31DE355F: qemuProcessLaunch (qemu_process.c:6292)
==6423== by 0x31DE7881: qemuProcessStart (qemu_process.c:6686)
and
==6423== 17,328 bytes in 1,083 blocks are definitely lost in loss record 2,276 of 2,297
==6423== at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==6423== by 0x83B20C9: strdup (in /usr/lib64/libc-2.17.so)
==6423== by 0x533C144: virStrdup (virstring.c:977)
==6423== by 0x54BDD53: virGetNWFilterBinding (datatypes.c:865)
==6423== by 0x318D641F: nwfilterBindingLookupByPortDev (nwfilter_driver.c:678)
==6423== by 0x54F3B63: virNWFilterBindingLookupByPortDev (libvirt-nwfilter.c:593)
==6423== by 0x539CBC5: virDomainConfNWFilterTeardownImpl.isra.0 (domain_nwfilter.c:136)
==6423== by 0x539CFA5: virDomainConfVMNWFilterTeardown (domain_nwfilter.c:170)
==6423== by 0x31DE5651: qemuProcessStop (qemu_process.c:6912)
==6423== by 0x31E37974: qemuDomainDestroyFlags (qemu_driver.c:2229)
==6423== by 0x54C24BB: virDomainDestroy (libvirt-domain.c:475)
==6423== by 0x1589A2: remoteDispatchDomainDestroy (remote_daemon_dispatch_stubs.h:4827)
==6423== by 0x1589A2: remoteDispatchDomainDestroyHelper (remote_daemon_dispatch_stubs.h:4803)
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Marc Hartmayer [Tue, 3 Jul 2018 11:37:38 +0000 (13:37 +0200)]
rpc: Fix name of include guard
The include guard should match the file name and comment.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Tue, 3 Jul 2018 11:37:37 +0000 (13:37 +0200)]
virt-admin: Fix two error messages
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Tue, 3 Jul 2018 11:37:36 +0000 (13:37 +0200)]
daemon: Raise an error if 'max_workers' < 1 in libvirtd.conf
Hypervisor drivers (e.g. QEMU) assume that they run in a separate
thread from the main event loop thread otherwise deadlocks can
occur. Therefore let's report an error if max_workers < 1 is set in
the libvirtd configuration file.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Tue, 3 Jul 2018 11:37:35 +0000 (13:37 +0200)]
virThreadPool: Prevent switching between zero and non-zero maxWorkers
...since maxWorkers=0 is only intended for virtlockd or virlogd which
must not be multithreaded.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Marc Hartmayer [Tue, 3 Jul 2018 11:37:33 +0000 (13:37 +0200)]
rpc: Fix deadlock if there is no worker pool available
@srv must be unlocked for the call virNetServerProcessMsg otherwise a
deadlock can occur.
Since the pointer 'srv->workers' will never be changed after
initialization and the thread pool has it's own locking we can release
the lock of 'srv' earlier. This also fixes the deadlock.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Peter Krempa [Fri, 13 Jul 2018 15:25:33 +0000 (17:25 +0200)]
utils: storage: Add helper for checking if storage source is the same
To allow checking whether a storage source points to the same location
add a helper which checks the relevant fields. This will allow replacing
a similar check done by formatting the command line arguments for
qemu-like syntax.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 15 Jun 2018 12:55:19 +0000 (14:55 +0200)]
qemu: monitor: Add the 'query-nodes' argument for query-blockstats
The 'query-blockstats' command does not return statistics for the
explicitly named nodes unless the new argument is specified. Add
infrastrucuture that will allow us to use the new approach if desired.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 26 Jun 2018 04:58:47 +0000 (06:58 +0200)]
qemu: domain: Add helper for getting the disk backend alias
The disk backend alias was historically the alias of the -drive backing
the storage. For setups with -blockdev this will become more complex as
it will depend on other configs and generally will differ.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 24 Apr 2018 12:07:22 +0000 (14:07 +0200)]
qemu: domain: Move out clearing of backing chain in qemuDomainDetermineDiskChain
In some cases backing chain needs to be cleared prior to re-detection.
Move this step out of qemuDomainDetermineDiskChain as only certain
places need it and the function itself is able to skip to the end of the
chain to perform detection.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 18 Jun 2018 11:06:53 +0000 (13:06 +0200)]
qemu: driver: Reuse qemuDomainBlocksStatsGather in qemuDomainGetBlockInfo
Allow updating capacity for the block devices returned by
qemuDomainBlocksStatsGather and replace the open-coded call to
qemuMonitorGetAllBlockStatsInfo by the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 13 Jul 2018 14:19:38 +0000 (16:19 +0200)]
utils: storage: Add copying of PR definition to virStorageSource
Despite the warning that virStorageSourceCopy needs to be populated on
additions to the structure commit 687730540e4 neglected to implement the
copy function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 13 Jul 2018 12:14:17 +0000 (14:14 +0200)]
qemu: hotplug: Refactor/simplify PR managed addition to VM
Similarly to qemuDomainDiskRemoveManagedPR make it enter monitor on
its own so that it can be reused. Future users will be in the snapshot
code and in removable media change code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 11 Jul 2018 12:24:49 +0000 (14:24 +0200)]
qemu: hotplug: Simplify removal of managed PR infrastructure on unplug
Extract the (possible) removal of the PR backend and daemon into a
separate helper which enters monitor on its own. This simplifies the
code and allows reuse of this function in the future e.g. for blockjobs
where removing a image with PR may result into PR not being necessary.
Since the PR is not used often the overhead of entering monitor again
should be negligible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 11 Jul 2018 09:16:41 +0000 (11:16 +0200)]
qemu: hotplug: Reuse qemuHotplugDiskSourceRemove for disk backend removal
Add code which will convert a disk definition into
qemuHotplugDiskSourceData and then reuse qemuHotplugDiskSourceRemove to
remove all the backend related objects.
This unifies the detach code as much as possible with the already
existing helpers and will allow reuse this infrastructure when changing
removable disk media.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 11 Jul 2018 16:26:12 +0000 (18:26 +0200)]
qemu: hotplug: Prepare for multiple backing chain member hotplug
Similarly to how we've intergrated data belonging to a single
virStorageSource for purposes of attaching it to a qemu instance we will
need to agregate data relevant for the whole disk. With blockdev there
will be some disk-wide backing chain members such as the copy-on-read
handler.
Introduce qemuHotplugDiskSourceData which agregates the backing chain
and other data relevant for the disk and functions which generate it
and apply and rollback it.
In addition to disk hotplug this will also be reused for media changing
where we need to exchange the full disk backend.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 11 Jul 2018 15:30:19 +0000 (17:30 +0200)]
qemu: hotplug: Don't format NULL in %s in qemuHotplugPrepareDiskAccess
The warning messages which include the disk source could potentially
format NULL using %s as virDomainDiskGetSource may return NULL for e.g.
NBD disks. As most of the APIs are NOOP for remote disks the usage of
the source string only should be fine for now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 3 Jul 2018 12:21:56 +0000 (14:21 +0200)]
qemu: monitor: Remove old code for dual handling of 'transaction' data
Now that we use only the separate function for creating data for the
'transaction' command we can remove all the boilerplate which was
necessary before.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 3 Jul 2018 12:13:29 +0000 (14:13 +0200)]
qemu: block: Create helper for creating data for legacy snapshots
With 'transaction' support we don't need to keep around the multipurpose
code which would create the snapshot if 'transaction' is not supported.
To simplify this add a new helper that just wraps the arguments for
'blockdev-snapshot-sync' operation in 'transaction' and use it instead
of qemuBlockSnapshotAddLegacy.
Additionally this allows to format the arguments prior to creating the
file for simpler cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 3 Jul 2018 07:24:40 +0000 (09:24 +0200)]
qemu: monitor: Add API to help creating 'transaction' arguments
Add a new helper that will be solely used to create arguments for the
transaction command. Later on this will make it possible to remove the
overloading which was caused by the fact that snapshots were created
without transaction and also will help in blockdevification of snapshots.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 3 Jul 2018 05:52:11 +0000 (07:52 +0200)]
qemu: snapshot: Unify conditions checking whether snapshot needs to be taken
In the cleanup path we already checked whether a snapshot needed to be
taken by looking into the collected data. Use the same approach when
creating the snapshot command data and when committing the changes to the
domain definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 3 Jul 2018 05:17:29 +0000 (07:17 +0200)]
qemu: snapshot: Require support of 'transaction' command for external snapshots
While qemu supports the 'transaction' command since v1.1.0
(52e7c241ac766406f05fa) and the 'blockdev-snapshot-sync' command since
v0.14.0-rc0 we need to keep the capability bits present since some qemu
downstreams (RHEL/CentOS 7 for example) chose to cripple qemu by
arbitrarily compiling out some stuff which was already present at that
time.
To simplify the crazy code just require both commands to be present at
the beginning of an external snapshot so that we can remove the case when
'transaction' would not be supported.
This also allows to drop any logic connected to the
VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC flag since snapshots are atomic with
the 'transaction' command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Julio Faracco [Thu, 19 Jul 2018 04:21:48 +0000 (01:21 -0300)]
qemu: Fix broken autostart symlink after renaming domain
If a domain is configured to start on boot, it has a symlink to the
domain definition inside the autostart directory. If you rename this
domain, the definition is renamed too. The symlink need to be pointed to
this renamed file. This commit recreates the symlink after renaming the
XML file.
util: set OOM in virCopyLastError if error is not set
virCopyLastError is intended to be used after last error is set.
However due to virLastErrorObject failures (very unlikely though
as thread local error is allocated on first use) we can have zero
fields in a copy as a result. In particular code field can be set
to VIR_ERR_OK.
In some places (qemu monitor, qemu agent and qemu migaration code
for example) we use copy result as a flag and this leads to bugs.
Let's set OOM-like error in copy in case of virLastErrorObject failures.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
locking: don't create qemu-sanlock.conf file when QEMU isn't enabled
The test targets result in the qemu-sanlock.conf file being created
when sanlock is enabled, even if QEMU is not enabled. As a result it
never gets cleaned up when distclean is run, breaking distcheck.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Erik Skultety [Mon, 21 May 2018 12:29:26 +0000 (14:29 +0200)]
conf: Introduce new video type 'none'
Historically, we've always enabled an emulated video device every time we
see that graphics should be supported with a guest. With the appearance
of mediated devices which can support QEMU's vfio-display capability,
users might want to use such a device as the only video device.
Therefore introduce a new, effectively a 'disable', type for video
device.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since QEMU 2.12, QEMU understands a new vfio-pci device option 'display'
which can be used to turn on display capabilities on vgpu-enabled
mediated devices, IOW emulated GPU devices like QXL will no longer be
needed with vgpu-enable mdevs.
QEMU defaults to 'auto' for the 'display' attribute, which is not
foolproof, so we need to play it safe here and default to display='off'
if this attribute wasn't provided in the XML explicitly.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com>
Erik Skultety [Thu, 24 May 2018 14:04:26 +0000 (16:04 +0200)]
conf: Introduce new <hostdev> attribute 'display'
QEMU 2.12 introduced a new type of display for mediated devices using
vfio-pci backend which allows a mediated device to be used as a VGA
compatible device as an alternative to an emulated video device. QEMU
exposes this feature via a vfio device property 'display' with supported
values 'on/off/auto' (libvirt will default to 'off').
This patch adds the necessary bits to domain config handling in order to
expose this feature. Since there's no convenient way for libvirt to come
up with usable defaults for the display setting, simply because libvirt
is not able to figure out which of the display implementations - dma-buf
which requires OpenGL support vs vfio regions which doesn't need OpenGL
(works with OpenGL enabled too) - the underlying mdev uses.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com>
A simple helper which will loop through all the graphics elements and
checks whether at least one of them enables OpenGL support, either by
containing <gl enable='yes'/> or being of type 'egl-headless'.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Acked-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Erik Skultety [Sat, 30 Jun 2018 14:23:01 +0000 (16:23 +0200)]
qemu: Introduce a new graphics display type 'headless'
Since 2.10 QEMU supports a new display type egl-headless which uses the
drm nodes for OpenGL rendering copying back the rendered bits back to
QEMU into a dma-buf which can be accessed by standard "display" apps
like VNC or SPICE. Although this display type can be used on its own,
for any practical use case it makes sense to pair it with either VNC or
SPICE display. The clear benefit of this display is that VNC gains
OpenGL support, which it natively doesn't have, and SPICE gains remote
OpenGL support (native OpenGL support only works locally through a UNIX
socket, i.e. listen type=socket/none).
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com>
Erik Skultety [Sat, 30 Jun 2018 14:16:24 +0000 (16:16 +0200)]
qemu: caps: Introduce a capability for egl-headless
Since QEMU 2.10, it's possible to use a new type of display -
egl-headless which uses drm nodes to provide OpenGL support. This patch
adds a capability for that. However, since QEMU doesn't provide a QMP
command to probe it, we have to base the capability on specific QEMU
version.
Acked-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Erik Skultety <eskultet@redhat.com>
Ján Tomko [Fri, 11 May 2018 12:27:02 +0000 (14:27 +0200)]
build: require Jansson if QEMU driver is enabled
If the QEMU driver was requested, require Jansson, since we need to use
the JSON monitor to probe capabilities for all QEMU version supported
by libvirt.
projects / libvirt.git / log