[Qemu-devel] Re: [libvirt] Re: [PATCH 2/3] Introduce monitor 'wait' command

[Anthony Liguori <aliguori@us.ibm.com>]

Gerd Hoffmann wrote:
> On 04/08/09 16:33, Daniel P. Berrange wrote:
>> On Wed, Apr 08, 2009 at 09:16:43AM -0500, 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.
>>>
>>> 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.
>>>
>>> To make use of the new notification mechanism, this patch introduces a
>>> qemu_notify_event() API.  This API takes three parameters: a class 
>>> which is
>>> meant to classify the type of event being generated, a name which is 
>>> meant to
>>> distinguish which event in the class that has been generated, and a 
>>> details
>>> parameters which is meant to allow events to send arbitrary data 
>>> with a given
>>> event.
>>
>> Perhaps we should have the ability to turn on/off events, via a 
>> 'notify EVENT'
>> command, and a way turn off the prompt on the channel used for receiving
>> events.
>
> That would nicely solve the "queue events indefinitely" issue.  By 
> default no events are generated.  Apps which want receive them (and 
> thus receive them) can enable them as needed.

It doesn't.  When an app enables events, we would start queuing them, 
but if it didn't consume them in a timely manner (or at all), we would 
start leaking memory badly.

We want to be robust even in the face of poorly written management 
apps/scripts so we need some expiration function too.

>> And then in the 2nd monitor channel, a single 'wait' command would turn
>> off the monitor prompt and make the channel dedicated for just events,
>> one per line
>>
>>    (qemu) wait
>>    rtc-change UTC+0100
>>    vnc-client connect 192.46.12.4:9353
>>    vnc-client disconnect 192.46.12.4:9353
>>    vnc-client connect 192.46.12.2:9353
>>    vnc-client disconnect 192.46.12.2:9353
>
> IMHO this is more useful than having "wait" just get one event.  
> You'll need a dedicated monitor channel for events anyway, so with 
> one-event-per-wait the management app would have to issue wait in a loop.

There two issues with this syntax.  The first is that 'notify EVENT' 
logically acts on the global event set.  That's not necessarily what you 
want.  For instance, libvirt may attach to a monitor and issue a 'wait 
"vm-state vnc-events"' and I may have some wiz-bang app that wants to 
connect on another monitor, and issue a 'wait "watchdog-events"'.  My 
super-deluxe app may sit watching for watchdog events to do some sort of 
fancy RAS stuff or something like that.

The 'notify EVENT' model makes this difficult unless you have notify act 
only on the current monitor session.  Monitor "sessions" are ill-defined 
though b/c of things like tcp:// reconnects so I wouldn't want to do that.

The second issue is that there is no clear way to deliminate events 
other than a new line.  If we wanted to send data along with events, we 
really want to be able to span multiple lines.  Especially if we want 
that data to be in the same format as some of the existing monitor 
commands.  You could get around this by introducing a new deliminator 
like '.' but everyone can already handle '(qemu)'.

That said, I can see a few advantages in the above model.  Obviously, 
the ability to read a large chunk of output and then parse multiple 
events in a single round trip is a big positive.  You could get that 
with my syntax by sending multiple wait commands at once but that's a 
bit hackish.

Also, I think where the above really shines is if you're a human user 
and you want to see all the events while debugging.  You really don't 
want to keep typing wait in the monitor.

So as a compromise, I think we need to introduce a mode where we can do 
the above but I'd like to wait until after the first round of these go 
in.  I'm thinking along the lines of 'wait N' where N can be -1 to 
signify an unlimited number of events or something like that.

> BTW: "wait" is quite generic.  Maybe we should name the commands 
> notify-*, i.e. have

Good point, I like wait_event personally.

Regards,

Anthony Liguori

>   notify-enable $class
>   notify-disable $class
>   notify-getevents
>
> cheers,
>   Gerd


-- 
Regards,

Anthony Liguori