[libvirt] [PATCH v3 7/7] qemu: Set up the migration TLS objects for source

[John Ferlan]

On 03/22/2017 12:26 PM, Jiri Denemark wrote:
> On Fri, Mar 17, 2017 at 14:39:01 -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1300769
>>
>> If the migration flags indicate this migration will be using TLS,
>> then while we have connection in the Begin phase check and setup the
>> TLS environment that will be used by virMigrationRun during the Perform
>> phase for the source to configure TLS.
>>
>> This creates at least an "-object tls-creds-x509,endpoint=client,..."
>> and potentially an "-object secret,..." to handle the passphrase response
>> to access the TLS credentials. The alias/id used for the TLS objects
>> will contain "libvirt_migrate" as a mechanism to signify that libvirt
>> started the migration on the source (reaping benefits possibly).
>>
>> Once the objects are created, the code will set the "tls-creds" and
>> "tls-hostname" migration parameters to signify usage of TLS.
> 
> Looks like a copy&paste from the previous patch. Is it necessary?
> 

I can reword

>>
>> Since qemuProcessRecoverMigrationOut will cancel outgoing migrations
>> that are still in the QEMU_MIGRATION_PHASE_PERFORM{2|3} stages, there's
>> no need to do anything special as the Perform cleanup and Cancel phases
>> will reset the environment.
> 
> Sure, TLS will be reset because you added a call to
> qemuMigrationResetTLS() in qemuMigrationCancel(). Just drop this
> paragraph which contradicts what you actually did.
> 

OK - so as I figured out things the RecoverMigrationOut only mattered
when certain migration phase requirements were met - that wouldn't
happen because the setting and clearing of the parameters is done in the
PrepareAny and Run phases as well as Cancel.

>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/qemu/qemu_migration.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index 42074f0..5acae6e 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
> ...
>> @@ -5075,6 +5086,38 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>>      if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig, graphicsuri) < 0)
>>          VIR_WARN("unable to provide data for graphics client relocation");
>>  
>> +    if (flags & VIR_MIGRATE_TLS) {
>> +        cfg = virQEMUDriverGetConfig(driver);
>> +
>> +        /* Begin/CheckSetupTLS already set up migTLSAlias, the following
>> +         * assumes that and adds the TLS objects to the domain. */
>> +        if (qemuMigrationAddTLSObjects(driver, vm, cfg, false,
>> +                                       QEMU_ASYNC_JOB_MIGRATION_OUT,
>> +                                       &tlsAlias, &secAlias, migParams) < 0)
>> +            goto cleanup;
>> +
>> +        /* We need to add the tls-hostname only for special circumstances,
>> +         * e.g. for a fd: or exec: based migration. As it turns out the
>> +         * CONNECT_HOST turns into an FD migration (see below). */
> 
> Specifically, we need to add tls-hostname whenever QEMU itself does not
> connect directly to the destination, which means
> MIGRATION_DEST_CONNECT_HOST and MIGRATION_DEST_FD.
> 

Sure - something that wasn't obvious on my first pass through the code.
In fact having CONNECT_HOST change into FD later on wasn't as obvious to
me until I dug through the code.

I can change the comment to:

        /* We need to add tls-hostname whenever QEMU itself does not
         * connect directly to the destination. NB: The CONNECT_HOST
         * turns into a FD migration below in qemuMigrationConnect */


>> +        if (spec->destType == MIGRATION_DEST_CONNECT_HOST ||
>> +            spec->destType == MIGRATION_DEST_FD) {
>> +            if (VIR_STRDUP(migParams->migrateTLSHostname,
>> +                           spec->dest.host.name) < 0)
>> +                goto cleanup;
>> +        } else {
>> +            /* Be sure there's nothing from a previous migration */
>> +            if (VIR_STRDUP(migParams->migrateTLSHostname, "") < 0)
>> +                goto cleanup;
>> +        }
>> +    } else {
>> +        /* If we support setting the tls-creds, be sure to always reset
>> +         * the migration parameters when this migration isn't using TLS */
>> +        if ((qemuMigrationCheckTLSCreds(driver, vm,
>> +                                        QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) ||
>> +            (qemuMigrationSetEmptyTLSParams(priv, migParams) < 0))

and I've fixed the extra () here as well

John
>> +            goto cleanup;
>> +    }
>> +
>>      if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK |
>>                           QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) {
>>          if (mig->nbd) {
> 
> Jirka
>