[libvirt] [PATCH 3/3] qemu: Introduce qemuBuildMasterKeyCommandLine

[Daniel P. Berrange]

On Thu, Mar 24, 2016 at 01:53:21PM -0400, John Ferlan wrote:
> If the -object secret capability exists, then get the path to the
> base64 encoded masterKey file and provide that to qemu. Checking
> for the existence of the file before passing to qemu could be done,
> but causes issues in mock test environment.
> 
> Since the qemuDomainObjPrivate is not available when building the
> command line, the qemuBuildHasMasterKey API will have to suffice
> as the primary arbiter for whether the capability exists in order
> to find/return the path to the master key for usage.
> 
> Created the qemuDomainGetMasterKeyAlias API which will be used by
> later patches to define the 'keyid' (eg, masterKey) to be used by
> other secrets to provide the id to qemu for the master key.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_alias.c                              | 17 ++++++
>  src/qemu/qemu_alias.h                              |  3 +
>  src/qemu/qemu_command.c                            | 68 ++++++++++++++++++++++
>  .../qemuxml2argvdata/qemuxml2argv-master-key.args  | 23 ++++++++
>  tests/qemuxml2argvdata/qemuxml2argv-master-key.xml | 30 ++++++++++
>  tests/qemuxml2argvtest.c                           |  2 +
>  6 files changed, 143 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-master-key.xml
> 
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index efd9222..b57b967 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -484,3 +484,20 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
>  
>      return 0;
>  }
> +
> +
> +/* qemuDomainGetMasterKeyAlias:
> + *
> + * Generate and return the masterKey alias
> + *
> + * Returns NULL or a string containing the master key alias
> + */
> +char *
> +qemuDomainGetMasterKeyAlias(void)
> +{
> +    char *alias;
> +
> +    ignore_value(VIR_STRDUP(alias, "masterKey0"));
> +
> +    return alias;
> +}
> diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
> index a2eaa27..299a6d4 100644
> --- a/src/qemu/qemu_alias.h
> +++ b/src/qemu/qemu_alias.h
> @@ -61,4 +61,7 @@ int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps);
>  
>  int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info,
>                                 const char *prefix);
> +
> +char *qemuDomainGetMasterKeyAlias(void);
> +
>  #endif /* __QEMU_ALIAS_H__*/
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0331789..2b1dc93 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -151,6 +151,71 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
>                "interleave");
>  
>  /**
> + * qemuBuildHasMasterKey:
> + * @qemuCaps: QEMU binary capabilities
> + *
> + * Return true if this binary supports the secret -object, false otherwise.
> + */
> +static bool
> +qemuBuildHasMasterKey(virQEMUCapsPtr qemuCaps)
> +{
> +    return virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET);
> +}
> +
> +
> +/**
> + * qemuBuildMasterKeyCommandLine:
> + * @cmd: the command to modify
> + * @qemuCaps qemu capabilities object
> + * @domainLibDir: location to find the master key
> +
> + * Formats the command line for a master key if available
> + *
> + * Returns 0 on success, -1 w/ error message on failure
> + */
> +static int
> +qemuBuildMasterKeyCommandLine(virCommandPtr cmd,
> +                              virQEMUCapsPtr qemuCaps,
> +                              const char *domainLibDir)
> +{
> +    int ret = -1;
> +    char *alias = NULL;
> +    char *path = NULL;
> +
> +    /* If the -object secret does not exist, then just return. This just
> +     * means the domain won't be able to use a secret master key and is
> +     * not a failure.
> +     */
> +    if (!qemuBuildHasMasterKey(qemuCaps)) {
> +        VIR_INFO("secret object is not supported by this QEMU binary");
> +        return 0;
> +    }
> +
> +    if (!(alias = qemuDomainGetMasterKeyAlias()))
> +        return -1;
> +
> +    /* Get the path. NB, the mocked test will not have the created
> +     * file so we cannot check for existence, which is no different
> +     * than other command line options which do not check for the
> +     * existence of socket files before using.
> +     */
> +    if (!(path = qemuDomainGetMasterKeyFilePath(domainLibDir)))
> +        goto cleanup;
> +
> +    virCommandAddArg(cmd, "-object");
> +    virCommandAddArgFormat(cmd, "secret,id=%s,format=base64,file=%s",

Reference my question in previous patch about whether we should
just use format=raw instead of base64

> +                           alias, path);
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(alias);
> +    VIR_FREE(path);
> +    return ret;
> +}


ACK in general though


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|