Re: [Qemu-devel] [PATCH 2/6] Introduce monitor 'wait' command (v2)

[Jan Kiszka]

Avi Kivity wrote:
> 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.

It should be feasible to handle some key sequence like ^C while the
monitor is suspended (ie. blocked on things like event-wait or migrate).
I thought about such a feature while reworking the involved parts but
postponed it as I saw no urgent need for it.

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

We do have multiple monitors already, try '-monitor X -serial mon:Y', or
also attach via gdb and issue 'monitor whatever'. And we should continue
to design the monitor interface for this (which means here: event
notification should be configured per monitor session).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux