Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole

[Alon Levy]

----- "Anthony Liguori" <address@hidden> wrote:

> On 08/02/2010 03:33 AM, Alon Levy wrote:
> > Hi,
> >
> >   This patch adds three CHR_IOCTLs and uses them in virtserial
> devices, to be used
> > by a chardev backend, such as a spice vm channel (spice is a vdi
> solution).
> >
> >   Basically virtio-serial provides three driver initiated events for
> guest open of
> > a device, guest close, and guest ready (driver port init complete)
> that before this
> > patch are not exposed to the chardev backend.
> >
> >   With the spicevmc backend this is used like this:
> > qemu -chardev spicevmc,id=vdiport,name=vdiport -device
> virtserialport,chardev=vdiport,name=com.redhat.spice.0
> >
> >   I'd appreciate any feedback if this seems the right way to
> accomplish this, and
> > for the numbers I grabbed.
> >    
> 
> I really hate to add connection semantics via IOCTLs.  I would rather
> we 
> add them as first class semantics to the char device layer.  This
> would 
> allow us to use char devices for VNC, for instance.
> 

Ok, that's actually what I wanted to do at first, how about:

diff --git a/qemu-char.h b/qemu-char.h
index 1df53ae..22973cd 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -16,6 +16,8 @@
 #define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
 #define CHR_EVENT_CLOSED  5 /* connection closed */
 
+#define CHR_DEVICE_EVENT_OPENED 0
+#define CHR_DEVICE_EVENT_CLOSED 1
 
 #define CHR_IOCTL_SERIAL_SET_PARAMS   1
 typedef struct {
@@ -59,6 +61,7 @@ struct CharDriverState {
     int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
     void (*chr_update_read_handler)(struct CharDriverState *s);
     int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
+    int (*chr_device_event)(struct CharDriverState *s, int cmd, void *arg);
     int (*get_msgfd)(struct CharDriverState *s);
     IOEventHandler *chr_event;
     IOCanReadHandler *chr_can_read;
@@ -83,6 +86,7 @@ void qemu_chr_close(CharDriverState *chr);
 void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
 void qemu_chr_send_event(CharDriverState *s, int event);
+void qemu_chr_send_device_event(CharDriverState *s, int event, void *arg);
 void qemu_chr_add_handlers(CharDriverState *s,
                            IOCanReadHandler *fd_can_read,
                            IOReadHandler *fd_read,


> Regards,
> 
> Anthony Liguori
> 
> > Alon
> >
> > -------------- commit message --------------------------------
> >  From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:00
> 2001
> > From: Alon Levy<address@hidden>
> > Date: Mon, 2 Aug 2010 11:22:58 +0300
> > Subject: [PATCH] virtio-console: add IOCTL's for
> guest_{ready,open,close}
> >
> > Add three IOCTL corresponding to the three control events of:
> >   guest_ready ->  CHR_IOCTL_VIRT_SERIAL_READY
> >   guest_open  ->  CHR_IOCTL_VIRT_SERIAL_OPEN
> >   guest_close ->  CHR_IOCTL_VIRT_SERIAL_CLOSE
> >
> > Can be used by a matching backend.
> > ---
> >   hw/virtio-console.c |   33 +++++++++++++++++++++++++++++++++
> >   qemu-char.h         |    4 ++++
> >   2 files changed, 37 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> > index caea11f..4c3686d 100644
> > --- a/hw/virtio-console.c
> > +++ b/hw/virtio-console.c
> > @@ -58,6 +58,33 @@ static void chr_event(void *opaque, int event)
> >       }
> >   }
> >
> > +static void virtconsole_guest_open(VirtIOSerialPort *port)
> > +{
> > +    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> > +
> > +    if (vcon->chr) {
> > +        qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_OPEN,
> NULL);
> > +    }
> > +}
> > +
> > +static void virtconsole_guest_close(VirtIOSerialPort *port)
> > +{
> > +    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> > +
> > +    if (vcon->chr) {
> > +        qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_CLOSE,
> NULL);
> > +    }
> > +}
> > +
> > +static void virtconsole_guest_ready(VirtIOSerialPort *port)
> > +{
> > +    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
> > +
> > +    if (vcon->chr) {
> > +        qemu_chr_ioctl(vcon->chr, CHR_IOCTL_VIRT_SERIAL_READY,
> NULL);
> > +    }
> > +}
> > +
> >   /* Virtio Console Ports */
> >   static int virtconsole_initfn(VirtIOSerialDevice *dev)
> >   {
> > @@ -94,6 +121,9 @@ static VirtIOSerialPortInfo virtconsole_info = {
> >       .qdev.size     = sizeof(VirtConsole),
> >       .init          = virtconsole_initfn,
> >       .exit          = virtconsole_exitfn,
> > +    .guest_open    = virtconsole_guest_open,
> > +    .guest_close   = virtconsole_guest_close,
> > +    .guest_ready   = virtconsole_guest_ready,
> >       .qdev.props = (Property[]) {
> >           DEFINE_PROP_UINT8("is_console", VirtConsole,
> port.is_console, 1),
> >           DEFINE_PROP_UINT32("nr", VirtConsole, port.id,
> VIRTIO_CONSOLE_BAD_ID),
> > @@ -130,6 +160,9 @@ static VirtIOSerialPortInfo virtserialport_info
> = {
> >       .qdev.size     = sizeof(VirtConsole),
> >       .init          = virtserialport_initfn,
> >       .exit          = virtconsole_exitfn,
> > +    .guest_open    = virtconsole_guest_open,
> > +    .guest_close   = virtconsole_guest_close,
> > +    .guest_ready   = virtconsole_guest_ready,
> >       .qdev.props = (Property[]) {
> >           DEFINE_PROP_UINT32("nr", VirtConsole, port.id,
> VIRTIO_CONSOLE_BAD_ID),
> >           DEFINE_PROP_CHR("chardev", VirtConsole, chr),
> > diff --git a/qemu-char.h b/qemu-char.h
> > index e3a0783..1df53ae 100644
> > --- a/qemu-char.h
> > +++ b/qemu-char.h
> > @@ -41,6 +41,10 @@ typedef struct {
> >   #define CHR_IOCTL_SERIAL_SET_TIOCM   13
> >   #define CHR_IOCTL_SERIAL_GET_TIOCM   14
> >
> > +#define CHR_IOCTL_VIRT_SERIAL_OPEN   15
> > +#define CHR_IOCTL_VIRT_SERIAL_CLOSE  16
> > +#define CHR_IOCTL_VIRT_SERIAL_READY  17
> > +
> >   #define CHR_TIOCM_CTS     0x020
> >   #define CHR_TIOCM_CAR     0x040
> >   #define CHR_TIOCM_DSR     0x100
> >