[libvirt] [PATCH 08/24] qemu: Use -incoming defer for migrations

[Jiri Denemark]

On Fri, Nov 13, 2015 at 11:25:14 +0100, Peter Krempa wrote:
> On Thu, Nov 12, 2015 at 19:37:10 +0100, Jiri Denemark wrote:
> > Traditionally, we pass incoming migration URI on QEMU command line,
> > which has some drawbacks. Depending on the URI QEMU may initialize its
> > migration state immediately without giving us a chance to set any
> > additional migration parameters (this applies mainly for fd: URIs). For
> > some URIs the monitor may be completely blocked from the beginning until
> > migration is finished, which means we may be stuck in qmp_capabilities
> > command without being able to send any QMP commands.
> > 
> > QEMU solved this by introducing "defer" parameter for -incoming command
> > line option. This will tell QEMU to prepare for an incoming migration
> > while the actual incoming URI is sent using migrate-incoming QMP
> > command. Before calling this command we can normally talk to the
> > monitor and even set any migration parameters which will be honored by
> > the incoming migration.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >  src/qemu/qemu_capabilities.c                 |  3 +++
> >  src/qemu/qemu_capabilities.h                 |  3 +++
> >  src/qemu/qemu_migration.c                    | 36 ++++++++++++++++++++++++++++
> >  src/qemu/qemu_migration.h                    |  5 ++++
> >  src/qemu/qemu_process.c                      | 12 ++++++++++
> >  src/qemu/qemu_process.h                      |  1 +
> >  tests/qemucapabilitiesdata/caps_2.4.0-1.caps |  1 +
> >  7 files changed, 61 insertions(+)
> > 
> 
> [...]
> 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 4d5b966..0c4c94a 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -2951,6 +2951,42 @@ qemuMigrationIncomingURI(const char *migrateFrom,
> >  }
> >  
> >  
> > +int
> > +qemuMigrationRunIncoming(virQEMUDriverPtr driver,
> > +                         virDomainObjPtr vm,
> > +                         const char *uri,
> > +                         qemuDomainAsyncJob asyncJob)
> 
> Is there a reason why this doesn't take the new fancy struct containing
> all the data and doing the decisions internally ... 
> 
...
> > @@ -4929,6 +4936,11 @@ int qemuProcessStart(virConnectPtr conn,
> >      if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0)
> >          goto error;
> >  
> > +    if (incoming &&
> > +        incoming->deferredURI &&
> 
> ... rather than having to check here ...
> 
> > +        qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0)
> 
> And extract the params?

Yeah, there are two reasons. First, I wrote this commit before the one
which introduces qemuProcessIncomingDef :-) Second, I wanted to make the
decision in the caller so that it is more obvious that
qemuMigrationRunIncoming is not going to be run at this point in some
cases because incoming migration was already initiated by
qemuProcessLaunch (since QEMU does not support -incoming defer).

None of the reasons is particularly strong though and if you think I
should still change after knowing the reasons, I'll happily do that.

Jirka