- Devel - libvirt List Archives

[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

[libvirt] [PATCH v2 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 v1: https://www.redhat.com/archives/libvir-list/2018-May/msg01914.html v2: - Remove incorrect warning message 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 | 3 +++ src/util/virpci.c | 19 +++++++++++++++++++ src/util/virpci.h | 2 ++ 8 files changed, 57 insertions(+) -- 2.17.0

2 3

[libvirt] [PATCH 0/4] Remove 'bootHash'
by Ján Tomko 29 May '18

29 May '18

An alternative to Peter's: [PATCH 1/4] conf: domain: Remove code accessing 'bootHash' from the post-parse infrestructure <8b4627fedd199117af1ab46f5562e3838679357e.1527500017.git.pkrempa(a)redhat.com> Ján Tomko (3): vmx: add VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER conf: introduce virDomainDefCheckBootOrder conf: remove 'bootHash' completely Peter Krempa (1): conf: remove 'bootHash' from the post-parse infrastructure src/conf/domain_conf.c | 209 +++++++++++++++------------ src/conf/domain_conf.h | 1 + src/vmx/vmx.c | 3 +- tests/qemuargv2xmldata/nomachine-aarch64.xml | 1 + tests/qemuargv2xmldata/nomachine-ppc64.xml | 1 + tests/qemuargv2xmldata/nomachine-x86_64.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 + 7 files changed, 120 insertions(+), 97 deletions(-) -- 2.16.1

2 13

[libvirt] plan for next release
by Daniel Veillard 28 May '18

28 May '18

Seems the end of the month is coming soon ! I suggest to enter freeze tomorrow morningm with plan for an RC2 on Thursday and a final release on the week-end, Hope this works for everybody, thanks ! Daniel -- Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/ veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

2 3

[libvirt] [PATCH] qemu: Don't build cache= cmd line for scsi-block
by Michal Privoznik 28 May '18

28 May '18

Trying to set any cache for <disk device='lun'/> makes no sense. Such disk translates into -device scsi-block on the command line and the device lacks any cache setting because it's merely a middle man between qemu and real SCSI device. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_command.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7408f6bc70..c7ff074e29 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1824,6 +1824,11 @@ qemuBuildDriveDevCacheStr(virDomainDiskDefPtr disk, if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_DEFAULT) return 0; + /* VIR_DOMAIN_DISK_DEVICE_LUN translates into 'scsi-block' + * where any caching setting makes no sense. */ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) + return 0; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_WRITE_CACHE)) return 0; -- 2.16.1

2 1

[libvirt] [PATCH 0/4] conf: Invoke post-parse callbacks after parsing private XML
by Peter Krempa 28 May '18

28 May '18

This will allow the qemu post-parse callbacks to access 'qemuCaps' which is stored in the status XML when we restore the status of VMs. It will become useful in cases when we need to decide whether storage authentication/encryption secret aliases will need to be regenerated. Peter Krempa (4): conf: domain: Remove code accessing 'bootHash' from the post-parse infrestructure conf: domain: Invoke post-parse callbacks after parsing private XML parts conf: domain: Allow passing in 'parseOpaque' for post-parse of status XML qemu: domain: Pass 'qemuCaps' to post parse callbacks when parsing status XML src/conf/domain_conf.c | 97 ++++++++++++++++++++++++++------------------------ src/conf/domain_conf.h | 5 +++ src/qemu/qemu_domain.c | 10 ++++++ 3 files changed, 65 insertions(+), 47 deletions(-) -- 2.16.2

2 8

[libvirt] [PATCHv2 0/3] Finish the conversion to virConfGetValue* functions
by Fabiano Fidêncio 28 May '18

28 May '18

This patchset finishes the conversion to virConfGetValue* functions, started by Daniel Berrange a few months ago. Please, mind that although we could make virConfGetValue* functions more generic in order to support numbers and booleans as strings, that doesn't seem the safest path to take. The side-effect of this is that we will have to live with some specific code doing that as part of vmx and xen_common. Once this patchset gets merged, https://wiki.libvirt.org/page/BiteSizedTasks#Finish_conversion_to_virConfGe… can be removed. - Changes since v1: All the "values" from virConfGetValueString() are freed Fabiano Fidêncio (3): xen_vm: convert to typesafe virConf accessors vmx: convert to typesafe virConf accessors xen_common: convert to typesafe virConf accessors src/vmx/vmx.c | 196 ++++++-------- src/xenconfig/xen_common.c | 637 ++++++++++++++++++++++----------------------- src/xenconfig/xen_xm.c | 268 ++++++++++--------- 3 files changed, 512 insertions(+), 589 deletions(-) -- 2.14.3

2 11