Re: [libvirt] [PATCH 04/13] qemu: Introduce qemuDomainSecretMigrate{Prepare|Destroy}

On Fri, Feb 17, 2017 at 14:39:21 -0500, John Ferlan wrote: > Introduce API's to Prepare/Destroy a qemuDomainSecretInfoPtr to be > used with a migrate or nbd TLS object > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/qemu/qemu_domain.c | 73 +++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_domain.h | 88 +++++++++++++++++++++++++++++--------------------- > 2 files changed, 124 insertions(+), 37 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index be44843..dd3cfd5 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -1370,6 +1370,77 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn, > } > > > +/* qemuDomainSecretMigrateDestroy: > + * @migSecinfo: Pointer to the secinfo from the incoming def > + * > + * Clear and destroy memory associated with the secret > + */ > +void > +qemuDomainSecretMigrateDestroy(qemuDomainSecretInfoPtr *migSecinfo) > +{ > + if (!*migSecinfo) > + return; > + > + qemuDomainSecretInfoFree(migSecinfo); > +} This is a useless wrapper, please drop it.

> +/* qemuDomainSecretMigratePrepare > + * @conn: Pointer to connection > + * @priv: pointer to domain private object > + * @srcAlias: Alias to use (either migrate or nbd) > + * @secretUUID: UUID for the secret from the cfg (migrate or nbd) > + * > + * Create and prepare the qemuDomainSecretInfoPtr to be used for either > + * a migration or nbd. Unlike other domain secret prepare functions, this > + * is only expected to be called for a single object/instance. Theoretically > + * the object could be reused, although that results in keeping a secret > + * stored in memory for perhaps longer than expected or necessary. > + * > + * Returns 0 on success, -1 on failure > + */ > +int > +qemuDomainSecretMigratePrepare(virConnectPtr conn, > + qemuDomainObjPrivatePtr priv, > + const char *srcAlias, > + const char *secretUUID) > +{ > + virSecretLookupTypeDef seclookupdef = {0}; > + qemuDomainSecretInfoPtr secinfo = NULL; > + > + if (virUUIDParse(secretUUID, seclookupdef.u.uuid) < 0) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("malformed %s TLS secret uuid in qemu.conf"), [1] > + srcAlias); > + return -1; > + } > + seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID; > + > + if (VIR_ALLOC(secinfo) < 0) > + return -1; > + > + if (qemuDomainSecretSetup(conn, priv, secinfo, srcAlias, > + VIR_SECRET_USAGE_TYPE_TLS, NULL, > + &seclookupdef, false) < 0) > + goto error; > + > + if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("TLS X.509 requires encrypted secrets " > + "to be supported")); > + goto error; > + } > + priv->migSecinfo = secinfo; > + > + return 0; > + > + error: > + qemuDomainSecretInfoFree(&secinfo); > + return -1; > +} Almost all lines in this functions were just copy-pasted from qemuDomainSecretChardevPrepare. Could you merge the two? Ideally you can just make it a function which lookups the secinfo and you can do the rest in the caller. Jirka