[Qemu-devel] Re: [PATCH 10/15] virtio-serial: Add QMP events for failed port/device add

[Amit Shah]

On (Thu) Mar 25 2010 [15:55:41], Luiz Capitulino wrote:
> On Wed, 24 Mar 2010 20:19:28 +0530
> Amit Shah <address@hidden> wrote:
> 
> > When adding a port or a device to the guest fails, management software
> > might be interested in knowing and then cleaning up the host-side of the
> > port. Introduce QMP events to signal such errors.
> > 
> > Signed-off-by: Amit Shah <address@hidden>
> > CC: Luiz Capitulino <address@hidden>
> > ---
> >  QMP/qmp-events.txt     |   48 
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/virtio-serial-bus.c |   15 +++++++++++++++
> >  monitor.c              |    3 +++
> >  monitor.h              |    1 +
> >  4 files changed, 67 insertions(+), 0 deletions(-)
> > 
> > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> > index a94e9b4..f13cf45 100644
> > --- a/QMP/qmp-events.txt
> > +++ b/QMP/qmp-events.txt
> > @@ -188,3 +188,51 @@ Example:
> >  
> >  Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
> >  followed respectively by the RESET, SHUTDOWN, or STOP events.
> > +
> > +VIRTIO_SERIAL
> > +-------------
> 
>  It should be VIRTIO_SERIAL_ADD.

What about other events that VIRTIO_SERIAL generates? Should they have a
different event by themselves? Or should they ride on top of
VIRTIO_SERIAL and mention different 'operations' that caused the event?

> > +Emitted when errors occur in guest port add or guest device add.
> > +
> > +Data:
> > +
> > +- "device": The virtio-serial device that triggered the event {json-string}
> > +      This is the name given to the bus on the command line:
> > +        -device virtio-serial,id="foo"
> > +      here, the device name is "foo"
> > +
> > +- "port": The port number that triggered the event {json-number}
> > +      This is the number given to the port on the command line:
> > +        -device virtserialport,nr=2
> > +      here, the port number is 2. If not mentioned on the command
> > +      line, the number is auto-assigned.
> 
>  We use (json-number) instead of {json-number}.

Fixed.

> > +
> > +- "result": The result of the operation {json-string}
> > +      This is one of the following:
> > +         "pass", "fail"
> 
>  "result" could be a boolean "success".

OK; success/fail? Also, by boolean, do you mean the data type? How is
that represented?

(Note: I put the port number as '%ld' instead of '%u' since %u isn't
parsed..)

> > +- "operation": The operation that triggered the event {json-sring}
> > +      This is one of the following:
> > +         "port_add", "device_add"
> 
>  You can drop the '_add', as this information is in the event name.

The answer to the first question above will answer this too.

> > +Example:
> > +
> > +Port 0 add failure in the guest:
> > +
> > +{ "timestamp": {"seconds": 1269438649, "microseconds": 851170},
> > +  "event": "VIRTIO_SERIAL",
> > +  "data": {
> > +            "device": "virtio-serial-bus.0",
> > +            "port": 0,
> > +            "result": "fail",
> > +            "operation": "port_add" } }
> 
>  If you look at the other events you will see that I put the event
> first and the timestamp later, I know this is not how the event is
> going to be on the wire but improves the readability of this file
> (and the spec says that clients should not assume the ordering of
> dicts or lists).

OK; I'll do that.

>  Implementation looks ok.

OK, thanks.

                Amit