Re: [libvirt PATCH 69/80] qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Finish phase

2022-05-19 Thread Jiri Denemark 
On Thu, May 12, 2022 at 17:20:34 +0200, Peter Krempa wrote:
> On Tue, May 10, 2022 at 17:21:30 +0200, Jiri Denemark wrote:
> > Everything was already done in the normal Finish phase and vCPUs are
> > running. We just need to wait for all remaining data to be transferred.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/qemu/qemu_migration.c | 46 ++-
> >  1 file changed, 40 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index a8481f7515..430dfb1abb 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -6600,6 +6600,22 @@ qemuMigrationDstFinishFresh(virQEMUDriver *driver,
> >  }
> >  
> >  
> > +static int
> > +qemuMigrationDstFinishResume(virQEMUDriver *driver,
> > + virDomainObj *vm)
> > +{
> > +VIR_DEBUG("vm=%p", vm);
> > +
> > +if (qemuMigrationDstWaitForCompletion(driver, vm,
> > +  VIR_ASYNC_JOB_MIGRATION_IN,
> > +  false) < 0) {
> > +return -1;
> > +}
> 
> As I mentioned in another reply, IMO it would be useful to allow
> adoption of a unattended running migration precisely for this case, so
> that mgmt apps don't have to encode more logic to wait for events if
> migration was actually running fine.

I think event processing in mgmt apps is inevitable anyway as the
migration may finish before an app even tries to recover the migration
in case only libvirt side of migration was broken. And it may still be
broken to make recovery impossible while migration is progressing just
fine (esp. if migration streams use separate network). Even plain
post-copy which never fails requires event processing since the
management has to explicitly switch migration to post-copy mode. Also
events are often the only way to get statistics about the completed
migration.

That said, I think it would be nice to have this functionality anyway so
I'll try to look at it as a follow up series.

Jirka

Re: [libvirt PATCH 69/80] qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Finish phase

2022-05-12 Thread Peter Krempa 
On Tue, May 10, 2022 at 17:21:30 +0200, Jiri Denemark wrote:
> Everything was already done in the normal Finish phase and vCPUs are
> running. We just need to wait for all remaining data to be transferred.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_migration.c | 46 ++-
>  1 file changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index a8481f7515..430dfb1abb 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -6600,6 +6600,22 @@ qemuMigrationDstFinishFresh(virQEMUDriver *driver,
>  }
>  
>  
> +static int
> +qemuMigrationDstFinishResume(virQEMUDriver *driver,
> + virDomainObj *vm)
> +{
> +VIR_DEBUG("vm=%p", vm);
> +
> +if (qemuMigrationDstWaitForCompletion(driver, vm,
> +  VIR_ASYNC_JOB_MIGRATION_IN,
> +  false) < 0) {
> +return -1;
> +}

As I mentioned in another reply, IMO it would be useful to allow
adoption of a unattended running migration precisely for this case, so
that mgmt apps don't have to encode more logic to wait for events if
migration was actually running fine.

> +
> +return 0;
> +}
> +
> +
>  static virDomainPtr
>  qemuMigrationDstFinishActive(virQEMUDriver *driver,
>   virConnectPtr dconn,
> @@ -6647,8 +6663,14 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver,
>  goto error;
>  }
>  
> -rc = qemuMigrationDstFinishFresh(driver, vm, mig, flags, v3proto,
> - timeReceived, , );
> +if (flags & VIR_MIGRATE_POSTCOPY_RESUME) {
> +rc = qemuMigrationDstFinishResume(driver, vm);
> +inPostCopy = true;
> +} else {
> +rc = qemuMigrationDstFinishFresh(driver, vm, mig, flags, v3proto,
> + timeReceived, , );
> +}
> +
>  if (rc < 0 ||
>  !(dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, 
> vm->def->id)))
>  goto error;
> @@ -6719,6 +6741,8 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
>  qemuDomainObjPrivate *priv = vm->privateData;
>  unsigned short port;
>  unsigned long long timeReceived = 0;
> +int phase = v3proto ? QEMU_MIGRATION_PHASE_FINISH3
> +: QEMU_MIGRATION_PHASE_FINISH2;


Avoid use of ?

>  
>  VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
>"cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d",

Reviewed-by: Peter Krempa

[libvirt PATCH 69/80] qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Finish phase

2022-05-10 Thread Jiri Denemark 
Everything was already done in the normal Finish phase and vCPUs are
running. We just need to wait for all remaining data to be transferred.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 46 ++-
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a8481f7515..430dfb1abb 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -6600,6 +6600,22 @@ qemuMigrationDstFinishFresh(virQEMUDriver *driver,
 }
 
 
+static int
+qemuMigrationDstFinishResume(virQEMUDriver *driver,
+ virDomainObj *vm)
+{
+VIR_DEBUG("vm=%p", vm);
+
+if (qemuMigrationDstWaitForCompletion(driver, vm,
+  VIR_ASYNC_JOB_MIGRATION_IN,
+  false) < 0) {
+return -1;
+}
+
+return 0;
+}
+
+
 static virDomainPtr
 qemuMigrationDstFinishActive(virQEMUDriver *driver,
  virConnectPtr dconn,
@@ -6647,8 +6663,14 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver,
 goto error;
 }
 
-rc = qemuMigrationDstFinishFresh(driver, vm, mig, flags, v3proto,
- timeReceived, , );
+if (flags & VIR_MIGRATE_POSTCOPY_RESUME) {
+rc = qemuMigrationDstFinishResume(driver, vm);
+inPostCopy = true;
+} else {
+rc = qemuMigrationDstFinishFresh(driver, vm, mig, flags, v3proto,
+ timeReceived, , );
+}
+
 if (rc < 0 ||
 !(dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, 
vm->def->id)))
 goto error;
@@ -6719,6 +6741,8 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
 qemuDomainObjPrivate *priv = vm->privateData;
 unsigned short port;
 unsigned long long timeReceived = 0;
+int phase = v3proto ? QEMU_MIGRATION_PHASE_FINISH3
+: QEMU_MIGRATION_PHASE_FINISH2;
 
 VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
   "cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d",
@@ -6733,14 +6757,24 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
 goto cleanup;
 }
 
+if (flags & VIR_MIGRATE_POSTCOPY_RESUME) {
+if (!qemuMigrationAnyCanResume(vm, VIR_ASYNC_JOB_MIGRATION_IN, flags,
+   QEMU_MIGRATION_PHASE_PREPARE_RESUME))
+goto cleanup;
+phase = QEMU_MIGRATION_PHASE_FINISH_RESUME;
+}
 ignore_value(virTimeMillisNow());
 
-if (qemuMigrationJobStartPhase(vm,
-   v3proto ? QEMU_MIGRATION_PHASE_FINISH3
-   : QEMU_MIGRATION_PHASE_FINISH2) < 0)
+if (qemuMigrationJobStartPhase(vm, phase) < 0)
 goto cleanup;
 
-qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup);
+if (flags & VIR_MIGRATE_POSTCOPY_RESUME) {
+virCloseCallbacksUnset(driver->closeCallbacks, vm,
+   qemuMigrationAnyConnectionClosed);
+qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob);
+} else {
+qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup);
+}
 g_clear_pointer(>job.completed, virDomainJobDataFree);
 
 cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK |
-- 
2.35.1