[libvirt] [PATCH 2/4] qemu: Introduce qemuBuildSecretObjectProps

[Peter Krempa]

On Fri, May 27, 2016 at 09:57:08 -0400, John Ferlan wrote:
> Need to commonalize the code a bit more in order to use a common function
> to build the JSON property from either a qemuDomainSecretInfoPtr or a
> virSecretKeyDef
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_command.c | 65 +++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 60 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c55f42e..06d135b 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -508,6 +508,64 @@ qemuNetworkDriveGetPort(int protocol,
>  
>  
>  /**
> + * qemuBuildSecretObjectProps
> + * @data: Pointer to data string
> + * @isfile: Boolean to indicate whether data is raw data or a filepath string
> + * @fmt: Format for the data/file (may be NULL)
> + * @keyid: Master key alias id (may be NULL)
> + * @iv: Initialization vector (may be NULL)
> + * @propsret: location to store the created/built property object
> + *
> + * There's many ways to build a secret object for qemu depending on need,
> + *
> + *    -object secret,id=$alias,data=$data

This doesn't make sense to use since it doesn't support binary data and
puts the secret on the command line.

> + *    -object secret,id=$alias,data=$data[,format=base64]

Almost the same as above except for binary data. Both should not be
allowed at all.

> + *    -object secret,id=$alias,file=$file

This makes sense.

> + *    -object secret,id=$alias,file=$file[,format=base64]
> + *    -object secret,id=$alias,data=$data,keyid=$keyid,[iv=$iv],format=base64
> + *
> + * When a keyid and/or iv are provided, they are assumed to be base64 encoded
> + *
> + * Build the JSON object property thusly and return
> + *
> + * Returns 0 on success, -1 on failure w/ error set
> + */
> +static int
> +qemuBuildSecretObjectProps(const char *data,
> +                           bool isfile,

Hm why not pass the file as a different pointer rather than a bool?

> +                           const char *fmt,
> +                           const char *keyid,
> +                           const char *iv,
> +                           virJSONValuePtr *propsret)
> +{
> +    if (!(*propsret = virJSONValueNewObject()))
> +        return -1;
> +
> +    if (isfile && virJSONValueObjectAdd(*propsret, "s:file", data, NULL) < 0)
> +        goto error;
> +    else if (virJSONValueObjectAdd(*propsret, "s:data", data, NULL) < 0)
> +        goto error;
> +
> +    if (keyid && virJSONValueObjectAdd(*propsret, "s:keyid", keyid, NULL) < 0)
> +        goto error;
> +
> +    if (iv && virJSONValueObjectAdd(*propsret, "s:iv", iv, NULL) < 0)
> +        goto error;
> +
> +    /* NB: QEMU will assume "raw" when fmt not provided! */
> +    if (fmt && virJSONValueObjectAdd(*propsret, "s:format", fmt, NULL) < 0)
> +        goto error;
> +
> +    return 0;
> +
> + error:
> +    virJSONValueFree(*propsret);
> +
> +    return -1;

I don't quite see a reason for this helper since the same can be
achieved ...

> +}
> +
> +
> +/**
>   * qemuBuildSecretInfoProps:
>   * @secinfo: pointer to the secret info object
>   * @type: returns a pointer to a character string for object name
> @@ -531,11 +589,8 @@ qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
>      if (!(keyid = qemuDomainGetMasterKeyAlias()))
>          return -1;
>  
> -    if (virJSONValueObjectCreate(propsret,
> -                                 "s:data", secinfo->s.aes.ciphertext,
> -                                 "s:keyid", keyid,
> -                                 "s:iv", secinfo->s.aes.iv,
> -                                 "s:format", "base64", NULL) < 0)

... with a similar single call to a function.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160531/64371313/attachment-0001.sig>