[libvirt PATCH 26/80] qemu: Separate success and failure path in qemuMigrationDstFinish

Jiri Denemark posted 80 patches 3 years, 9 months ago
There is a newer version of this series
[libvirt PATCH 26/80] qemu: Separate success and failure path in qemuMigrationDstFinish
Posted by Jiri Denemark 3 years, 9 months ago
Most of the code in "endjob" label is executed only on failure. Let's
duplicate the rest so that the label can be used only in error path
making the success path easier to follow and refactor.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_migration.c | 45 ++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index cb17cbd189..0c6d2c041b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5854,7 +5854,9 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
                                   QEMU_MIGRATION_COOKIE_STATS) < 0)
                 VIR_WARN("Unable to encode migration cookie");
         }
-        goto endjob;
+
+        qemuMigrationJobFinish(vm);
+        goto cleanup;
     }
 
     if (retcode != 0) {
@@ -6035,12 +6037,31 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
     if (inPostCopy)
         g_clear_pointer(&priv->job.completed, virDomainJobDataFree);
 
+    qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN,
+                             jobPriv->migParams, priv->job.apiFlags);
+
     dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id);
 
+    qemuMigrationJobFinish(vm);
+
+ cleanup:
+    g_clear_pointer(&jobData, virDomainJobDataFree);
+    virPortAllocatorRelease(port);
+    if (priv->mon)
+        qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL);
+    VIR_FREE(priv->origname);
+    virDomainObjEndAPI(&vm);
+    virErrorRestore(&orig_err);
+
+    /* Set a special error if Finish is expected to return NULL as a result of
+     * successful call with retcode != 0
+     */
+    if (retcode != 0 && !dom && virGetLastErrorCode() == VIR_ERR_OK)
+        virReportError(VIR_ERR_MIGRATE_FINISH_OK, NULL);
+    return dom;
+
  endjob:
-    if (!dom &&
-        !(flags & VIR_MIGRATE_OFFLINE) &&
-        virDomainObjIsActive(vm)) {
+    if (virDomainObjIsActive(vm)) {
         if (doKill) {
             qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
                             VIR_ASYNC_JOB_MIGRATION_IN,
@@ -6069,21 +6090,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
     if (!virDomainObjIsActive(vm))
         qemuDomainRemoveInactive(driver, vm);
 
- cleanup:
-    g_clear_pointer(&jobData, virDomainJobDataFree);
-    virPortAllocatorRelease(port);
-    if (priv->mon)
-        qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL);
-    VIR_FREE(priv->origname);
-    virDomainObjEndAPI(&vm);
-    virErrorRestore(&orig_err);
-
-    /* Set a special error if Finish is expected to return NULL as a result of
-     * successful call with retcode != 0
-     */
-    if (retcode != 0 && !dom && virGetLastErrorCode() == VIR_ERR_OK)
-        virReportError(VIR_ERR_MIGRATE_FINISH_OK, NULL);
-    return dom;
+    goto cleanup;
 }
 
 
-- 
2.35.1
Re: [libvirt PATCH 26/80] qemu: Separate success and failure path in qemuMigrationDstFinish
Posted by Peter Krempa 3 years, 9 months ago
On Tue, May 10, 2022 at 17:20:47 +0200, Jiri Denemark wrote:
> Most of the code in "endjob" label is executed only on failure. Let's
> duplicate the rest so that the label can be used only in error path
> making the success path easier to follow and refactor.
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/qemu/qemu_migration.c | 45 ++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 19 deletions(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>