[Qemu-devel] [PATCH 0/6] RFC: add "spiceport" chardev

[Qemu-devel] [PATCH 0/6] RFC: add "spiceport" chardev

[Marc-André Lureau]

Hi,

Add a new chardev to allow arbitrary communication between the
host and the Spice client (via the spice server).

Note: the related Spice protocol and server patches are in review,
this patch series serves as a RFC for the reviewers. Except the first
3 patches, the spice dependencies will need to be bump to the upcoming
protocol and server releases.

Some examples:

This allows the Spice client to have a special port for the qemu
monitor:

... -chardev spiceport,name=org.qemu.monitor,id=monitorport
    -mon chardev=monitorport

Or to allow arbitrary communication outside of qemu:

... -chardev spiceport,name=org.ovirt.controller,id=...,chardev=ovcsocket
    -chardev socket,server,host=0.0.0.0,port=4242,id=ovcsocket,nowait

Marc-André Lureau (6):
  qemu-char: add qemu_chr_remove_clients()
  spice-qemu-char: write to chardev whatever amount it can read
  spice-qemu-char: factor out CharDriverState creation
  spice-qemu-char: add spiceport chardev
  spice-qemu-char: keep a list of spice chardev
  spice-qemu-char: register spicevmc ports during qemu_spice_init()

 qemu-char.c       |  29 ++++++--
 qemu-char.h       |   2 +
 qemu-config.c     |   3 +
 qemu-options.hx   |  14 ++++
 spice-qemu-char.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 trace-events      |   3 +
 ui/qemu-spice.h   |   2 +
 ui/spice-core.c   |   2 +
 8 files changed, 229 insertions(+), 21 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH 1/6] qemu-char: add qemu_chr_remove_clients()

[Marc-André Lureau]

Make it possible for chardev user to disconnect all the clients.

The spiceport will remove associated chardev clients when the spice
client is disconnected.

(since qemu-char could have several clients implementation later, as
chr_add_client() name suggests, I chose to have generic name and
behaviour that could apply to a single or many clients implementation)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qemu-char.c | 28 ++++++++++++++++++++++++----
 qemu-char.h |  2 ++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 242b799..1414ca1 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -176,6 +176,11 @@ int qemu_chr_add_client(CharDriverState *s, int fd)
     return s->chr_add_client ? s->chr_add_client(s, fd) : -1;
 }
 
+int qemu_chr_remove_clients(CharDriverState *s)
+{
+    return s->chr_remove_clients ? s->chr_remove_clients(s) : -1;
+}
+
 void qemu_chr_accept_input(CharDriverState *s)
 {
     if (s->chr_accept_input)
@@ -2378,6 +2383,22 @@ static int tcp_chr_add_client(CharDriverState *chr, int fd)
     return 0;
 }
 
+static int tcp_chr_remove_clients(CharDriverState *chr)
+{
+    TCPCharDriver *s = chr->opaque;
+
+    if (s->fd >= 0) {
+        qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+        closesocket(s->fd);
+        s->fd = -1;
+    }
+
+    /* listen for new clients */
+    qemu_set_fd_handler2(s->listen_fd, NULL, tcp_chr_accept, NULL, chr);
+
+    return 0;
+}
+
 static void tcp_chr_accept(void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -2417,10 +2438,8 @@ static void tcp_chr_accept(void *opaque)
 static void tcp_chr_close(CharDriverState *chr)
 {
     TCPCharDriver *s = chr->opaque;
-    if (s->fd >= 0) {
-        qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-        closesocket(s->fd);
-    }
+
+    tcp_chr_remove_clients(chr);
     if (s->listen_fd >= 0) {
         qemu_set_fd_handler2(s->listen_fd, NULL, NULL, NULL, NULL);
         closesocket(s->listen_fd);
@@ -2484,6 +2503,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     chr->chr_close = tcp_chr_close;
     chr->get_msgfd = tcp_get_msgfd;
     chr->chr_add_client = tcp_chr_add_client;
+    chr->chr_remove_clients = tcp_chr_remove_clients;
 
     if (is_listen) {
         s->listen_fd = fd;
diff --git a/qemu-char.h b/qemu-char.h
index a121e04..b01c45c 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -60,6 +60,7 @@ struct CharDriverState {
     int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
     int (*get_msgfd)(struct CharDriverState *s);
     int (*chr_add_client)(struct CharDriverState *chr, int fd);
+    int (*chr_remove_clients)(struct CharDriverState *chr);
     IOEventHandler *chr_event;
     IOCanReadHandler *chr_can_read;
     IOReadHandler *chr_read;
@@ -232,6 +233,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
 void qemu_chr_generic_open(CharDriverState *s);
 void qemu_chr_accept_input(CharDriverState *s);
 int qemu_chr_add_client(CharDriverState *s, int fd);
+int qemu_chr_remove_clients(CharDriverState *s);
 void qemu_chr_info_print(Monitor *mon, const QObject *ret_data);
 void qemu_chr_info(Monitor *mon, QObject **ret_data);
 CharDriverState *qemu_chr_find(const char *name);
-- 
1.7.11.7

[Qemu-devel] [PATCH 2/6] spice-qemu-char: write to chardev whatever amount it can read

[Marc-André Lureau]

The current code waits until the chardev can read MIN(len, VMC_MAX)
But some chardev may never reach than amount, in fact some of them
will only ever accept write of 1. Fix the min computation and remove
the VMC_MAX constant.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 spice-qemu-char.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 09aa22d..665efd3 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -14,8 +14,6 @@
         }                                                               \
     } while (0)
 
-#define VMC_MAX_HOST_WRITE    2048
-
 typedef struct SpiceCharDriver {
     CharDriverState*      chr;
     SpiceCharDeviceInstance     sin;
@@ -35,8 +33,8 @@ static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len)
     uint8_t* p = (uint8_t*)buf;
 
     while (len > 0) {
-        last_out = MIN(len, VMC_MAX_HOST_WRITE);
-        if (qemu_chr_be_can_write(scd->chr) < last_out) {
+        last_out = MIN(len, qemu_chr_be_can_write(scd->chr));
+        if (last_out <= 0) {
             break;
         }
         qemu_chr_be_write(scd->chr, p, last_out);
-- 
1.7.11.7

[Qemu-devel] [PATCH 3/6] spice-qemu-char: factor out CharDriverState creation

[Marc-André Lureau]

Make the CharDriverState creation code reusable by spicevmc port.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 spice-qemu-char.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 665efd3..b86e83a 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -186,13 +186,32 @@ static void print_allowed_subtypes(void)
     fprintf(stderr, "\n");
 }
 
-CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
+static CharDriverState *chr_open(QemuOpts *opts, const char *subtype)
 {
     CharDriverState *chr;
     SpiceCharDriver *s;
-    const char* name = qemu_opt_get(opts, "name");
     uint32_t debug = qemu_opt_get_number(opts, "debug", 0);
-    const char** psubtype = spice_server_char_device_recognized_subtypes();
+
+    chr = g_malloc0(sizeof(CharDriverState));
+    s = g_malloc0(sizeof(SpiceCharDriver));
+    s->chr = chr;
+    s->debug = debug;
+    s->active = false;
+    s->sin.subtype = subtype;
+    chr->opaque = s;
+    chr->chr_write = spice_chr_write;
+    chr->chr_close = spice_chr_close;
+    chr->chr_guest_open = spice_chr_guest_open;
+    chr->chr_guest_close = spice_chr_guest_close;
+
+    return chr;
+}
+
+CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
+{
+    CharDriverState *chr;
+    const char *name = qemu_opt_get(opts, "name");
+    const char **psubtype = spice_server_char_device_recognized_subtypes();
     const char *subtype = NULL;
 
     if (name == NULL) {
@@ -212,17 +231,7 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
         return NULL;
     }
 
-    chr = g_malloc0(sizeof(CharDriverState));
-    s = g_malloc0(sizeof(SpiceCharDriver));
-    s->chr = chr;
-    s->debug = debug;
-    s->active = false;
-    s->sin.subtype = subtype;
-    chr->opaque = s;
-    chr->chr_write = spice_chr_write;
-    chr->chr_close = spice_chr_close;
-    chr->chr_guest_open = spice_chr_guest_open;
-    chr->chr_guest_close = spice_chr_guest_close;
+    chr = chr_open(opts, subtype);
 
 #if SPICE_SERVER_VERSION < 0x000901
     /* See comment in vmc_state() */
-- 
1.7.11.7

[Qemu-devel] [PATCH 4/6] spice-qemu-char: add spiceport chardev

[Marc-André Lureau]

Add a new spice chardev to allow arbitrary communication between the
host and the Spice client (via the spice server).

Examples:

This allows the Spice client to have a special port for the qemu
monitor:

... -chardev spiceport,name=org.qemu.monitor,id=monitorport
    -mon chardev=monitorport

Or to allow arbitrary communication outside of qemu:

... -chardev spiceport,name=org.ovirt.controller,id=...,chardev=ovcsocket
    -chardev socket,server,host=0.0.0.0,port=4242,id=ovcsocket,nowait

The spice client is notified when the qemu socket server has gain or
lost a client. The qemu socket client is disconnected when the spice
client is disconnected.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qemu-char.c       |   1 +
 qemu-config.c     |   3 ++
 qemu-options.hx   |  14 ++++++
 spice-qemu-char.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 trace-events      |   3 ++
 ui/qemu-spice.h   |   1 +
 6 files changed, 161 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index 1414ca1..bc1c74b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2782,6 +2782,7 @@ static const struct {
 #endif
 #ifdef CONFIG_SPICE
     { .name = "spicevmc",     .open = qemu_chr_open_spice },
+    { .name = "spiceport",     .open = qemu_chr_open_spice_port },
 #endif
 };
 
diff --git a/qemu-config.c b/qemu-config.c
index 10d1ba4..b4e7af3 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -217,6 +217,9 @@ static QemuOptsList qemu_chardev_opts = {
         },{
             .name = "debug",
             .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "chardev",
+            .type = QEMU_OPT_STRING,
         },
         { /* end of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index fbcf079..d2f09f6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1749,6 +1749,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
 #endif
 #if defined(CONFIG_SPICE)
     "-chardev spicevmc,id=id,name=name[,debug=debug]\n"
+    "-chardev spiceport,id=id,name=name[,chardev=name,debug=debug]\n"
 #endif
     , QEMU_ARCH_ALL
 )
@@ -1776,6 +1777,7 @@ Backend is one of:
 @option{tty},
 @option{parport},
 @option{spicevmc}.
+@option{spiceport}.
 The specific backend will determine the applicable options.
 
 All devices must have an id, which can be any string up to 127 characters long.
@@ -1961,6 +1963,18 @@ required.
 
 Connect to a spice virtual machine channel, such as vdiport.
 
+@item -chardev spiceport ,id=@var{id} ,debug=@var{debug}, name=@var{name}, chardev=@var{chardev}
+
+@option{spiceport} is only available when spice support is built in.
+
+@option{debug} debug level for spicevmc
+
+@option{name} name of spice port to connect to
+
+@option{chardev} name of chardev to connect to
+
+Connect to a spice port, fixme.
+
 @end table
 ETEXI
 
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index b86e83a..629b500 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -3,6 +3,7 @@
 #include "ui/qemu-spice.h"
 #include <spice.h>
 #include <spice-experimental.h>
+#include <spice/protocol.h>
 
 #include "osdep.h"
 
@@ -23,6 +24,7 @@ typedef struct SpiceCharDriver {
     uint8_t               *datapos;
     ssize_t               bufsize, datalen;
     uint32_t              debug;
+    CharDriverState       *chardev;
 } SpiceCharDriver;
 
 static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len)
@@ -67,6 +69,25 @@ static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t *buf, int len)
     return bytes;
 }
 
+static void vmc_event(SpiceCharDeviceInstance *sin, uint8_t event)
+{
+    SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
+    int chr_event;
+
+    switch (event) {
+    case SPICE_PORT_EVENT_BREAK:
+        chr_event = CHR_EVENT_BREAK;
+        break;
+    default:
+        dprintf(scd, 2, "%s: unknown %d\n", __func__, event);
+        return;
+    }
+
+    dprintf(scd, 2, "%s: %d\n", __func__, event);
+    trace_spice_vmc_event(chr_event);
+    qemu_chr_be_event(scd->chr, chr_event);
+}
+
 static void vmc_state(SpiceCharDeviceInstance *sin, int connected)
 {
     SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
@@ -103,6 +124,7 @@ static SpiceCharDeviceInterface vmc_interface = {
     .state              = vmc_state,
     .write              = vmc_write,
     .read               = vmc_read,
+    .event              = vmc_event,
 };
 
 
@@ -116,6 +138,10 @@ static void vmc_register_interface(SpiceCharDriver *scd)
     qemu_spice_add_interface(&scd->sin.base);
     scd->active = true;
     trace_spice_vmc_register_interface(scd);
+
+    if (scd->chardev != NULL && scd->chardev->opened) {
+        spice_server_port_event(&scd->sin, SPICE_PORT_EVENT_OPENED);
+    }
 }
 
 static void vmc_unregister_interface(SpiceCharDriver *scd)
@@ -242,3 +268,116 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
 
     return chr;
 }
+static int port_can_read(void *opaque)
+{
+    CharDriverState *chr = opaque;
+    SpiceCharDriver *scd = chr->opaque;
+
+    return qemu_chr_be_can_write(scd->chardev);
+}
+
+static void port_read(void *opaque, const uint8_t *buf, int size)
+{
+    CharDriverState *chr = opaque;
+    SpiceCharDriver *scd = chr->opaque;
+
+    dprintf(scd, 2, "%s: %d\n", __func__, size);
+    trace_spice_port_chardev_write(size);
+    qemu_chr_fe_write(scd->chardev, buf, size);
+}
+
+static void port_event(void *opaque, int event)
+{
+    CharDriverState *chr = opaque;
+    SpiceCharDriver *scd = chr->opaque;
+
+    dprintf(scd, 2, "%s: %d\n", __func__, event);
+
+    switch (event) {
+    case CHR_EVENT_CLOSED:
+        if (scd->chardev != NULL) {
+            qemu_chr_remove_clients(scd->chardev);
+        }
+        break;
+    }
+}
+
+static int chardev_can_read(void *opaque)
+{
+    CharDriverState *chr = opaque;
+    SpiceCharDriver *s = chr->opaque;
+
+    if (!chr->opened) {
+        return 0;
+    }
+
+    if (s->datalen != 0) {
+        return 0;
+    }
+
+    /* FIXME: assume spice can take chunks of 4096 */
+    return  4096;
+}
+
+/* Send data from a char device over to the spice port */
+static void chardev_read(void *opaque, const uint8_t *buf, int size)
+{
+    CharDriverState *chr = opaque;
+
+    trace_spice_port_chardev_read(size);
+    /* spicevmc port always send/queue all data */
+    qemu_chr_fe_write(chr, buf, size);
+}
+
+static void chardev_event(void *opaque, int event)
+{
+    CharDriverState *chr = opaque;
+    SpiceCharDriver *scd = chr->opaque;
+
+    dprintf(scd, 2, "%s: %d\n", __func__, event);
+
+    switch (event) {
+    case CHR_EVENT_OPENED:
+        spice_server_port_event(&scd->sin, SPICE_PORT_EVENT_OPENED);
+        break;
+    case CHR_EVENT_CLOSED:
+        spice_server_port_event(&scd->sin, SPICE_PORT_EVENT_CLOSED);
+        break;
+    }
+}
+
+CharDriverState *qemu_chr_open_spice_port(QemuOpts *opts)
+{
+    CharDriverState *chr, *chardev = NULL;
+    SpiceCharDriver *s;
+    const char *name = qemu_opt_get(opts, "name");
+    const char *chrdev = qemu_opt_get(opts, "chardev");
+
+    if (name == NULL) {
+        fprintf(stderr, "spice-qemu-char: missing name parameter\n");
+        return NULL;
+    }
+
+    if (chrdev != NULL) {
+        chardev = qemu_chr_find(chrdev);
+        if (chardev == NULL) {
+            fprintf(stderr, "spice-qemu-char: chardev \"%s\" not found\n",
+                    chrdev);
+            return NULL;
+        }
+    }
+
+    chr = chr_open(opts, "port");
+    s = chr->opaque;
+    s->sin.portname = name;
+
+    if (chardev != NULL) {
+        s->chardev = chardev;
+        qemu_chr_add_handlers(chardev, chardev_can_read,
+                              chardev_read, chardev_event, chr);
+    }
+
+    qemu_chr_add_handlers(chr, port_can_read, port_read, port_event, chr);
+
+    return chr;
+}
diff --git a/trace-events b/trace-events
index 6c6cbf1..26ca363 100644
--- a/trace-events
+++ b/trace-events
@@ -535,6 +535,9 @@ spice_vmc_write(ssize_t out, int len) "spice wrottn %zd of requested %d"
 spice_vmc_read(int bytes, int len) "spice read %d of requested %d"
 spice_vmc_register_interface(void *scd) "spice vmc registered interface %p"
 spice_vmc_unregister_interface(void *scd) "spice vmc unregistered interface %p"
+spice_vmc_event(int event) "spice vmc event %d"
+spice_port_chardev_read(int bytes) "spice port read %d from chardev"
+spice_port_chardev_write(int bytes) "spice port wrote %d to chardev"
 
 # hw/lm32_pic.c
 lm32_pic_raise_irq(void) "Raise CPU interrupt"
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index 3299da8..ab1943a 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -46,6 +46,7 @@ void do_info_spice_print(Monitor *mon, const QObject *data);
 void do_info_spice(Monitor *mon, QObject **ret_data);
 
 CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
+CharDriverState *qemu_chr_open_spice_port(QemuOpts *opts);
 
 #else  /* CONFIG_SPICE */
 #include "monitor.h"
-- 
1.7.11.7

[Qemu-devel] [PATCH 5/6] spice-qemu-char: keep a list of spice chardev

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 spice-qemu-char.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 629b500..13de037 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -25,8 +25,12 @@ typedef struct SpiceCharDriver {
     ssize_t               bufsize, datalen;
     uint32_t              debug;
     CharDriverState       *chardev;
+    QLIST_ENTRY(SpiceCharDriver) next;
 } SpiceCharDriver;
 
+static QLIST_HEAD(, SpiceCharDriver) spice_chars =
+    QLIST_HEAD_INITIALIZER(spice_chars);
+
 static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len)
 {
     SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
@@ -180,6 +184,7 @@ static void spice_chr_close(struct CharDriverState *chr)
 
     printf("%s\n", __func__);
     vmc_unregister_interface(s);
+    QLIST_REMOVE(s, next);
     g_free(s);
 }
 
@@ -230,6 +235,8 @@ static CharDriverState *chr_open(QemuOpts *opts, const char *subtype)
     chr->chr_guest_open = spice_chr_guest_open;
     chr->chr_guest_close = spice_chr_guest_close;
 
+    QLIST_INSERT_HEAD(&spice_chars, s, next);
+
     return chr;
 }
 
-- 
1.7.11.7

[Qemu-devel] [PATCH 6/6] spice-qemu-char: register spicevmc ports during qemu_spice_init()

[Marc-André Lureau]

Do the delayed registration of spicevmc ports after Spice server is
initialized.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 spice-qemu-char.c | 13 +++++++++++++
 ui/qemu-spice.h   |  1 +
 ui/spice-core.c   |  2 ++
 3 files changed, 16 insertions(+)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 13de037..8577664 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -275,6 +275,7 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
 
     return chr;
 }
+
 static int port_can_read(void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -388,3 +389,15 @@ CharDriverState *qemu_chr_open_spice_port(QemuOpts *opts)
 
     return chr;
 }
+
+void qemu_spice_register_ports(void)
+{
+    SpiceCharDriver *s;
+
+    QLIST_FOREACH(s, &spice_chars, next) {
+        if (s->sin.portname == NULL) {
+            continue;
+        }
+        vmc_register_interface(s);
+    }
+}
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index ab1943a..5d60e99 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -47,6 +47,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data);
 
 CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
 CharDriverState *qemu_chr_open_spice_port(QemuOpts *opts);
+void qemu_spice_register_ports(void);
 
 #else  /* CONFIG_SPICE */
 #include "monitor.h"
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 261c6f2..979c689 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -714,6 +714,8 @@ void qemu_spice_init(void)
     g_free(x509_key_file);
     g_free(x509_cert_file);
     g_free(x509_cacert_file);
+
+    qemu_spice_register_ports();
 }
 
 int qemu_spice_add_interface(SpiceBaseInstance *sin)
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 0/6] RFC: add "spiceport" chardev

[Gerd Hoffmann]

  Hi,

> This allows the Spice client to have a special port for the qemu
> monitor:
> 
> ... -chardev spiceport,name=org.qemu.monitor,id=monitorport
>     -mon chardev=monitorport

Nice.  I think we should have a registry of names, at least for the
org.qemu.* namespace.  A simple text file in docs/ should do.  Monitor
needs to specify the protocol (hmp vs qmp).  Also adding a number
doesn't hurt.  So ...

   org.qemu.monitor.hmp.0
   org.qemu.monitor.qmp.0

Also nice to have:

   org.qemu.console.serial.0

... and spice client redirecting that to a vte.  You might have to add
some termios control messages to spiceport, so one can turn on/off echo,
send breaks, xon/xoff flow control, etc to make it work really nicely.

> Or to allow arbitrary communication outside of qemu:
> 
> ... -chardev spiceport,name=org.ovirt.controller,id=...,chardev=ovcsocket
>     -chardev socket,server,host=0.0.0.0,port=4242,id=ovcsocket,nowait

Hmm, so that will make qemu just hook those chardevs back-to-back and
forward data without looking at it?  I'm not sure we want that ...

What is the use case?  Any reason why the spice client can not (or
should not) speak to ovirt directly?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/6] RFC: add "spiceport" chardev

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2042 bytes --]

Hi

On Fri, Nov 30, 2012 at 2:58 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:

> Also nice to have:
>
>    org.qemu.console.serial.0
>
> ... and spice client redirecting that to a vte.  You might have to add
> some termios control messages to spiceport, so one can turn on/off echo,
> send breaks, xon/xoff flow control, etc to make it work really nicely.
>

I haven't played with a console yet, but this is indeed an important use
case.

> Or to allow arbitrary communication outside of qemu:
> >
> > ... -chardev spiceport
> ,name=org.ovirt.controller,id=...,chardev=ovcsocket
> >     -chardev socket,server,host=0.0.0.0,port=4242,id=ovcsocket,nowait
>
> Hmm, so that will make qemu just hook those chardevs back-to-back and
> forward data without looking at it?  I'm not sure we want that ...
>


> What is the use case?  Any reason why the spice client can not (or
> should not) speak to ovirt directly?
>

Ah, in fact, it's the main reason why I worked on this. Currently, the
Spice client has to communicate with ovirt via the browser, which is a pain
to deal with: it's a completely different route, it needs a running
browser, a compatible extension (xpi vs activex vs the rest not supported),
leading to duplicated work, license problems, regular breakage between
browser versions, hard to test, difficult to upgrade... Instead, we are
investigating the use of a configuration file provided by ovirt portal for
setting up the client, and the dynamic interaction could take place either
via the propose Spice port, or directly via ovirt.

Some of the dynamic ovirt functionality are interesting for direct clients,
like the "spice controller menu" (a customizable client UI menu,
virt-viewer and Boxes could benefit it). It may not be the best solution to
route the "ovirt/spice controller" through qemu host, but at least I wanted
to try that option. It could be that in the end, it is prefered that the
client just talk directly to ovirt, whatever fits best.

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2934 bytes --]

Re: [Qemu-devel] [PATCH 0/6] RFC: add "spiceport" chardev

[Gerd Hoffmann]

  Hi,

>> What is the use case?  Any reason why the spice client can not (or
>> should not) speak to ovirt directly?
> 
> Ah, in fact, it's the main reason why I worked on this. Currently, the
> Spice client has to communicate with ovirt via the browser, which is a pain
> to deal with: it's a completely different route, it needs a running
> browser, a compatible extension (xpi vs activex vs the rest not supported),
> leading to duplicated work, license problems, regular breakage between
> browser versions, hard to test, difficult to upgrade...

Understood.

> Instead, we are
> investigating the use of a configuration file provided by ovirt portal for
> setting up the client, and the dynamic interaction could take place either
> via the propose Spice port, or directly via ovirt.
> 
> Some of the dynamic ovirt functionality are interesting for direct clients,
> like the "spice controller menu" (a customizable client UI menu,
> virt-viewer and Boxes could benefit it). It may not be the best solution to
> route the "ovirt/spice controller" through qemu host, but at least I wanted
> to try that option. It could be that in the end, it is prefered that the
> client just talk directly to ovirt, whatever fits best.

I'd go for a direct connection.  Going the indirect route via qemu
probably isn't as bad as going indirectly via browser, but still.
Unless there is a very good reason to use qemu as middle man I simply
wouldn't do that.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/6] spice-qemu-char: write to chardev whatever amount it can read

[Alon Levy]

> The current code waits until the chardev can read MIN(len, VMC_MAX)
> But some chardev may never reach than amount, in fact some of them
> will only ever accept write of 1. Fix the min computation and remove
> the VMC_MAX constant.

Looks good to me.

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  spice-qemu-char.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 09aa22d..665efd3 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -14,8 +14,6 @@
>          }
>                                                                        \
>      } while (0)
>  
> -#define VMC_MAX_HOST_WRITE    2048
> -
>  typedef struct SpiceCharDriver {
>      CharDriverState*      chr;
>      SpiceCharDeviceInstance     sin;
> @@ -35,8 +33,8 @@ static int vmc_write(SpiceCharDeviceInstance *sin,
> const uint8_t *buf, int len)
>      uint8_t* p = (uint8_t*)buf;
>  
>      while (len > 0) {
> -        last_out = MIN(len, VMC_MAX_HOST_WRITE);
> -        if (qemu_chr_be_can_write(scd->chr) < last_out) {
> +        last_out = MIN(len, qemu_chr_be_can_write(scd->chr));
> +        if (last_out <= 0) {
>              break;
>          }
>          qemu_chr_be_write(scd->chr, p, last_out);
> --
> 1.7.11.7
> 
> 
>