[libvirt] [PATCH v3 0/8] Alter TLS/UUID algorithms to be a bit more generic

This is a *split* from the v2 series for migration TLS. I figure what's here may be useful for the Veritas code... The most recent qemu patches ar using TLS, which means when the libvirt patches are updated they'll need similar type support. v2: http://www.redhat.com/archives/libvir-list/2017-February/msg01374.html Adjustments here are mainly in patch 2 which is new - the former patch2 is migrate TLS specific so it's moved later, but not in this series. The patch2 here does the TLS setup with the UUID. Essentially the end result is creation of a TLS/UUID secret info setup API and the splitting of the hotplug code for chardev to allow creation of TLS objects. John Ferlan (8): qemu: Introduce qemuDomainSecretInfoNew qemu: Introduce qemuDomainSecretInfoTLSNew qemu: Move exit monitor calls in failure paths qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects qemu: Refactor qemuDomainGetChardevTLSObjects to converge code qemu: Move qemuDomainSecretChardevPrepare call qemu: Move qemuDomainPrepareChardevSourceTLS call qemu: Introduce qemuDomainGetTLSObjects src/qemu/qemu_domain.c | 182 +++++++++++++++----------- src/qemu/qemu_hotplug.c | 341 +++++++++++++++++++++++++----------------------- src/qemu/qemu_hotplug.h | 24 ++++ 3 files changed, 309 insertions(+), 238 deletions(-) -- 2.9.3

Create a helper which will create the secinfo used for disks, hostdevs, and chardevs. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 137 +++++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 66 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c187214..f8ac0f4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1112,6 +1112,52 @@ qemuDomainSecretSetup(virConnectPtr conn, } +/* qemuDomainSecretInfoNew: + * @conn: Pointer to connection + * @priv: pointer to domain private object + * @srcAlias: Alias base to use for TLS object + * @lookupType: Type of secret lookup + * @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 + * + * Returns @secinfo on success, NULL on failure. Caller is responsible + * to eventually free @secinfo. + */ +static qemuDomainSecretInfoPtr +qemuDomainSecretInfoNew(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + const char *srcAlias, + virSecretUsageType secretUsageType, + const char *username, + virSecretLookupTypeDefPtr lookupDef, + bool isLuks) +{ + qemuDomainSecretInfoPtr secinfo = NULL; + + if (VIR_ALLOC(secinfo) < 0) + return NULL; + + if (qemuDomainSecretSetup(conn, priv, secinfo, srcAlias, secretUsageType, + 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; + } + + return secinfo; + + error: + qemuDomainSecretInfoFree(&secinfo); + return NULL; +} + + /* qemuDomainSecretDiskDestroy: * @disk: Pointer to a disk definition * @@ -1171,51 +1217,30 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, { virStorageSourcePtr src = disk->src; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - qemuDomainSecretInfoPtr secinfo = NULL; if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI; - if (VIR_ALLOC(secinfo) < 0) - return -1; - if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) secretUsageType = VIR_SECRET_USAGE_TYPE_CEPH; - if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, - secretUsageType, src->auth->username, - &src->auth->seclookupdef, false) < 0) - goto error; - - diskPriv->secinfo = secinfo; + if (!(diskPriv->secinfo = + qemuDomainSecretInfoNew(conn, priv, disk->info.alias, + secretUsageType, src->auth->username, + &src->auth->seclookupdef, false))) + return -1; } if (qemuDomainDiskHasEncryptionSecret(src)) { - - if (VIR_ALLOC(secinfo) < 0) - return -1; - - if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias, - VIR_SECRET_USAGE_TYPE_VOLUME, NULL, - &src->encryption->secrets[0]->seclookupdef, - true) < 0) - goto error; - - if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("luks encryption requires encrypted secrets " - "to be supported")); - goto error; - } - - diskPriv->encinfo = secinfo; + if (!(diskPriv->encinfo = + qemuDomainSecretInfoNew(conn, priv, disk->info.alias, + VIR_SECRET_USAGE_TYPE_VOLUME, NULL, + &src->encryption->secrets[0]->seclookupdef, + true))) + return -1; } return 0; - - error: - qemuDomainSecretInfoFree(&secinfo); - return -1; } @@ -1251,8 +1276,6 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, qemuDomainObjPrivatePtr priv, virDomainHostdevDefPtr hostdev) { - qemuDomainSecretInfoPtr secinfo = NULL; - if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; @@ -1263,24 +1286,17 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev); - if (VIR_ALLOC(secinfo) < 0) + if (!(hostdevPriv->secinfo = + qemuDomainSecretInfoNew(conn, priv, hostdev->info->alias, + VIR_SECRET_USAGE_TYPE_ISCSI, + iscsisrc->auth->username, + &iscsisrc->auth->seclookupdef, + false))) return -1; - - if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias, - VIR_SECRET_USAGE_TYPE_ISCSI, - iscsisrc->auth->username, - &iscsisrc->auth->seclookupdef, false) < 0) - goto error; - - hostdevPriv->secinfo = secinfo; } } return 0; - - error: - qemuDomainSecretInfoFree(&secinfo); - return -1; } @@ -1322,7 +1338,6 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn, virDomainChrSourceDefPtr dev) { virSecretLookupTypeDef seclookupdef = {0}; - qemuDomainSecretInfoPtr secinfo = NULL; char *charAlias = NULL; if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP) @@ -1337,36 +1352,26 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn, seclookupdef.u.uuid) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("malformed chardev TLS secret uuid in qemu.conf")); - goto error; + return -1; } seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID; - if (VIR_ALLOC(secinfo) < 0) - goto error; - if (!(charAlias = qemuAliasChardevFromDevAlias(chrAlias))) - goto error; - - if (qemuDomainSecretSetup(conn, priv, secinfo, charAlias, - VIR_SECRET_USAGE_TYPE_TLS, NULL, - &seclookupdef, false) < 0) - goto error; + return -1; - if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("TLS X.509 requires encrypted secrets " - "to be supported")); + if (!(chrSourcePriv->secinfo = + qemuDomainSecretInfoNew(conn, priv, charAlias, + VIR_SECRET_USAGE_TYPE_TLS, NULL, + &seclookupdef, false))) goto error; - } - chrSourcePriv->secinfo = secinfo; + VIR_FREE(charAlias); } - VIR_FREE(charAlias); return 0; error: - qemuDomainSecretInfoFree(&secinfo); + VIR_FREE(charAlias); return -1; } -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:19 -0500, John Ferlan wrote:
Create a helper which will create the secinfo used for disks, hostdevs, and chardevs.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 137 +++++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 66 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c187214..f8ac0f4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1112,6 +1112,52 @@ qemuDomainSecretSetup(virConnectPtr conn, }
+/* qemuDomainSecretInfoNew: + * @conn: Pointer to connection + * @priv: pointer to domain private object + * @srcAlias: Alias base to use for TLS object + * @lookupType: Type of secret lookup
You forgot to change the documentation here.
+ * @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 + * + * Returns @secinfo on success, NULL on failure. Caller is responsible + * to eventually free @secinfo. + */ +static qemuDomainSecretInfoPtr +qemuDomainSecretInfoNew(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + const char *srcAlias, + virSecretUsageType secretUsageType,
secretUsageType seems to be a bit too verbose given that we are in a *SecretInfoNew function. Keep it if you like it or change it to usageType.
+ const char *username, + virSecretLookupTypeDefPtr lookupDef, + bool isLuks) ...
ACK Jirka

Building upon the qemuDomainSecretInfoNew, create a helper which will build the secret used for TLS. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 59 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f8ac0f4..f5c2961 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1158,6 +1158,40 @@ qemuDomainSecretInfoNew(virConnectPtr conn, } +/** + * qemuDomainSecretInfoTLSNew: + * @conn: Pointer to connection + * @priv: pointer to domain private object + * @srcAlias: Alias base to use for TLS object + * @secretUUID: Provide a secretUUID value to look up/create the secretInfo + * + * Using the passed @secretUUID, generate a seclookupdef that can be used + * to generate the returned qemuDomainSecretInfoPtr for a TLS based secret. + * + * Returns qemuDomainSecretInfoPtr or NULL on error. + */ +static qemuDomainSecretInfoPtr +qemuDomainSecretInfoTLSNew(virConnectPtr conn, + qemuDomainObjPrivatePtr priv, + const char *srcAlias, + const char *secretUUID) +{ + virSecretLookupTypeDef seclookupdef = {0}; + + if (virUUIDParse(secretUUID, seclookupdef.u.uuid) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("malformed TLS secret uuid '%s' provided"), + secretUUID); + return NULL; + } + seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID; + + return qemuDomainSecretInfoNew(conn, priv, srcAlias, + VIR_SECRET_USAGE_TYPE_TLS, NULL, + &seclookupdef, false); +} + + /* qemuDomainSecretDiskDestroy: * @disk: Pointer to a disk definition * @@ -1337,7 +1371,6 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn, const char *chrAlias, virDomainChrSourceDefPtr dev) { - virSecretLookupTypeDef seclookupdef = {0}; char *charAlias = NULL; if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP) @@ -1348,31 +1381,19 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn, qemuDomainChrSourcePrivatePtr chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev); - if (virUUIDParse(cfg->chardevTLSx509secretUUID, - seclookupdef.u.uuid) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("malformed chardev TLS secret uuid in qemu.conf")); - return -1; - } - seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID; - if (!(charAlias = qemuAliasChardevFromDevAlias(chrAlias))) return -1; - if (!(chrSourcePriv->secinfo = - qemuDomainSecretInfoNew(conn, priv, charAlias, - VIR_SECRET_USAGE_TYPE_TLS, NULL, - &seclookupdef, false))) - goto error; - + chrSourcePriv->secinfo = + qemuDomainSecretInfoTLSNew(conn, priv, charAlias, + cfg->chardevTLSx509secretUUID); VIR_FREE(charAlias); + + if (!chrSourcePriv->secinfo) + return -1; } return 0; - - error: - VIR_FREE(charAlias); - return -1; } -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:20 -0500, John Ferlan wrote:
Building upon the qemuDomainSecretInfoNew, create a helper which will build the secret used for TLS.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 59 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 19 deletions(-)
ACK Jirka

Since qemuDomainObjExitMonitor can also generate error messages, let's move it inside any error message saving code on error paths for various hotplug add activities. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0c08856..fbd5180 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -442,13 +442,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias)); if (encobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + releaseaddr = false; if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - if (qemuDomainObjExitMonitor(driver, vm) < 0) - releaseaddr = false; virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -728,13 +728,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias)); if (encobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - ignore_value(qemuDomainObjExitMonitor(driver, vm)); - virDomainAuditDisk(vm, NULL, disk->src, "attach", false); error: @@ -822,12 +821,12 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, VIR_WARN("Unable to remove drive %s (%s) after failed " "qemuMonitorAddDevice", drivealias, drivestr); } + ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditDisk(vm, NULL, disk->src, "attach", false); error: @@ -1679,11 +1678,11 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); if (secobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto audit; } @@ -1973,12 +1972,12 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); if (secobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto audit; } @@ -2159,13 +2158,13 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); if (secobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + releaseaddr = false; if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - if (qemuDomainObjExitMonitor(driver, vm) < 0) - releaseaddr = false; goto audit; } @@ -2279,14 +2278,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, orig_err = virSaveLastError(); if (objAdded) ignore_value(qemuMonitorDelObject(priv->mon, objalias)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + mem = NULL; if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - if (qemuDomainObjExitMonitor(driver, vm) < 0) { - mem = NULL; + if (!mem) goto audit; - } removedef: if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0) @@ -2509,12 +2508,12 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, "qemuMonitorAddDevice", drvstr, devstr); } + ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditHostdev(vm, hostdev, "attach", false); goto cleanup; @@ -2801,14 +2800,14 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, memAlias)); } + if (qemuDomainObjExitMonitor(driver, vm) < 0) + release_address = false; + if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } - if (qemuDomainObjExitMonitor(driver, vm) < 0) - release_address = false; - goto audit; } -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:21 -0500, John Ferlan wrote:
Since qemuDomainObjExitMonitor can also generate error messages, let's move it inside any error message saving code on error paths for various hotplug add activities.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
ACK Jirka

Refactor the TLS object adding code to make two separate API's that will handle the add/remove of the "secret" and "tls-creds-x509" objects including the Enter/Exit monitor commands. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 165 +++++++++++++++++++++++++++--------------------- src/qemu/qemu_hotplug.h | 13 ++++ 2 files changed, 107 insertions(+), 71 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fbd5180..75a2596 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1528,6 +1528,85 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, } +void +qemuDomainDelTLSObjects(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *secAlias, + const char *tlsAlias) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err; + + if (!tlsAlias && !secAlias) + return; + + orig_err = virSaveLastError(); + + qemuDomainObjEnterMonitor(driver, vm); + + if (tlsAlias) + ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); + + if (secAlias) + ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); + + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } +} + + +int +qemuDomainAddTLSObjects(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *secAlias, + virJSONValuePtr *secProps, + const char *tlsAlias, + virJSONValuePtr *tlsProps) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int rc; + virErrorPtr orig_err; + + if (!tlsAlias && !secAlias) + return 0; + + qemuDomainObjEnterMonitor(driver, vm); + + if (secAlias) { + rc = qemuMonitorAddObject(priv->mon, "secret", + secAlias, *secProps); + *secProps = NULL; /* qemuMonitorAddObject consumes */ + if (rc < 0) + goto error; + } + + if (tlsAlias) { + rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509", + tlsAlias, *tlsProps); + *tlsProps = NULL; /* qemuMonitorAddObject consumes */ + if (rc < 0) + goto error; + } + + return qemuDomainObjExitMonitor(driver, vm); + + error: + orig_err = virSaveLastError(); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } + qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias); + + return -1; +} + + static int qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, qemuDomainObjPrivatePtr priv, @@ -1584,8 +1663,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, char *charAlias = NULL; char *devstr = NULL; bool chardevAdded = false; - bool tlsobjAdded = false; - bool secobjAdded = false; virJSONValuePtr tlsProps = NULL; virJSONValuePtr secProps = NULL; char *tlsAlias = NULL; @@ -1621,25 +1698,11 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, &secProps, &secAlias) < 0) goto cleanup; - qemuDomainObjEnterMonitor(driver, vm); - - if (secAlias) { - rc = qemuMonitorAddObject(priv->mon, "secret", - secAlias, secProps); - secProps = NULL; - if (rc < 0) - goto exit_monitor; - secobjAdded = true; - } + if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps, + tlsAlias, &tlsProps) < 0) + goto audit; - if (tlsAlias) { - rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509", - tlsAlias, tlsProps); - tlsProps = NULL; /* qemuMonitorAddObject consumes */ - if (rc < 0) - goto exit_monitor; - tlsobjAdded = true; - } + qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorAttachCharDev(priv->mon, charAlias, @@ -1674,15 +1737,12 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, /* detach associated chardev on error */ if (chardevAdded) ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); - if (tlsobjAdded) - ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); - if (secobjAdded) - ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } + qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias); goto audit; } @@ -1860,10 +1920,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, virDomainChrSourceDefPtr dev = chr->source; char *charAlias = NULL; bool chardevAttached = false; - bool tlsobjAdded = false; bool teardowncgroup = false; bool teardowndevice = false; - bool secobjAdded = false; virJSONValuePtr tlsProps = NULL; char *tlsAlias = NULL; virJSONValuePtr secProps = NULL; @@ -1910,24 +1968,11 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, &secProps, &secAlias) < 0) goto cleanup; - qemuDomainObjEnterMonitor(driver, vm); - if (secAlias) { - rc = qemuMonitorAddObject(priv->mon, "secret", - secAlias, secProps); - secProps = NULL; - if (rc < 0) - goto exit_monitor; - secobjAdded = true; - } + if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps, + tlsAlias, &tlsProps) < 0) + goto audit; - if (tlsAlias) { - rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509", - tlsAlias, tlsProps); - tlsProps = NULL; /* qemuMonitorAddObject consumes */ - if (rc < 0) - goto exit_monitor; - tlsobjAdded = true; - } + qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorAttachCharDev(priv->mon, charAlias, chr->source) < 0) goto exit_monitor; @@ -1968,16 +2013,13 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, /* detach associated chardev on error */ if (chardevAttached) qemuMonitorDetachCharDev(priv->mon, charAlias); - if (tlsobjAdded) - ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); - if (secobjAdded) - ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (orig_err) { virSetError(orig_err); virFreeError(orig_err); } + qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias); goto audit; } @@ -2002,8 +2044,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, bool teardowndevice = false; bool chardevAdded = false; bool objAdded = false; - bool tlsobjAdded = false; - bool secobjAdded = false; virJSONValuePtr props = NULL; virJSONValuePtr tlsProps = NULL; virJSONValuePtr secProps = NULL; @@ -2078,27 +2118,13 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, charAlias, &tlsProps, &tlsAlias, &secProps, &secAlias) < 0) goto cleanup; - } - qemuDomainObjEnterMonitor(driver, vm); - - if (secAlias) { - rv = qemuMonitorAddObject(priv->mon, "secret", - secAlias, secProps); - secProps = NULL; - if (rv < 0) - goto exit_monitor; - secobjAdded = true; + if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps, + tlsAlias, &tlsProps) < 0) + goto audit; } - if (tlsAlias) { - rv = qemuMonitorAddObject(priv->mon, "tls-creds-x509", - tlsAlias, tlsProps); - tlsProps = NULL; /* qemuMonitorAddObject consumes */ - if (rv < 0) - goto exit_monitor; - tlsobjAdded = true; - } + qemuDomainObjEnterMonitor(driver, vm); if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && qemuMonitorAttachCharDev(priv->mon, charAlias, @@ -2154,10 +2180,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && chardevAdded) ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); - if (tlsobjAdded) - ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); - if (secobjAdded) - ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); if (qemuDomainObjExitMonitor(driver, vm) < 0) releaseaddr = false; if (orig_err) { @@ -2165,6 +2187,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, virFreeError(orig_err); } + qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias); goto audit; } diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 0b11c1e..24cf033 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -33,6 +33,19 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, bool force); + +void qemuDomainDelTLSObjects(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *secAlias, + const char *tlsAlias); + +int qemuDomainAddTLSObjects(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *secAlias, + virJSONValuePtr *secProps, + const char *tlsAlias, + virJSONValuePtr *tlsProps); + int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainControllerDefPtr controller); -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:22 -0500, John Ferlan wrote:
Refactor the TLS object adding code to make two separate API's that will handle the add/remove of the "secret" and "tls-creds-x509" objects including the Enter/Exit monitor commands.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 165 +++++++++++++++++++++++++++--------------------- src/qemu/qemu_hotplug.h | 13 ++++ 2 files changed, 107 insertions(+), 71 deletions(-)
ACK Jirka

Create a qemuDomainAddChardevTLSObjects which will encapsulate the qemuDomainGetChardevTLSObjects and qemuDomainAddTLSObjects so that the callers don't need to worry about the props. Move the dev->type and haveTLS checks in to the Add function to avoid an unnecessary call to qemuDomainAddTLSObjects Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 80 ++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 75a2596..3ae6b2a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1620,10 +1620,6 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, qemuDomainChrSourcePrivatePtr chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev); - if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP || - dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) - return 0; - /* Add a secret object in order to access the TLS environment. * The secinfo will only be created for serial TCP device. */ if (chrSourcePriv && chrSourcePriv->secinfo) { @@ -1650,6 +1646,43 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, } +static int +qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver, + virQEMUDriverConfigPtr cfg, + virDomainObjPtr vm, + virDomainChrSourceDefPtr dev, + char *charAlias, + char **tlsAlias, + char **secAlias) +{ + int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + virJSONValuePtr tlsProps = NULL; + virJSONValuePtr secProps = NULL; + + if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP || + dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) + return 0; + + if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias, + &tlsProps, tlsAlias, + &secProps, secAlias) < 0) + goto cleanup; + + if (qemuDomainAddTLSObjects(driver, vm, *secAlias, &secProps, + *tlsAlias, &tlsProps) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virJSONValueFree(tlsProps); + virJSONValueFree(secProps); + + return ret; +} + + int qemuDomainAttachRedirdevDevice(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -1663,8 +1696,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, char *charAlias = NULL; char *devstr = NULL; bool chardevAdded = false; - virJSONValuePtr tlsProps = NULL; - virJSONValuePtr secProps = NULL; char *tlsAlias = NULL; char *secAlias = NULL; bool need_release = false; @@ -1693,13 +1724,8 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, redirdev->source) < 0) goto cleanup; - if (qemuDomainGetChardevTLSObjects(cfg, priv, redirdev->source, - charAlias, &tlsProps, &tlsAlias, - &secProps, &secAlias) < 0) - goto cleanup; - - if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps, - tlsAlias, &tlsProps) < 0) + if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, redirdev->source, + charAlias, &tlsAlias, &secAlias) < 0) goto audit; qemuDomainObjEnterMonitor(driver, vm); @@ -1724,9 +1750,7 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, if (ret < 0 && need_release) qemuDomainReleaseDeviceAddress(vm, &redirdev->info, NULL); VIR_FREE(tlsAlias); - virJSONValueFree(tlsProps); VIR_FREE(secAlias); - virJSONValueFree(secProps); VIR_FREE(charAlias); VIR_FREE(devstr); virObjectUnref(cfg); @@ -1922,9 +1946,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, bool chardevAttached = false; bool teardowncgroup = false; bool teardowndevice = false; - virJSONValuePtr tlsProps = NULL; char *tlsAlias = NULL; - virJSONValuePtr secProps = NULL; char *secAlias = NULL; bool need_release = false; @@ -1963,13 +1985,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, dev) < 0) goto cleanup; - if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias, - &tlsProps, &tlsAlias, - &secProps, &secAlias) < 0) - goto cleanup; - - if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps, - tlsAlias, &tlsProps) < 0) + if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, dev, charAlias, + &tlsAlias, &secAlias) < 0) goto audit; qemuDomainObjEnterMonitor(driver, vm); @@ -2000,9 +2017,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, VIR_WARN("Unable to remove chr device from /dev"); } VIR_FREE(tlsAlias); - virJSONValueFree(tlsProps); VIR_FREE(secAlias); - virJSONValueFree(secProps); VIR_FREE(charAlias); VIR_FREE(devstr); virObjectUnref(cfg); @@ -2045,8 +2060,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, bool chardevAdded = false; bool objAdded = false; virJSONValuePtr props = NULL; - virJSONValuePtr tlsProps = NULL; - virJSONValuePtr secProps = NULL; virDomainCCWAddressSetPtr ccwaddrs = NULL; const char *type; int ret = -1; @@ -2114,13 +2127,8 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, rng->source.chardev) < 0) goto cleanup; - if (qemuDomainGetChardevTLSObjects(cfg, priv, rng->source.chardev, - charAlias, &tlsProps, &tlsAlias, - &secProps, &secAlias) < 0) - goto cleanup; - - if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps, - tlsAlias, &tlsProps) < 0) + if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, rng->source.chardev, + charAlias, &tlsAlias, &secAlias) < 0) goto audit; } @@ -2153,8 +2161,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, audit: virDomainAuditRNG(vm, NULL, rng, "attach", ret == 0); cleanup: - virJSONValueFree(tlsProps); - virJSONValueFree(secProps); virJSONValueFree(props); if (ret < 0) { if (releaseaddr) -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:23 -0500, John Ferlan wrote:
Create a qemuDomainAddChardevTLSObjects which will encapsulate the qemuDomainGetChardevTLSObjects and qemuDomainAddTLSObjects so that the callers don't need to worry about the props.
Move the dev->type and haveTLS checks in to the Add function to avoid an unnecessary call to qemuDomainAddTLSObjects
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 80 ++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 37 deletions(-)
ACK Jirka

Move the call to inside the qemuDomainAddChardevTLSObjects in order to further converge the code. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3ae6b2a..5924031 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1647,10 +1647,12 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, static int -qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver, +qemuDomainAddChardevTLSObjects(virConnectPtr conn, + virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, virDomainChrSourceDefPtr dev, + char *devAlias, char *charAlias, char **tlsAlias, char **secAlias) @@ -1664,6 +1666,9 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver, dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) return 0; + if (qemuDomainSecretChardevPrepare(conn, cfg, priv, devAlias, dev) < 0) + goto cleanup; + if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias, &tlsProps, tlsAlias, &secProps, secAlias) < 0) @@ -1720,12 +1725,9 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, if (VIR_REALLOC_N(def->redirdevs, def->nredirdevs+1) < 0) goto cleanup; - if (qemuDomainSecretChardevPrepare(conn, cfg, priv, redirdev->info.alias, - redirdev->source) < 0) - goto cleanup; - - if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, redirdev->source, - charAlias, &tlsAlias, &secAlias) < 0) + if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, redirdev->source, + redirdev->info.alias, charAlias, + &tlsAlias, &secAlias) < 0) goto audit; qemuDomainObjEnterMonitor(driver, vm); @@ -1981,11 +1983,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, if (qemuDomainChrPreInsert(vmdef, chr) < 0) goto cleanup; - if (qemuDomainSecretChardevPrepare(conn, cfg, priv, chr->info.alias, - dev) < 0) - goto cleanup; - - if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, dev, charAlias, + if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, dev, + chr->info.alias, charAlias, &tlsAlias, &secAlias) < 0) goto audit; @@ -2123,12 +2122,10 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, goto cleanup; if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) { - if (qemuDomainSecretChardevPrepare(conn, cfg, priv, rng->info.alias, - rng->source.chardev) < 0) - goto cleanup; - - if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, rng->source.chardev, - charAlias, &tlsAlias, &secAlias) < 0) + if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, + rng->source.chardev, + rng->info.alias, charAlias, + &tlsAlias, &secAlias) < 0) goto audit; } -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:24 -0500, John Ferlan wrote:
Move the call to inside the qemuDomainAddChardevTLSObjects in order to further converge the code.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-)
ACK Jirka

Move the call to inside the qemuDomainAddChardevTLSObjects in order to further converge the code. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5924031..83ac1c8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1649,7 +1649,6 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, static int qemuDomainAddChardevTLSObjects(virConnectPtr conn, virQEMUDriverPtr driver, - virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, virDomainChrSourceDefPtr dev, char *devAlias, @@ -1658,13 +1657,19 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn, char **secAlias) { int ret = -1; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virJSONValuePtr tlsProps = NULL; virJSONValuePtr secProps = NULL; + /* NB: This may alter haveTLS based on cfg */ + qemuDomainPrepareChardevSourceTLS(dev, cfg); + if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP || - dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) - return 0; + dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) { + ret = 0; + goto cleanup; + } if (qemuDomainSecretChardevPrepare(conn, cfg, priv, devAlias, dev) < 0) goto cleanup; @@ -1683,6 +1688,7 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn, cleanup: virJSONValueFree(tlsProps); virJSONValueFree(secProps); + virObjectUnref(cfg); return ret; } @@ -1695,7 +1701,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, { int ret = -1; int rc; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; char *charAlias = NULL; @@ -1706,8 +1711,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, bool need_release = false; virErrorPtr orig_err; - qemuDomainPrepareChardevSourceTLS(redirdev->source, cfg); - if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0) goto cleanup; @@ -1725,7 +1728,7 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, if (VIR_REALLOC_N(def->redirdevs, def->nredirdevs+1) < 0) goto cleanup; - if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, redirdev->source, + if (qemuDomainAddChardevTLSObjects(conn, driver, vm, redirdev->source, redirdev->info.alias, charAlias, &tlsAlias, &secAlias) < 0) goto audit; @@ -1755,7 +1758,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn, VIR_FREE(secAlias); VIR_FREE(charAlias); VIR_FREE(devstr); - virObjectUnref(cfg); return ret; exit_monitor: @@ -1938,7 +1940,6 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, virDomainChrDefPtr chr) { int ret = -1, rc; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virErrorPtr orig_err; virDomainDefPtr vmdef = vm->def; @@ -1956,8 +1957,6 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0) goto cleanup; - qemuDomainPrepareChardevSourceTLS(dev, cfg); - if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0) goto cleanup; @@ -1983,7 +1982,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, if (qemuDomainChrPreInsert(vmdef, chr) < 0) goto cleanup; - if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, dev, + if (qemuDomainAddChardevTLSObjects(conn, driver, vm, dev, chr->info.alias, charAlias, &tlsAlias, &secAlias) < 0) goto audit; @@ -2019,7 +2018,6 @@ int qemuDomainAttachChrDevice(virConnectPtr conn, VIR_FREE(secAlias); VIR_FREE(charAlias); VIR_FREE(devstr); - virObjectUnref(cfg); return ret; exit_monitor: @@ -2044,7 +2042,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, virDomainObjPtr vm, virDomainRNGDefPtr rng) { - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_RNG, { .rng = rng } }; virErrorPtr orig_err; @@ -2105,9 +2102,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, goto cleanup; teardowncgroup = true; - if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) - qemuDomainPrepareChardevSourceTLS(rng->source.chardev, cfg); - /* build required metadata */ if (!(devstr = qemuBuildRNGDevStr(vm->def, rng, priv->qemuCaps))) goto cleanup; @@ -2122,7 +2116,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, goto cleanup; if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) { - if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, + if (qemuDomainAddChardevTLSObjects(conn, driver, vm, rng->source.chardev, rng->info.alias, charAlias, &tlsAlias, &secAlias) < 0) @@ -2174,7 +2168,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, VIR_FREE(objAlias); VIR_FREE(devstr); virDomainCCWAddressSetFree(ccwaddrs); - virObjectUnref(cfg); return ret; exit_monitor: -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:25 -0500, John Ferlan wrote:
Move the call to inside the qemuDomainAddChardevTLSObjects in order to further converge the code.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-)
ACK Jirka

Split apart and rename qemuDomainGetChardevTLSObjects in order to make a more generic API that can create the TLS JSON prop objects (secret and tls-creds-x509) to be used to create the objects Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 55 ++++++++++++++++++++++++++----------------------- src/qemu/qemu_hotplug.h | 11 ++++++++++ 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 83ac1c8..f056a73 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1607,40 +1607,34 @@ qemuDomainAddTLSObjects(virQEMUDriverPtr driver, } -static int -qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, - qemuDomainObjPrivatePtr priv, - virDomainChrSourceDefPtr dev, - char *charAlias, - virJSONValuePtr *tlsProps, - char **tlsAlias, - virJSONValuePtr *secProps, - char **secAlias) +int +qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, + qemuDomainSecretInfoPtr secinfo, + const char *tlsCertdir, + bool tlsListen, + bool tlsVerify, + const char *srcAlias, + virJSONValuePtr *tlsProps, + char **tlsAlias, + virJSONValuePtr *secProps, + char **secAlias) { - qemuDomainChrSourcePrivatePtr chrSourcePriv = - QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev); - /* Add a secret object in order to access the TLS environment. * The secinfo will only be created for serial TCP device. */ - if (chrSourcePriv && chrSourcePriv->secinfo) { - if (qemuBuildSecretInfoProps(chrSourcePriv->secinfo, secProps) < 0) + if (secinfo) { + if (qemuBuildSecretInfoProps(secinfo, secProps) < 0) return -1; - if (!(*secAlias = qemuDomainGetSecretAESAlias(charAlias, false))) + if (!(*secAlias = qemuDomainGetSecretAESAlias(srcAlias, false))) return -1; } - if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir, - dev->data.tcp.listen, - cfg->chardevTLSx509verify, - *secAlias, - priv->qemuCaps, - tlsProps) < 0) + if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify, + *secAlias, qemuCaps, tlsProps) < 0) return -1; - if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(charAlias))) + if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias))) return -1; - dev->data.tcp.tlscreds = true; return 0; } @@ -1659,6 +1653,8 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn, int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainChrSourcePrivatePtr chrSourcePriv; + qemuDomainSecretInfoPtr secinfo = NULL; virJSONValuePtr tlsProps = NULL; virJSONValuePtr secProps = NULL; @@ -1674,10 +1670,17 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn, if (qemuDomainSecretChardevPrepare(conn, cfg, priv, devAlias, dev) < 0) goto cleanup; - if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias, - &tlsProps, tlsAlias, - &secProps, secAlias) < 0) + if ((chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev))) + secinfo = chrSourcePriv->secinfo; + + if (qemuDomainGetTLSObjects(priv->qemuCaps, secinfo, + cfg->chardevTLSx509certdir, + dev->data.tcp.listen, + cfg->chardevTLSx509verify, + charAlias, &tlsProps, tlsAlias, + &secProps, secAlias) < 0) goto cleanup; + dev->data.tcp.tlscreds = true; if (qemuDomainAddTLSObjects(driver, vm, *secAlias, &secProps, *tlsAlias, &tlsProps) < 0) diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 24cf033..73f2b1f 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -46,6 +46,17 @@ int qemuDomainAddTLSObjects(virQEMUDriverPtr driver, const char *tlsAlias, virJSONValuePtr *tlsProps); +int qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps, + qemuDomainSecretInfoPtr secinfo, + const char *tlsCertdir, + bool tlsListen, + bool tlsVerify, + const char *srcAlias, + virJSONValuePtr *tlsProps, + char **tlsAlias, + virJSONValuePtr *secProps, + char **secAlias); + int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainControllerDefPtr controller); -- 2.9.3

On Wed, Mar 01, 2017 at 18:30:26 -0500, John Ferlan wrote:
Split apart and rename qemuDomainGetChardevTLSObjects in order to make a more generic API that can create the TLS JSON prop objects (secret and tls-creds-x509) to be used to create the objects
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 55 ++++++++++++++++++++++++++----------------------- src/qemu/qemu_hotplug.h | 11 ++++++++++ 2 files changed, 40 insertions(+), 26 deletions(-)
ACK Jirka
participants (2)
-
Jiri Denemark
-
John Ferlan