[libvirt] [PATCH 0/8] qemu: Properly reset all migration capabilities
We need to make sure to reset all migration parameters to prevent future operations calling the "migrate" QMP command from accidentally using stale parameters. This series resets only TLS parameters and all capabilities. A followup series will reset all other parameters (such as compression) too. https://bugzilla.redhat.com/show_bug.cgi?id=1425003 Jiri Denemark (8): qemu: Properly reset TLS in qemuProcessRecoverMigrationIn qemu: Drop resume label in qemuProcessRecoverMigrationOut qemu: Always reset TLS in qemuProcessRecoverMigrationOut qemu: Don't reset TLS in qemuMigrationRun qemu: Don't reset TLS in qemuMigrationCancel qemu: Introduce qemuMigrationReset qemu: Simplify qemuMigrationResetTLS qemu: Properly reset all migration capabilities src/qemu/qemu_migration.c | 85 ++++++++++++++++++++++++++--------------------- src/qemu/qemu_migration.h | 10 +++--- src/qemu/qemu_process.c | 36 ++++++++++---------- 3 files changed, 70 insertions(+), 61 deletions(-) -- 2.12.2
There is no async job running when a freshly started libvirtd is trying to recover from an interrupted incoming migration. While at it, let's call qemuMigrationResetTLS every time we don't kill the domain. This is not strictly necessary since TLS is not supported when v2 migration protocol is used, but doing so makes more sense. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e450d0647..a37496107 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2973,9 +2973,6 @@ qemuProcessRecoverMigrationIn(virQEMUDriverPtr driver, /* migration finished, we started resuming the domain but didn't * confirm success or failure yet; killing it seems safest unless * we already started guest CPUs or we were in post-copy mode */ - ignore_value(qemuMigrationResetTLS(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_IN, - NULL, NULL)); if (postcopy) { qemuMigrationPostcopyFailed(driver, vm); } else if (state != VIR_DOMAIN_RUNNING) { @@ -2985,6 +2982,7 @@ qemuProcessRecoverMigrationIn(virQEMUDriverPtr driver, break; } + qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, NULL); return 0; } -- 2.12.2
On Wed, Apr 05, 2017 at 15:09:56 +0200, Jiri Denemark wrote:
There is no async job running when a freshly started libvirtd is trying to recover from an interrupted incoming migration. While at it, let's call qemuMigrationResetTLS every time we don't kill the domain. This is not strictly necessary since TLS is not supported when v2 migration protocol is used, but doing so makes more sense.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
ACK
Let's use a bool variable to create a single shared path returning 0. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a37496107..f2a80ad22 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2998,6 +2998,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver, bool postcopy = state == VIR_DOMAIN_PAUSED && (reason == VIR_DOMAIN_PAUSED_POSTCOPY || reason == VIR_DOMAIN_PAUSED_POSTCOPY_FAILED); + bool resume = false; switch (phase) { case QEMU_MIGRATION_PHASE_NONE: @@ -3028,7 +3029,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver, VIR_WARN("Could not cancel ongoing migration of domain %s", vm->def->name); } - goto resume; + resume = true; } break; @@ -3051,7 +3052,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver, } else { VIR_DEBUG("Resuming domain %s after failed migration", vm->def->name); - goto resume; + resume = true; } break; @@ -3061,21 +3062,21 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver, return -1; } - return 0; - - resume: - /* resume the domain but only if it was paused as a result of - * migration - */ - if (state == VIR_DOMAIN_PAUSED && - (reason == VIR_DOMAIN_PAUSED_MIGRATION || - reason == VIR_DOMAIN_PAUSED_UNKNOWN)) { - if (qemuProcessStartCPUs(driver, vm, conn, - VIR_DOMAIN_RUNNING_UNPAUSED, - QEMU_ASYNC_JOB_NONE) < 0) { - VIR_WARN("Could not resume domain %s", vm->def->name); + if (resume) { + /* resume the domain but only if it was paused as a result of + * migration + */ + if (state == VIR_DOMAIN_PAUSED && + (reason == VIR_DOMAIN_PAUSED_MIGRATION || + reason == VIR_DOMAIN_PAUSED_UNKNOWN)) { + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_UNPAUSED, + QEMU_ASYNC_JOB_NONE) < 0) { + VIR_WARN("Could not resume domain %s", vm->def->name); + } } } + return 0; } -- 2.12.2
On Wed, Apr 05, 2017 at 15:09:57 +0200, Jiri Denemark wrote:
Let's use a bool variable to create a single shared path returning 0.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
ACK
qemuProcessRecoverMigrationOut doesn't explicitly call qemuMigrationResetTLS relying on two things: - qemuMigrationCancel resets TLS parameters - our migration code resets TLS before entering QEMU_MIGRATION_PHASE_PERFORM3_DONE phase But this is not obvious and the assumptions will be broken soon. Let's explicitly reset TLS parameters on all paths which do not kill the domain. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f2a80ad22..935993f16 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3077,6 +3077,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver, } } + qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, NULL); return 0; } -- 2.12.2
On Wed, Apr 05, 2017 at 15:09:58 +0200, Jiri Denemark wrote:
qemuProcessRecoverMigrationOut doesn't explicitly call qemuMigrationResetTLS relying on two things:
- qemuMigrationCancel resets TLS parameters - our migration code resets TLS before entering QEMU_MIGRATION_PHASE_PERFORM3_DONE phase
But this is not obvious and the assumptions will be broken soon. Let's explicitly reset TLS parameters on all paths which do not kill the domain.
We are still fond of the XEN heritage?
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 1 + 1 file changed, 1 insertion(+)
ACK
Finished qemuMigrationRun does not mean the migration itself finished (it might have just switched to post-copy mode). While resetting TLS parameters is probably OK at this point even if migration is still running, we want to consolidate the code which resets various migration parameters. Thus qemuMigrationResetTLS will be called from the Confirm phase (or at the end of the Perform phase in case of v2 protocol), when migration is either canceled or finished. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 68e72b37f..87506c73a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3216,6 +3216,9 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, qemuDomainEventQueue(driver, event); } + qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, + NULL, NULL); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) VIR_WARN("Failed to save status on vm %s", vm->def->name); } @@ -3848,10 +3851,6 @@ qemuMigrationRun(virQEMUDriverPtr driver, ret = -1; } - if (qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, - tlsAlias, secAlias) < 0) - ret = -1; - VIR_FREE(tlsAlias); VIR_FREE(secAlias); virObjectUnref(cfg); @@ -4827,6 +4826,13 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, if (ret < 0) orig_err = virSaveLastError(); + /* v2 proto has no confirm phase so we need to reset migration parameters + * here + */ + if (!v3proto && ret < 0) + qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, + NULL, NULL); + if (qemuMigrationRestoreDomainState(conn, vm)) { event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, -- 2.12.2
On Wed, Apr 05, 2017 at 15:09:59 +0200, Jiri Denemark wrote:
Finished qemuMigrationRun does not mean the migration itself finished (it might have just switched to post-copy mode). While resetting TLS parameters is probably OK at this point even if migration is still running, we want to consolidate the code which resets various migration parameters. Thus qemuMigrationResetTLS will be called from the Confirm phase (or at the end of the Perform phase in case of v2 protocol), when migration is either canceled or finished.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 68e72b37f..87506c73a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c
[...]
@@ -4827,6 +4826,13 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, if (ret < 0) orig_err = virSaveLastError();
+ /* v2 proto has no confirm phase so we need to reset migration parameters + * here + */ + if (!v3proto && ret < 0) + qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, + NULL, NULL);
At this point this is slightly non-obvious as you've stated previously that v2 does not support TLS. The refactor to qemuMigration puts the sense into this. ACK
Migration parameters are either reset by the main migration code path or from qemuProcessRecoverMigration* in case libvirtd is restarted during migration. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 87506c73a..392632566 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5560,9 +5560,6 @@ qemuMigrationCancel(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0 || (storage && !blockJobs)) goto endsyncjob; - ignore_value(qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_NONE, - NULL, NULL)); - if (!storage) { ret = 0; goto cleanup; -- 2.12.2
On Wed, Apr 05, 2017 at 15:10:00 +0200, Jiri Denemark wrote:
Migration parameters are either reset by the main migration code path or from qemuProcessRecoverMigration* in case libvirtd is restarted during migration.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 3 --- 1 file changed, 3 deletions(-)
ACK
This new API is supposed to reset all migration parameters to make sure future migrations won't accidentally use them. This patch makes the first step and moves qemuMigrationResetTLS call inside qemuMigrationReset. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 31 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 5 +++++ src/qemu/qemu_process.c | 4 ++-- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 392632566..739b595ac 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2836,9 +2836,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, return ret; stopjob: - ignore_value(qemuMigrationResetTLS(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_IN, - tlsAlias, secAlias)); + qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN); if (stopProcess) { unsigned int stopFlags = VIR_QEMU_PROCESS_STOP_MIGRATED; @@ -3216,8 +3214,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, qemuDomainEventQueue(driver, event); } - qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, - NULL, NULL); + qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) VIR_WARN("Failed to save status on vm %s", vm->def->name); @@ -4830,8 +4827,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, * here */ if (!v3proto && ret < 0) - qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, - NULL, NULL); + qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT); if (qemuMigrationRestoreDomainState(conn, vm)) { event = virDomainEventLifecycleNewFromObj(vm, @@ -5362,7 +5358,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_IN); } - qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, NULL, NULL); + qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN); qemuMigrationJobFinish(driver, vm); if (!virDomainObjIsActive(vm)) @@ -5875,3 +5871,22 @@ qemuMigrationCompressionDump(qemuMigrationCompressionPtr compression, return 0; } + + +/* + * qemuMigrationReset: + * + * Reset all migration parameters so that the next job which internally uses + * migration (save, managedsave, snapshots, dump) will not try to use them. + */ +void +qemuMigrationReset(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job) +{ + if (!virDomainObjIsActive(vm)) + return; + + if (qemuMigrationResetTLS(driver, vm, job, NULL, NULL) < 0) + return; +} diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 5248f399d..28eb55056 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -285,4 +285,9 @@ qemuMigrationResetTLS(virQEMUDriverPtr driver, char *in_tlsAlias, char *in_secAlias); +void +qemuMigrationReset(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job); + #endif /* __QEMU_MIGRATION_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 935993f16..18ff1e143 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2982,7 +2982,7 @@ qemuProcessRecoverMigrationIn(virQEMUDriverPtr driver, break; } - qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, NULL); + qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_NONE); return 0; } @@ -3077,7 +3077,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriverPtr driver, } } - qemuMigrationResetTLS(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, NULL); + qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_NONE); return 0; } -- 2.12.2
On Wed, Apr 05, 2017 at 15:10:01 +0200, Jiri Denemark wrote:
This new API is supposed to reset all migration parameters to make sure future migrations won't accidentally use them. This patch makes the first step and moves qemuMigrationResetTLS call inside qemuMigrationReset.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 31 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 5 +++++ src/qemu/qemu_process.c | 4 ++-- 3 files changed, 30 insertions(+), 10 deletions(-)
[...]
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 5248f399d..28eb55056 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -285,4 +285,9 @@ qemuMigrationResetTLS(virQEMUDriverPtr driver, char *in_tlsAlias, char *in_secAlias);
+void +qemuMigrationReset(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job); +
This does not conform to the prevailing coding style in the file.
#endif /* __QEMU_MIGRATION_H__ */
ACK with ^^
On Fri, Apr 07, 2017 at 10:46:55 +0200, Peter Krempa wrote:
On Wed, Apr 05, 2017 at 15:10:01 +0200, Jiri Denemark wrote:
This new API is supposed to reset all migration parameters to make sure future migrations won't accidentally use them. This patch makes the first step and moves qemuMigrationResetTLS call inside qemuMigrationReset.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 31 +++++++++++++++++++++++-------- src/qemu/qemu_migration.h | 5 +++++ src/qemu/qemu_process.c | 4 ++-- 3 files changed, 30 insertions(+), 10 deletions(-)
[...]
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 5248f399d..28eb55056 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -285,4 +285,9 @@ qemuMigrationResetTLS(virQEMUDriverPtr driver, char *in_tlsAlias, char *in_secAlias);
+void +qemuMigrationReset(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job); +
This does not conform to the prevailing coding style in the file.
It does since the style is pretty inconsistent there. Anyway, see patch 10/8 :-) Jirka
It's only called from qemuMigrationReset now so it doesn't need to be exported and {tls,sec}Alias are always NULL. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 37 +++++++++++++------------------------ src/qemu/qemu_migration.h | 7 ------- 2 files changed, 13 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 739b595ac..cc3e58c16 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2406,45 +2406,36 @@ qemuMigrationSetParams(virQEMUDriverPtr driver, * @driver: pointer to qemu driver * @vm: domain object * @asyncJob: migration job to join - * @tlsAlias: alias generated for TLS object (may be NULL) - * @secAlias: alias generated for a secinfo object (may be NULL) * * Deconstruct all the setup possibly done for TLS - delete the TLS and - * security objects, fre the secinfo, and reset the migration params to "". + * security objects, free the secinfo, and reset the migration params to "". * * Returns 0 on success, -1 on failure */ -int +static int qemuMigrationResetTLS(virQEMUDriverPtr driver, virDomainObjPtr vm, - qemuDomainAsyncJob asyncJob, - char *in_tlsAlias, - char *in_secAlias) + qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - char *tlsAlias = in_tlsAlias; - char *secAlias = in_secAlias; + char *tlsAlias = NULL; + char *secAlias = NULL; qemuMonitorMigrationParams migParams = { 0 }; int ret = -1; - /* If coming from a path that doesn't know whether it's been used or not, - * let's first check we need to do this. If the tls-creds doesn't exist - * or if they're set to "" then there's nothing to do since we never set - * anything up */ - if (!in_tlsAlias && qemuMigrationCheckTLSCreds(driver, vm, asyncJob) < 0) + if (qemuMigrationCheckTLSCreds(driver, vm, asyncJob) < 0) return -1; + /* If the tls-creds doesn't exist or if they're set to "" then there's + * nothing to do since we never set anything up */ if (!priv->migTLSAlias || !*priv->migTLSAlias) return 0; /* NB: If either or both fail to allocate memory we can still proceed * since the next time we migrate another deletion attempt will be * made after successfully generating the aliases. */ - if (!tlsAlias) - tlsAlias = qemuAliasTLSObjFromSrcAlias(QEMU_MIGRATION_TLS_ALIAS_BASE); - if (!secAlias) - secAlias = qemuDomainGetSecretAESAlias(QEMU_MIGRATION_TLS_ALIAS_BASE, - false); + tlsAlias = qemuAliasTLSObjFromSrcAlias(QEMU_MIGRATION_TLS_ALIAS_BASE); + secAlias = qemuDomainGetSecretAESAlias(QEMU_MIGRATION_TLS_ALIAS_BASE, false); qemuDomainDelTLSObjects(driver, vm, asyncJob, secAlias, tlsAlias); qemuDomainSecretInfoFree(&priv->migSecinfo); @@ -2457,10 +2448,8 @@ qemuMigrationResetTLS(virQEMUDriverPtr driver, ret = 0; cleanup: - if (!in_tlsAlias) - VIR_FREE(tlsAlias); - if (!in_secAlias) - VIR_FREE(secAlias); + VIR_FREE(tlsAlias); + VIR_FREE(secAlias); qemuMigrationParamsClear(&migParams); return ret; @@ -5887,6 +5876,6 @@ qemuMigrationReset(virQEMUDriverPtr driver, if (!virDomainObjIsActive(vm)) return; - if (qemuMigrationResetTLS(driver, vm, job, NULL, NULL) < 0) + if (qemuMigrationResetTLS(driver, vm, job) < 0) return; } diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 28eb55056..b392b8fff 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -278,13 +278,6 @@ int qemuMigrationRunIncoming(virQEMUDriverPtr driver, void qemuMigrationPostcopyFailed(virQEMUDriverPtr driver, virDomainObjPtr vm); -int -qemuMigrationResetTLS(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainAsyncJob asyncJob, - char *in_tlsAlias, - char *in_secAlias); - void qemuMigrationReset(virQEMUDriverPtr driver, virDomainObjPtr vm, -- 2.12.2
On Wed, Apr 05, 2017 at 15:10:02 +0200, Jiri Denemark wrote:
It's only called from qemuMigrationReset now so it doesn't need to be exported and {tls,sec}Alias are always NULL.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 37 +++++++++++++------------------------ src/qemu/qemu_migration.h | 7 ------- 2 files changed, 13 insertions(+), 31 deletions(-)
ACK
So far only QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY was reset, but only in a single code path leaving post-copy enabled in quite a few cases. https://bugzilla.redhat.com/show_bug.cgi?id=1425003 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index cc3e58c16..1feb320b8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5342,9 +5342,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver, */ if (inPostCopy) VIR_FREE(priv->job.completed); - - qemuMigrationSetPostCopy(driver, vm, false, - QEMU_ASYNC_JOB_MIGRATION_IN); } qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN); @@ -5873,9 +5870,16 @@ qemuMigrationReset(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob job) { + qemuMonitorMigrationCaps cap; + if (!virDomainObjIsActive(vm)) return; if (qemuMigrationResetTLS(driver, vm, job) < 0) return; + + for (cap = 0; cap < QEMU_MONITOR_MIGRATION_CAPS_LAST; cap++) { + if (qemuMigrationSetOption(driver, vm, cap, false, job) < 0) + return; + } } -- 2.12.2
On Wed, Apr 05, 2017 at 15:10:03 +0200, Jiri Denemark wrote:
So far only QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY was reset, but only in a single code path leaving post-copy enabled in quite a few cases.
https://bugzilla.redhat.com/show_bug.cgi?id=1425003
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index cc3e58c16..1feb320b8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5342,9 +5342,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver, */ if (inPostCopy) VIR_FREE(priv->job.completed); - - qemuMigrationSetPostCopy(driver, vm, false, - QEMU_ASYNC_JOB_MIGRATION_IN); }
qemuMigrationReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN); @@ -5873,9 +5870,16 @@ qemuMigrationReset(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob job) { + qemuMonitorMigrationCaps cap; + if (!virDomainObjIsActive(vm)) return;
if (qemuMigrationResetTLS(driver, vm, job) < 0) return; + + for (cap = 0; cap < QEMU_MONITOR_MIGRATION_CAPS_LAST; cap++) { + if (qemuMigrationSetOption(driver, vm, cap, false, job) < 0)
Well, this allways queries whether the capability is supported ...
+ return; + } }
ACK
https://bugzilla.redhat.com/show_bug.cgi?id=1439130 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1feb320b8..d8222fe3b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5871,15 +5871,22 @@ qemuMigrationReset(virQEMUDriverPtr driver, qemuDomainAsyncJob job) { qemuMonitorMigrationCaps cap; + virErrorPtr err = virSaveLastError(); if (!virDomainObjIsActive(vm)) - return; + goto cleanup; if (qemuMigrationResetTLS(driver, vm, job) < 0) - return; + goto cleanup; for (cap = 0; cap < QEMU_MONITOR_MIGRATION_CAPS_LAST; cap++) { if (qemuMigrationSetOption(driver, vm, cap, false, job) < 0) - return; + goto cleanup; + } + + cleanup: + if (err) { + virSetError(err); + virFreeError(err); } } -- 2.12.2
On Thu, Apr 06, 2017 at 09:57:48 +0200, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1439130
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
ACK
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.h | 318 ++++++++++++++++++++++++++-------------------- 1 file changed, 177 insertions(+), 141 deletions(-) diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 5248f399d..8afe80f85 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -115,11 +115,12 @@ qemuMigrationCompressionPtr qemuMigrationCompressionParse(virTypedParameterPtr params, int nparams, unsigned long flags); -int qemuMigrationCompressionDump(qemuMigrationCompressionPtr compression, - virTypedParameterPtr *params, - int *nparams, - int *maxparams, - unsigned long *flags); +int +qemuMigrationCompressionDump(qemuMigrationCompressionPtr compression, + virTypedParameterPtr *params, + int *nparams, + int *maxparams, + unsigned long *flags); void qemuMigrationParamsClear(qemuMonitorMigrationParamsPtr migParams); @@ -132,151 +133,186 @@ qemuMigrationParams(virTypedParameterPtr params, int nparams, unsigned long flags); -int qemuMigrationJobStart(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainAsyncJob job) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -void qemuMigrationJobSetPhase(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuMigrationJobPhase phase) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -void qemuMigrationJobStartPhase(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuMigrationJobPhase phase) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -void qemuMigrationJobContinue(virDomainObjPtr obj) - ATTRIBUTE_NONNULL(1); -bool qemuMigrationJobIsActive(virDomainObjPtr vm, - qemuDomainAsyncJob job) - ATTRIBUTE_NONNULL(1); -void qemuMigrationJobFinish(virQEMUDriverPtr driver, virDomainObjPtr obj) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - -int qemuMigrationSetOffline(virQEMUDriverPtr driver, - virDomainObjPtr vm); - -char *qemuMigrationBegin(virConnectPtr conn, - virDomainObjPtr vm, - const char *xmlin, - const char *dname, - char **cookieout, - int *cookieoutlen, - size_t nmigrate_disks, - const char **migrate_disks, - unsigned long flags); - -virDomainDefPtr qemuMigrationPrepareDef(virQEMUDriverPtr driver, - const char *dom_xml, - const char *dname, - char **origname); - -int qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, - virConnectPtr dconn, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - virStreamPtr st, - virDomainDefPtr *def, - const char *origname, - unsigned long flags); - -int qemuMigrationPrepareDirect(virQEMUDriverPtr driver, - virConnectPtr dconn, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - const char *uri_in, - char **uri_out, - virDomainDefPtr *def, - const char *origname, - const char *listenAddress, - size_t nmigrate_disks, - const char **migrate_disks, - int nbdPort, - qemuMigrationCompressionPtr compression, - unsigned long flags); - -int qemuMigrationPerform(virQEMUDriverPtr driver, - virConnectPtr conn, - virDomainObjPtr vm, - const char *xmlin, - const char *persist_xml, - const char *dconnuri, - const char *uri, - const char *graphicsuri, - const char *listenAddress, - size_t nmigrate_disks, - const char **migrate_disks, - int nbdPort, - qemuMigrationCompressionPtr compression, - qemuMonitorMigrationParamsPtr migParams, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - unsigned long flags, - const char *dname, - unsigned long resource, - bool v3proto); - -virDomainPtr qemuMigrationFinish(virQEMUDriverPtr driver, - virConnectPtr dconn, - virDomainObjPtr vm, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - unsigned long flags, - int retcode, - bool v3proto); - -int qemuMigrationConfirm(virConnectPtr conn, - virDomainObjPtr vm, - const char *cookiein, - int cookieinlen, - unsigned int flags, - int cancelled); - -bool qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, - bool remote, unsigned int flags); - -int qemuMigrationToFile(virQEMUDriverPtr driver, - virDomainObjPtr vm, - int fd, - const char *compressor, - qemuDomainAsyncJob asyncJob) +int +qemuMigrationJobStart(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob job) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -int qemuMigrationCancel(virQEMUDriverPtr driver, +void +qemuMigrationJobSetPhase(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuMigrationJobPhase phase) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +void +qemuMigrationJobStartPhase(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuMigrationJobPhase phase) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +void +qemuMigrationJobContinue(virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1); + +bool +qemuMigrationJobIsActive(virDomainObjPtr vm, + qemuDomainAsyncJob job) + ATTRIBUTE_NONNULL(1); + +void +qemuMigrationJobFinish(virQEMUDriverPtr driver, + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int +qemuMigrationSetOffline(virQEMUDriverPtr driver, virDomainObjPtr vm); -int qemuMigrationFetchJobStatus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainAsyncJob asyncJob, - qemuDomainJobInfoPtr jobInfo); +char * +qemuMigrationBegin(virConnectPtr conn, + virDomainObjPtr vm, + const char *xmlin, + const char *dname, + char **cookieout, + int *cookieoutlen, + size_t nmigrate_disks, + const char **migrate_disks, + unsigned long flags); -int qemuMigrationErrorInit(virQEMUDriverPtr driver); -void qemuMigrationErrorSave(virQEMUDriverPtr driver, - const char *name, - virErrorPtr err); -void qemuMigrationErrorReport(virQEMUDriverPtr driver, - const char *name); +virDomainDefPtr +qemuMigrationPrepareDef(virQEMUDriverPtr driver, + const char *dom_xml, + const char *dname, + char **origname); -int qemuMigrationCheckIncoming(virQEMUCapsPtr qemuCaps, - const char *migrateFrom); +int +qemuMigrationPrepareTunnel(virQEMUDriverPtr driver, + virConnectPtr dconn, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + virStreamPtr st, + virDomainDefPtr *def, + const char *origname, + unsigned long flags); -char *qemuMigrationIncomingURI(const char *migrateFrom, - int migrateFd); +int +qemuMigrationPrepareDirect(virQEMUDriverPtr driver, + virConnectPtr dconn, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + const char *uri_in, + char **uri_out, + virDomainDefPtr *def, + const char *origname, + const char *listenAddress, + size_t nmigrate_disks, + const char **migrate_disks, + int nbdPort, + qemuMigrationCompressionPtr compression, + unsigned long flags); -int qemuMigrationRunIncoming(virQEMUDriverPtr driver, - virDomainObjPtr vm, - const char *uri, - qemuDomainAsyncJob asyncJob); +int +qemuMigrationPerform(virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainObjPtr vm, + const char *xmlin, + const char *persist_xml, + const char *dconnuri, + const char *uri, + const char *graphicsuri, + const char *listenAddress, + size_t nmigrate_disks, + const char **migrate_disks, + int nbdPort, + qemuMigrationCompressionPtr compression, + qemuMonitorMigrationParamsPtr migParams, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + const char *dname, + unsigned long resource, + bool v3proto); -void qemuMigrationPostcopyFailed(virQEMUDriverPtr driver, - virDomainObjPtr vm); +virDomainPtr +qemuMigrationFinish(virQEMUDriverPtr driver, + virConnectPtr dconn, + virDomainObjPtr vm, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + int retcode, + bool v3proto); + +int +qemuMigrationConfirm(virConnectPtr conn, + virDomainObjPtr vm, + const char *cookiein, + int cookieinlen, + unsigned int flags, + int cancelled); + +bool +qemuMigrationIsAllowed(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool remote, + unsigned int flags); + +int +qemuMigrationToFile(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int fd, + const char *compressor, + qemuDomainAsyncJob asyncJob) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int +qemuMigrationCancel(virQEMUDriverPtr driver, + virDomainObjPtr vm); + +int +qemuMigrationFetchJobStatus(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuDomainJobInfoPtr jobInfo); + +int +qemuMigrationErrorInit(virQEMUDriverPtr driver); + +void +qemuMigrationErrorSave(virQEMUDriverPtr driver, + const char *name, + virErrorPtr err); + +void +qemuMigrationErrorReport(virQEMUDriverPtr driver, + const char *name); + +int +qemuMigrationCheckIncoming(virQEMUCapsPtr qemuCaps, + const char *migrateFrom); + +char * +qemuMigrationIncomingURI(const char *migrateFrom, + int migrateFd); + +int +qemuMigrationRunIncoming(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *uri, + qemuDomainAsyncJob asyncJob); + +void +qemuMigrationPostcopyFailed(virQEMUDriverPtr driver, + virDomainObjPtr vm); int qemuMigrationResetTLS(virQEMUDriverPtr driver, -- 2.12.2
On Fri, Apr 07, 2017 at 12:14:21 +0200, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.h | 318 ++++++++++++++++++++++++++-------------------- 1 file changed, 177 insertions(+), 141 deletions(-)
ACK
participants (2)
-
Jiri Denemark -
Peter Krempa