The encryption secret is setup only for LUKS and thus requires the new approach. Use qemuDomainSecretInfoNew for initializing it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 67bf2f6718..d9b10ae96d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1575,10 +1575,10 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivatePtr priv, if (hasEnc) { if (!(srcPriv->encinfo = - qemuDomainSecretInfoNewPlain(priv, encalias, - VIR_SECRET_USAGE_TYPE_VOLUME, NULL, - &src->encryption->secrets[0]->seclookupdef, - true))) + qemuDomainSecretInfoNew(priv, encalias, + VIR_SECRET_USAGE_TYPE_VOLUME, NULL, + &src->encryption->secrets[0]->seclookupdef, + true))) return -1; } -- 2.16.2

Move the logic that determines which secret shall be used into the caller and make this function work only for plain secrets. This untangles the control flow by only checking relevant data. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 94 ++++++++++++-------------------------------------- 1 file changed, 22 insertions(+), 72 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d9b10ae96d..e4588f7428 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1308,94 +1308,33 @@ qemuDomainSupportsEncryptedSecret(qemuDomainObjPrivatePtr priv) } -/* qemuDomainSecretSetup: - * @priv: pointer to domain private object - * @secinfo: Pointer to secret info - * @srcalias: Alias of the disk/hostdev used to generate the secret alias - * @usageType: The virSecretUsageType - * @username: username to use for authentication (may be NULL) - * @seclookupdef: Pointer to seclookupdef data - * @isLuks: True when is luks (generates different alias) - * - * If we have the encryption API present and can support a secret object, then - * build the AES secret; otherwise, build the Plain secret. This is the magic - * decision point for utilizing the AES secrets for an RBD disk. For now iSCSI - * disks and hostdevs will not be able to utilize this mechanism. - * - * Returns 0 on success, -1 on failure - */ -static int -qemuDomainSecretSetup(qemuDomainObjPrivatePtr priv, - qemuDomainSecretInfoPtr secinfo, - const char *srcalias, - virSecretUsageType usageType, - const char *username, - virSecretLookupTypeDefPtr seclookupdef, - bool isLuks) -{ - bool iscsiHasPS = virQEMUCapsGet(priv->qemuCaps, - QEMU_CAPS_ISCSI_PASSWORD_SECRET); - - if (qemuDomainSupportsEncryptedSecret(priv) && - (usageType == VIR_SECRET_USAGE_TYPE_CEPH || - (usageType == VIR_SECRET_USAGE_TYPE_ISCSI && iscsiHasPS) || - usageType == VIR_SECRET_USAGE_TYPE_VOLUME || - usageType == VIR_SECRET_USAGE_TYPE_TLS)) { - if (qemuDomainSecretAESSetup(priv, secinfo, srcalias, - usageType, username, - seclookupdef, isLuks) < 0) - return -1; - } else { - if (qemuDomainSecretPlainSetup(secinfo, usageType, - username, seclookupdef) < 0) - return -1; - } - return 0; -} - - /* qemuDomainSecretInfoNewPlain: - * @priv: pointer to domain private object - * @srcAlias: Alias base to use for TLS object * @usageType: Secret usage type * @username: username for plain secrets (only) * @looupdef: lookup def describing secret - * @isLuks: boolean for luks lookup * * Helper function to create a secinfo to be used for secinfo consumers. This - * possibly sets up a 'plain' (unencrypted) secret for legacy consumers. + * up a 'plain' (unencrypted) secret for legacy consumers. * * Returns @secinfo on success, NULL on failure. Caller is responsible * to eventually free @secinfo. */ static qemuDomainSecretInfoPtr -qemuDomainSecretInfoNewPlain(qemuDomainObjPrivatePtr priv, - const char *srcAlias, - virSecretUsageType usageType, +qemuDomainSecretInfoNewPlain(virSecretUsageType usageType, const char *username, - virSecretLookupTypeDefPtr lookupDef, - bool isLuks) + virSecretLookupTypeDefPtr lookupDef) { qemuDomainSecretInfoPtr secinfo = NULL; if (VIR_ALLOC(secinfo) < 0) return NULL; - if (qemuDomainSecretSetup(priv, secinfo, srcAlias, usageType, - username, lookupDef, isLuks) < 0) - goto error; - - if (!username && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("encrypted secrets are not supported")); - goto error; + if (qemuDomainSecretPlainSetup(secinfo, usageType, username, lookupDef) < 0) { + qemuDomainSecretInfoFree(&secinfo); + return NULL; } return secinfo; - - error: - qemuDomainSecretInfoFree(&secinfo); - return NULL; } @@ -1549,6 +1488,7 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivatePtr priv, const char *encalias) { qemuDomainStorageSourcePrivatePtr srcPriv; + bool iscsiHasPS = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET); bool hasAuth = qemuDomainStorageSourceHasAuth(src); bool hasEnc = qemuDomainDiskHasEncryptionSecret(src); @@ -1566,11 +1506,21 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivatePtr priv, if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) usageType = VIR_SECRET_USAGE_TYPE_CEPH; - if (!(srcPriv->secinfo = - qemuDomainSecretInfoNewPlain(priv, authalias, - usageType, src->auth->username, - &src->auth->seclookupdef, false))) - return -1; + if (!qemuDomainSupportsEncryptedSecret(priv) || + (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && !iscsiHasPS)) { + srcPriv->secinfo = qemuDomainSecretInfoNewPlain(usageType, + src->auth->username, + &src->auth->seclookupdef); + } else { + srcPriv->secinfo = qemuDomainSecretInfoNew(priv, authalias, + usageType, + src->auth->username, + &src->auth->seclookupdef, + false); + } + + if (!srcPriv->secinfo) + return -1; } if (hasEnc) { -- 2.16.2

It's desired to keep the alias around to allow referencing of the secret object used with qemu. Add set of APIs which will destroy all data except the alias. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 43 +++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_domain.h | 2 ++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e4588f7428..4318818e85 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -949,38 +949,65 @@ qemuDomainSecretPlainClear(qemuDomainSecretPlain secret) static void -qemuDomainSecretAESClear(qemuDomainSecretAES secret) +qemuDomainSecretAESClear(qemuDomainSecretAES secret, + bool keepAlias) { + if (!keepAlias) + VIR_FREE(secret.alias); + VIR_FREE(secret.username); - VIR_FREE(secret.alias); VIR_FREE(secret.iv); VIR_FREE(secret.ciphertext); } -void -qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo) +static void +qemuDomainSecretInfoClear(qemuDomainSecretInfoPtr secinfo, + bool keepAlias) { - if (!*secinfo) + if (!secinfo) return; - switch ((qemuDomainSecretInfoType) (*secinfo)->type) { + switch ((qemuDomainSecretInfoType) secinfo->type) { case VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN: - qemuDomainSecretPlainClear((*secinfo)->s.plain); + qemuDomainSecretPlainClear(secinfo->s.plain); break; case VIR_DOMAIN_SECRET_INFO_TYPE_AES: - qemuDomainSecretAESClear((*secinfo)->s.aes); + qemuDomainSecretAESClear(secinfo->s.aes, keepAlias); break; case VIR_DOMAIN_SECRET_INFO_TYPE_LAST: break; } +} + + +void +qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo) +{ + if (!*secinfo) + return; + + qemuDomainSecretInfoClear(*secinfo, false); VIR_FREE(*secinfo); } +/** + * qemuDomainSecretInfoDestroy: + * @secinfo: object to destroy + * + * Removes any data unnecessary for further use, but keeps alias allocated. + */ +void +qemuDomainSecretInfoDestroy(qemuDomainSecretInfoPtr secinfo) +{ + qemuDomainSecretInfoClear(secinfo, true); +} + + static virClassPtr qemuDomainDiskPrivateClass; static void qemuDomainDiskPrivateDispose(void *obj); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f76404e1ac..3e139e0c57 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -836,6 +836,8 @@ bool qemuDomainSupportsEncryptedSecret(qemuDomainObjPrivatePtr priv); void qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo) ATTRIBUTE_NONNULL(1); +void qemuDomainSecretInfoDestroy(qemuDomainSecretInfoPtr secinfo); + void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); -- 2.16.2

We need to reference the secret objects by name when hot-unplugging disks. Don't remove the alias so that it does not need to be recalculated. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4318818e85..9ebb5d150c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1440,31 +1440,23 @@ qemuDomainSecretInfoTLSNew(qemuDomainObjPrivatePtr priv, } -static void -qemuDomainSecretStorageSourceDestroy(virStorageSourcePtr src) -{ - qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); - - if (srcPriv && srcPriv->secinfo) - qemuDomainSecretInfoFree(&srcPriv->secinfo); - - if (srcPriv && srcPriv->encinfo) - qemuDomainSecretInfoFree(&srcPriv->encinfo); -} - - /* qemuDomainSecretDiskDestroy: * @disk: Pointer to a disk definition * - * Clear and destroy memory associated with the secret + * Clears unnecessary data associated with disk secret objects. */ void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) { - virStorageSourcePtr next; + qemuDomainStorageSourcePrivatePtr srcPriv; + virStorageSourcePtr n; - for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) - qemuDomainSecretStorageSourceDestroy(next); + for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { + if ((srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(n))) { + qemuDomainSecretInfoDestroy(srcPriv->secinfo); + qemuDomainSecretInfoDestroy(srcPriv->encinfo); + } + } } @@ -1698,8 +1690,7 @@ qemuDomainSecretChardevPrepare(virQEMUDriverConfigPtr cfg, /* qemuDomainSecretDestroy: * @vm: Domain object * - * Once completed with the generation of the command line it is - * expect to remove the secrets + * Removes all unnecessary data which was needed to generate 'secret' objects. */ void qemuDomainSecretDestroy(virDomainObjPtr vm) -- 2.16.2

Rather than trying to figure out which alias was used, store it in the status XML. --- src/qemu/qemu_domain.c | 90 +++++++++++++++++++++++++++++-- tests/qemustatusxml2xmldata/modern-in.xml | 4 ++ 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9ebb5d150c..a6494ff5fc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1991,20 +1991,84 @@ qemuDomainObjPrivateFree(void *data) } +static int +qemuStorageSourcePrivateDataAssignSecinfo(qemuDomainSecretInfoPtr *secinfo, + char **alias) +{ + if (!*alias) + return 0; + + if (!*secinfo) { + if (VIR_ALLOC(*secinfo) < 0) + return -1; + + (*secinfo)->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; + } + + if ((*secinfo)->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) + VIR_STEAL_PTR((*secinfo)->s.aes.alias, *alias); + + return 0; +} + + static int qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, virStorageSourcePtr src) { + qemuDomainStorageSourcePrivatePtr priv; + char *authalias = NULL; + char *encalias = NULL; + int ret = -1; + src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); if (src->pr) src->pr->mgralias = virXPathString("string(./reservations/@mgralias)", ctxt); + authalias = virXPathString("string(./objects/secret[@type='auth']/@alias)", ctxt); + encalias = virXPathString("string(./objects/secret[@type='encryption']/@alias)", ctxt); + + if (authalias || encalias) { + if (!src->privateData && + !(src->privateData = qemuDomainStorageSourcePrivateNew())) + goto cleanup; + + priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->secinfo, &authalias) < 0) + goto cleanup; + + if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->encinfo, &encalias) < 0) + goto cleanup; + } + if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) - return -1; + goto cleanup; - return 0; + ret = 0; + + cleanup: + VIR_FREE(authalias); + VIR_FREE(encalias); + + return ret; +} + + +static void +qemuStorageSourcePrivateDataFormatSecinfo(virBufferPtr buf, + qemuDomainSecretInfoPtr secinfo, + const char *type) +{ + if (!secinfo || + secinfo->type != VIR_DOMAIN_SECRET_INFO_TYPE_AES || + !secinfo->s.aes.alias) + return; + + virBufferAsprintf(buf, "<secret type='%s' alias='%s'/>\n", + type, secinfo->s.aes.alias); } @@ -2012,6 +2076,10 @@ static int qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, virBufferPtr buf) { + virBuffer tmp = VIR_BUFFER_INITIALIZER; + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + int ret = -1; + if (src->nodestorage || src->nodeformat) { virBufferAddLit(buf, "<nodenames>\n"); virBufferAdjustIndent(buf, 2); @@ -2025,9 +2093,23 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, virBufferAsprintf(buf, "<reservations mgralias='%s'/>\n", src->pr->mgralias); if (virStorageSourcePrivateDataFormatRelPath(src, buf) < 0) - return -1; + goto cleanup; - return 0; + virBufferSetChildIndent(&tmp, buf); + + if (srcPriv) { + qemuStorageSourcePrivateDataFormatSecinfo(&tmp, srcPriv->secinfo, "auth"); + qemuStorageSourcePrivateDataFormatSecinfo(&tmp, srcPriv->encinfo, "encryption"); + } + + if (virXMLFormatElement(buf, "objects", NULL, &tmp) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virBufferFreeAndReset(&tmp); + return ret; } diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index 5b7e2a34cb..42869261d0 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -319,6 +319,10 @@ </nodenames> <reservations mgralias='test-alias'/> <relPath>base.qcow2</relPath> + <objects> + <secret type='auth' alias='test-auth-alias'/> + <secret type='encryption' alias='test-encryption-alias'/> + </objects> </privateData> </source> <backingStore/> -- 2.16.2

Previously we did not store the aliases but rather re-generated them when unplug was necessary. This is very cumbersome since the knowledge when and which alias to use needs to be stored in the hotplug code as well. While this patch will not strictly improve this situation since there still will be two places containing this code it at least will allow to remove the mess from the disk-unplug code and will prevent introducing more mess when adding blockdev support. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 90 +++++++++++++++++++++- .../disk-secinfo-upgrade-out.xml | 16 ++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a6494ff5fc..d070c013a1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5838,8 +5838,91 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr, } +/** + * qemuDomainDeviceDiskDefPostParseRestoreSecAlias: + * + * Re-generate aliases for objects related to the storage source if they + * were not stored in the status XML by an older libvirt. + * + * Note that qemuCaps should be always present for a status XML. + */ +static int +qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + unsigned int parseFlags) +{ + qemuDomainStorageSourcePrivatePtr priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + bool restoreAuthSecret = false; + bool restoreEncSecret = false; + char *authalias = NULL; + char *encalias = NULL; + int ret = -1; + + if (!(parseFlags & VIR_DOMAIN_DEF_PARSE_STATUS) || + !qemuCaps || + virStorageSourceIsEmpty(disk->src) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET)) + return 0; + + /* network storage authentication secret */ + if (disk->src->auth && + (!priv || !priv->secinfo)) { + + /* only RBD and iSCSI (with capability) were supporting authentication + * using secret object at the time we did not format the alias into the + * status XML */ + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_NETWORK && + (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD || + (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET)))) + restoreAuthSecret = true; + } + + /* disk encryption secret */ + if (disk->src->encryption && + disk->src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + (!priv || !priv->encinfo)) + restoreEncSecret = true; + + if (!restoreAuthSecret && !restoreEncSecret) + return 0; + + if (!priv) { + if (!(disk->src->privateData = qemuDomainStorageSourcePrivateNew())) + return -1; + + priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + } + + if (restoreAuthSecret) { + if (!(authalias = qemuDomainGetSecretAESAlias(disk->info.alias, false))) + goto cleanup; + + if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->secinfo, &authalias) < 0) + goto cleanup; + } + + if (restoreEncSecret) { + if (!(encalias = qemuDomainGetSecretAESAlias(disk->info.alias, true))) + goto cleanup; + + if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->encinfo, &encalias) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(authalias); + VIR_FREE(encalias); + return ret; +} + + static int qemuDomainDeviceDiskDefPostParse(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + unsigned int parseFlags, virQEMUDriverConfigPtr cfg) { /* set default disk types and drivers */ @@ -5873,6 +5956,10 @@ qemuDomainDeviceDiskDefPostParse(virDomainDiskDefPtr disk, disk->mirror->format = VIR_STORAGE_FILE_RAW; } + if (qemuDomainDeviceDiskDefPostParseRestoreSecAlias(disk, qemuCaps, + parseFlags) < 0) + return -1; + return 0; } @@ -5964,7 +6051,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, break; case VIR_DOMAIN_DEVICE_DISK: - ret = qemuDomainDeviceDiskDefPostParse(dev->data.disk, cfg); + ret = qemuDomainDeviceDiskDefPostParse(dev->data.disk, qemuCaps, + parseFlags, cfg); break; case VIR_DOMAIN_DEVICE_VIDEO: diff --git a/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml b/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml index d364fc7644..a554bca99c 100644 --- a/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml +++ b/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml @@ -317,6 +317,11 @@ <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> </encryption> + <privateData> + <objects> + <secret type='encryption' alias='virtio-disk1-luks-secret0'/> + </objects> + </privateData> </source> <backingStore/> <target dev='vdb' bus='virtio'/> @@ -350,6 +355,12 @@ <encryption format='luks'> <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> </encryption> + <privateData> + <objects> + <secret type='auth' alias='virtio-disk3-secret0'/> + <secret type='encryption' alias='virtio-disk3-luks-secret0'/> + </objects> + </privateData> </source> <backingStore/> <target dev='vdd' bus='virtio'/> @@ -381,6 +392,11 @@ <auth username='testuser-rbd'> <secret type='ceph' usage='testuser-rbd-secret'/> </auth> + <privateData> + <objects> + <secret type='auth' alias='virtio-disk5-secret0'/> + </objects> + </privateData> </source> <backingStore/> <target dev='vdf' bus='virtio'/> -- 2.16.2

Now that we remember the alias we've used to attach the secret objects we should reuse them rather than trying to infer them from the disk configuration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 43 ++++++++++++------------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2899f49fff..5e2ca1b988 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3831,14 +3831,15 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk) { + qemuDomainStorageSourcePrivatePtr diskPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); virDomainDeviceDef dev; virObjectEventPtr event; size_t i; const char *src = virDomainDiskGetSource(disk); qemuDomainObjPrivatePtr priv = vm->privateData; char *drivestr; - char *objAlias = NULL; - char *encAlias = NULL; + const char *authAlias = NULL; + const char *encAlias = NULL; VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); @@ -3848,32 +3849,14 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, if (!(drivestr = qemuAliasFromDisk(disk))) return -1; - /* Let's look for some markers for a secret object and create an alias - * object to be used to attempt to delete the object that was created. - * We cannot just use the disk private secret info since it would have - * been removed during cleanup of qemuProcessLaunch. Likewise, libvirtd - * restart wouldn't have them, so no assumption can be made. */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) && - qemuDomainStorageSourceHasAuth(disk->src)) { - - if (!(objAlias = - qemuDomainGetSecretAESAlias(disk->info.alias, false))) { - VIR_FREE(drivestr); - return -1; - } - } - - /* Similarly, if this is possible a device using LUKS encryption, we - * can remove the luks object password too - */ - if (qemuDomainDiskHasEncryptionSecret(disk->src)) { + if (diskPriv) { + if (diskPriv->secinfo && + diskPriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) + authAlias = diskPriv->secinfo->s.aes.alias; - if (!(encAlias = - qemuDomainGetSecretAESAlias(disk->info.alias, true))) { - VIR_FREE(objAlias); - VIR_FREE(drivestr); - return -1; - } + if (diskPriv->encinfo && + diskPriv->encinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) + encAlias = diskPriv->encinfo->s.aes.alias; } qemuDomainObjEnterMonitor(driver, vm); @@ -3882,14 +3865,12 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, VIR_FREE(drivestr); /* If it fails, then so be it - it was a best shot */ - if (objAlias) - ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); - VIR_FREE(objAlias); + if (authAlias) + ignore_value(qemuMonitorDelObject(priv->mon, authAlias)); /* If it fails, then so be it - it was a best shot */ if (encAlias) ignore_value(qemuMonitorDelObject(priv->mon, encAlias)); - VIR_FREE(encAlias); /* If it fails, then so be it - it was a best shot */ if (disk->src->pr) -- 2.16.2

Using 'haveTLS' to do this is pointless if the alias is not set. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5e2ca1b988..f8f1d2c323 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3876,7 +3876,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, if (disk->src->pr) ignore_value(qemuMonitorDelObject(priv->mon, disk->src->pr->mgralias)); - if (disk->src->haveTLS) + if (disk->src->tlsAlias) ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias)); if (qemuDomainObjExitMonitor(driver, vm) < 0) -- 2.16.2

Libvirt uses the stored alias to detach the tlx x509 object on disk unplug. As the alias was not stored, the object would not be detached if unplugging disks after libvirtd restart. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 5 +++++ tests/qemustatusxml2xmldata/modern-in.xml | 1 + 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d070c013a1..a98424cc62 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2023,6 +2023,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); + src->tlsAlias = virXPathString("string(./objects/tlsX509/@alias)", ctxt); if (src->pr) src->pr->mgralias = virXPathString("string(./reservations/@mgralias)", ctxt); @@ -2102,6 +2103,10 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, qemuStorageSourcePrivateDataFormatSecinfo(&tmp, srcPriv->encinfo, "encryption"); } + if (src->tlsAlias) + virBufferAsprintf(&tmp, "<tlsX509 alias='%s'/>\n", src->tlsAlias); + + if (virXMLFormatElement(buf, "objects", NULL, &tmp) < 0) goto cleanup; diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index 42869261d0..e5c00db6a4 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -322,6 +322,7 @@ <objects> <secret type='auth' alias='test-auth-alias'/> <secret type='encryption' alias='test-encryption-alias'/> + <tlsX509 alias='transport-alias'/> </objects> </privateData> </source> -- 2.16.2

qemuDomainPrepareDiskSourceChain should set up the disk zero detection mode only for the top level image. Since it's invoked also for the middle of the chain we need to check that it's really only the top level image. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99656fcd6d..e459e1eeee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12404,7 +12404,8 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, src = disk->src; /* transfer properties valid only for the top level image */ - src->detect_zeroes = disk->detect_zeroes; + if (src == disk->src) + src->detect_zeroes = disk->detect_zeroes; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { if (cfg && -- 2.16.2

Convert the function to just prepare data for the disk. Callers need to do the looping since there's more to do than just copy the data around. The code path in qemuDomainPrepareDiskSource doesn't need to loop over the chain yet, since there currently is no chain at this point. This will be addressed later in the blockdev series where we will setup much more stuff. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 63 +++++++++++++++++++++++--------------------------- src/qemu/qemu_domain.h | 8 +++---- tests/qemublocktest.c | 6 ++--- 3 files changed, 36 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e459e1eeee..d327440ec4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8017,6 +8017,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virStorageSourcePtr src = disk->src; + virStorageSourcePtr n; qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; uid_t uid; @@ -8099,9 +8100,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, report_broken) < 0) goto cleanup; - /* fill in data for the rest of the chain */ - if (qemuDomainPrepareDiskSourceChain(disk, src, cfg, priv->qemuCaps) < 0) - goto cleanup; + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (qemuDomainPrepareDiskSourceData(disk, n, cfg, priv->qemuCaps) < 0) + goto cleanup; + } ret = 0; @@ -12382,51 +12384,44 @@ qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def, /** - * qemuDomainPrepareDiskSourceChain: + * qemuDomainPrepareDiskSourceData: * * @disk: Disk config object * @src: source to start from * @cfg: qemu driver config object * - * Prepares various aspects of the disk source and it's backing chain. This - * function should be also called for detected backing chains. If @src is NULL - * the root source is used. + * Prepares various aspects of a storage source belonging to a disk backing + * chain. This function should be also called for detected backing chain + * members. */ int -qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, - virStorageSourcePtr src, - virQEMUDriverConfigPtr cfg, - virQEMUCapsPtr qemuCaps) +qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg, + virQEMUCapsPtr qemuCaps) { - virStorageSourcePtr n; - - if (!src) - src = disk->src; - /* transfer properties valid only for the top level image */ if (src == disk->src) src->detect_zeroes = disk->detect_zeroes; - for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (cfg && - n->type == VIR_STORAGE_TYPE_NETWORK && - n->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { - n->debug = true; - n->debugLevel = cfg->glusterDebugLevel; - } + if (cfg && + src->type == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { + src->debug = true; + src->debugLevel = cfg->glusterDebugLevel; + } - if (qemuDomainValidateStorageSource(n, qemuCaps) < 0) - return -1; + if (qemuDomainValidateStorageSource(src, qemuCaps) < 0) + return -1; - /* transfer properties valid for the full chain */ - n->iomode = disk->iomode; - n->cachemode = disk->cachemode; - n->discard = disk->discard; + /* transfer properties valid for the full chain */ + src->iomode = disk->iomode; + src->cachemode = disk->cachemode; + src->discard = disk->discard; - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - n->floppyimg = true; - } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + src->floppyimg = true; return 0; } @@ -12476,7 +12471,7 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, if (qemuDomainSecretDiskPrepare(priv, disk) < 0) return -1; - if (qemuDomainPrepareDiskSourceChain(disk, NULL, cfg, priv->qemuCaps) < 0) + if (qemuDomainPrepareDiskSourceData(disk, disk->src, cfg, priv->qemuCaps) < 0) return -1; if (qemuDomainPrepareStorageSourcePR(disk->src, priv, disk->info.alias) < 0) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3e139e0c57..36b000be60 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1003,10 +1003,10 @@ bool qemuDomainCheckCCWS390AddressSupport(const virDomainDef *def, const char *devicename); int -qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, - virStorageSourcePtr src, - virQEMUDriverConfigPtr cfg, - virQEMUCapsPtr qemuCaps) +qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg, + virQEMUCapsPtr qemuCaps) ATTRIBUTE_RETURN_CHECK; int diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index d671505969..7f39f61018 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -215,13 +215,13 @@ testQemuDiskXMLToProps(const void *opaque) goto cleanup; } - if (qemuDomainPrepareDiskSourceChain(disk, NULL, NULL, data->qemuCaps) < 0) - goto cleanup; - for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { if (testQemuDiskXMLToJSONFakeSecrets(n) < 0) goto cleanup; + if (qemuDomainPrepareDiskSourceData(disk, n, NULL, data->qemuCaps) < 0) + goto cleanup; + if (!(formatProps = qemuBlockStorageSourceGetBlockdevProps(n)) || !(storageProps = qemuBlockStorageSourceGetBackendProps(n, false))) { if (!data->fail) { -- 2.16.2

Remove the call to the validating function from the function which sets stuff up. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 11 +++++++---- src/qemu/qemu_domain.h | 6 ++++++ tests/qemublocktest.c | 3 +++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d327440ec4..f616641c26 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4477,7 +4477,7 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) } -static int +int qemuDomainValidateStorageSource(virStorageSourcePtr src, virQEMUCapsPtr qemuCaps) { @@ -8101,6 +8101,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, goto cleanup; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (qemuDomainValidateStorageSource(n, priv->qemuCaps) < 0) + goto cleanup; + if (qemuDomainPrepareDiskSourceData(disk, n, cfg, priv->qemuCaps) < 0) goto cleanup; } @@ -12412,9 +12415,6 @@ qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, src->debugLevel = cfg->glusterDebugLevel; } - if (qemuDomainValidateStorageSource(src, qemuCaps) < 0) - return -1; - /* transfer properties valid for the full chain */ src->iomode = disk->iomode; src->cachemode = disk->cachemode; @@ -12465,6 +12465,9 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, { qemuDomainPrepareDiskCachemode(disk); + if (qemuDomainValidateStorageSource(disk->src, priv->qemuCaps) < 0) + return -1; + if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0) return -1; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 36b000be60..f17157b951 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1009,6 +1009,12 @@ qemuDomainPrepareDiskSourceData(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps) ATTRIBUTE_RETURN_CHECK; + +int +qemuDomainValidateStorageSource(virStorageSourcePtr src, + virQEMUCapsPtr qemuCaps); + + int qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, qemuDomainObjPrivatePtr priv, diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 7f39f61018..ec882b43e1 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -219,6 +219,9 @@ testQemuDiskXMLToProps(const void *opaque) if (testQemuDiskXMLToJSONFakeSecrets(n) < 0) goto cleanup; + if (qemuDomainValidateStorageSource(n, data->qemuCaps) < 0) + goto cleanup; + if (qemuDomainPrepareDiskSourceData(disk, n, NULL, data->qemuCaps) < 0) goto cleanup; -- 2.16.2

When using blockdev the approach to base aliases will change. Add a helper function that will aggregate all code which needs to be called with the disk alias for the -drive to setup internal data. qemuDomainSecretDiskPrepare wrapper is no longer necessary as the contents were moved to a function which is designed to use the old aliases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 61 ++++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f616641c26..15c2e28604 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1555,25 +1555,6 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivatePtr priv, } -/* qemuDomainSecretDiskPrepare: - * @priv: pointer to domain private object - * @disk: Pointer to a disk definition - * - * For the right disk, generate the qemuDomainSecretInfo structure. - * - * Returns 0 on success, -1 on failure - */ - -static int -qemuDomainSecretDiskPrepare(qemuDomainObjPrivatePtr priv, - virDomainDiskDefPtr disk) -{ - return qemuDomainSecretStorageSourcePrepare(priv, disk->src, - disk->info.alias, - disk->info.alias); -} - - /* qemuDomainSecretHostdevDestroy: * @disk: Pointer to a hostdev definition * @@ -12458,26 +12439,48 @@ qemuDomainPrepareStorageSourcePR(virStorageSourcePtr src, } -int -qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, - qemuDomainObjPrivatePtr priv, - virQEMUDriverConfigPtr cfg) +/** + * qemuDomainPrepareDiskSourceLegacy: + * @disk: disk to prepare + * @priv: VM private data + * @cfg: qemu driver config + * + * Prepare any disk source relevant data for use with the -drive command line. + */ +static int +qemuDomainPrepareDiskSourceLegacy(virDomainDiskDefPtr disk, + qemuDomainObjPrivatePtr priv, + virQEMUDriverConfigPtr cfg) { - qemuDomainPrepareDiskCachemode(disk); - if (qemuDomainValidateStorageSource(disk->src, priv->qemuCaps) < 0) return -1; - if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0) + if (qemuDomainPrepareDiskSourceData(disk, disk->src, cfg, priv->qemuCaps) < 0) return -1; - if (qemuDomainSecretDiskPrepare(priv, disk) < 0) + if (qemuDomainSecretStorageSourcePrepare(priv, disk->src, + disk->info.alias, + disk->info.alias) < 0) return -1; - if (qemuDomainPrepareDiskSourceData(disk, disk->src, cfg, priv->qemuCaps) < 0) + if (qemuDomainPrepareStorageSourcePR(disk->src, priv, disk->info.alias) < 0) return -1; - if (qemuDomainPrepareStorageSourcePR(disk->src, priv, disk->info.alias) < 0) + return 0; +} + + +int +qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, + qemuDomainObjPrivatePtr priv, + virQEMUDriverConfigPtr cfg) +{ + qemuDomainPrepareDiskCachemode(disk); + + if (qemuDomainPrepareDiskSourceLegacy(disk, priv, cfg) < 0) + return -1; + + if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0) return -1; return 0; -- 2.16.2

Split out the code into a separate function so that all steps for a storage protocol are contained and the original function is easily extendable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 51 ++++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 15c2e28604..a4499e7916 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9911,6 +9911,32 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def, } +static int +qemuProcessPrepareStorageSourceTlsVxhs(virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg) +{ + /* VxHS uses only client certificates and thus has no need for + * the server-key.pem nor a secret that could be used to decrypt + * the it, so no need to add a secinfo for a secret UUID. */ + if (src->haveTLS == VIR_TRISTATE_BOOL_ABSENT) { + if (cfg->vxhsTLS) + src->haveTLS = VIR_TRISTATE_BOOL_YES; + else + src->haveTLS = VIR_TRISTATE_BOOL_NO; + src->tlsFromConfig = true; + } + + if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { + if (VIR_STRDUP(src->tlsCertdir, cfg->vxhsTLSx509certdir) < 0) + return -1; + + src->tlsVerify = true; + } + + return 0; +} + + /* qemuProcessPrepareDiskSourceTLS: * @source: pointer to host interface data for disk device * @cfg: driver configuration @@ -9928,29 +9954,10 @@ qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, virStorageSourcePtr next; for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) { - /* VxHS uses only client certificates and thus has no need for - * the server-key.pem nor a secret that could be used to decrypt - * the it, so no need to add a secinfo for a secret UUID. */ if (next->type == VIR_STORAGE_TYPE_NETWORK && - next->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) { - - if (next->haveTLS == VIR_TRISTATE_BOOL_ABSENT) { - if (cfg->vxhsTLS) - next->haveTLS = VIR_TRISTATE_BOOL_YES; - else - next->haveTLS = VIR_TRISTATE_BOOL_NO; - next->tlsFromConfig = true; - } - - if (next->haveTLS == VIR_TRISTATE_BOOL_YES) { - /* Grab the vxhsTLSx509certdir and set the verify/listen values. - * NB: tlsAlias filled in during qemuDomainGetTLSObjects. */ - if (VIR_STRDUP(next->tlsCertdir, cfg->vxhsTLSx509certdir) < 0) - return -1; - - next->tlsVerify = true; - } - } + next->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && + qemuProcessPrepareStorageSourceTlsVxhs(next, cfg) < 0) + return -1; } return 0; -- 2.16.2

Select protocol using a swtich with all cases enumerated. This will simplify checking unsupported protocols and adding new support. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a4499e7916..2737d7b7f2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9951,13 +9951,37 @@ static int qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, virQEMUDriverConfigPtr cfg) { - virStorageSourcePtr next; + virStorageSourcePtr n; - for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) { - if (next->type == VIR_STORAGE_TYPE_NETWORK && - next->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && - qemuProcessPrepareStorageSourceTlsVxhs(next, cfg) < 0) + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (virStorageSourceGetActualType(n) != VIR_STORAGE_TYPE_NETWORK) + continue; + + switch ((virStorageNetProtocol) n->protocol) { + case VIR_STORAGE_NET_PROTOCOL_VXHS: + if (qemuProcessPrepareStorageSourceTlsVxhs(n, cfg) < 0) + return -1; + break; + + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_RBD: + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + case VIR_STORAGE_NET_PROTOCOL_HTTP: + case VIR_STORAGE_NET_PROTOCOL_HTTPS: + case VIR_STORAGE_NET_PROTOCOL_FTP: + case VIR_STORAGE_NET_PROTOCOL_FTPS: + case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: + break; + + case VIR_STORAGE_NET_PROTOCOL_NONE: + case VIR_STORAGE_NET_PROTOCOL_LAST: + default: + virReportEnumRangeError(virStorageNetProtocol, n->protocol); return -1; + } } return 0; -- 2.16.2

Remove the loop from qemuDomainPrepareDiskSourceTLS and rename it to qemuDomainPrepareStorageSourceTLS. Currently there is no backing chain to prepare so fixing one device is equivalent. In the future it will be reused in a function which will do the looping. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 62 +++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2737d7b7f2..5e8ff675c8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9937,8 +9937,8 @@ qemuProcessPrepareStorageSourceTlsVxhs(virStorageSourcePtr src, } -/* qemuProcessPrepareDiskSourceTLS: - * @source: pointer to host interface data for disk device +/* qemuProcessPrepareStorageSourceTLS: + * @source: source for a disk * @cfg: driver configuration * * Updates host interface TLS encryption setting based on qemu.conf @@ -9948,40 +9948,36 @@ qemuProcessPrepareStorageSourceTlsVxhs(virStorageSourcePtr src, * Returns 0 on success, -1 on bad config/failure */ static int -qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, - virQEMUDriverConfigPtr cfg) +qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg) { - virStorageSourcePtr n; - - for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (virStorageSourceGetActualType(n) != VIR_STORAGE_TYPE_NETWORK) - continue; + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) + return 0; - switch ((virStorageNetProtocol) n->protocol) { - case VIR_STORAGE_NET_PROTOCOL_VXHS: - if (qemuProcessPrepareStorageSourceTlsVxhs(n, cfg) < 0) - return -1; - break; + switch ((virStorageNetProtocol) src->protocol) { + case VIR_STORAGE_NET_PROTOCOL_VXHS: + if (qemuProcessPrepareStorageSourceTlsVxhs(src, cfg) < 0) + return -1; + break; - case VIR_STORAGE_NET_PROTOCOL_NBD: - case VIR_STORAGE_NET_PROTOCOL_RBD: - case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: - case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - case VIR_STORAGE_NET_PROTOCOL_ISCSI: - case VIR_STORAGE_NET_PROTOCOL_HTTP: - case VIR_STORAGE_NET_PROTOCOL_HTTPS: - case VIR_STORAGE_NET_PROTOCOL_FTP: - case VIR_STORAGE_NET_PROTOCOL_FTPS: - case VIR_STORAGE_NET_PROTOCOL_TFTP: - case VIR_STORAGE_NET_PROTOCOL_SSH: - break; + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_RBD: + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + case VIR_STORAGE_NET_PROTOCOL_HTTP: + case VIR_STORAGE_NET_PROTOCOL_HTTPS: + case VIR_STORAGE_NET_PROTOCOL_FTP: + case VIR_STORAGE_NET_PROTOCOL_FTPS: + case VIR_STORAGE_NET_PROTOCOL_TFTP: + case VIR_STORAGE_NET_PROTOCOL_SSH: + break; - case VIR_STORAGE_NET_PROTOCOL_NONE: - case VIR_STORAGE_NET_PROTOCOL_LAST: - default: - virReportEnumRangeError(virStorageNetProtocol, n->protocol); - return -1; - } + case VIR_STORAGE_NET_PROTOCOL_NONE: + case VIR_STORAGE_NET_PROTOCOL_LAST: + default: + virReportEnumRangeError(virStorageNetProtocol, src->protocol); + return -1; } return 0; @@ -12511,7 +12507,7 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, if (qemuDomainPrepareDiskSourceLegacy(disk, priv, cfg) < 0) return -1; - if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0) + if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg) < 0) return -1; return 0; -- 2.16.2

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5e8ff675c8..55e47a482d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9971,6 +9971,12 @@ qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_SSH: + if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("TLS transport is not supported for disk protocol '%s'"), + virStorageNetProtocolTypeToString(src->protocol)); + return -1; + } break; case VIR_STORAGE_NET_PROTOCOL_NONE: -- 2.16.2

Always parse the 'tls' source field and let the drivers decide whether they support it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 544f63a2a9..51e3d47930 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8682,17 +8682,11 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, goto cleanup; } - /* Check tls=yes|no domain setting for the block device - * At present only VxHS. Other block devices may be added later */ - if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && - (haveTLS = virXMLPropString(node, "tls"))) { - if ((src->haveTLS = - virTristateBoolTypeFromString(haveTLS)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown disk source 'tls' setting '%s'"), - haveTLS); + if ((haveTLS = virXMLPropString(node, "tls")) && + (src->haveTLS = virTristateBoolTypeFromString(haveTLS)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk source 'tls' setting '%s'"), haveTLS); goto cleanup; - } } if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) && -- 2.16.2

For some reason the function returned an error if secAlias was not passed in. It's not an error, in fact it's desired. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f8f1d2c323..8cfb81d545 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1513,7 +1513,7 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, if (qemuBuildSecretInfoProps(secinfo, secProps) < 0) return -1; - if (!secAlias || + if (secAlias && !(*secAlias = qemuDomainGetSecretAESAlias(srcAlias, false))) return -1; } -- 2.16.2

Some callers will not need to generate the alias again. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8cfb81d545..f52e0c773d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1523,7 +1523,8 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, tlsProps) < 0) return -1; - if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias))) + if (tlsAlias && + !(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias))) return -1; return 0; -- 2.16.2

Move the TLS object alias setup earlier. Also make sure that the alias is not overwritten on hotplug. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 3 --- src/qemu/qemu_domain.c | 14 ++++++++++---- src/qemu/qemu_hotplug.c | 8 +++----- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b5ec4f2ba..9ec1d30c80 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -791,9 +791,6 @@ qemuBuildDiskSrcTLSx509CommandLine(virCommandPtr cmd, /* other protocols may be added later */ if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && src->haveTLS == VIR_TRISTATE_BOOL_YES) { - if (!(src->tlsAlias = qemuAliasTLSObjFromSrcAlias(srcalias))) - return -1; - return qemuBuildTLSx509CommandLine(cmd, src->tlsCertdir, false, src->tlsVerify, false, srcalias, qemuCaps); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 55e47a482d..e329cdf958 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9940,6 +9940,7 @@ qemuProcessPrepareStorageSourceTlsVxhs(virStorageSourcePtr src, /* qemuProcessPrepareStorageSourceTLS: * @source: source for a disk * @cfg: driver configuration + * @parentAlias: alias of the parent device * * Updates host interface TLS encryption setting based on qemu.conf * for disk devices. This will be presented as "tls='yes|no'" in @@ -9949,7 +9950,8 @@ qemuProcessPrepareStorageSourceTlsVxhs(virStorageSourcePtr src, */ static int qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, - virQEMUDriverConfigPtr cfg) + virQEMUDriverConfigPtr cfg, + const char *parentAlias) { if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) return 0; @@ -9986,6 +9988,10 @@ qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, return -1; } + if (src->haveTLS == VIR_TRISTATE_BOOL_YES && + !(src->tlsAlias = qemuAliasTLSObjFromSrcAlias(parentAlias))) + return -1; + return 0; } @@ -12499,6 +12505,9 @@ qemuDomainPrepareDiskSourceLegacy(virDomainDiskDefPtr disk, if (qemuDomainPrepareStorageSourcePR(disk->src, priv, disk->info.alias) < 0) return -1; + if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg, disk->info.alias) < 0) + return -1; + return 0; } @@ -12513,9 +12522,6 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, if (qemuDomainPrepareDiskSourceLegacy(disk, priv, cfg) < 0) return -1; - if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg) < 0) - return -1; - return 0; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f52e0c773d..996063b117 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -156,8 +156,7 @@ qemuHotplugPrepareDiskAccess(virQEMUDriverPtr driver, static int qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver, virDomainObjPtr vm, - virStorageSourcePtr src, - const char *srcalias) + virStorageSourcePtr src) { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -167,7 +166,7 @@ qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver, src->tlsCertdir, false, src->tlsVerify, - srcalias, &tlsProps, &src->tlsAlias, + NULL, &tlsProps, NULL, NULL, NULL) < 0) goto cleanup; @@ -471,8 +470,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, prdStarted = true; if (disk->src->haveTLS && - qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src, - disk->info.alias) < 0) + qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src) < 0) goto error; if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) -- 2.16.2

qemuBuildTLSx509CommandLine has no business guessing which alias should be used. The alias needs to be passed in. Note that there's a lingering bad design of this, since the secret object alias is based on the device name and not on the fact that the secret is used for decrypting of the TLS private key. If we ever add authentication for chardevs this will bite us. Thankfully disk code does not support encrypted private keys for TLS so it can be happily refactored there. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9ec1d30c80..c63963adfa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -723,7 +723,8 @@ qemuBuildTLSx509BackendProps(const char *tlspath, * @tlspath: path to the TLS credentials * @listen: boolen listen for client or server setting * @verifypeer: boolean to enable peer verification (form of authorization) - * @addpasswordid: boolean to handle adding passwordid to object + * @certEncSecretAlias: alias of a 'secret' object for decrypting TLS private key + * (optional) * @inalias: Alias for the parent to generate object alias * @qemuCaps: capabilities * @@ -736,7 +737,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, const char *tlspath, bool isListen, bool verifypeer, - bool addpasswordid, + const char *certEncSecretAlias, const char *inalias, virQEMUCapsPtr qemuCaps) { @@ -744,13 +745,9 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, char *objalias = NULL; virJSONValuePtr props = NULL; char *tmp = NULL; - char *secalias = NULL; - if (addpasswordid && - !(secalias = qemuDomainGetSecretAESAlias(inalias, false))) - return -1; - - if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias, + if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, + certEncSecretAlias, qemuCaps, &props) < 0) goto cleanup; @@ -769,7 +766,6 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, virJSONValueFree(props); VIR_FREE(objalias); VIR_FREE(tmp); - VIR_FREE(secalias); return ret; } @@ -793,7 +789,7 @@ qemuBuildDiskSrcTLSx509CommandLine(virCommandPtr cmd, src->haveTLS == VIR_TRISTATE_BOOL_YES) { return qemuBuildTLSx509CommandLine(cmd, src->tlsCertdir, false, src->tlsVerify, - false, srcalias, qemuCaps); + NULL, srcalias, qemuCaps); } return 0; @@ -4986,20 +4982,24 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, qemuDomainChrSourcePrivatePtr chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev); char *objalias = NULL; + const char *tlsCertEncSecAlias = NULL; /* Add the secret object first if necessary. The * secinfo is added only to a TCP serial device during * qemuDomainSecretChardevPrepare. Subsequently called * functions can just check the config fields */ - if (chrSourcePriv && chrSourcePriv->secinfo && - qemuBuildObjectSecretCommandLine(cmd, - chrSourcePriv->secinfo) < 0) - goto cleanup; + if (chrSourcePriv && chrSourcePriv->secinfo) { + if (qemuBuildObjectSecretCommandLine(cmd, + chrSourcePriv->secinfo) < 0) + goto cleanup; + + tlsCertEncSecAlias = chrSourcePriv->secinfo->s.aes.alias; + } if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, - !!cfg->chardevTLSx509secretUUID, + tlsCertEncSecAlias, charAlias, qemuCaps) < 0) goto cleanup; -- 2.16.2

Callers need to know the alias anyways so it does not make much sense to generate it inside of this function. Note that there's a lingering bad design of this, since the secret object alias is based on the device name and not on the fact that the secret is used for decrypting of the TLS private key. If we ever add authentication for chardevs this will bite us. Thankfully disk code does not support encrypted private keys for TLS so it can be happily refactored there. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c63963adfa..2ed58befd9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -725,7 +725,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath, * @verifypeer: boolean to enable peer verification (form of authorization) * @certEncSecretAlias: alias of a 'secret' object for decrypting TLS private key * (optional) - * @inalias: Alias for the parent to generate object alias + * @alias: TLS object alias * @qemuCaps: capabilities * * Create the command line for a TLS object @@ -738,11 +738,10 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, bool isListen, bool verifypeer, const char *certEncSecretAlias, - const char *inalias, + const char *alias, virQEMUCapsPtr qemuCaps) { int ret = -1; - char *objalias = NULL; virJSONValuePtr props = NULL; char *tmp = NULL; @@ -751,11 +750,8 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, qemuCaps, &props) < 0) goto cleanup; - if (!(objalias = qemuAliasTLSObjFromSrcAlias(inalias))) - goto cleanup; - if (!(tmp = virQEMUBuildObjectCommandlineFromJSON("tls-creds-x509", - objalias, props))) + alias, props))) goto cleanup; virCommandAddArgList(cmd, "-object", tmp, NULL); @@ -764,7 +760,6 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, cleanup: virJSONValueFree(props); - VIR_FREE(objalias); VIR_FREE(tmp); return ret; } @@ -779,7 +774,6 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, static int qemuBuildDiskSrcTLSx509CommandLine(virCommandPtr cmd, virStorageSourcePtr src, - const char *srcalias, virQEMUCapsPtr qemuCaps) { @@ -789,7 +783,7 @@ qemuBuildDiskSrcTLSx509CommandLine(virCommandPtr cmd, src->haveTLS == VIR_TRISTATE_BOOL_YES) { return qemuBuildTLSx509CommandLine(cmd, src->tlsCertdir, false, src->tlsVerify, - NULL, srcalias, qemuCaps); + NULL, src->tlsAlias, qemuCaps); } return 0; @@ -2291,8 +2285,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0) return -1; - if (qemuBuildDiskSrcTLSx509CommandLine(cmd, disk->src, disk->info.alias, - qemuCaps) < 0) + if (qemuBuildDiskSrcTLSx509CommandLine(cmd, disk->src, qemuCaps) < 0) return -1; virCommandAddArg(cmd, "-drive"); @@ -4996,15 +4989,18 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, tlsCertEncSecAlias = chrSourcePriv->secinfo->s.aes.alias; } + if (!(objalias = qemuAliasTLSObjFromSrcAlias(charAlias))) + goto cleanup; + if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, tlsCertEncSecAlias, - charAlias, qemuCaps) < 0) + objalias, qemuCaps) < 0) { + VIR_FREE(objalias); goto cleanup; + } - if (!(objalias = qemuAliasTLSObjFromSrcAlias(charAlias))) - goto cleanup; virBufferAsprintf(&buf, ",tls-creds=%s", objalias); VIR_FREE(objalias); } -- 2.16.2

We make sure that the disk supports TLS when preparing the environment so there's no need to duplicate checks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2ed58befd9..134e1a3a20 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -766,8 +766,6 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, /* qemuBuildDiskSrcTLSx509CommandLine: - * - * Add TLS object if the disk src uses a secure communication channel * * Returns 0 on success, -1 w/ error on some sort of failure. */ @@ -776,17 +774,12 @@ qemuBuildDiskSrcTLSx509CommandLine(virCommandPtr cmd, virStorageSourcePtr src, virQEMUCapsPtr qemuCaps) { + if (src->haveTLS != VIR_TRISTATE_BOOL_YES) + return 0; - - /* other protocols may be added later */ - if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && - src->haveTLS == VIR_TRISTATE_BOOL_YES) { - return qemuBuildTLSx509CommandLine(cmd, src->tlsCertdir, - false, src->tlsVerify, - NULL, src->tlsAlias, qemuCaps); - } - - return 0; + return qemuBuildTLSx509CommandLine(cmd, src->tlsCertdir, + false, src->tlsVerify, + NULL, src->tlsAlias, qemuCaps); } -- 2.16.2

The alias of the secret for decrypting the TLS passphrase is useless besides for TLS setup. Stop passing it around. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 8 ++------ src/qemu/qemu_migration_params.c | 21 +++++++++++---------- src/qemu/qemu_migration_params.h | 1 - 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 68663eac47..5cf9be56b4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2296,7 +2296,6 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, bool relabel = false; int rv; char *tlsAlias = NULL; - char *secAlias = NULL; virNWFilterReadLockFilterUpdates(); @@ -2505,7 +2504,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, if (flags & VIR_MIGRATE_TLS) { if (qemuMigrationParamsEnableTLS(driver, vm, true, QEMU_ASYNC_JOB_MIGRATION_IN, - &tlsAlias, &secAlias, NULL, + &tlsAlias, NULL, migParams) < 0) goto stopjob; } else { @@ -2596,7 +2595,6 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, cleanup: VIR_FREE(tlsAlias); - VIR_FREE(secAlias); qemuProcessIncomingDefFree(incoming); VIR_FREE(xmlout); VIR_FORCE_CLOSE(dataFD[0]); @@ -3371,7 +3369,6 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; qemuMigrationCookiePtr mig = NULL; char *tlsAlias = NULL; - char *secAlias = NULL; qemuMigrationIOThreadPtr iothread = NULL; int fd = -1; unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth; @@ -3455,7 +3452,7 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, if (qemuMigrationParamsEnableTLS(driver, vm, false, QEMU_ASYNC_JOB_MIGRATION_OUT, - &tlsAlias, &secAlias, hostname, + &tlsAlias, hostname, migParams) < 0) goto error; } else { @@ -3675,7 +3672,6 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, cleanup: VIR_FREE(tlsAlias); - VIR_FREE(secAlias); VIR_FORCE_CLOSE(fd); virDomainDefFree(persistDef); qemuMigrationCookieFree(mig); diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 578cd6671f..f3c62f26f0 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -809,7 +809,6 @@ qemuMigrationParamsSetString(qemuMigrationParamsPtr migParams, * @tlsListen: server or client * @asyncJob: Migration job to join * @tlsAlias: alias to be generated for TLS object - * @secAlias: alias to be generated for a secinfo object * @hostname: hostname of the migration destination * @migParams: migration parameters to set * @@ -825,7 +824,6 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, bool tlsListen, int asyncJob, char **tlsAlias, - char **secAlias, const char *hostname, qemuMigrationParamsPtr migParams) { @@ -833,6 +831,7 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, virJSONValuePtr tlsProps = NULL; virJSONValuePtr secProps = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *secAlias = NULL; int ret = -1; if (!cfg->migrateTLSx509certdir) { @@ -849,26 +848,28 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, } /* If there's a secret, then grab/store it now using the connection */ - if (cfg->migrateTLSx509secretUUID && - !(priv->migSecinfo = - qemuDomainSecretInfoTLSNew(priv, QEMU_MIGRATION_TLS_ALIAS_BASE, - cfg->migrateTLSx509secretUUID))) - goto error; + if (cfg->migrateTLSx509secretUUID) { + if (!(priv->migSecinfo = + qemuDomainSecretInfoTLSNew(priv, QEMU_MIGRATION_TLS_ALIAS_BASE, + cfg->migrateTLSx509secretUUID))) + goto error; + secAlias = priv->migSecinfo->s.aes.alias; + } if (qemuDomainGetTLSObjects(priv->qemuCaps, priv->migSecinfo, cfg->migrateTLSx509certdir, tlsListen, cfg->migrateTLSx509verify, QEMU_MIGRATION_TLS_ALIAS_BASE, - &tlsProps, tlsAlias, &secProps, secAlias) < 0) + &tlsProps, tlsAlias, &secProps, NULL) < 0) goto error; /* Ensure the domain doesn't already have the TLS objects defined... * This should prevent any issues just in case some cleanup wasn't * properly completed (both src and dst use the same alias) or * some other error path between now and perform . */ - qemuDomainDelTLSObjects(driver, vm, asyncJob, *secAlias, *tlsAlias); + qemuDomainDelTLSObjects(driver, vm, asyncJob, secAlias, *tlsAlias); - if (qemuDomainAddTLSObjects(driver, vm, asyncJob, *secAlias, &secProps, + if (qemuDomainAddTLSObjects(driver, vm, asyncJob, secAlias, &secProps, *tlsAlias, &tlsProps) < 0) goto error; diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index 9a865b19f3..da4c734e3a 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -98,7 +98,6 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, bool tlsListen, int asyncJob, char **tlsAlias, - char **secAlias, const char *hostname, qemuMigrationParamsPtr migParams); -- 2.16.2

Setting up the 'secinfo' for the TLS private key password also generates the given alias, so we don't need to generate another one. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 996063b117..1b36e7fdfa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1536,7 +1536,7 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver, char *devAlias, char *charAlias, char **tlsAlias, - char **secAlias) + const char **secAlias) { int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -1561,12 +1561,15 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver, if ((chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev))) secinfo = chrSourcePriv->secinfo; + if (secinfo) + *secAlias = secinfo->s.aes.alias; + if (qemuDomainGetTLSObjects(priv->qemuCaps, secinfo, cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, charAlias, &tlsProps, tlsAlias, - &secProps, secAlias) < 0) + &secProps, NULL) < 0) goto cleanup; dev->data.tcp.tlscreds = true; @@ -1644,7 +1647,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, char *devstr = NULL; bool chardevAdded = false; char *tlsAlias = NULL; - char *secAlias = NULL; + const char *secAlias = NULL; bool need_release = false; virErrorPtr orig_err; @@ -1691,7 +1694,6 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, if (ret < 0 && need_release) qemuDomainReleaseDeviceAddress(vm, &redirdev->info, NULL); VIR_FREE(tlsAlias); - VIR_FREE(secAlias); VIR_FREE(charAlias); VIR_FREE(devstr); return ret; @@ -1885,7 +1887,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, bool teardowndevice = false; bool teardownlabel = false; char *tlsAlias = NULL; - char *secAlias = NULL; + const char *secAlias = NULL; bool need_release = false; if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && @@ -1956,7 +1958,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, VIR_WARN("Unable to remove chr device from /dev"); } VIR_FREE(tlsAlias); - VIR_FREE(secAlias); VIR_FREE(charAlias); VIR_FREE(devstr); return ret; @@ -1987,7 +1988,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, char *charAlias = NULL; char *objAlias = NULL; char *tlsAlias = NULL; - char *secAlias = NULL; + const char *secAlias = NULL; bool releaseaddr = false; bool teardowncgroup = false; bool teardowndevice = false; @@ -2077,7 +2078,6 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, } VIR_FREE(tlsAlias); - VIR_FREE(secAlias); VIR_FREE(charAlias); VIR_FREE(objAlias); VIR_FREE(devstr); -- 2.16.2

'secinfo' is present also for migrations. Delete the misleading comment. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1b36e7fdfa..cb3d3f581a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1505,8 +1505,6 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, virJSONValuePtr *secProps, char **secAlias) { - /* Add a secret object in order to access the TLS environment. - * The secinfo will only be created for serial TCP device. */ if (secinfo) { if (qemuBuildSecretInfoProps(secinfo, secProps) < 0) return -1; -- 2.16.2

No callers are using it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 17 +++++++---------- src/qemu/qemu_hotplug.h | 3 +-- src/qemu/qemu_migration_params.c | 2 +- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cb3d3f581a..94705abb8a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -166,8 +166,7 @@ qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver, src->tlsCertdir, false, src->tlsVerify, - NULL, &tlsProps, NULL, - NULL, NULL) < 0) + NULL, &tlsProps, NULL, NULL) < 0) goto cleanup; if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, @@ -1502,21 +1501,19 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, const char *srcAlias, virJSONValuePtr *tlsProps, char **tlsAlias, - virJSONValuePtr *secProps, - char **secAlias) + virJSONValuePtr *secProps) { + const char *secAlias = NULL; + if (secinfo) { if (qemuBuildSecretInfoProps(secinfo, secProps) < 0) return -1; - if (secAlias && - !(*secAlias = qemuDomainGetSecretAESAlias(srcAlias, false))) - return -1; + secAlias = secinfo->s.aes.alias; } if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify, - secAlias ? *secAlias : NULL, qemuCaps, - tlsProps) < 0) + secAlias, qemuCaps, tlsProps) < 0) return -1; if (tlsAlias && @@ -1567,7 +1564,7 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver, dev->data.tcp.listen, cfg->chardevTLSx509verify, charAlias, &tlsProps, tlsAlias, - &secProps, NULL) < 0) + &secProps) < 0) goto cleanup; dev->data.tcp.tlscreds = true; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 751cbf61d4..ec5a9b8198 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -56,8 +56,7 @@ int qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, const char *srcAlias, virJSONValuePtr *tlsProps, char **tlsAlias, - virJSONValuePtr *secProps, - char **secAlias); + virJSONValuePtr *secProps); int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index f3c62f26f0..42d72436fb 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -860,7 +860,7 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, cfg->migrateTLSx509certdir, tlsListen, cfg->migrateTLSx509verify, QEMU_MIGRATION_TLS_ALIAS_BASE, - &tlsProps, tlsAlias, &secProps, NULL) < 0) + &tlsProps, tlsAlias, &secProps) < 0) goto error; /* Ensure the domain doesn't already have the TLS objects defined... -- 2.16.2

Callers should generate the alias separately. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 14 +++++--------- src/qemu/qemu_hotplug.h | 2 -- src/qemu/qemu_migration_params.c | 6 ++++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 94705abb8a..4f58cfbd9a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -166,7 +166,7 @@ qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver, src->tlsCertdir, false, src->tlsVerify, - NULL, &tlsProps, NULL, NULL) < 0) + &tlsProps, NULL) < 0) goto cleanup; if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, @@ -1498,9 +1498,7 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, const char *tlsCertdir, bool tlsListen, bool tlsVerify, - const char *srcAlias, virJSONValuePtr *tlsProps, - char **tlsAlias, virJSONValuePtr *secProps) { const char *secAlias = NULL; @@ -1516,10 +1514,6 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, secAlias, qemuCaps, tlsProps) < 0) return -1; - if (tlsAlias && - !(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias))) - return -1; - return 0; } @@ -1559,12 +1553,14 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver, if (secinfo) *secAlias = secinfo->s.aes.alias; + if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(charAlias))) + goto cleanup; + if (qemuDomainGetTLSObjects(priv->qemuCaps, secinfo, cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, - charAlias, &tlsProps, tlsAlias, - &secProps) < 0) + &tlsProps, &secProps) < 0) goto cleanup; dev->data.tcp.tlscreds = true; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index ec5a9b8198..2059baf47f 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -53,9 +53,7 @@ int qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, const char *tlsCertdir, bool tlsListen, bool tlsVerify, - const char *srcAlias, virJSONValuePtr *tlsProps, - char **tlsAlias, virJSONValuePtr *secProps); int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 42d72436fb..5976bfdaf2 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -856,11 +856,13 @@ qemuMigrationParamsEnableTLS(virQEMUDriverPtr driver, secAlias = priv->migSecinfo->s.aes.alias; } + if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(QEMU_MIGRATION_TLS_ALIAS_BASE))) + goto error; + if (qemuDomainGetTLSObjects(priv->qemuCaps, priv->migSecinfo, cfg->migrateTLSx509certdir, tlsListen, cfg->migrateTLSx509verify, - QEMU_MIGRATION_TLS_ALIAS_BASE, - &tlsProps, tlsAlias, &secProps) < 0) + &tlsProps, &secProps) < 0) goto error; /* Ensure the domain doesn't already have the TLS objects defined... -- 2.16.2

Drop the 'vxhs' suffix so other network protocols using TLS can be put into the same test. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- ...-drive-network-tlsx509-vxhs.args => disk-drive-network-tlsx509.args} | 0 ...sk-drive-network-tlsx509-vxhs.xml => disk-drive-network-tlsx509.xml} | 0 tests/qemuxml2argvtest.c | 2 +- ...sk-drive-network-tlsx509-vxhs.xml => disk-drive-network-tlsx509.xml} | 0 tests/qemuxml2xmltest.c | 2 +- 5 files changed, 2 insertions(+), 2 deletions(-) rename tests/qemuxml2argvdata/{disk-drive-network-tlsx509-vxhs.args => disk-drive-network-tlsx509.args} (100%) rename tests/qemuxml2argvdata/{disk-drive-network-tlsx509-vxhs.xml => disk-drive-network-tlsx509.xml} (100%) rename tests/qemuxml2xmloutdata/{disk-drive-network-tlsx509-vxhs.xml => disk-drive-network-tlsx509.xml} (100%) diff --git a/tests/qemuxml2argvdata/disk-drive-network-tlsx509-vxhs.args b/tests/qemuxml2argvdata/disk-drive-network-tlsx509.args similarity index 100% rename from tests/qemuxml2argvdata/disk-drive-network-tlsx509-vxhs.args rename to tests/qemuxml2argvdata/disk-drive-network-tlsx509.args diff --git a/tests/qemuxml2argvdata/disk-drive-network-tlsx509-vxhs.xml b/tests/qemuxml2argvdata/disk-drive-network-tlsx509.xml similarity index 100% rename from tests/qemuxml2argvdata/disk-drive-network-tlsx509-vxhs.xml rename to tests/qemuxml2argvdata/disk-drive-network-tlsx509.xml diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ddd2b88c0a..a67d95f471 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1050,7 +1050,7 @@ mymain(void) DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE); DO_TEST("disk-drive-network-vxhs", QEMU_CAPS_VXHS); driver.config->vxhsTLS = 1; - DO_TEST("disk-drive-network-tlsx509-vxhs", QEMU_CAPS_VXHS, + DO_TEST("disk-drive-network-tlsx509", QEMU_CAPS_VXHS, QEMU_CAPS_OBJECT_TLS_CREDS_X509); driver.config->vxhsTLS = 0; VIR_FREE(driver.config->vxhsTLSx509certdir); diff --git a/tests/qemuxml2xmloutdata/disk-drive-network-tlsx509-vxhs.xml b/tests/qemuxml2xmloutdata/disk-drive-network-tlsx509.xml similarity index 100% rename from tests/qemuxml2xmloutdata/disk-drive-network-tlsx509-vxhs.xml rename to tests/qemuxml2xmloutdata/disk-drive-network-tlsx509.xml diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 1c08724c25..89a44ce043 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -381,7 +381,7 @@ mymain(void) DO_TEST("disk-drive-network-source-auth", NONE); DO_TEST("disk-drive-network-sheepdog", NONE); DO_TEST("disk-drive-network-vxhs", NONE); - DO_TEST("disk-drive-network-tlsx509-vxhs", NONE); + DO_TEST("disk-drive-network-tlsx509", NONE); DO_TEST("disk-scsi-device", QEMU_CAPS_SCSI_LSI); DO_TEST("disk-scsi-vscsi", NONE); -- 2.16.2

Use the default TLS env if TLS is required for NBD. The rest of the implementation is rather simple since all pieces were in place. Note that separate configuration knobs in qemu.conf can be added later if it's desired to configure them. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/domaincommon.rng | 5 ++++ src/qemu/qemu_command.c | 5 ++++ src/qemu/qemu_domain.c | 33 ++++++++++++++++++++-- .../disk-drive-network-tlsx509.args | 9 +++++- .../disk-drive-network-tlsx509.xml | 8 ++++++ tests/qemuxml2argvtest.c | 2 +- .../disk-drive-network-tlsx509.xml | 8 ++++++ 7 files changed, 66 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 703a1bb6f8..ce2d1e91e0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1706,6 +1706,11 @@ <optional> <attribute name="name"/> </optional> + <optional> + <attribute name="tls"> + <ref name="virYesNo"/> + </attribute> + </optional> <ref name="diskSourceNetworkHost"/> <optional> <ref name="encryption"/> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 134e1a3a20..07fa35c6b3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1392,6 +1392,11 @@ qemuDiskSourceNeedsProps(virStorageSourcePtr src, virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET)) return true; + if (actualType == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_NBD && + src->haveTLS == VIR_TRISTATE_BOOL_YES) + return true; + return false; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e329cdf958..db7884a9a1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9937,6 +9937,29 @@ qemuProcessPrepareStorageSourceTlsVxhs(virStorageSourcePtr src, } +static int +qemuProcessPrepareStorageSourceTlsNbd(virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg, + virQEMUCapsPtr qemuCaps) +{ + /* XXX: for NBD we don't have the qemu.conf knobs for private TLS env */ + if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NBD_TLS)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu does not support TLS transport for nbd")); + return -1; + } + + if (VIR_STRDUP(src->tlsCertdir, cfg->defaultTLSx509certdir) < 0) + return -1; + + src->tlsVerify = true; + } + + return 0; +} + + /* qemuProcessPrepareStorageSourceTLS: * @source: source for a disk * @cfg: driver configuration @@ -9951,7 +9974,8 @@ qemuProcessPrepareStorageSourceTlsVxhs(virStorageSourcePtr src, static int qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, virQEMUDriverConfigPtr cfg, - const char *parentAlias) + const char *parentAlias, + virQEMUCapsPtr qemuCaps) { if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) return 0; @@ -9963,6 +9987,10 @@ qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, break; case VIR_STORAGE_NET_PROTOCOL_NBD: + if (qemuProcessPrepareStorageSourceTlsNbd(src, cfg, qemuCaps) < 0) + return -1; + break; + case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: case VIR_STORAGE_NET_PROTOCOL_GLUSTER: @@ -12505,7 +12533,8 @@ qemuDomainPrepareDiskSourceLegacy(virDomainDiskDefPtr disk, if (qemuDomainPrepareStorageSourcePR(disk->src, priv, disk->info.alias) < 0) return -1; - if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg, disk->info.alias) < 0) + if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg, disk->info.alias, + priv->qemuCaps) < 0) return -1; return 0; diff --git a/tests/qemuxml2argvdata/disk-drive-network-tlsx509.args b/tests/qemuxml2argvdata/disk-drive-network-tlsx509.args index 91d3a8a70a..970b8a32a6 100644 --- a/tests/qemuxml2argvdata/disk-drive-network-tlsx509.args +++ b/tests/qemuxml2argvdata/disk-drive-network-tlsx509.args @@ -43,4 +43,11 @@ id=virtio-disk1 \ file.server.host=192.168.0.3,file.server.port=9999,format=raw,if=none,\ id=drive-virtio-disk2,serial=eb90327c-8302-4725-9e1b-4e85ed4dc252,cache=none \ -device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\ -id=virtio-disk2 +id=virtio-disk2 \ +-object tls-creds-x509,id=objvirtio-disk3_tls0,dir=/etc/pki/qemu,\ +endpoint=client,verify-peer=yes \ +-drive file.driver=nbd,file.server.type=inet,file.server.host=example.com,\ +file.server.port=1234,file.tls-creds=objvirtio-disk3_tls0,format=raw,if=none,\ +id=drive-virtio-disk3,cache=none \ +-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk3,\ +id=virtio-disk3 diff --git a/tests/qemuxml2argvdata/disk-drive-network-tlsx509.xml b/tests/qemuxml2argvdata/disk-drive-network-tlsx509.xml index a66e81f065..9f6f298b54 100644 --- a/tests/qemuxml2argvdata/disk-drive-network-tlsx509.xml +++ b/tests/qemuxml2argvdata/disk-drive-network-tlsx509.xml @@ -41,6 +41,14 @@ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='nbd' tls='yes'> + <host name='example.com' port='1234'/> + </source> + <target dev='vdd' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </disk> <controller type='usb' index='0'/> <controller type='pci' index='0' model='pci-root'/> <input type='mouse' bus='ps2'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a67d95f471..53b8b31a46 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1051,7 +1051,7 @@ mymain(void) DO_TEST("disk-drive-network-vxhs", QEMU_CAPS_VXHS); driver.config->vxhsTLS = 1; DO_TEST("disk-drive-network-tlsx509", QEMU_CAPS_VXHS, - QEMU_CAPS_OBJECT_TLS_CREDS_X509); + QEMU_CAPS_OBJECT_TLS_CREDS_X509, QEMU_CAPS_NBD_TLS); driver.config->vxhsTLS = 0; VIR_FREE(driver.config->vxhsTLSx509certdir); DO_TEST("disk-drive-no-boot", diff --git a/tests/qemuxml2xmloutdata/disk-drive-network-tlsx509.xml b/tests/qemuxml2xmloutdata/disk-drive-network-tlsx509.xml index 7053affd17..a9b8d32646 100644 --- a/tests/qemuxml2xmloutdata/disk-drive-network-tlsx509.xml +++ b/tests/qemuxml2xmloutdata/disk-drive-network-tlsx509.xml @@ -41,6 +41,14 @@ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='nbd' tls='yes'> + <host name='example.com' port='1234'/> + </source> + <target dev='vdd' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> -- 2.16.2