Refactors qemuMigrationDstFinish by moving some parts to a dedicated
function for easier introduction of postcopy resume code without
duplicating common parts of the Finish phase. The goal is to have the
following call graph:

    - qemuMigrationDstFinish
        - qemuMigrationDstFinishOffline
        - qemuMigrationDstFinishActive
            - qemuMigrationDstFinishFresh
            - qemuMigrationDstFinishResume

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

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 385bd91a6b..dcd7ff3597 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5839,6 +5839,32 @@ qemuMigrationDstComplete(virQEMUDriver *driver,
 }
 
 
+static virDomainPtr
+qemuMigrationDstFinishOffline(virQEMUDriver *driver,
+                              virConnectPtr dconn,
+                              virDomainObj *vm,
+                              qemuMigrationCookie *mig,
+                              char **cookieout,
+                              int *cookieoutlen)
+{
+    virDomainPtr dom = NULL;
+
+    if (qemuMigrationDstPersist(driver, vm, mig, false) < 0)
+        return NULL;
+
+    dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, -1);
+
+    if (dom &&
+        qemuMigrationCookieFormat(mig, driver, vm,
+                                  QEMU_MIGRATION_DESTINATION,
+                                  cookieout, cookieoutlen,
+                                  QEMU_MIGRATION_COOKIE_STATS) < 0)
+        VIR_WARN("Unable to encode migration cookie");
+
+    return dom;
+}
+
+
 /*
  * Perform Finish phase of a fresh (i.e., not recovery) migration of an active
  * domain.
@@ -6029,16 +6055,9 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
         goto error;
 
     if (flags & VIR_MIGRATE_OFFLINE) {
-        if (retcode == 0 &&
-            qemuMigrationDstPersist(driver, vm, mig, false) == 0) {
-            dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, -1);
-
-            if (dom &&
-                qemuMigrationCookieFormat(mig, driver, vm,
-                                  QEMU_MIGRATION_DESTINATION,
-                                  cookieout, cookieoutlen,
-                                  QEMU_MIGRATION_COOKIE_STATS) < 0)
-                VIR_WARN("Unable to encode migration cookie");
+        if (retcode == 0) {
+            dom = qemuMigrationDstFinishOffline(driver, dconn, vm, mig,
+                                                cookieout, cookieoutlen);
         }
 
         qemuMigrationJobFinish(vm);
-- 
2.35.1

On Tue, May 10, 2022 at 17:20:54 +0200, Jiri Denemark wrote:
> Refactors qemuMigrationDstFinish by moving some parts to a dedicated
> function for easier introduction of postcopy resume code without
> duplicating common parts of the Finish phase. The goal is to have the
> following call graph:
> 
>     - qemuMigrationDstFinish
>         - qemuMigrationDstFinishOffline
>         - qemuMigrationDstFinishActive
>             - qemuMigrationDstFinishFresh
>             - qemuMigrationDstFinishResume
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/qemu/qemu_migration.c | 39 +++++++++++++++++++++++++++++----------
>  1 file changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 385bd91a6b..dcd7ff3597 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -5839,6 +5839,32 @@ qemuMigrationDstComplete(virQEMUDriver *driver,
>  }
>  
>  
> +static virDomainPtr
> +qemuMigrationDstFinishOffline(virQEMUDriver *driver,
> +                              virConnectPtr dconn,
> +                              virDomainObj *vm,
> +                              qemuMigrationCookie *mig,
> +                              char **cookieout,
> +                              int *cookieoutlen)
> +{
> +    virDomainPtr dom = NULL;
> +
> +    if (qemuMigrationDstPersist(driver, vm, mig, false) < 0)
> +        return NULL;
> +
> +    dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, -1);
> +
> +    if (dom &&

Earlier I've commented about uselessness of this check. Other code
formats the cookie unconditionally.

> +        qemuMigrationCookieFormat(mig, driver, vm,
> +                                  QEMU_MIGRATION_DESTINATION,
> +                                  cookieout, cookieoutlen,
> +                                  QEMU_MIGRATION_COOKIE_STATS) < 0)
> +        VIR_WARN("Unable to encode migration cookie");
> +
> +    return dom;
> +}


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

On Wed, May 11, 2022 at 16:53:48 +0200, Peter Krempa wrote:
> On Tue, May 10, 2022 at 17:20:54 +0200, Jiri Denemark wrote:
> > Refactors qemuMigrationDstFinish by moving some parts to a dedicated
> > function for easier introduction of postcopy resume code without
> > duplicating common parts of the Finish phase. The goal is to have the
> > following call graph:
> > 
> >     - qemuMigrationDstFinish
> >         - qemuMigrationDstFinishOffline
> >         - qemuMigrationDstFinishActive
> >             - qemuMigrationDstFinishFresh
> >             - qemuMigrationDstFinishResume
> > 
> > Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> >  src/qemu/qemu_migration.c | 39 +++++++++++++++++++++++++++++----------
> >  1 file changed, 29 insertions(+), 10 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 385bd91a6b..dcd7ff3597 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -5839,6 +5839,32 @@ qemuMigrationDstComplete(virQEMUDriver *driver,
> >  }
> >  
> >  
> > +static virDomainPtr
> > +qemuMigrationDstFinishOffline(virQEMUDriver *driver,
> > +                              virConnectPtr dconn,
> > +                              virDomainObj *vm,
> > +                              qemuMigrationCookie *mig,
> > +                              char **cookieout,
> > +                              int *cookieoutlen)
> > +{
> > +    virDomainPtr dom = NULL;
> > +
> > +    if (qemuMigrationDstPersist(driver, vm, mig, false) < 0)
> > +        return NULL;
> > +
> > +    dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, -1);
> > +
> > +    if (dom &&
> 
> Earlier I've commented about uselessness of this check. Other code
> formats the cookie unconditionally.

That's because the cookie is formatted before calling virGetDomain
there. We could swap it here too, or drop the check, but we can as well
keep it as is and avoid formatting the cookie when virGetDomain fails
(even though the failure is mostly theoretical).

Jirka