August 2018 - Devel - libvirt List Archives

[libvirt] [PATCH v2 0/4] Fix a SIGSEGV in libvirtd when querying AMD SEV info
by Erik Skultety 20 Aug '18

20 Aug '18

This series fixes the following BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1612009 TL;DR: We don't format SEV platform data (PDH, certificate chain,...) into our qemu caps cache which poses a problem after libvirtd restart when we restore from the cache and get a segfault upon issuing virNodeGetSEVInfo. Since v1: - reworked patch 3 so that qemuMonitorJSONGetSEVCapabilities returns more values in order to distinguish between error and whether SEV is actually supported - added the missing backtrace to patch 4 - no other patches were changed apart from patch 3 Erik Skultety (4): tests: sev: Test launch-security with specific QEMU version qemu: Define and use a auto cleanup function with virSEVCapability qemu: Fix probing of AMD SEV support qemu: caps: Format SEV platform data into qemuCaps cache src/conf/domain_capabilities.h | 4 + src/qemu/qemu_capabilities.c | 127 +++++++++++++++++++-- src/qemu/qemu_monitor_json.c | 31 +++-- tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml | 5 +- tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 6 + tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 - ...args => launch-security-sev.x86_64-2.12.0.args} | 19 +-- tests/qemuxml2argvtest.c | 4 +- 8 files changed, 164 insertions(+), 33 deletions(-) rename tests/qemuxml2argvdata/{launch-security-sev.args => launch-security-sev.x86_64-2.12.0.args} (54%) -- 2.14.4

2 6

[libvirt] X86CPU "feature-words" property on QEMU (was Re: [PATCH v3 1/3] x86: Data structure changes to support MSR based) features
by Eduardo Habkost 17 Aug '18

17 Aug '18

On Fri, Aug 17, 2018 at 07:34:13PM +0200, Paolo Bonzini wrote: > On 17/08/2018 17:55, Eduardo Habkost wrote: > >> The names will be X86_CPU_FEATURE_WORD_TYPE_CPUID and > >> X86_CPU_FEATURE_WORD_TYPE_MSR. > > I wouldn't like to make this an external API unless really > > necessary. I would rather deprecate the "feature-words" property > > because nobody ended up using it. > > I think we should either remove it directly, or extend it to support MSR > features. Deprecating and only supporting CPUID features is the worst > of both worlds... So let's do what's necessary to remove it. But I don't think the removal of "feature-words" should block the inclusion of this series. Now, should QOM properties follow our feature deprecation policy, or they were never a supported external API and we can remove it immediately? CCing Jiri and libvir-list, because I just found that there's code on libvirt that uses it, but I don't know exactly it does with that info. -- Eduardo

2 2

[libvirt] [PATCHv2 00/11] BaselineHypervisorCPU using QEMU QMP exchanges
by Chris Venteicher 17 Aug '18

17 Aug '18

Some architectures (S390) depend on QEMU to compute baseline CPU model and expand a models feature set. Interacting with QEMU requires starting the QEMU process and completing one or more query-cpu-model-baseline QMP exchanges with QEMU in addition to a query-cpu-model-expansion QMP exchange to expand all features in the model. See "s390x CPU models: exposing features" patch set on Qemu-devel for discussion of QEMU aspects. This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999 Version 2 address all issues raised by Collin as well as all other issues identified with additional testing. Chris Venteicher (11): qemu_monitor_json: Introduce qemuMonitorCPUModelInfo / JSON conversion qemu_monitor: Introduce qemuMonitorGetCPUModelBaseline (query-cpu-model-baseline) qemu_monitor: Indicate when CPUModelInfo props report migratablity qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents qemu_monitor: CPUModelExpansion on both model name and properties qemu_monitor: Introduce qemuMonitorCPUModelInfoRemovePropByBoolValue qemu_capabilities: Introduce virCPUDef / qemuMonitorCPUModelInfo conversions qemu_capabilities: QMPCommandPtr without maintaining shadow qmperr qemu_capabilities: Persist QEMU instance over multiple QMP Cmds qemu_capabilities: Introduce virQEMUCapsQMPBaselineCPUModel (baseline using QEMU) qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP src/qemu/qemu_capabilities.c | 349 +++++++++++++++++++++++++++++------ src/qemu/qemu_capabilities.h | 33 ++++ src/qemu/qemu_driver.c | 74 +++++++- src/qemu/qemu_monitor.c | 117 +++++++++++- src/qemu/qemu_monitor.h | 21 ++- src/qemu/qemu_monitor_json.c | 232 +++++++++++++++++++---- src/qemu/qemu_monitor_json.h | 14 +- tests/cputest.c | 7 +- 8 files changed, 724 insertions(+), 123 deletions(-) -- 2.17.1

4 41

[libvirt] [PATCH] storage: Don't hold storage pool locked during wipeVol
by Michal Privoznik 17 Aug '18

17 Aug '18

Currently the way virStorageVolWipe() works is it looks up pool containing given volume and hold it locked throughout while API execution. This is suboptimal because wiping a volume means writing data to it which can take ages. And if the whole pool is locked during that operation no other API can be issued (nor pool-list). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/storage/storage_backend_iscsi_direct.c | 5 +++++ src/storage/storage_backend_rbd.c | 7 ++++++- src/storage/storage_util.c | 8 +++++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index 1624066e9c..58d25557f1 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -691,6 +691,9 @@ virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPtr pool, if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, NULL))) return -1; + vol->in_use++; + virObjectUnlock(pool); + switch ((virStorageVolWipeAlgorithm) algorithm) { case VIR_STORAGE_VOL_WIPE_ALG_ZERO: if (virStorageBackendISCSIDirectVolWipeZero(vol, iscsi) < 0) { @@ -719,6 +722,8 @@ virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPtr pool, cleanup: virISCSIDirectDisconnect(iscsi); iscsi_destroy_context(iscsi); + virObjectLock(pool); + vol->in_use--; return ret; } diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 642cacb673..30c94c109b 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1212,7 +1212,10 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, VIR_DEBUG("Wiping RBD image %s/%s", def->source.name, vol->name); if (!(ptr = virStorageBackendRBDNewState(pool))) - goto cleanup; + return -1; + + vol->in_use++; + virObjectUnlock(pool); if ((r = rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { virReportSystemError(-r, _("failed to open the RBD image %s"), @@ -1271,6 +1274,8 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, rbd_close(image); virStorageBackendRBDFreeState(&ptr); + virObjectLock(pool); + vol->in_use--; return ret; } diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 42a9b6abf0..5c1fb7b7d3 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -2750,7 +2750,7 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol, int -virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int algorithm, unsigned int flags) @@ -2759,6 +2759,9 @@ virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virCheckFlags(0, -1); + vol->in_use++; + virObjectUnlock(pool); + VIR_DEBUG("Wiping volume with path '%s' and algorithm %u", vol->target.path, algorithm); @@ -2769,6 +2772,9 @@ virStorageBackendVolWipeLocal(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, vol->target.allocation, false); } + virObjectLock(pool); + vol->in_use--; + return ret; } -- 2.16.4

2 3

[libvirt] Investigation and possible fix of 1361592 - apparmor profiles do not include backing files
by Povilas Kanapickas 17 Aug '18

17 Aug '18

Hi, I've looked into why apparmor profiles do not contain exceptions for backing files of images which later leads to permission errors due to apparmor containment. As of newest libvirt git master, only the first level backing image is included, the subsequent images are omitted. Below is my investigation of why this issue happens. It is based on libvirt git master as of 2018-08-15 (e3e48d7cb82d58). The possible fix is easy, but I don't have the background how to write tests for it, so it's best that someone who knows the project well completes the fix In my case I have the following file hierarchy: - image-master.qcow2 (has backing file image-backing-1.qcow2) - image-backing-1.qcow2 (has backing file image-backing-2.qcow2 - image-backing-2.qcow2 The apparmor profiles are created by the virt-aa-helper tool. The get_files function (src/security/virt-aa-helper.c:949) creates a list of files that need to be accessed by the virtual machine. The call to virDomainDiskDefForeachPath() is responsible for creating the list of files required to access each disk given the disk metadata. The disk->src argument contains the source file. The expected file hierarchy would be this: disk->src->path == path/to/image-master.qcow2 disk->src->backingStore->path == path/to/image-backing-1.qcow2 disk->src->backingStore->backingStore->path == path/to/image-backing-2.qcow2 Unfortunately only the first two levels are present and disk->src->backingStore->backingStore points to a dummy object. The backing store details are filled in virStorageFileGetMetadata() call. It calls into virStorageFileGetMetadataRecurse (src/util/virstoragefile.c:4789) which will collect metadata for a single image and recurse into itself for backing files. For us, the following part of virStorageFileGetMetadataRecurse is important (simplified for brevity): ``` virStorageFileGetMetadataInternal(src, ..., &backingFormat); if (src->backingStoreRaw) { backingStore = ... if (backingFormat == VIR_STORAGE_FILE_AUTO) backingStore->format = VIR_STORAGE_FILE_RAW; [1] else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) backingStore->format = VIR_STORAGE_FILE_AUTO; else backingStore->format = backingFormat; virStorageFileGetMetadataRecurse(backingStore, ...) [2] } ``` The crux of the issue seems that the call to virStorageFileGetMetadataInternal() for the image-master.qcow2 image will set the `backingFormat` return value to VIR_STORAGE_FILE_AUTO. The code responsible is in qcowXGetBackingStore (src/util/virstoragefile.c:491) called via `fileTypeInfo[meta->format].getBackingStore(...)` at src/util/virstoragefile.c:1042. It backingFormat is then downgraded to VIR_STORAGE_FILE_RAW at src/util/virstoragefile.c:4854 ([1] above). This causes the next recurse call to virStorageFileGetMetadataRecurse() ([2] above) to not investigate the backing images at all in virStorageFileGetMetadataInternal() as fileTypeInfo[VIR_STORAGE_FILE_RAW].getBackingStore will be NULL. The possible solution is to return VIR_STORAGE_FILE_AUTO_SAFE from qcowXGetBackingStore. It probably does not make much sense to prevent probing of qcow2 backing images as we probe the first level of backing images anyway, so that return value only affect second and subsequent levels of backing images. Looking into the implementation of qcowXGetBackingStore it also does not seem that it performs any operations that are unsafe by design. Currently VIR_STORAGE_FILE_AUTO is used only in qedGetBackingStore and it does not seem that probing of qcow images is unsafe enough What do the developers think about this? Could someone complete the fix with tests or advise me about the testing frameworks if there's not enough time? Thanks a lot! Povilas

2 3

[libvirt] [PATCH] qemu: hotplug: Fix asynchronous unplug of 'shmem'
by Peter Krempa 17 Aug '18

17 Aug '18

commit 5c81c342a7 forgot to skip the detaching of the shmem backend when async unplug is requested which meant that we've tried to unplug the backend prior to delivery of the DEVICE_DELETED event. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1618622 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_hotplug.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5b52fe9edc..fa490c53e1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5479,11 +5479,16 @@ qemuDomainDetachShmemDevice(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) - ret = qemuDomainRemoveShmemDevice(driver, vm, shmem); + if (async) { + ret = 0; + } else { + if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) + ret = qemuDomainRemoveShmemDevice(driver, vm, shmem); + } cleanup: - qemuDomainResetDeviceRemoval(vm); + if (!async) + qemuDomainResetDeviceRemoval(vm); return ret; } -- 2.16.2

2 1

[libvirt] [PATCH 0/3] conf: qemu: support new Hyper-V enlightenments
by Vitaly Kuznetsov 16 Aug '18

16 Aug '18

Several new Hyper-V enlightenments were recently added to Qemu: - hv-frequencies - hv-reenlightenment - hv-tlbflush Support these in libvirt. Vitaly Kuznetsov (3): conf: qemu: add support for Hyper-V frequency MSRs conf: qemu: add support for Hyper-V reenlightenment notifications conf: qemu: add support for Hyper-V PV TLB flush docs/formatdomain.html.in | 21 +++++++++++++++++++++ docs/schemas/domaincommon.rng | 15 +++++++++++++++ src/conf/domain_conf.c | 14 +++++++++++++- src/conf/domain_conf.h | 3 +++ src/cpu/cpu_x86.c | 9 +++++++++ src/cpu/cpu_x86_data.h | 3 +++ src/qemu/qemu_command.c | 3 +++ src/qemu/qemu_parse_command.c | 3 +++ src/qemu/qemu_process.c | 3 +++ tests/qemuxml2argvdata/hyperv-off.xml | 3 +++ tests/qemuxml2argvdata/hyperv.args | 3 ++- tests/qemuxml2argvdata/hyperv.xml | 3 +++ tests/qemuxml2xmloutdata/hyperv-off.xml | 3 +++ tests/qemuxml2xmloutdata/hyperv.xml | 3 +++ 14 files changed, 87 insertions(+), 2 deletions(-) -- 2.14.4

3 5

[libvirt] [PATCH 0/4] Fix a SIGSEGV in libvirtd when querying AMD SEV info
by Erik Skultety 16 Aug '18

16 Aug '18

This series fixes the following BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1612009 TL;DR: We don't format SEV platform data (PDH, certificate chain,...) into our qemu caps cache which poses a problem after libvirtd restart when we restore from the cache and get a segfault upon issuing virNodeGetSEVInfo. I performed some tests on an AMD machine, but CC'ing Brijesh, he might give it a test too. Erik Skultety (4): tests: sev: Test launch-security with specific QEMU version qemu: Define and use a auto cleanup function with virSEVCapability qemu: Fix probing of AMD SEV support qemu: caps: Format SEV platform data into qemuCaps cache src/conf/domain_capabilities.h | 4 + src/qemu/qemu_capabilities.c | 112 +++++++++++++++++++-- src/qemu/qemu_monitor_json.c | 20 ++-- tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml | 5 +- tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 6 ++ tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 - ...args => launch-security-sev.x86_64-2.12.0.args} | 19 ++-- tests/qemuxml2argvtest.c | 4 +- 8 files changed, 142 insertions(+), 29 deletions(-) rename tests/qemuxml2argvdata/{launch-security-sev.args => launch-security-sev.x86_64-2.12.0.args} (54%) -- 2.14.4

3 12

[libvirt] [RFC PATCH 00/17] Add vhost-user-gpu support
by marcandre.lureau＠redhat.com 16 Aug '18

16 Aug '18

From: Marc-André Lureau <marcandre.lureau(a)redhat.com> Hi, This series of patches add support for running a virtio GPU in a seperate process, using vhost-user. The QEMU series "[PATCH v4 00/29] vhost-user for input & GPU" is still under review, and will hopefully land in 3.1. There are several benefits of running the GPU process in an external process, since Mesa is rather heavy on the qemu main loop, and may block for a while or crash. I observe x5 performance improvements with Unigine Heaven 4 benchmark. The external GPU process is started with one end of the vhost-user socket pair, the other end is given to a QEMU chardev. It is also added to the emulator cgroup to restrict its CPU usage. vhost-user requires shared VM memory. A few preliminary patches ease and improve shared memory setup, when no explicit domain NUMA configuration is given. Also, if there is no need for file-backed memory, teach libvirt to make use of memfd memory backend, which has better security guarantees and is easier to setup. Review welcome! Marc-André Lureau (17): qemu: setup shared memory without explicit numa configuration qemu: add memory-backend-memfd capability check qemu: use memory-backend-memfd if possible qemu: add vhost-user-gpu capabilities domain: add "vhost-user" video type qemu: fill the vhost-user video type capability qemu: check that qemu is vhost-user-vga capable qemu: vhost-user is valid as non-primary video device qemu: validate vhost-user video model qemu: add qemuSecurityStartVhostUserGPU helper qemu: add vhost-user-gpu helper unit qemu: restrict 'virgl=' option to 'virtio' video type qemu: set default address type on vhost-user video model qemu: start/stop the vhost-user-gpu external device qemu: build vhost-user-backend for vhost-user-gpu qemu: build vhost-user-gpu video device arguments tests: add vhost-user-gpu xml2argv tests docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 1 + src/conf/device_conf.h | 1 + src/conf/domain_conf.c | 4 +- src/conf/domain_conf.h | 1 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_capabilities.c | 10 + src/qemu/qemu_capabilities.h | 5 + src/qemu/qemu_command.c | 191 ++++++++--- src/qemu/qemu_domain.c | 8 +- src/qemu/qemu_domain_address.c | 4 +- src/qemu/qemu_extdevice.c | 47 ++- src/qemu/qemu_process.c | 6 +- src/qemu/qemu_security.c | 48 +++ src/qemu/qemu_security.h | 6 + src/qemu/qemu_vhost_user_gpu.c | 318 ++++++++++++++++++ src/qemu/qemu_vhost_user_gpu.h | 48 +++ tests/domaincapsschemadata/full.xml | 1 + .../caps_2.12.0.aarch64.xml | 1 + .../caps_2.12.0.ppc64.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../caps_2.12.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../fd-memory-no-numa-topology.args | 4 + tests/qemuxml2argvdata/memfd.args | 28 ++ tests/qemuxml2argvdata/memfd.xml | 32 ++ .../vhost-user-gpu-secondary.args | 34 ++ .../vhost-user-gpu-secondary.xml | 38 +++ tests/qemuxml2argvdata/vhost-user-vga.args | 31 ++ tests/qemuxml2argvdata/vhost-user-vga.xml | 35 ++ tests/qemuxml2argvtest.c | 14 + 32 files changed, 877 insertions(+), 51 deletions(-) create mode 100644 src/qemu/qemu_vhost_user_gpu.c create mode 100644 src/qemu/qemu_vhost_user_gpu.h create mode 100644 tests/qemuxml2argvdata/memfd.args create mode 100644 tests/qemuxml2argvdata/memfd.xml create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.args create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.args create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.xml -- 2.18.0.129.ge3331758f1

5 25

[libvirt] [libvirt-glib][PATCH] configure: Drop GLIB2_TEST_REQUIRED
by Michal Privoznik 16 Aug '18

16 Aug '18

Introduced in eb1f97a4b49a6e it is not needed anymore. The minimal required version of glib is now the same as the one required for tests. This practically reverts the referenced commit. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- configure.ac | 8 +------- tests/Makefile.am | 3 --- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index dd1a85a..adc9e6b 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,6 @@ LIBVIRT_REQUIRED=1.2.5 AC_SUBST([LIBVIRT_REQUIRED]) dnl used in the .spec file GLIB2_REQUIRED=2.38.0 AC_SUBST([GLIB2_REQUIRED]) dnl used in the .spec file -GLIB2_TEST_REQUIRED=2.38.0 GOBJECT_INTROSPECTION_REQUIRED=1.36.0 LIBXML2_REQUIRED=2.0.0 @@ -107,17 +106,12 @@ PKG_CHECK_EXISTS(libvirt >= 1.2.6, AC_DEFINE([HAVE_VIR_NETWORK_GET_DHCP_LEASES], 1, [Have virNetworkGetDHCPLeases?]) AC_MSG_RESULT([yes]) ],[AC_MSG_RESULT([no])]) -enable_tests=no -PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_TEST_REQUIRED, - [enable_tests=yes], - [PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED)]) +PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED) PKG_CHECK_MODULES(GTHREAD2, gthread-2.0 >= $GLIB2_REQUIRED) PKG_CHECK_MODULES(GOBJECT2, gobject-2.0 >= $GLIB2_REQUIRED) PKG_CHECK_MODULES(GIO2, gio-2.0 >= $GLIB2_REQUIRED) PKG_CHECK_MODULES(LIBXML2, libxml-2.0 >= $LIBXML2_REQUIRED) -AM_CONDITIONAL([ENABLE_TESTS], [test "$enable_tests" = "yes"]) - LIBVIRT_GLIB_GETTEXT dnl Should be in m4/virt-gettext.m4 but intltoolize is too dnl dumb to find it there diff --git a/tests/Makefile.am b/tests/Makefile.am index 3f4ef6c..2fa847a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,5 +1,3 @@ -if ENABLE_TESTS - include $(top_srcdir)/build-aux/glib-tap.mk AM_CFLAGS = \ @@ -22,4 +20,3 @@ test_programs = test-gconfig test-events EXTRA_DIST += \ xml \ $(NULL) -endif -- 2.16.4

2 1