7:41 a.m.
On 03/22/2016 09:01 AM, Peter Krempa wrote:
On Mon, Mar 21, 2016 at 14:29:02 -0400, John Ferlan wrote:
> Using the masterKey and if the capability exists build an -object secret
> command line to pass a masterKey file to qemu. The qemuBuildHasMasterKey
> is expected to be reused by other objects which would need to know not
> only if the masterKey has been provided, but if the secret object can be
> used for their own purposes.
>
> Additionally, generate qemuDomainGetMasterKeyAlias 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.
>
> Although the path to the domain libDir and the masterKey file are created
> after qemuBuildCommandLine completes successfully, it's still possible to
> generate the path and use it. No different than code paths that use the
> domain libDir to create socket files (e.g. monitor.sock).
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 67 ++++++++++++++++++++++
> src/qemu/qemu_domain.c | 17 ++++++
> src/qemu/qemu_domain.h | 2 +
> .../qemuxml2argvdata/qemuxml2argv-master-key.args | 23 ++++++++
> tests/qemuxml2argvdata/qemuxml2argv-master-key.xml | 30 ++++++++++
> tests/qemuxml2argvtest.c | 2 +
> 6 files changed, 141 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_command.c b/src/qemu/qemu_command.c
> index eb02553..04e75fd 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -151,6 +151,70 @@ 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);
> +}
This doesn't seem to be terribly useful. Especially since you have to
check for priv->masterKey anyways.
I thought it might be useful for other CommmandLine functions that may
someday need to determine if the secret -object is supported before
making decisions whether to use it or go with the existing mechanism.
But given other feedback, this'll probably be moot anyway.
> +
> +
> +/**
> + * 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;
Here is the correct place to generate the key. I don't really see the
benefit of doing it even for qemu that does not support this feature.
Cannot disagree, but my conundrum was not having the priv object in
qemuBuildCommandLine, so choose something (libDir) that I can use.
This'll all get flipped in the next pass, but figured I'd at least try
to explain what was rattling through the rafters while making an initial
pass.
> + }
> +
> + if (!(alias = qemuDomainGetMasterKeyAlias()))
> + return -1;
> +
> + /* Get the path... NB, the path will not exist yet as domainLibDir
> + * and the master secret file gets created after we've successfully
> + * built the command line
> + */
> + if (!(path = qemuDomainGetMasterKeyFilePath(domainLibDir)))
> + goto cleanup;
> +
> + virCommandAddArg(cmd, "-object");
> + virCommandAddArgFormat(cmd, "secret,id=%s,format=base64,file=%s",
> + alias, path);
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(alias);
> + VIR_FREE(path);
> + return ret;
> +}
> +
> +
> +/**
> * qemuVirCommandGetFDSet:
> * @cmd: the command to modify
> * @fd: fd to reassign to the child
[...]
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 507ae9e..53d6705 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -468,6 +468,23 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
> }
>
>
> +/* qemuDomainGetMasterKeyAlias:
> + *
> + * Generate and return the masterKey alias
> + *
> + * Returns NULL or a string containing the master key alias
> + */
> +char *
> +qemuDomainGetMasterKeyAlias(void)
You've added src/qemu/qemu_alias.c, so this belongs there.
Coin flip - I chose here mainly because qemu_alias.c is primarily
dealing with Device aliases, but also because qemuDomainStorageAlias is
here. IDC either way and will move it there.
Tks -
John
> +{
> + char *alias;
> +
> + ignore_value(VIR_STRDUP(alias, "masterKey0"));
> +
> + return alias;
> +}
> +
> +
> /* qemuDomainGetMasterKeyFilePath:
> * @libDir: Directory path to domain lib files
> *
Peter