8:42 p.m.
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(a)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