Re: [libvirt] [PATCH v3 7/7] qemu: Set up the migration TLS objects for source
On Wed, Mar 22, 2017 at 21:42:49 -0400, John Ferlan wrote:
> On 03/22/2017 12:26 PM, Jiri Denemark wrote:
> > On Fri, Mar 17, 2017 at 14:39:01 -0400, John Ferlan wrote:
...
> >> @@ -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,
> >> + , , 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 */
The important thing is the semantics of MIGRATION_DEST_CONNECT_HOST and
not the fact that it changes into MIGRATION_DEST_FD. I guess having the
semantics documented for each member of the qemuMigrationDestinationType
enum would help quite a bit :-) With this documentation in place, the
"NB:..." part would not be needed.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 7/7] qemu: Set up the migration TLS objects for source
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 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
>> ---
>> 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,
>> + , , 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
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 7/7] qemu: Set up the migration TLS objects for source
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 from the previous patch. Is it necessary?
>
> 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.
>
> Signed-off-by: John Ferlan
> ---
> 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,
> + , , 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.
> +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))
> +goto cleanup;
> +}
> +
> if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK |
> QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) {
> if (mig->nbd) {
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 7/7] qemu: Set up the migration TLS objects for source
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.
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.
Signed-off-by: John Ferlan
---
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
@@ -3453,6 +3453,7 @@ qemuMigrationBegin(virConnectPtr conn,
unsigned long flags)
{
virQEMUDriverPtr driver = conn->privateData;
+virQEMUDriverConfigPtr cfg = NULL;
char *xml = NULL;
qemuDomainAsyncJob asyncJob;
@@ -3486,6 +3487,12 @@ qemuMigrationBegin(virConnectPtr conn,
nmigrate_disks, migrate_disks, flags)))
goto endjob;
+if (flags & VIR_MIGRATE_TLS) {
+cfg = virQEMUDriverGetConfig(driver);
+if (qemuMigrationCheckSetupTLS(conn, driver, cfg, vm, asyncJob) < 0)
+goto endjob;
+}
+
if ((flags & VIR_MIGRATE_CHANGE_PROTECTION)) {
/* We keep the job active across API calls until the confirm() call.
* This prevents any other APIs being invoked while migration is taking
@@ -3502,6 +3509,7 @@ qemuMigrationBegin(virConnectPtr conn,
}
cleanup:
+virObjectUnref(cfg);
virDomainObjEndAPI();
return xml;
@@ -5010,8 +5018,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
{
int ret = -1;
unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
+virQEMUDriverConfigPtr cfg = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuMigrationCookiePtr mig = NULL;
+char *tlsAlias = NULL;
+char *secAlias = NULL;
qemuMigrationIOThreadPtr iothread = NULL;
int fd = -1;
unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth;
@@ -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,
+ , , 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). */
+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))
+goto cleanup;
+}
+
if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK |
QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) {
if (mig->nbd) {
@@ -5255,6 +5298,14 @@ qemuMigrationRun(virQEMUDriverPtr driver,
ret = -1;
}
+if (qemuMigrationDeconstructTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
+tlsAlias, secAlias) < 0)
+ret = -1;
+
+
