[Qemu-devel] Re: [PATCH 3/5] QMP: Introduce MIGRATION events

[Luiz Capitulino]

On Wed, 26 May 2010 11:55:31 -0500
Anthony Liguori <address@hidden> wrote:

> On 05/26/2010 10:15 AM, Daniel P. Berrange wrote:
> > On Wed, May 26, 2010 at 09:54:22AM -0500, Anthony Liguori wrote:
> >    
> >> On 05/26/2010 05:33 AM, Daniel P. Berrange wrote:
> >>      
> >>>>> I'm not sure why you would need a notification of when migration
> >>>>> starts (since you know when you've started migration).
> >>>>>
> >>>>>            
> >>>> But you don't know if the other end "knows" that it has also started.
> >>>>
> >>>> started is needed only in incoming part, because .... we don't have a
> >>>> monitor to ask if migration has started.
> >>>>
> >>>>          
> >>> If we ever want to get closer to allowing multiple monitors, or allowing
> >>> apps to issue QMP commands directly via libvirt, then we still need the
> >>> 'migration started' event on the source, because something else can
> >>> have issued the 'migrate' command without the mgmt app knowing.
> >>>
> >>>        
> >> Migration started doesn't help multiple monitors.  You need locking of
> >> some sort.
> >>
> >> Part of the problem is the QMP migrate command is implemented as a
> >> synchronous command.  It really ought to be an asynchronous command.
> >> That tells you when the migration has actually completed without polling.
> >>      
> > Handling asynchronous commands is alot more complicated and error
> > prone for client apps, than providing a asynchronous event notification
> > of the lifecycle stages. If you want to also query status while waiting
> > for the completion, it means you can have to deal with overlapping
> > command  execute+return pairs within a single monitor connection.
> > AFAICT this requires a change to QMP to require a unique ID to be
> > sent with the {'execute'..} command and be sent back with the later
> > corresponding {'return'...} data,  so you can actually correlate
> > reliably.
> >    
> 
> That's exactly how the protocol is designed.  That was one of the major 
> improvements of QMP over the human monior.

 Yes and it already has 'id' support:

{ "execute": "cont", "id": "luiz" }
{"timestamp": {"seconds": 1274966635, "microseconds": 776813}, "event": 
"RESUME"}
{"return": {}, "id": "luiz"}

 But it doesn't detect duplicates, this is something I think it's up
to the client to do, do you agree?

> This is how the info balloon command works, BTW.

 I won't remember the details now, but that interface has some issues and it
has to be reviewed.

> Since there's a clear correlation between the request and the result of 
> the request, an asynchronous command is what makes the most sense.  It 
> eliminates the problem of how to pass QErrors via an event which is one 
> of the problems with the current event proposal.

 Not exactly, this is a problem with QError not the event proposal. We'll
have the same issue if we decide to include errno in the migrate errors and
the problem still exists with the BLOCK_IO_ERROR event.

 That said, I do agree that migrate should be asynchronous. This yet another
thing we may want to fix before 0.13.

[...]

> >> For tcp: and unix:, a CONNECTED event absolutely makes sense (every
> >> socket server should emit a CONNECTED event).  Unfortunately, after
> >> CONNECTED you lose the monitor until migration is complete.  If
> >> something bad happens, you have to exit qemu so once the monitor
> >> returns, migration has completed successfully.
> >>
> >> If we introduce live incoming migration, we'll need to rethink things.
> >> I would actually suggest that we deprecate the incoming command if we do
> >> that and make incoming migration a monitor command.  I would think it
> >> should have the same semantics as migrate (as an asynchronous command).
> >> A CONNECTED event still makes sense for tcp and unix protocols but I
> >> don't think events make sense for start stop vs. an asynchronous command
> >> completion.
> >>      
> > Do you actually mean 'deprecate -incoming arg' here ?
> >    
> 
> Yes.  And by deprecate, I really mean that -incoming just becomes 
> syntactic sugar for executing a monitor command immediately.

 But we can't change -incoming itself, since our command-line is supposed
to be stable, right?

 Also, Juan has said that replacing that arg with a monitor command
doesn't work, as qemu would have to be started in paused monitor for this
to work.

 So, what about introducing a -incoming-monitor command, which puts qemu
in the right state for migration, but requires a migrate_incoming command
to actually start migration?