[Qemu-devel] Re: [PATCH] migration: Issue 'cont' only on successful incoming migration

[Luiz Capitulino <lcapitulino@redhat.com>]

On Sat, 24 Jul 2010 13:01:24 +0530
Amit Shah <amit.shah@redhat.com> wrote:

> On (Fri) Jul 23 2010 [15:08:18], Luiz Capitulino wrote:
> > > diff --git a/monitor.c b/monitor.c
> > > index 45fd482..d12a7b5 100644
> > > --- a/monitor.c
> > > +++ b/monitor.c
> > > @@ -1056,6 +1056,10 @@ static int do_cont(Monitor *mon, const QDict *qdict, QObject **ret_data)
> > >  {
> > >      struct bdrv_iterate_context context = { mon, 0 };
> > >  
> > > +    if (incoming_expected && !incoming_done) {
> > > +        autostart = 1;
> > 
> > Why do we need to set autostart? We should just fail if we're unable to run.
> > 
> > > +        return 1; /* Waiting for incoming migration */
> > 
> > You should return -1 and use qerror_report(), so that we have a meaningful
> > error in the user Monitor and QMP (otherwise we'll get UndefinedError).
> 
> That would mean old/existing libvirt will be confused on why guests
> wouldn't start even though it issued cont.

Yes, although delaying to start could cause a problem too and this is
also introducing an new error in QMP already.

I really would like to avoid adding weird semantics, specially in QMP where
cont will return an error but will put the VM to run later. We could fix this
there only, but then it will get complex w/o reason.

We should fix it properly right now, IMO.

> If it's not a problem for the libvirt folks, I can do that.

Laine, could you please check that?

> 
> > And incomding_done is not needed.
> 
> Yes, not in this version.
> 
> 		Amit
>