Jie Wang [Fri, 31 May 2019 07:33:31 +0000 (15:33 +0800)]
qemu: Try harder to remove pr-helper object and kill pr-helper process
If libvirt receives DISCONNECTED event and prDaemonRunning is set
to false, and qemuDomainRemoveDiskDevice() is performing in the
meantime, then qemuDomainRemoveDiskDevice() will fail to remove
pr-helper object because prDaemonRunning is false. But removing
that check from qemuHotplugRemoveManagedPR() is not enough,
because after removing the object through monitor the
qemuProcessKillManagedPRDaemon() is called which contains the
same check. Thus the pr-helper process might be left behind.
Signed-off-by: Jie Wang <wangjie88@huawei.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Tue, 11 Jun 2019 15:16:57 +0000 (17:16 +0200)]
tests: Include LDADDS in qemu_LDADDS
At this point, all test programs that use qemu_LDADDS also
use LDADDS, so we can remove a bunch of repetition by simply
including the latter in the former.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Andrea Bolognani [Wed, 12 Jun 2019 09:42:39 +0000 (11:42 +0200)]
tests: Tweak vircapstest_LDADD
We optionally include QEMU and LXC support in this test and
depending on which is enabled (if either is enabled at all) we
need to link in different objects.
Right now we implicitly depend on the fact that qemu_LDADDS is
empty when QEMU is not enabled to get the correct set of objects,
but it's better to be explicit about it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Andrea Bolognani [Tue, 11 Jun 2019 14:03:24 +0000 (16:03 +0200)]
tests: Tweak cputest_LDADDS
We want all test programs using qemu_LDADDS to also use LDADDS,
and cputest is the only existing exception.
We can't just replace GNULIB_LIBS with LDADDS though, even though
the latter is a superset of the former, because that would result
in a linking error due to including the same object twice:
/usr/bin/ld:
../src/libvirt_probes.o:.../src/libvirt_probes.o.dtrace-temp.c:141:
multiple definition of `libvirt_object_new_semaphore';
../src/libvirt_probes.o:.../src/libvirt_probes.o.dtrace-temp.c:141:
first defined here
To work around this, we include both qemu_LDADDS and LDADDS when
QEMU support is enabled, and just LDADDS otherwise.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Since the behaviour of launch security is heavily dependent on 3rd party
vendors (e.g. AMD SEV) where the data returned can be essentially
anything, the most reasonable approach here in the test driver is not to
try return any data.
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Peter Krempa [Tue, 11 Jun 2019 14:42:53 +0000 (16:42 +0200)]
qemu: Use proper block job name when reconnecting to VM
The hash table returned by qemuMonitorGetAllBlockJobInfo is organized by
the frontend name (which skipps the 'drive-' prefix). While our code
properly matches the jobs to the disk, qemu needs the full job name
including the 'drive-' prefix to be able to identify jobs.
Fix this by adding an argument to qemuMonitorGetAllBlockJobInfo which
does not modify the job name before filling the hash.
This fixes a regression where users would not be able to cancel/pivot
block jobs after restarting libvirtd while a blockjob is running.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 11 Jun 2019 14:38:02 +0000 (16:38 +0200)]
qemu: domain: Allow forcing images to read-write in qemuDomainStorageSourceAccessAllow
In commit 76b9aba2ba6 I refactored how the function treats the readonly
flag which introduced a bug when we'd not allow to force read-write
state for an image.
This created problems with blockjobs where we need to temporarily
force images to have read-write permissions.
Rename QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_READ_ONLY to
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_FORCE_READ_ONLY and also introduce
a complement QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_FORCE_READ_WRITE which
will allow to force write access.
Peter Krempa [Thu, 30 May 2019 11:57:08 +0000 (13:57 +0200)]
tests: uri: Add test for urlencoded URIs
When specifying extra params for spcie TLS verification, it's necessary
to pass a weird URI to it. Let's add a test for this case where the TLS
string contains a space.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ilias Stamatis [Mon, 10 Jun 2019 09:04:59 +0000 (11:04 +0200)]
test_driver: extract image loading code into a separate function
Extracting the code logic for opening and parsing a test image from
testDomainRestoreFlags into a separate function, allows us to reuse this
code in other functions such as testDomainSaveImageGetXMLDesc.
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Ilias Stamatis [Mon, 10 Jun 2019 09:04:58 +0000 (11:04 +0200)]
test_driver: extract image saving code into a separate function
Extracting the code logic for writing a test image to disk from
testDomainSaveFlags into a separate function, allows us to reuse this
code in other functions such as testDomainSaveImageDefineXML.
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Eric Blake [Thu, 6 Jun 2019 02:25:05 +0000 (21:25 -0500)]
backup: Add new parameters to qemu monitor nbd-server-add
The upcoming virDomainBackup() API needs to take advantage of the
ability to expose a bitmap as part of nbd-server-add for a pull-mode
backup (this is the recently-added QEMU_CAPS_NBD_BITMAP capability).
Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Peter Krempa <pkrempa@redhat.com>
Eric Blake [Thu, 6 Jun 2019 02:25:05 +0000 (21:25 -0500)]
backup: Add new qemu monitor bitmap
The upcoming virDomainBackup() API needs to take advantage of various
qcow2 bitmap manipulations as the basis to virDomainCheckpoints and
incremental backups. Add four functions to expose
block-dirty-bitmap-{add,enable,disable,merge} (this is the
recently-added QEMU_CAPS_BITMAP_MERGE capability).
Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Peter Krempa <pkrempa@redhat.com>
Eric Blake [Thu, 6 Jun 2019 02:25:05 +0000 (21:25 -0500)]
backup: Add two new qemu capabilities
Add two capabilities for testing features required for the upcoming
virDomainBackupBegin: use block-dirty-bitmap-merge as the generic
witness of bitmap support needed for checkpoints (since all of the
bitmap management functionalities were finalized in the same qemu 4.0
release), and the bitmap parameter to nbd-server-add for pull-mode
backup support. Even though both capabilities are likely to be
present or absent together (that is, it is unlikely to encounter a
qemu that backports only one of the two), it still makes sense to keep
two capabilities as the two uses are orthogonal (full backups don't
require checkpoints, push mode backups don't require NBD bitmap
support, and checkpoints can be used for more than just incremental
backups).
Existing code is not affected by the new capabilities.
Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Peter Krempa <pkrempa@redhat.com>
Eric Blake [Tue, 9 Apr 2019 21:21:38 +0000 (16:21 -0500)]
backup: Prepare for Unix sockets in QMP nbd-server-start
Migration always uses a TCP socket for NBD servers, because we don't
support same-host migration. But upcoming pull-mode incremental backup
needs to also support a Unix socket, for retrieving the backup from
the same host. Support this by plumbing virStorageNetHostDef through
the monitor calls, since that is a nice reusable struct that can track
both TCP and Unix sockets.
Update qemumonitorjsontest to verify both forms of the QMP command.
Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Peter Krempa <pkrempa@redhat.com>
Eric Blake [Tue, 11 Jun 2019 03:01:08 +0000 (22:01 -0500)]
qemumonitorjsontest: Simplify returns
Time to remove the cleanup labels rendered useless in the previous
patch. There are still plenty of other tests that could be further
simplified, but I've already spent enough time in this file for now.
Eric Blake [Wed, 12 Jun 2019 01:59:11 +0000 (20:59 -0500)]
qemumonitorjsontest: Validate more commands against schema
The DO_TEST() macro in qemumonitorjsontest.c was not passing the
schema through, which meant that we were not validating any of those
tests for correct usage according to the schema.
In the process of mechanically altering tests to pass the schema
through, use VIR_AUTOPTR on all of the affected test instances. The
next patch will do some further cleanups that it exposes.
Tested by using this hack, where the test mistakenly passed pre-patch,
but correctly diagnosed the garbage post-patch:
Peter Krempa [Wed, 5 Jun 2019 13:29:37 +0000 (15:29 +0200)]
qemu: snapshot: Don't overload 'ret' in qemuDomainSnapshotCreateDiskActive
Introduce 'rc' for collecting state from monitor commands so that we can
initialize 'ret' to -1. This also fixes few cases which could return 0
from the function despite an error condition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 5 Jun 2019 13:16:34 +0000 (15:16 +0200)]
qemu: snapshot: Move all cleanup of snapshot disk data to qemuDomainSnapshotDiskDataFree
qemuDomainSnapshotDiskDataFree also removes the resources associated
with the disk data. Move the unlinking of the just-created file so that
we can unify the cleanup paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 10 Jun 2019 10:28:10 +0000 (12:28 +0200)]
qemu: Rename qemuDomainSnapshotDiskDataFree to qemuDomainSnapshotDiskDataCleanup
In commit cbb4d229de30b5 I named the function with 'free' suffix, but at
that time it already did some non-freeing tasks. Rename it to make it
obvious that it's not just memory managemet.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 5 Jun 2019 12:24:25 +0000 (14:24 +0200)]
qemu: snapshot: Densely pack data in qemuDomainSnapshotDiskDataCollect
The function skips disks which are not selected for snapshot. Rather
than creating a sparse array and check whether the given field is filled
compress the entries.
Note that this does not allocate a smaller array, but the memory
allocation is short-lived.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
This configuration can be used by gitdm to generate reports about
libvirt development.
The goal I was working with was being able to generate a report
for every single libvirt release and having zero "email address
as company" entries; picking different commit ranges might result
in some contributions not being accounted for.
I had to make some judgement calls when the situation was not
entirely clear-cut: when in doubt, and not finding any obvious
signs of the opposite being true, I mostly ended up dumping
people in the "unaffiliated contributions" bin. If I got it
wrong, and companies want to get recognition for their sponsored
contributions to libvirt, they can send patches.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Fabiano Fidencio was working for Red Hat when he contributed to
libvirt, Shi Lei's non-company email address contains the company
name so it's fair to assume contributions using it were made on
company time, and Adrian Brzezinski's personal email was used
for the S-o-b while the git authorship information clearly pointed
at the company being involved.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu: Fix NULL pointer access in qemuProcessInitCpuAffinity()
Commit 2f2254c7f4e5 attempted to fix a memory leak by ensuring
cpumapToSet is always a freshly allocated bitmap, but regrettably
introduced a NULL pointer access while doing so, because it called
virBitmapCopy() without allocating the destination bitmap first.
Solve the issue by using virBitmapNewCopy() instead.
Reported-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
In two out of three scenarios we are cleaning up properly after
ourselves, but commit 5f2212c062c7 has changed the remaining one
in a way that caused it to start leaking cpumapToSet.
Refactor the logic so that cpumapToSet is always a freshly
allocated bitmap that gets cleaned up automatically thanks to
VIR_AUTOPTR(); this also allows us to remove the hostcpumap
variable.
Reported-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Right now, if numad fails, we raise an error but return an
empty string to the caller instead of a NULL pointer, which
means processing will continue and the user will see
# virsh start guest
error: Failed to start domain guest
error: invalid argument: Failed to parse bitmap ''
instead of a more reasonable
# virsh start guest
error: Failed to start domain guest
error: operation failed: Failed to query numad for the advisory nodeset
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Thu, 30 May 2019 17:20:34 +0000 (19:20 +0200)]
qemu: Fix qemuProcessInitCpuAffinity()
Ever since the feature was introduced with commit 0f8e7ae33ace,
it has contained a logic error in that it attempted to use a NUMA
node map where a CPU map was expected.
Because of that, guests using <numatune> might fail to start:
# virsh start guest
error: Failed to start domain guest
error: cannot set CPU affinity on process 40055: Invalid argument
This was particularly easy to trigger on POWER 8 machines, where
secondary threads always show up as offline in the host: having
in the guest configuration, for example, would result in libvirt
trying to set the process affinity so that it would prefer
running on CPU 1, but since that's a secondary thread and thus
shows up as offline, the operation would fail, and so would
starting the guest.
Use the newly introduced virNumaNodesetToCPUset() to convert the
NUMA node map to a CPU map, which in the example above would be
48,56,64,72,80,88 - a valid input for virProcessSetAffinity().
Ilias Stamatis [Sat, 1 Jun 2019 12:40:12 +0000 (14:40 +0200)]
virDomainSendKey: validate codeset argument
This argument wasn't validated anywhere, neither in the generic
implementation nor in the individual drivers. As a result a call to this
function with a large enough codeset value prior to this change causes
libvirtd to crash.
This happens because all drivers call virKeycodeValueTranslate which
uses codeset as an index to the virKeymapValues array, causing an
out-of-bounds error.
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Jiri Denemark [Thu, 30 May 2019 22:03:59 +0000 (00:03 +0200)]
qemu: Check TSC frequency before starting QEMU
When migrating a domain with invtsc CPU feature enabled, the TSC
frequency of the destination host must match the frequency used when the
domain was started on the source host or the destination host has to
support TSC scaling.
If the frequencies do not match and the destination host does not
support TSC scaling, QEMU will fail to set the right TSC frequency when
starting vCPUs on the destination and thus migration will fail. However,
this is quite late since both host might have spent significant time
transferring memory and perhaps even storage data.
By adding the check to libvirt we can let migration fail before any data
starts to be sent over. If for some reason libvirt is unable to detect
the host's TSC frequency or scaling support, we'll just let QEMU try and
the migration will either succeed or fail later.
Luckily, we mandate TSC frequency to be explicitly set in the domain XML
to even allow migration of domains with invtsc. We can just check
whether the requested frequency is compatible with the current host
before starting QEMU.
Jiri Denemark [Thu, 30 May 2019 19:47:38 +0000 (21:47 +0200)]
cpu_x86: Probe TSC frequency and scaling support
When the host CPU supports invariant TSC the host CPU definition created
by virCPUx86GetHost will contain (unless probing fails for some reason)
addition TSC related data.
Jiri Denemark [Thu, 30 May 2019 14:34:59 +0000 (16:34 +0200)]
util: Add virHostCPUGetTscInfo
On a KVM x86_64 host which supports invariant TSC this function can be
used to detect the TSC frequency and the availability of TSC scaling.
The magic MSR numbers required to check if VMX scaling is supported on
the host are documented in Volume 3 of the Intel® 64 and IA-32
Architectures Software Developer’s Manual.
Jiri Denemark [Mon, 3 Jun 2019 11:15:19 +0000 (13:15 +0200)]
qemuargv2xmltest: Use mocked virQEMUCapsProbeHostCPU
The qemuTestParseCapabilitiesArch call would eventually lead to the host
CPU being probed via virCPUGetHost. Let's divert this to a mocked
version already used by the qemuxml2argvtest.
Jiri Denemark [Mon, 3 Jun 2019 11:13:38 +0000 (13:13 +0200)]
qemu: Make virQEMUCapsProbeHostCPUForEmulator more generic
The function is renamed as virQEMUCapsProbeHostCPU and it does not get
the list of allowed CPU models from qemuCaps anymore. This is
responsibility is moved to the caller. The result is just a very thin
wrapper around virCPUGetHost mostly required mocking in tests.
The generic function is used in place of a direct call to virCPUGetHost
in virQEMUCapsInitHostCPUModel to make sure tests don't accidentally
probe host CPU.
Turns out, some aarch64 systems have SMBIOS info. That means we
can use dmidecode to fetch some information. If that fails, fall
back to the old behaviour.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Fri, 10 May 2019 12:08:09 +0000 (14:08 +0200)]
virsysinfo: Rename virSysinfoReadX86 to virSysinfoReadDMI
There's nothing x86 specific about this function. Rename the
function so that it has DMI suffix which enables it to be reused
on different arches (as using X86 from say ARM would look
suspicious).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Thu, 30 May 2019 08:00:58 +0000 (10:00 +0200)]
tests: Tweak x86 sysinfo test case
We have a single mock dmidecode script right now, but we're
going to add another one soon, so we need to make sure its
name contains the test case name as a prefix, just like we
already do with all data files.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Acked-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Tue, 28 May 2019 14:00:57 +0000 (16:00 +0200)]
qemu: Move capability checks for IOMMU features
All current IOMMU features are specific to Intel IOMMU, so
understandably we check for the corresponding capabilities
inside the Intel-specific switch() branch; however, we want
to make sure SMMUv3 IOMMU users get an error if they try to
enable any of those features in their guest, and performing
the capability checks unconditionally is both the easiest
way to achieve that, as well as the one least likely to
result in us inadvertently letting users enable some new
Intel-specific IOMMU feature for ARM guests later on.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Fri, 17 May 2019 07:15:27 +0000 (09:15 +0200)]
qemu: Introduce QEMU_CAPS_MACHINE_VIRT_IOMMU
This capability can be used to figure out whether the
QEMU binary at hand supports the machine type property
we need in order to enable SMMUv3 IOMMU support.
Unfortunately we can't avoid probing the RISC-V binaries
along with the ARM ones, since both architectures have
their own 'virt' machine type.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>