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

[John Ferlan]

On 02/20/2017 10:43 AM, Jiri Denemark wrote:
> 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 at redhat.com>
>> ---
>>  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.
> 

Sure - it's like an automatic reaction - have allocation need free
function... Will use qemuDomainSecretInfoFree  instead...

It does get used later in patch 12 and come to think of it, probably
needs to be added to patch 13 in the cleanup of the Run.

>> +/* 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
> 

Sure. no problem... I'll create:

static qemuDomainSecretInfoPtr
qemuDomainSecretTLSObjectPrepare(virConnectPtr conn,
                                 qemuDomainObjPrivatePtr priv,
                                 const char *srcAlias,
                                 const char *secretUUID)


which grabs the guts and does the right thing... I'll also adjust the
UUIDParse error message to just be "malformed secret uuid '%s' in
qemu.conf" passing the secretUUID string for the error message


Tks -

John