Peter Krempa [Tue, 22 Sep 2020 12:53:57 +0000 (14:53 +0200)]
qemu: hotplug: Remove overlay of <transient> disk on disk unplug
Remove the overlay if the disk was <transient/>. Note that even if we'd
forbid unplug of such a disk through the API, the disk can still be
ejected from the guest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Tested-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 21 Sep 2020 17:39:02 +0000 (19:39 +0200)]
qemu: snapshot: Introduce helpers for creating overlays on <transient/> disks
To implement <transient/> disks we'll need to install an overlay on top
of the original disk image which will be discarded after the VM is
turned off. This was initially implemented by qemu but libvirt never
picked up this option as the overlays were created by qemu without
libvirt involvment which didn't work with SELinux.
With blockdev the qemu feature became unsupported so we need to do this
via the snapshot code anyways.
The helpers introduced in this patch prepare a fake snapshot disk
definition for a disk which is configured as <transient/> and use it to
create a snapshot (without actually modifying metadata or persistent
def).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Tested-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 22 Sep 2020 12:39:27 +0000 (14:39 +0200)]
qemu: prepare cleanup for <transient/> disk overlays
Later patches will implement support for <transient/> disks in libvirt
by installing an overlay on top of the configured image. This will
require cleanup after the VM will be stopped so that the state is
correctly discarded.
Since the overlay will be installed only during the startup phase of the
VM we need to ensure that qemuProcessStop doesn't delete the original
file on some previous failure. This is solved by adding
'inhibitDiskTransientDelete' VM private data member which is set prior
to any startup step and will be cleared once transient disk overlays are
established.
Based on that we can then delete the overlays for any <transient/> disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Tested-by: Ján Tomko <jtomko@redhat.com>
Ján Tomko [Fri, 18 Sep 2020 15:44:56 +0000 (17:44 +0200)]
rpc: gendispatch: handle empty flags
CVE-2020-25637
Prepare for omission of the <flagname> in remote_protocol.x
@acl annotations:
@acl: <object>:<permission>:<flagname>
so that we can add more fields after, e.g.:
@acl: <object>:<permission>::<field>
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
meson: don't check for libutil.h except on FreeBSD
The libutil.h we are after is explicitly only something we want on
FreeBSD, we don't want to accidentally pick up this header on other
platforms as it can lead to build failures.
Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Jim Fehlig [Thu, 24 Sep 2020 16:39:13 +0000 (10:39 -0600)]
libxl: Don't free def member of virDomainObj
The refactoring in commit de49d5bad3 accidentally dropped the statement
setting def to NULL after successfully adding it to the virDomainObjList,
causing it to be freed while still in use. The resulting memory
corruption caused unpredictable behavior, often resulting in a libvirtd
crash.
Jim Fehlig [Tue, 22 Sep 2020 22:34:22 +0000 (16:34 -0600)]
apparmor: Allow /usr/libexec for libxl-save-helper and pygrub
Like other distros, openSUSE Tumbleweed recently changed libexecdir from
/usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper
and pygrub.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Peter Krempa [Thu, 24 Sep 2020 11:09:47 +0000 (13:09 +0200)]
qemuxml2argvtest: Append newline to tested error messages
'virTestCompareToFile' automatically fixes newline if it is not present
in the input string but is present in the file. In this case we need to
append the erorr messages with a newline so that
VIR_TEST_REGENERATE_OUTPUT produces files which will pass syntax-check.
Fixes: 9ec77eef2df Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 24 Sep 2020 10:50:41 +0000 (12:50 +0200)]
qemuSnapshotDiskContextNew: Don't set 'ndd'
'ndd' tracks the actual number of snapshot disks filled into the
structure and is incremented by the functions filling the context, thus
it must not be set when initializing the context.
Fixes: 8c2ecdf131c Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Thu, 24 Sep 2020 03:29:42 +0000 (21:29 -0600)]
tests: Adjust libxlxml2domconfigtest to work with Xen < 4.10
Commit f253dc90f5 introduced a test regression in environments with
Xen < 4.10. The logic in libxl_conf.c correctly maps ACPI and APIC
from virDomainObj to libxl_domain_conf based on
LIBXL_HAVE_BUILDINFO_APIC, but the tests did not account for the
different libxl_domain_conf JSON representations.
One approach to fixing the test regression is to duplicate JSON test
data files, having one set for Xen <= 4.9 and another for Xen 4.10
and greater. To avoid duplicate data files, this patch takes the
approach of modifying the libxl_domain_conf object based on
LIBXL_HAVE_BUILDINFO_APIC, before retrieving the JSON representation.
It allows using the same test data files for all supported versions
of Xen by adjusting the intermediate form of libxl_domain_conf object
as needed.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 21 Sep 2020 13:52:43 +0000 (15:52 +0200)]
qemu: snapshot: Introduce qemuSnapshotDiskContext
Add a container struct which holds all data needed to create and clean
up after a (for now external) snapshot. This will aggregate all the
'qemuSnapshotDiskDataPtr' the 'actions' of a transaction QMP command and
everything needed for cleanup at any given point.
This aggregation allows to simplify the arguments of the functions which
prepare the snapshot data and additionally will simplify the code
necessary for creating overlays on top of <transient/> disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ján Tomko [Tue, 22 Sep 2020 20:38:34 +0000 (22:38 +0200)]
vbox: remove VBoxCGlueTerm
cppcheck reports:
src/vbox/vbox_XPCOMCGlue.c:226:21: style:
The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is
logically equivalent to 'hVBoxXPCOMC=NULL'.
[duplicateConditionalAssign]
It does not matter anyway because this function
is never called.
Fixes: e1506cb4eb7eab96e7ded27a23f0d8ac9697ac2a Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Jim Fehlig [Fri, 11 Sep 2020 17:35:47 +0000 (11:35 -0600)]
xen: Don't add dom0 twice on driver reload
When the xen driver loads, it probes libxl for some info about dom0 and
adds it to the virDomainObjList. The driver then looks for any domains
in stateDir and if they are still alive adds them to the list as well.
This logic is a bit flawed wrt handling driver reload and causes the
following error
When an error is expected, the error message will be checked.
This is expressed by creating an additional ".err" file containing
the expected error message.
It is added in order to make sure the expected errors
are not masked by other errors during test execution while
leveraging the existing framework.
In order to keep it simple, an input file cannot be reused
anymore to cover several expected error cases configured
in the test code. An input file can still be reused by creating
a test case specific symlink.
For consistency, the mock needs to report an error now, too,
as every failure must have an error; otherwise a test case will
fail.
Require LC_ALL=C explicitly to make sure error messages are not
localized for testing.
Signed-off-by: Sebastian Mitterle <smitterl@redhat.com> Suggested-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
tests: qemuxml2argvmock: Report error in virNumaNodesetIsAvailable
The code path is invoked by one of the test cases. Upcoming testing of
error messages would fail.
Signed-off-by: Sebastian Mitterle <smitterl@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
virDomainCCWAddressAssign: Drop spurious space at end of error message
Signed-off-by: Sebastian Mitterle <smitterl@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Peter Krempa [Mon, 21 Sep 2020 17:36:17 +0000 (19:36 +0200)]
virDomainSnapshotAlignDisks: refactor extension to all disks
Last step of the algorithm in virDomainSnapshotAlignDisks is to extend
the array of disks to all VM's disk and provide defaults. This was done
by extending the array, adding defaults at the end and then sorting it.
This requires the 'idx' variable and also a separate sorting function.
If we store the pointer to existing snapshot disk definitions in a hash
table and create a new array of snapshot disk definitions, we can fill
the new array directly by either copying the definition from the old
array or adding the default.
This avoids the sorting step and thus even the need to store the index
of the domain disk altogether.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Mon, 21 Sep 2020 16:59:37 +0000 (18:59 +0200)]
virDomainSnapshotAlignDisks: clarify handing of snapshot location
Remove the use of the 'disk_snapshot' temporary variable since accessing
the disk definition now isn't that much longer to write and use explicit
value checks instead of the (non-)zero check to make it more obvious
what the code is doing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
The 'disk' variable usually refers to a definition of a disk from the
domain definition. Rename it to 'snapdisk' to be clear that we are
talking about the snapshot disk definition especially since this
function also accesses the domain disk definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
While this function resides in the snapshot config module, the 'def'
variable is referencing the VM definition in most places. Change the
name to 'snapdef' to avoid ambiguity especially since we are also
dealing with the domain definition in this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 21 Sep 2020 17:38:11 +0000 (19:38 +0200)]
qemuSnapshotCreateInactiveExternal: Don't access 'idx' of snapshot
After virDomainSnapshotAlignDisks is called the definitions of disks in
the snapshot definition and in the domain definition are in the same
order so they can be addressed using the same index.
This frees up 'idx' to be removed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 22 Sep 2020 09:04:17 +0000 (11:04 +0200)]
virStorageSourceNew: Abort on failure
Add an abort() on the class/object allocation failures so that
virStorageSourceNew() always returns a virStorageSource and remove
checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu: substitute missing model name for host-passthrough
Before:
$ uname -m
s390x
$ cat passthrough-cpu.xml
<cpu check="none" mode="host-passthrough" />
$ virsh hypervisor-cpu-compare passthrough-cpu.xml
error: Failed to compare hypervisor CPU with passthrough-cpu.xml
error: internal error: unable to execute QEMU command 'query-cpu-model-comp
arison': Invalid parameter type for 'modelb.name', expected: string
After:
$ virsh hypervisor-cpu-compare passthrough-cpu.xml
CPU described in passthrough-cpu.xml is identical to the CPU provided by hy
pervisor on the host
domain_conf.c: auto-align pSeries NVDIMM in virDomainMemoryDefPostParse()
The alignment for the pSeries NVDIMM does not depend on runtime
constraints. This means that it can be done in device parse
time, instead of runtime, allowing the domain XML to reflect
what the auto-alignment would do when the domain starts.
This brings consistency between the NVDIMM size reported by the
domain XML and what the guest sees, without impacting existing
guests that are using an unaligned size - they'll work as usual,
but the domain XML will be updated with the actual size of the
NVDIMM.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c
We'll use the auto-alignment function during parse time, in
domain_conf.c. Let's move the function to that file, renaming
it to virDomainNVDimmAlignSizePseries(). This will also make it
clearer that, although QEMU is the only driver that currently
supports it, pSeries NVDIMM restrictions aren't tied to QEMU.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Ján Tomko [Wed, 23 Sep 2020 14:30:22 +0000 (16:30 +0200)]
util: do not unref event thread after joining it
g_thread_join() eats a reference.
==295055== Invalid read of size 4
==295055== at 0x4DA4AE4: g_thread_unref (in /usr/lib64/libglib-2.0.so.0.6400.5)
==295055== by 0x491D5FA: vir_event_thread_finalize (vireventthread.c:47)
==295055== by 0x4E6BCFF: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6400.5)
==295055== by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055== by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP (qemu_process.h:237)
...
==295055== by 0x22E98A29: qemuDomainPostParseDataAlloc (qemu_domain.c:5476)
==295055== by 0x49ABF83: virDomainDefPostParse (domain_conf.c:6023)
==295055== Address 0x2acb1c68 is 24 bytes inside a block of size 88 free'd
==295055== at 0x483B9F5: free (vg_replace_malloc.c:538)
==295055== by 0x4D80A4C: g_free (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055== by 0x491D5F1: vir_event_thread_finalize (vireventthread.c:46)
==295055== by 0x4E6BCFF: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6400.5)
==295055== by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055== by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP (qemu_process.h:237)
...
==295055== Block was alloc'd at
==295055== at 0x483A809: malloc (vg_replace_malloc.c:307)
==295055== by 0x4D80958: g_malloc (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055== by 0x4DA4C32: g_thread_try_new (in /usr/lib64/libglib-2.0.so.0.6400.5)
==295055== by 0x491D3BC: virEventThreadStart (vireventthread.c:159)
==295055== by 0x491D3BC: virEventThreadNew (vireventthread.c:185)
...
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: f4fc3db9204407874181117085756c9ced78adad Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Pavel Hrdina [Wed, 23 Sep 2020 14:31:21 +0000 (16:31 +0200)]
virnetdaemon: fix memory leak in virNetDaemonCallInhibit
g_variant_new() returns a weak reference which can be consumed by passing
to other g_variant* functions or to g_dbus_connection_call* functions.
This make it possible to call g_variant_new() directly as argument to
the functions above. Because this might be confusing I explicitly call
g_variant_ref_sink() to make it normal reference in both
virGDBusCallMethod() and virGDBusCallMethodWithFD() so the caller is
always responsible for the data.
Reported-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Peter Krempa [Wed, 23 Sep 2020 13:13:57 +0000 (15:13 +0200)]
docs: manpages: Strip table of contents from manpages
After meson conversion the man pages started to contain the table of
contents.
In autoconf we prevented this by a 'grep -v ::contents' in the command
building the manpages.
A more cultured solution is to strip out the 'contents' docutils element
directly.
Reported-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Ján Tomko [Wed, 23 Sep 2020 08:29:56 +0000 (10:29 +0200)]
tests: esxutilstest: depend on esx_gen_headers
Sometimes parallel compilation randomly fails on platforms
that do not have many drivers enabled, like macOS:
In file included from ../tests/esxutilstest.c:13:
../src/esx/esx_vi_types.h:62:10: fatal error: 'esx_vi_types.generated.typedef' file not found
#include "esx_vi_types.generated.typedef"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
List esx_gen_headers as a source to stop meson from building
it before the headers are generated.
Pavel Hrdina [Mon, 21 Sep 2020 12:39:49 +0000 (14:39 +0200)]
virfirewalld: fix g_variant_get call
We need to pass pointer to `array`.
Reported-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Tested-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Tue, 22 Sep 2020 11:41:49 +0000 (13:41 +0200)]
tests: Don't advertise VIR_TEST_EXPENSIVE to users
Right now, the logic that takes care of deciding whether expensive
tests should be run or not is not working correctly: more
specifically, it's not possible to use something like
$ VIR_TEST_EXPENSIVE=1 ninja test
to override the default choice, because in meson.build we always
pass an explicit value that overrides whatever is present in the
environment.
We could implement logic to make this work properly, but that
would require some refactoring of our test infrastructure and is
arguably of little value given that running
$ meson build -Dexpensive_tests=enabled
is very fast, so let's just stop telling users about the variable
instead and call it a day.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
projects / thirdparty / libvirt.git / log
