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

[Peter Krempa]

On Fri, Nov 13, 2015 at 19:52:20 +0100, Jiri Denemark wrote:
> 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

I thought so :)

> 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.

Actually I don't care that much, do whichever you like.

ACK

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151119/8b9b6b28/attachment-0001.sig>