May 2018 - Devel - libvirt List Archives

[libvirt] [PATCH] tests: initialize some variables
by Ján Tomko 30 May '18

30 May '18

Found by cppcheck: [tests/metadatatest.c:284]: (error) Uninitialized variable: test [tests/objecteventtest.c:855]: (error) Uninitialized variable: test Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- tests/metadatatest.c | 2 +- tests/objecteventtest.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/metadatatest.c b/tests/metadatatest.c index d5a6f37119..b17a85c883 100644 --- a/tests/metadatatest.c +++ b/tests/metadatatest.c @@ -268,7 +268,7 @@ testTextMetadata(const void *data) static int mymain(void) { - struct metadataTest test; + struct metadataTest test = { 0 }; int ret = EXIT_SUCCESS; if (!(test.conn = virConnectOpen("test:///default"))) diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c index 4b12572eb4..e6de11ec39 100644 --- a/tests/objecteventtest.c +++ b/tests/objecteventtest.c @@ -835,7 +835,7 @@ timeout(int id ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) static int mymain(void) { - objecteventTest test; + objecteventTest test = { 0 }; int ret = EXIT_SUCCESS; int timer; -- 2.16.1

2 1

[libvirt] [PATCH] storage: Add specific check for LUKS encryption support
by John Ferlan 30 May '18

30 May '18

Modify virStorageBackendLogicalLVCreate to ensure if encryption is requested that only type LUKS is supported; otherwise, error. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/storage/storage_backend_logical.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index edd4971f1f..67ca7f514d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -938,6 +938,13 @@ virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, unsigned long long capacity = vol->target.capacity; virCommandPtr cmd = NULL; + if (vol->target.encryption && + vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage pool only supports LUKS encrypted volumes")); + return -1; + } + cmd = virCommandNewArgList(LVCREATE, "--name", vol->name, NULL); @@ -953,8 +960,7 @@ virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, /* If we're going to encrypt using LUKS, then we could need up to * an extra 2MB for the LUKS header - so account for that now */ - if (vol->target.encryption && - vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) + if (vol->target.encryption) capacity += 2 * 1024 * 1024; virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(capacity, 1024)); -- 2.14.3

2 1

Re: [libvirt] [PATCH] vfio/pci: Default display option to "off"
by Alex Williamson 30 May '18

30 May '18

[Cc +Erik,libvirt] Sorry, should have cc'd libvirt with this initially since display support is under development. I think "off" is the better compatibility option, but perhaps the damage is done since it was the 2.12 default. Thanks, Alex On Tue, 29 May 2018 09:18:10 -0600 Alex Williamson <alex.williamson(a)redhat.com> wrote: > Commit a9994687cb9b ("vfio/display: core & wireup") added display > support to vfio-pci with the default being "auto", which breaks > existing VMs when the vGPU requires GL support but had no previous > requirement for a GL compatible configuration. "Off" is the safer > default as we impose no new requirements to VM configurations. > > Fixes: a9994687cb9b ("vfio/display: core & wireup") > Cc: qemu-stable(a)nongnu.org > Cc: Gerd Hoffmann <kraxel(a)redhat.com> > Signed-off-by: Alex Williamson <alex.williamson(a)redhat.com> > --- > hw/vfio/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 84e27c7bb2d1..18c493b49ec1 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -3160,7 +3160,7 @@ static Property vfio_pci_dev_properties[] = { > DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host), > DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev), > DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice, > - display, ON_OFF_AUTO_AUTO), > + display, ON_OFF_AUTO_OFF), > DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice, > intx.mmap_timeout, 1100), > DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features, >

2 1

Re: [libvirt] [PATCH] virtlockd: acquire locks on re-exec
by Matthew Richardson 30 May '18

30 May '18

> Locks held by virtlockd are dropped on re-exec. > > virtlockd 94306 POSIX 5.4G WRITE 0 0 0 /tmp/test.qcow2 > virtlockd 94306 POSIX 5B WRITE 0 0 0 /run/virtlockd.pid > virtlockd 94306 POSIX 5B WRITE 0 0 0 /run/virtlockd.pid > > Acquire locks in PostExecRestart code path. > > Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx> I've just encountered this same unexpected behaviour (currently on RHEL7 box running libvirt 3.2), and found this thread while searching for a bug report. Has this patch been accepted, or the issue fixed in a later release? I notice there was some confusion in the conversation following as to the cause of this issue/the fix for it. If not, is there a bug open against this which I can follow/comment on? Cheers, Matthew The University of Edinburgh is a charitable body, registered in Scotland, with registration number SC005336.

2 1

[libvirt] [PATCH 00/22] New CPU related APIs
by Jiri Denemark 29 May '18

29 May '18

The current virConnectCompareCPU and virConnectBaselineCPU APIs are not very useful because they ignore what a hypervisor can do on the current host. This series adds two new APIs which are designed to work with capabilities of a specific hypervisor to provide usable results. The third CPU related API virConnectGetCPUModelNames is pretty useless too, but no new API with similar functionality is needed because domain capabilities XML already contains the relevant data. Jiri Denemark (22): virsh: Move cpu-{baseline,compare} commands virsh: Extract common code from cmdCPU{Compare,Baseline} virsh: Enhance documentation of cpu-compare command virsh: Enhance documentation of cpu-models command Improve documentation of virConnectGetCPUModelNames vshExtractCPUDefXML: Accept domain capabilities XML qemu_capabilities: Introduce virQEMUCapsCacheLookupDefault Introduce virConnectCompareHypervisorCPU public API remote: Implement virConnectCompareHypervisorCPU virsh: Introduce new hypervisor-cpu-compare command qemu: Implement virConnectCompareHypervisorCPU Introduce virConnectBaselineHypervisorCPU public API remote: Implement virConnectBaselineHypervisorCPU virsh: Introduce new hypervisor-cpu-baseline command cpu: Rename cpuBaseline as virCPUBaseline cpu_x86: Add support for passing guest CPUs to virCPUx86Baseline cpu: Add explicit arch parameter for virCPUBaseline cpu: Update style in virCPUBaseline cpu: Add optional list of allowed features to virCPUBaseline qemu_capabilities: Introduce virQEMUCapsGetCPUFeatures qemu: Implement virConnectBaselineHypervisorCPU news: Mention new CPU related APIs docs/news.xml | 9 + include/libvirt/libvirt-host.h | 15 ++ src/bhyve/bhyve_driver.c | 4 +- src/cpu/cpu.c | 34 ++- src/cpu/cpu.h | 21 +- src/cpu/cpu_arm.c | 11 +- src/cpu/cpu_ppc64.c | 5 +- src/cpu/cpu_x86.c | 35 ++- src/driver-hypervisor.h | 20 ++ src/libvirt-host.c | 161 +++++++++++- src/libvirt_private.syms | 2 +- src/libvirt_public.syms | 6 + src/libxl/libxl_driver.c | 4 +- src/qemu/qemu_capabilities.c | 170 +++++++++++++ src/qemu/qemu_capabilities.h | 12 + src/qemu/qemu_driver.c | 241 ++++++++++++------ src/remote/remote_driver.c | 4 +- src/remote/remote_protocol.x | 40 ++- src/remote_protocol-structs | 27 ++ src/test/test_driver.c | 2 +- src/vz/vz_driver.c | 2 +- tests/cputest.c | 6 +- tools/virsh-domain.c | 223 ----------------- tools/virsh-host.c | 441 +++++++++++++++++++++++++++++++++ tools/virsh.pod | 90 ++++++- 25 files changed, 1223 insertions(+), 362 deletions(-) -- 2.17.0

5 90

[libvirt] [PATCH 0/2] bhyve: add CPU topology support
by Roman Bogorodskiy 29 May '18

29 May '18

This series adds support for specifying CPU topology for bhyve guests. This also would need domxml-from-native support that I plan to add separately. I've updated docs to mention 4.4.0, but more probably it'll go to 4.5.0 as the freeze is close and I'm not sure I'll be able to turn around before it. Roman Bogorodskiy (2): bhyve: add CPU topology support docs: bhyve: document guest CPU topology feature docs/drvbhyve.html.in | 16 ++++++++++++ docs/news.xml | 9 +++++++ src/bhyve/bhyve_capabilities.c | 7 +++-- src/bhyve/bhyve_capabilities.h | 1 + src/bhyve/bhyve_command.c | 17 +++++++++++- .../bhyvexml2argv-cputopology.args | 9 +++++++ .../bhyvexml2argv-cputopology.ldargs | 3 +++ .../bhyvexml2argv-cputopology.xml | 26 +++++++++++++++++++ tests/bhyvexml2argvtest.c | 7 ++++- 9 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.xml -- 2.17.0

2 4

[libvirt] [PATCH v2] storage: Add capability to use LUKS encryption for disk backend
by John Ferlan 29 May '18

29 May '18

https://bugzilla.redhat.com/show_bug.cgi?id=1560946 Similar to the the Logical backend, use qemu-img on the created disk partition device to set up for LUKS encryption. Secret mgmt for the device can be complicated by a reboot possibly changing the path to the device if the infrastructure changes. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- Changes in v2: Don't alter the result 'endOffset' of the DiskPartBoundaries, rather modify the input capacity value. Alteration of the value for the error path won't matter since the volume would be deleted anyway. The call to virStorageBackendDiskPartBoundaries was also slightly modified to check the return value vs -1 rather than != 0 so that the call is similar to other calls. Separating that into it's own patch could have been done, but just felt like too much "busy work" to be worth the trouble. src/storage/storage_backend_disk.c | 49 ++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 2e3d1e04a4..e9578d01d6 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -879,28 +879,26 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, char *partFormat = NULL; unsigned long long startOffset = 0, endOffset = 0; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + virErrorPtr save_err; virCommandPtr cmd = virCommandNewArgList(PARTED, def->source.devices[0].path, "mkpart", "--script", NULL); - if (vol->target.encryption != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("storage pool does not support encrypted " - "volumes")); - goto cleanup; - } - if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0) goto cleanup; virCommandAddArg(cmd, partFormat); - if (virStorageBackendDiskPartBoundaries(pool, &startOffset, - &endOffset, - vol->target.capacity) != 0) { + /* If we're going to encrypt using LUKS, then we could need up to + * an extra 2MB for the LUKS header - so account for that now */ + if (vol->target.encryption && + vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) + vol->target.capacity += 2 * 1024 * 1024; + + if (virStorageBackendDiskPartBoundaries(pool, &startOffset, &endOffset, + vol->target.capacity) < 0) goto cleanup; - } virCommandAddArgFormat(cmd, "%lluB", startOffset); virCommandAddArgFormat(cmd, "%lluB", endOffset); @@ -919,15 +917,15 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, VIR_FREE(vol->target.path); /* Fetch actual extent info, generate key */ - if (virStorageBackendDiskReadPartitions(pool, vol) < 0) { - /* Best effort to remove the partition. Ignore any errors - * since we could be calling this with vol->target.path == NULL - */ - virErrorPtr save_err = virSaveLastError(); - ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0)); - virSetError(save_err); - virFreeError(save_err); - goto cleanup; + if (virStorageBackendDiskReadPartitions(pool, vol) < 0) + goto error; + + if (vol->target.encryption) { + /* Adjust the sizes to account for the LUKS header */ + vol->target.capacity -= 2 * 1024 * 1024; + vol->target.allocation -= 2 * 1024 * 1024; + if (virStorageBackendCreateVolUsingQemuImg(pool, vol, NULL, 0) < 0) + goto error; } res = 0; @@ -936,8 +934,19 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, VIR_FREE(partFormat); virCommandFree(cmd); return res; + + error: + /* Best effort to remove the partition. Ignore any errors + * since we could be calling this with vol->target.path == NULL + */ + save_err = virSaveLastError(); + ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0)); + virSetError(save_err); + virFreeError(save_err); + goto cleanup; } + static int virStorageBackendDiskBuildVolFrom(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, -- 2.14.3

2 2

[libvirt] [PATCH 0/2] capabilities: Provide info about host IOMMU support
by Filip Alac 29 May '18

29 May '18

Provide information about host IOMMU support in capabilities XML. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=967231 Filip Alac (2): capabilities: Provide info about host IOMMU support docs: news: Add entry about iommu_support docs/news.xml | 8 ++++++++ docs/schemas/capability.rng | 11 +++++++++++ src/conf/capabilities.c | 8 ++++++++ src/conf/capabilities.h | 5 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 4 ++++ src/util/virpci.c | 19 +++++++++++++++++++ src/util/virpci.h | 2 ++ 8 files changed, 58 insertions(+) -- 2.17.0

2 4

[libvirt] [PATCH] storage: Add capability to use LUKS encryption for disk backend
by John Ferlan 29 May '18

29 May '18

https://bugzilla.redhat.com/show_bug.cgi?id=1560946 Following the model of the Logical backend, use qemu-img on the created device to set up for LUKS encryption. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- works much better with the settle patch applied from: https://www.redhat.com/archives/libvir-list/2018-May/msg01847.html src/storage/storage_backend_disk.c | 43 ++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 7b4549c34d..a3003fd0b5 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -870,19 +870,13 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, char *partFormat = NULL; unsigned long long startOffset = 0, endOffset = 0; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + virErrorPtr save_err; virCommandPtr cmd = virCommandNewArgList(PARTED, def->source.devices[0].path, "mkpart", "--script", NULL); - if (vol->target.encryption != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("storage pool does not support encrypted " - "volumes")); - goto cleanup; - } - if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0) goto cleanup; virCommandAddArg(cmd, partFormat); @@ -893,6 +887,12 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, goto cleanup; } + /* If we're going to encrypt using LUKS, then we could need up to + * an extra 2MB for the LUKS header - so account for that now */ + if (vol->target.encryption && + vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) + endOffset += 2 * 1024 * 1024; + virCommandAddArgFormat(cmd, "%lluB", startOffset); virCommandAddArgFormat(cmd, "%lluB", endOffset); @@ -910,15 +910,15 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, VIR_FREE(vol->target.path); /* Fetch actual extent info, generate key */ - if (virStorageBackendDiskReadPartitions(pool, vol) < 0) { - /* Best effort to remove the partition. Ignore any errors - * since we could be calling this with vol->target.path == NULL - */ - virErrorPtr save_err = virSaveLastError(); - ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0)); - virSetError(save_err); - virFreeError(save_err); - goto cleanup; + if (virStorageBackendDiskReadPartitions(pool, vol) < 0) + goto error; + + if (vol->target.encryption) { + /* Adjust the sizes to account for the LUKS header */ + vol->target.capacity -= 2 * 1024 * 1024; + vol->target.allocation -= 2 * 1024 * 1024; + if (virStorageBackendCreateVolUsingQemuImg(pool, vol, NULL, 0) < 0) + goto error; } res = 0; @@ -927,8 +927,19 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, VIR_FREE(partFormat); virCommandFree(cmd); return res; + + error: + /* Best effort to remove the partition. Ignore any errors + * since we could be calling this with vol->target.path == NULL + */ + save_err = virSaveLastError(); + ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0)); + virSetError(save_err); + virFreeError(save_err); + goto cleanup; } + static int virStorageBackendDiskBuildVolFrom(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, -- 2.14.3

2 4

[libvirt] [PATCH] storage: Remove rwlocks during virStoragePoolObjListForEach
by John Ferlan 29 May '18

29 May '18

Remove the locks since they are unnecessary and would cause a hang for a driver reload/restart when a transient pool was previously active as a result of the call: virStoragePoolUpdateInactive: ... if (!virStoragePoolObjGetConfigFile(obj)) { virStoragePoolObjRemove(driver->pools, obj); ... stack trace: Thread 17 (Thread 0x7fffcc574700 (LWP 12465)): ...pthread_rwlock_wrlock ...virRWLockWrite ...virObjectRWLockWrite ...virStoragePoolObjRemove ...virStoragePoolUpdateInactive ...storagePoolUpdateStateCallback ...virStoragePoolObjListForEachCb ...virHashForEach ...virStoragePoolObjListForEach ...storagePoolUpdateAllState ...storageStateInitialize ...virStateInitialize ...daemonRunStateInit ...virThreadHelper ...start_thread ...clone Introduced by commit id 4b2e0ed6e3. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/conf/virstorageobj.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 6c937f105c..e66b2ebfb2 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -438,6 +438,12 @@ virStoragePoolObjListForEachCb(void *payload, * not as it's being used for Autostart and UpdateAllState callers * that want to iterate over all the @pools objects not stopping if * one happens to fail. + * + * NB: We cannot take the Storage Pool lock here because it's possible + * that some action as part of Autostart or UpdateAllState will need + * to modify/destroy a transient pool. Since these paths only occur + * during periods in which the storageDriverLock is held (Initialization, + * AutoStart, or Reload) this is OK. */ void virStoragePoolObjListForEach(virStoragePoolObjListPtr pools, @@ -447,9 +453,7 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools, struct _virStoragePoolObjListForEachData data = { .iter = iter, .opaque = opaque }; - virObjectRWLockRead(pools); virHashForEach(pools->objs, virStoragePoolObjListForEachCb, &data); - virObjectRWUnlock(pools); } -- 2.14.3

2 2