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@xxxxxxxxxx>
>> ---
>> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list