|
From: | Anthony Liguori |
Subject: | Re: [Qemu-devel] RFC adding ioctl's to virtserial/virtconsole |
Date: | Mon, 02 Aug 2010 12:42:57 -0500 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.11) Gecko/20100713 Lightning/1.0b1 Thunderbird/3.0.6 |
On 08/02/2010 12:28 PM, Alon Levy wrote:
----- "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 virtserialdevices, to be usedby a chardev backend, such as a spice vm channel (spice is a vdisolution).Basically virtio-serial provides three driver initiated events forguest open ofa device, guest close, and guest ready (driver port init complete)that before thispatch are not exposed to the chardev backend. With the spicevmc backend this is used like this: qemu -chardev spicevmc,id=vdiport,name=vdiport -devicevirtserialport,chardev=vdiport,name=com.redhat.spice.0I'd appreciate any feedback if this seems the right way toaccomplish this, andfor 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 LiguoriAlon -------------- commit message -------------------------------- From a90d4e26df727ed0d2b64b705e955f695289fa61 Mon Sep 17 00:00:002001From: Alon Levy<address@hidden> Date: Mon, 2 Aug 2010 11:22:58 +0300 Subject: [PATCH] virtio-console: add IOCTL's forguest_{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
[Prev in Thread] | Current Thread | [Next in Thread] |