RFC adding ioctl's to virtserial/virtconsole

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.

Alon

-------------- commit message --------------------------------
From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:00 2001
From: Alon Levy <alevy@redhat.com>
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(-)

On (Mon) Aug 02 2010 [04:33:49], 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.

Looks fine to me, maybe you don't need the notifications for
virtio-console ports, only the virtio-serial ports could be of interest,
but I'm fine with both notifying as well.

Also, this description could be made part of the patch description
itself.

		Amit

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.

Regards,

Anthony Liguori

> Alon
>
> -------------- commit message --------------------------------
>  From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:00 2001
> From: Alon Levy<alevy@redhat.com>
> 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
>