[libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy

[John Ferlan]

On 05/02/2016 09:52 AM, Ján Tomko wrote:
> On Mon, May 02, 2016 at 08:48:02AM -0400, John Ferlan wrote:
>> On 05/02/2016 07:38 AM, Ján Tomko wrote:
>>> On Sat, Apr 16, 2016 at 10:17:35AM -0400, John Ferlan wrote:
>>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>>> index 81d86c2..c9f43fa 100644
>>>> --- a/src/qemu/qemu_process.c
>>>> +++ b/src/qemu/qemu_process.c
>>>> @@ -5640,6 +5640,9 @@ qemuProcessStart(virConnectPtr conn,
>>>>      if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
>>>>          goto stop;
>>>>  
>>>> +    if (qemuDomainSecretPrepare(conn, vm) < 0)
>>>> +        goto cleanup;
>>>> +
>>>
>>> The call fits better in qemuProcessPrepareDomain,
>>> that way it will be called even for incoming migration.
>>>
>>
>> Understood; however, PrepareDomain doesn't have everything that will be
>> needed.  The qemuProcessPrepareHost must run first in order to create
>> "priv->libDir" in order to write the domain master key secret that will
>> be used in "future patches" (11/12 of this series) in order to generate
>> an Initialization Vector secret.
>>
>> I think it's possible to move the call into qemuProcessPrepareHost if
>> you think that works better/fine.  The qemuProcessCreatePretendCmd
>> already calls qemuDomainSecretPrepare, so that "should" cover the
>> testing scenario...
> 
> Another possibility could be splitting qemuDomainMasterKeyCreate
> into key creation (which does prepare the domain) and writing/labeling
> (which prepares the host environment).

Seems a bit of overkill to me to keep split that up. If that's something
you feel strongly about I can review whatever you send.

> 
> Either way, this patch removed formatting secrets from the migration
> path.
> 

Yes, I see - good catch... I just didn't keep up with all the other
changes made around this by also addressing.

For me the question comes down to whether I add the call to
qemuDomainSecretPrepare in qemuProcessPrepareHost or
qemuMigrationPrepareAny.  To a degree adding the secrets doesn't fit
with the "Host" type functions, so I'm starting to lean toward the latter...

John