[libvirt] Re: [Qemu-devel] [PATCH 2/6] Introduce monitor 'wait' command (v2)
Anthony Liguori wrote:
Avi Kivity wrote:
> Anthony Liguori wrote:
>> The wait command will pause the monitor the command was issued in
>> until a new
>> event becomes available. Events are queued if there isn't a waiter
>> present.
>> The wait command completes after a single event is available.
>>
>
> How do you stop a wait if there are no pending events?
You mean, cancel a wait? You cannot. I thought about whether it was
a problem or not. I'm not sure.
A management agent might want to detach from guests, upgrade, restart,
and reattach.
You could introduce a wait-cancel command, but then you need a way to
identify which wait you want to cancel. I can't think of a simple way
to do that today.
This, as well as the queueing that's necessary with this model,
indicates (to me) that it is too complicated.
>> Today, we queue events indefinitely but in the future, I suspect
>> we'll drop
>> events that are older than a certain amount of time to avoid infinitely
>> allocating memory for long running VMs.
>>
>
> This queueing plug the race where an event happens immediately after
> a wait completes. But it could be avoided completely by having
> asynchronous notifications on a single monitor.
There are multiple things I think are being confused: asynchronous
completion of monitor commands, events, monitor multiplexing, and
non-human mode.
There can only be one command active at any given time on a Monitor
context. We can have many Monitor contexts. There is currently only
one Monitor context connected to a character device at a given time.
Don't think of 'migrate -d' as a command to perform migration, instead
it's a command to start migration.
I also object to exposing internal qemu implementation details like
monitor contexts to the user (by forcing them to have multiple
connections). If we can't have more than one monitor command, we need
to fix that.
I think what you want to see is something like this:
<input> tag=4: info cpus
<input> tag=5: info kvm
<output> tag=5,rc=0: kvm enabled
<output> tag=4,rc=0: eip = 0x0000000444
<ouput> rc=0,class=vm-state,name=start: vm started
To me, this is a combination of events, which is introduced by this
patch, a non-human monitor mode, and finally multiplexing multiple
monitors into a single session.
Right now, you can have the same functional thing by having three
monitors. In the first monitor you'd see:
(qemu) info cpus
eip = 0x000000444
(qemu)
In the second you'd see:
(qemu) info kvm
kvm enabled
(qemu)
In the third you'd see:
(qemu) wait
23423423.23423: vm-state: start: vm started
(qemu)
Even those the two info commands today are synchronous, there's
nothing requiring them to be (see migrate as an example). So I think
we're in agreement but you just want to jump ahead 6 months ;-)
Commands which are inherently synchronous should remain so. Commands
which are inherently async should be coded like that. It was a mistake
IMO to have 'migrate' be a synchronous command, it should have always
behaved as if -d is given.
Having tagged replies is a good idea, but IMO, introducing multiple
monitors will create a lot of subtle problems, several of which we've
already identified.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.