On Wed, 26 May 2010 11:55:31 -0500
Anthony Liguori <anth...@codemonkey.ws> 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?
