On 08/02/2010 12:28 PM, Alon Levy wrote:
----- "Anthony Liguori"<anth...@codemonkey.ws> 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:
My main objection to ioctls is that you change states based on event
delivery. This results in weird things like what happens when you do a
chr_write while not ready or not connected.
So what I'd rather see is a move to an API that was connection
oriented. For instance, we could treat CharDriverState as an
established connection. So something like:
typedef struct CharServerState
{
int backlog; /* max simultaneous connections; -1 for unlimited */
void (*connect)(CharServerState *s, CharDriverState *session);
void (*disconnect)(CharServerState *s, CharDriverState *session);
} CharDriverState;
Obviously, more thought is needed but I hope the point comes across. We
should be able to reflect the connect/disconnect semantics with an
object that has a life cycle that matches the session instead of forcing
each user to keep track of the session's life cycle.
Regards,
Anthony Liguori
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<al...@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
