RFC adding ioctl's to virtserial/virtconsole

Submitted by Alon Levy on Aug. 2, 2010, 8:33 a.m.

Details

Message ID 1504146626.525061280738029828.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com
State New
Headers show

Commit Message

Alon Levy Aug. 2, 2010, 8:33 a.m.
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(-)

Comments

Amit Shah Aug. 2, 2010, 9:03 a.m.
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
Anthony Liguori Aug. 2, 2010, 3:04 p.m.
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
>

Patch hide | download patch | download mbox

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