Re: [libvirt] [PATCH v2 16/73] qemu: Reset all migration parameters

[Ján Tomko]

On Wed, Apr 11, 2018 at 04:41:06PM +0200, Jiri Denemark wrote:

Restore the original values of all migration parameters we store in
qemuDomainJobObj instead of explicitly resting only a limited set of
them.

The result is not strictly equivalent to the previous code wrt reseting
TLS state because the previous code would only reset it if we changed it
before while the new code will reset it always if QEMU supports TLS
migration. This is not a problem for the parameters themselves, but it
can cause spurious errors about missing TLS objects being logged at the
end of non-TLS migration. This issue will be fixed ~50 patches later.


It better be.



Signed-off-by: Jiri Denemark 
---
src/qemu/qemu_migration.c| 20 +
src/qemu/qemu_migration_params.c | 48 +++-
src/qemu/qemu_migration_params.h |  3 +-
src/qemu/qemu_process.c  |  4 +--
4 files changed, 34 insertions(+), 41 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 16/73] qemu: Reset all migration parameters

[Jiri Denemark]

Restore the original values of all migration parameters we store in
qemuDomainJobObj instead of explicitly resting only a limited set of
them.

The result is not strictly equivalent to the previous code wrt reseting
TLS state because the previous code would only reset it if we changed it
before while the new code will reset it always if QEMU supports TLS
migration. This is not a problem for the parameters themselves, but it
can cause spurious errors about missing TLS objects being logged at the
end of non-TLS migration. This issue will be fixed ~50 patches later.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c| 20 +
 src/qemu/qemu_migration_params.c | 48 +++-
 src/qemu/qemu_migration_params.h |  3 +-
 src/qemu/qemu_process.c  |  4 +--
 4 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 96ca5593cf..72fcae77f4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1834,7 +1834,8 @@ qemuMigrationSrcCleanup(virDomainObjPtr vm,
 VIR_WARN("Migration of domain %s finished but we don't know if the"
  " domain was successfully started on destination or not",
  vm->def->name);
-qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT);
+qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
+ priv->job.migParams);
 /* clear the job and let higher levels decide what to do */
 qemuDomainObjDiscardAsyncJob(driver, vm);
 break;
@@ -2593,7 +2594,8 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
 return ret;
 
  stopjob:
-qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN);
+qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
+ priv->job.migParams);
 
 if (stopProcess) {
 unsigned int stopFlags = VIR_QEMU_PROCESS_STOP_MIGRATED;
@@ -2969,7 +2971,8 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver,
 qemuDomainEventQueue(driver, event);
 }
 
-qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT);
+qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
+ priv->job.migParams);
 
 if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, 
driver->caps) < 0)
 VIR_WARN("Failed to save status on vm %s", vm->def->name);
@@ -4581,6 +4584,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
 int ret = -1;
 virErrorPtr orig_err = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+qemuDomainObjPrivatePtr priv = vm->privateData;
 
 if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
 goto cleanup;
@@ -4641,7 +4645,8 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
  * here
  */
 if (!v3proto && ret < 0)
-qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT);
+qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
+ priv->job.migParams);
 
 if (qemuMigrationSrcRestoreDomainState(driver, vm)) {
 event = virDomainEventLifecycleNewFromObj(vm,
@@ -4691,6 +4696,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
  unsigned long flags,
  unsigned long resource)
 {
+qemuDomainObjPrivatePtr priv = vm->privateData;
 virObjectEventPtr event = NULL;
 int ret = -1;
 
@@ -4731,7 +4737,8 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
 
  endjob:
 if (ret < 0) {
-qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT);
+qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
+ priv->job.migParams);
 qemuMigrationJobFinish(driver, vm);
 } else {
 qemuMigrationJobContinue(vm);
@@ -5187,7 +5194,8 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
 VIR_FREE(priv->job.completed);
 }
 
-qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN);
+qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
+ priv->job.migParams);
 
 qemuMigrationJobFinish(driver, vm);
 if (!virDomainObjIsActive(vm))
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 465132fd9c..72836ba9fa 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -378,30 +378,19 @@ qemuMigrationParamsSetCompression(virQEMUDriverPtr driver,
  *
  * Deconstruct all the setup possibly done for TLS - delete the TLS and
  * security objects, free the secinfo, and reset the migration params to "".
- *
- * Returns 0 on success, -1 on failure
  */
-static int
+static void