[Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

The following two patches make it possible to tunnel character devices
over VNC, using a new VNC extension. This is motivated by the existing
QEMU support for tunnelling audio streams over VNC, and the code follows
a very similar design.  The key requirement here is that it should not
be neccessary to specifically configure each character device to make
it available via VNC. The admin should be able to configure the char
devices with all current available backends (file, pty, null, tcp, udp,
unix, etc), and regardless of this config be able to snoop on data from
any active VNC client on demand.

The first patch adds a bunch of support APIs to qemu-char{.h,.c}

 - An API to iterate over all current character devices. This allows
   the VNC server to inform the client of the current list of available
   data streams upon connection

 - An API to monitor creation & deletion of character devices. This
   allows the VNC server to inform the client  when character devices
   are hot-plugged/unplugged.

 - An API to request capture data on a specific character device.
   This is how the VNC server gets the data coming out of the guest

The design for this stuff follows that used in the audio stream capture
APIs (audio/audio.h)

The second patch implements the VNC extension for tunnelling multiple
independant data streams over a connection. I have stolen one of
the psuedo encodings assigned to Anthony for this purpose, and also
added a new message sub-type to the 'aliguori' client/server message
type. Hopefully anthony will approve this allocation if the patches
look good :-)

The VNC extension is for generic data streams, so it can be implemented
by any VNC server that wants this. The QEMU implementation maps data
streams onto character devices. Every data stream has a name, and in
this case I'm using the QEMU character device label, since this can be
explicitly set by mgmt tools starting QEMU, and allows VNC client to
correlate the stream with devices, and with the monitor. When the client
requests capture on a specific device, the server assigns an integer
ID to the stream and that's used in place of the name for all future
messages. The idea is that if you're sending a few bytes at a time of
data, it is overkill to send a 20 byte string with the name whn you
could just send an int32 identifier. I might actually make it an int8.
The wire format for the messages is again closely following the model
used for the earlier audio stream extension.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 1/2] APIs to capture character device data

[Daniel P. Berrange]

commit b6d541a02ffa3dbff3f0e6a27d7441ff24481185
Author: Daniel P. Berrange <berrange@redhat.com>
Date:   Wed Jul 1 14:21:17 2009 +0100

    Add APIs for capturing data written to character devices, and
    for receiving notification of creation & deletion of character
    devices

diff --git a/qemu-char.c b/qemu-char.c
index 287e0cd..bee309c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -106,6 +106,23 @@
 /***********************************************************/
 /* character device */
 
+struct CharCaptureState {
+    struct CharCaptureOps *ops;
+    void *opaque;
+
+    TAILQ_ENTRY(CharCaptureState) next;
+};
+
+struct CharMonitorState {
+    struct CharMonitorOps *ops;
+    void *opaque;
+
+    TAILQ_ENTRY(CharMonitorState) next;
+};
+
+static TAILQ_HEAD(CharMonitorStateHead, CharMonitorState) charmonitors =
+    TAILQ_HEAD_INITIALIZER(charmonitors);
+
 static TAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
     TAILQ_HEAD_INITIALIZER(chardevs);
 static int initial_reset_issued;
@@ -146,6 +163,13 @@ void qemu_chr_initial_reset(void)
 
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len)
 {
+    CharCaptureState *cap;
+
+    TAILQ_FOREACH(cap, &s->capture, next) {
+	if (cap->ops->capture)
+	    (*cap->ops->capture)(cap->opaque, buf, len);
+    }
+
     return s->chr_write(s, buf, len);
 }
 
@@ -2215,17 +2239,41 @@ CharDriverState *qemu_chr_open(const char *label, const char *filename, void (*i
     }
 
     if (chr) {
+	CharMonitorState *mon;
+
         if (!chr->filename)
             chr->filename = qemu_strdup(filename);
         chr->init = init;
         chr->label = qemu_strdup(label);
         TAILQ_INSERT_TAIL(&chardevs, chr, next);
+
+	TAILQ_INIT(&chr->capture);
+
+	TAILQ_FOREACH(mon, &charmonitors, next) {
+	    if (mon->ops->opened)
+		(*mon->ops->opened)(chr, mon->opaque);
+	}
     }
     return chr;
 }
 
 void qemu_chr_close(CharDriverState *chr)
 {
+    CharMonitorState *mon;
+    CharCaptureState *cap, *captmp;
+
+    TAILQ_FOREACH_SAFE(cap, &chr->capture, next, captmp) {
+	if (cap->ops->destroy)
+	    (*cap->ops->destroy)(cap->opaque);
+
+	qemu_free(cap);
+    }
+
+    TAILQ_FOREACH(mon, &charmonitors, next) {
+	if (mon->ops->closing)
+	    (*mon->ops->closing)(chr, mon->opaque);
+    }
+
     TAILQ_REMOVE(&chardevs, chr, next);
     if (chr->chr_close)
         chr->chr_close(chr);
@@ -2242,3 +2290,77 @@ void qemu_chr_info(Monitor *mon)
         monitor_printf(mon, "%s: filename=%s\n", chr->label, chr->filename);
     }
 }
+
+
+
+CharCaptureState *qemu_chr_add_capture(CharDriverState *s,
+				       struct CharCaptureOps *ops,
+				       void *opaque)
+{
+    CharCaptureState *cap;
+
+    cap = qemu_mallocz(sizeof(*cap));
+
+    cap->ops = ops;
+    cap->opaque = opaque;
+
+    TAILQ_INSERT_TAIL(&s->capture, cap, next);
+
+    return cap;
+}
+
+
+void qemu_chr_del_capture(CharDriverState *s,
+			  CharCaptureState *cap)
+{
+    TAILQ_REMOVE(&s->capture, cap, next);
+
+    qemu_free(cap);
+}
+
+CharMonitorState *qemu_chr_add_monitor(struct CharMonitorOps *ops,
+				       void *opaque)
+{
+    CharMonitorState *mon;
+
+    mon = qemu_mallocz(sizeof(*mon));
+
+    mon->ops = ops;
+    mon->opaque = opaque;
+
+    TAILQ_INSERT_TAIL(&charmonitors, mon, next);
+
+    return mon;
+}
+
+
+void qemu_chr_remove_monitor(CharMonitorState *mon)
+{
+    TAILQ_REMOVE(&charmonitors, mon, next);
+
+    qemu_free(mon);
+}
+
+
+int qemu_chr_iterate(qemu_chr_iterator iter,
+		     void *opaque)
+{
+    CharDriverState *chr;
+
+    TAILQ_FOREACH(chr, &chardevs, next) {
+	if ((*iter)(chr, opaque) < 0)
+	    return -1;
+    }
+    return 0;
+}
+
+CharDriverState *qemu_chr_fetch(const char *label)
+{
+    CharDriverState *chr;
+
+    TAILQ_FOREACH(chr, &chardevs, next) {
+	if (strcmp(chr->label, label) == 0)
+	    return chr;
+    }
+    return NULL;
+}
diff --git a/qemu-char.h b/qemu-char.h
index e1aa8db..94613e7 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -46,6 +46,19 @@ typedef struct {
 
 typedef void IOEventHandler(void *opaque, int event);
 
+struct CharCaptureOps {
+    void (*capture) (void *opaque, const void *buf, int size);
+    void (*destroy) (void *opaque);
+};
+
+struct CharMonitorOps {
+    void (*opened) (CharDriverState *s, void *opaque);
+    void (*closing) (CharDriverState *s, void *opaque);
+};
+
+typedef struct CharMonitorState CharMonitorState;
+typedef struct CharCaptureState CharCaptureState;
+
 struct CharDriverState {
     void (*init)(struct CharDriverState *s);
     int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
@@ -64,6 +77,8 @@ struct CharDriverState {
     char *label;
     char *filename;
     TAILQ_ENTRY(CharDriverState) next;
+
+    TAILQ_HEAD(CharCaptureStateHead, CharCaptureState) capture;
 };
 
 CharDriverState *qemu_chr_open(const char *label, const char *filename, void (*init)(struct CharDriverState *s));
@@ -84,6 +99,48 @@ void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
 void qemu_chr_accept_input(CharDriverState *s);
 void qemu_chr_info(Monitor *mon);
 
+/*
+ * Register to capture all data written to the character
+ * device.
+ */
+CharCaptureState *qemu_chr_add_capture(CharDriverState *s,
+				       struct CharCaptureOps *ops,
+				       void *opaque);
+
+/*
+ * Unregister a capture register on the character device
+ */
+void qemu_chr_del_capture(CharDriverState *s,
+			  CharCaptureState *c);
+
+
+/*
+ * Register to get notifications of the creation and deletion
+ * of character devices
+ */
+CharMonitorState *qemu_chr_add_monitor(struct CharMonitorOps *ops,
+				       void *opaque);
+/*
+ * Unregister notification of creation and deletion of
+ * character devices
+ */
+void qemu_chr_del_monitor(CharMonitorState *m);
+
+
+/*
+ * Iterate over all existing character devices
+ */
+typedef int (*qemu_chr_iterator)(CharDriverState *s,
+				 void *opaque);
+int qemu_chr_iterate(qemu_chr_iterator iter,
+		     void *opaque);
+
+/*
+ * Fetch a character device based on its assigned label
+ */
+CharDriverState *qemu_chr_fetch(const char *label);
+
+
 extern int term_escape_char;
 
 /* async I/O support */


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 2/2] VNC char device data stream tunnelling

[Daniel P. Berrange]

commit 71000d52fb4f9a4bef1f4841dd97561416fe5eed
Author: Daniel P. Berrange <berrange@redhat.com>
Date:   Wed Jul 1 17:03:46 2009 +0100

    Add a new VNC extension to allowing tunnelling of gemeric data streams.
    Map QEMU character devices onto VNC data streams

diff --git a/Makefile b/Makefile
index 2a4b3f3..a7de1a5 100644
--- a/Makefile
+++ b/Makefile
@@ -156,7 +156,7 @@ obj-y += $(addprefix audio/, $(audio-obj-y))
 obj-y += keymaps.o
 obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
 obj-$(CONFIG_CURSES) += curses.o
-obj-y += vnc.o acl.o d3des.o
+obj-y += vnc.o vnc-char.o acl.o d3des.o
 obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
 obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
 obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/vnc-char.c b/vnc-char.c
new file mode 100644
index 0000000..b7b4606
--- /dev/null
+++ b/vnc-char.c
@@ -0,0 +1,401 @@
+/*
+ * QEMU VNC display driver: Char device capture
+ *
+ * Copyright (C) 2009 Red Hat, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "vnc.h"
+
+/*
+ * Encoding #: -260
+ *
+ * The server notifies the client of available streams using the
+ * framebuffer update for our psuedo encoding #.
+ *
+ * The 'x' field is '0' for a device addition, '1' for removal
+ *
+ * The payload of the update is the device name
+ *  - u32 - length of device name
+ *  - u8 * len - text of device name, not including \0
+ *
+ * When client indicates it accepts the stream encoding,
+ * the server will send back a stream addition update for
+ * each initial device. Hotplug/unplug for streams will
+ * trigger further updates.
+ *
+ * For QEMU, the streams extension maps 1-stream to 1 char device
+ *
+ * Stream capture uses an new 'aliguori' message sub-type. There
+ * are 3 client -> server messages, and 3 server -> client messages
+ *
+ * Client starts capture by supplying a stream name. Server replies
+ * givng the stream name -> stream ID mapping. All further messages
+ * use the unique stream ID.
+ *
+ * Client -> server messages:
+ *
+ *   Message   == u8: 255 (aliguouri)
+ *   sub-type  == u8: 2  (streams)
+ *   operation == u8:
+ *        0 == start capture
+ *        1 == end capture
+ *        2 == send data
+ *
+ * For operation == start capture:
+ *   content:
+ *      u32 - length of device name
+ *      u8 *len - device name to capture
+ *
+ * For operation == end capture
+ *   content:
+ *      u32 - capture session ID
+ *
+ * For operation == send data
+ *   content:
+ *      u32 - capture session ID
+ *      u32 - length of data
+ *      u8*len - raw data
+ *
+ *
+ *
+ * Server -> client messages:
+ *
+ *   Message   == u8: 255 (aliguouri)
+ *   sub-type  == u8: 2  (streams)
+ *   operation == u8:
+ *        0 == start capture
+ *        1 == end capture
+ *        2 == send data
+ *
+ * For operation == start capture
+ *   content:
+ *      u32 - capture session ID
+ *      u32 - length of device name
+ *      u8 *len - device name to capture
+ *
+ * For operation == end capture
+ *   content:
+ *      u32 - capture session ID
+ *
+ * For operation == send data
+ *   content:
+ *      u32 - capture session ID
+ *      u32 - length of data
+ *      u8*len - raw data
+ */
+
+static void vnc_send_start_capture(VncState *client,
+				   VncCharCaptureState *chr);
+static void vnc_send_end_capture(VncState *client,
+				 VncCharCaptureState *chr);
+static void vnc_send_data_capture(VncState *client,
+				  VncCharCaptureState *chr,
+				  const void *buf,
+				  int len);
+
+
+static VncCharCaptureState *vnc_char_get_capture(VncState *client,
+						 int id)
+{
+    VncCharCaptureState *cap;
+
+    TAILQ_FOREACH(cap, &client->char_cap, next) {
+	if (cap->id == id)
+	    return cap;
+    }
+
+    return NULL;
+}
+
+
+/*
+ * From guest device to VNC client
+ */
+static void vnc_char_capture(void *opaque, const void *buf, int len)
+{
+    VncCharCaptureState *cap = opaque;
+
+    if (!cap)
+	return;
+
+    vnc_send_data_capture(cap->client, cap, buf, len);
+}
+
+static void vnc_char_destroy(void *opaque)
+{
+    VncCharCaptureState *cap = opaque;
+
+    if (!cap)
+	return;
+
+    qemu_chr_del_capture(cap->chr, cap->cap);
+    cap->chr = NULL;
+    cap->cap = NULL;
+}
+
+static struct CharCaptureOps vnc_char_capture_ops = {
+    vnc_char_capture,
+    vnc_char_destroy,
+};
+
+
+
+int vnc_char_add_capture(VncState *client,
+			 const char *chrname)
+{
+    VncCharCaptureState *cap;
+    CharDriverState *chr;
+
+    chr = qemu_chr_fetch("serial0");
+    if (!chr)
+	return -1;
+
+    cap = qemu_mallocz(sizeof(*cap));
+    cap->id = client->char_cap_next_id++;
+    cap->chr = chr;
+    cap->cap = qemu_chr_add_capture(chr, &vnc_char_capture_ops, cap);
+    cap->client = client;
+
+    TAILQ_INSERT_TAIL(&client->char_cap, cap, next);
+
+    vnc_send_start_capture(client, cap);
+
+    return cap->id;
+}
+
+void vnc_char_del_capture(VncState *client,
+			  int id)
+{
+    VncCharCaptureState *cap = vnc_char_get_capture(client, id);
+    if (!cap)
+	return;
+
+    TAILQ_REMOVE(&client->char_cap, cap, next);
+
+    if (cap->chr)
+	qemu_chr_del_capture(cap->chr, cap->cap);
+
+    vnc_send_end_capture(client, cap);
+
+    qemu_free(cap);
+}
+
+void vnc_char_clear_capture(VncState *client)
+{
+    VncCharCaptureState *cap, *tmp;
+
+    TAILQ_FOREACH_SAFE(cap, &client->char_cap, next, tmp) {
+	if (cap->chr)
+	    qemu_chr_del_capture(cap->chr, cap->cap);
+
+	qemu_free(cap);
+    }
+}
+
+
+/*
+ * From VNC client to guest device
+ */
+void vnc_char_write(VncState *client,
+		    int id,
+		    const void *buf,
+		    int len)
+{
+    VncCharCaptureState *cap = vnc_char_get_capture(client, id);
+    if (!cap)
+	return; /* XXX kill client */
+
+    if (cap->chr)
+	qemu_chr_write(cap->chr, buf, len);
+}
+
+
+static void
+vnc_framebuffer_update_stream(VncState *client,
+			      CharDriverState *chr,
+			      int deleted)
+{
+    if (strcmp(chr->label, "monitor") == 0)
+	return;
+
+    vnc_write_u8(client, 0);
+    vnc_write_u8(client, 0);
+    vnc_write_u16(client, 1);
+    vnc_framebuffer_update(client,
+			   deleted,
+			   0,
+			   ds_get_width(client->ds),
+			   ds_get_height(client->ds),
+                           VNC_ENCODING_STREAMS);
+    vnc_write_u32(client, strlen(chr->label));
+    vnc_write(client, chr->label, strlen(chr->label));
+}
+
+static int vnc_chr_iterator(CharDriverState *chr, void *opaque) {
+    VncState *client = opaque;
+
+    vnc_framebuffer_update_stream(client, chr, 0);
+    return 0;
+}
+
+
+static void vnc_chr_opened(CharDriverState *chr, void *opaque) {
+    VncState *client = opaque;
+
+    vnc_framebuffer_update_stream(client, chr, 0);
+    vnc_flush(client);
+}
+
+static void vnc_chr_closing(CharDriverState *chr, void *opaque) {
+    VncState *client = opaque;
+
+    vnc_framebuffer_update_stream(client, chr, 1);
+    vnc_flush(client);
+}
+
+static struct CharMonitorOps vnc_monops = {
+    vnc_chr_opened, vnc_chr_closing
+};
+
+void vnc_streams_ack(VncState *client)
+{
+    if (client->char_mon)
+	return;
+
+    qemu_chr_iterate(vnc_chr_iterator, client);
+    vnc_flush(client);
+
+    client->char_mon = qemu_chr_add_monitor(&vnc_monops, client);
+}
+
+
+static void vnc_send_start_capture(VncState *client,
+				   VncCharCaptureState *chr) {
+    vnc_write_u8(client, 255);
+    vnc_write_u8(client, 2);
+    vnc_write_u8(client, 0);
+
+    vnc_write_u32(client, chr->id);
+    vnc_write_u32(client, strlen(chr->chr->label));
+    vnc_write(client, chr->chr->label, strlen(chr->chr->label));
+    vnc_flush(client);
+}
+
+
+static void vnc_send_end_capture(VncState *client,
+				 VncCharCaptureState *chr) {
+    vnc_write_u8(client, 255);
+    vnc_write_u8(client, 2);
+    vnc_write_u8(client, 1);
+
+    vnc_write_u32(client, chr->id);
+    vnc_flush(client);
+}
+
+
+static void vnc_send_data_capture(VncState *client,
+				  VncCharCaptureState *chr,
+				  const void *data,
+				  int len) {
+    vnc_write_u8(client, 255);
+    vnc_write_u8(client, 2);
+    vnc_write_u8(client, 2);
+
+    vnc_write_u32(client, chr->id);
+    vnc_write_u32(client, len);
+    vnc_write(client, data, len);
+    vnc_flush(client);
+}
+
+
+int vnc_streams_msg(VncState *client, uint8_t *data, int len) {
+    if (len == 2)
+	return 3;
+
+    switch (read_u8(data, 2)) {
+    case 0: {
+	uint32_t n_name;
+	char *name;
+
+	if (len == 3)
+	    return 7;
+
+	n_name = read_u32(data, 3);
+	if (n_name > 4096) {
+	    vnc_client_error(client);
+	    break;
+	}
+
+	if (len == 7)
+	    return 7 + n_name;
+
+	name = qemu_malloc(n_name + 1);
+	memcpy(name, data + 7, n_name);
+	name[n_name] = '\0';
+
+	if (strcmp(name, "monitor") == 0) {
+	    vnc_client_error(client);
+	    qemu_free(name);
+	    break;
+	}
+
+	vnc_char_add_capture(client, name);
+	qemu_free(name);
+    } break;
+
+    case 1: {
+	uint32_t id;
+	if (len == 3)
+	    return 7;
+
+	id = read_u32(data, 3);
+
+	vnc_char_del_capture(client, id);
+    } break;
+
+    case 2: {
+	uint32_t id;
+	uint32_t n_data;
+	if (len == 3)
+	    return 11;
+
+	id = read_u32(data, 3);
+	n_data = read_u32(data, 7);
+
+	if (n_data > 4096) {
+	    vnc_client_error(client);
+	    break;
+	}
+
+	if (len == 11)
+	    return 11 + n_data;
+
+	vnc_char_write(client, id, data + 11, n_data);
+    } break;
+
+    default:
+	printf ("Invalid audio message %d\n", read_u8(data, 4));
+	vnc_client_error(client);
+	break;
+    }
+
+    return 0;
+}
diff --git a/vnc-char.h b/vnc-char.h
new file mode 100644
index 0000000..b421448
--- /dev/null
+++ b/vnc-char.h
@@ -0,0 +1,60 @@
+/*
+ * QEMU VNC display driver: Char device capture
+ *
+ * Copyright (C) 2009 Red Hat, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+
+#ifndef __QEMU_VNC_CHAR_H__
+#define __QEMU_VNC_CHAR_H__
+
+typedef struct VncCharCaptureState VncCharCaptureState;
+struct VncCharCaptureState
+{
+    int id;
+    CharDriverState *chr;
+    CharCaptureState *cap;
+
+    VncState *client;
+
+    TAILQ_ENTRY(VncCharCaptureState) next;
+};
+
+
+int vnc_char_add_capture(VncState *client,
+			 const char *chrname);
+
+void vnc_char_del_capture(VncState *client,
+			  int id);
+
+void vnc_char_clear_capture(VncState *client);
+
+void vnc_char_write(VncState *client,
+		    int id,
+		    const void *buf,
+		    int len);
+
+void vnc_streams_ack(VncState *client);
+
+int vnc_streams_msg(VncState *client, uint8_t *data, int len);
+
+
+#endif /* __QEMU_VNC_CHAR_H__ */
diff --git a/vnc.c b/vnc.c
index de0ff87..d73ed80 100644
--- a/vnc.c
+++ b/vnc.c
@@ -296,8 +296,8 @@ static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h)
     }
 }
 
-static void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
-                                   int32_t encoding)
+void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
+			    int32_t encoding)
 {
     vnc_write_u16(vs, x);
     vnc_write_u16(vs, y);
@@ -901,6 +901,8 @@ static void vnc_disconnect_finish(VncState *vs)
 #endif /* CONFIG_VNC_SASL */
     audio_del(vs);
 
+    vnc_char_clear_capture(vs);
+
     VncState *p, *parent = NULL;
     for (p = vs->vd->clients; p != NULL; p = p->next) {
         if (p == vs) {
@@ -1523,6 +1525,7 @@ static void send_ext_audio_ack(VncState *vs)
     vnc_flush(vs);
 }
 
+
 static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
 {
     int i;
@@ -1564,6 +1567,9 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
         case VNC_ENCODING_AUDIO:
             send_ext_audio_ack(vs);
             break;
+        case VNC_ENCODING_STREAMS:
+	    vnc_streams_ack(vs);
+            break;
         case VNC_ENCODING_WMVi:
             vs->features |= VNC_FEATURE_WMVI_MASK;
             break;
@@ -1706,6 +1712,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
 {
     int i;
     uint16_t limit;
+    int ret;
 
     switch (data[0]) {
     case 0:
@@ -1822,6 +1829,12 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
             }
             break;
 
+        case 2:
+	    ret = vnc_streams_msg(vs, data, len);
+	    if (ret)
+		return ret;
+	    break;
+
         default:
             printf("Msg: %d\n", read_u16(data, 0));
             vnc_client_error(vs);
@@ -2082,6 +2095,8 @@ static void vnc_connect(VncDisplay *vd, int csock)
     vnc_read_when(vs, protocol_version, 12);
     reset_keys(vs);
 
+    TAILQ_INIT(&vs->char_cap);
+
     vs->next = vd->clients;
     vd->clients = vs;
 
diff --git a/vnc.h b/vnc.h
index 3ae95f3..73d9ee5 100644
--- a/vnc.h
+++ b/vnc.h
@@ -31,6 +31,7 @@
 #include "console.h"
 #include "monitor.h"
 #include "audio/audio.h"
+#include "qemu-char.h"
 #include <zlib.h>
 
 #include "keymaps.h"
@@ -84,6 +85,7 @@ typedef struct VncDisplay VncDisplay;
 #include "vnc-auth-sasl.h"
 #endif
 
+#include "vnc-char.h"
 
 struct VncDisplay
 {
@@ -152,6 +154,10 @@ struct VncState
     CaptureVoiceOut *audio_cap;
     struct audsettings as;
 
+    TAILQ_HEAD(VncCharCaptureStateHead, VncCharCaptureState) char_cap;
+    int char_cap_next_id;
+    CharMonitorState *char_mon;
+
     VncReadEvent *read_handler;
     size_t read_handler_expect;
     /* input */
@@ -224,6 +230,7 @@ enum {
 #define VNC_ENCODING_POINTER_TYPE_CHANGE  0XFFFFFEFF /* -257 */
 #define VNC_ENCODING_EXT_KEY_EVENT        0XFFFFFEFE /* -258 */
 #define VNC_ENCODING_AUDIO                0XFFFFFEFD /* -259 */
+#define VNC_ENCODING_STREAMS              0XFFFFFEFC /* -260 */
 #define VNC_ENCODING_WMVi                 0x574D5669
 
 /*****************************************************************************
@@ -303,6 +310,9 @@ int vnc_client_io_error(VncState *vs, int ret, int last_errno);
 void start_client_init(VncState *vs);
 void start_auth_vnc(VncState *vs);
 
+void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
+			    int32_t encoding);
+
 /* Buffer management */
 void buffer_reserve(Buffer *buffer, size_t len);
 int buffer_empty(Buffer *buffer);

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 2/2] VNC char device data stream tunnelling

[Anthony Liguori]

Daniel P. Berrange wrote:
> +/*
> + * Encoding #: -260
> + *
> + * The server notifies the client of available streams using the
> + * framebuffer update for our psuedo encoding #.
> + *
> + * The 'x' field is '0' for a device addition, '1' for removal
> + *
> + * The payload of the update is the device name
> + *  - u32 - length of device name
> + *  - u8 * len - text of device name, not including \0
> + *
> + * When client indicates it accepts the stream encoding,
> + * the server will send back a stream addition update for
> + * each initial device. Hotplug/unplug for streams will
> + * trigger further updates.
> + *
> + * For QEMU, the streams extension maps 1-stream to 1 char device
> + *
> + * Stream capture uses an new 'aliguori' message sub-type. There
> + * are 3 client -> server messages, and 3 server -> client messages
> + *
> + * Client starts capture by supplying a stream name. Server replies
> + * givng the stream name -> stream ID mapping. All further messages
> + * use the unique stream ID.
> + *
> + * Client -> server messages:
> + *
> + *   Message   == u8: 255 (aliguouri)
> + *   sub-type  == u8: 2  (streams)
> + *   operation == u8:
> + *        0 == start capture
> + *        1 == end capture
> + *        2 == send data
> + *
> + * For operation == start capture:
> + *   content:
> + *      u32 - length of device name
> + *      u8 *len - device name to capture
> + *
> + * For operation == end capture
> + *   content:
> + *      u32 - capture session ID
> + *
> + * For operation == send data
> + *   content:
> + *      u32 - capture session ID
> + *      u32 - length of data
> + *      u8*len - raw data
> + *
> + *
> + *
> + * Server -> client messages:
> + *
> + *   Message   == u8: 255 (aliguouri)
> + *   sub-type  == u8: 2  (streams)
> + *   operation == u8:
> + *        0 == start capture
> + *        1 == end capture
> + *        2 == send data
> + *
> + * For operation == start capture
> + *   content:
> + *      u32 - capture session ID
> + *      u32 - length of device name
> + *      u8 *len - device name to capture
> + *
> + * For operation == end capture
> + *   content:
> + *      u32 - capture session ID
> + *
> + * For operation == send data
> + *   content:
> + *      u32 - capture session ID
> + *      u32 - length of data
> + *      u8*len - raw data
> + *

Acked-by: Anthony Liguori <aliguori@us.ibm.com> on the protocol encoding

Do you have gtk-vnc patches yet?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

If anyone wants to actually try this code out, the following patch is
a quick proof of concept hack against GTK-VNC[1] and enables a single data
stream for device 'serial0' just printing the received data to stdout.

eg, start QEMU with

  ./i386-softmmu/qemu -kernel ~/vmlinuz  -serial file:foo.log -append 'console=ttyS0' -vnc :1

And then start GTK-VNC with

  ./examples/gvncviewer localhost:1

If all goes to plan, gvncviewer should output the kernel boot messages,
and QEMU will be saving them in 'foo.log' as per its config.

Regards,
Daniel

[1] http://git.gnome.org/cgit/gtk-vnc/

diff --git a/src/gvnc.c b/src/gvnc.c
index c3b3163..7cafa0e 100644
--- a/src/gvnc.c
+++ b/src/gvnc.c
@@ -2144,6 +2144,39 @@ static void gvnc_ext_key_event(struct gvnc *gvnc)
 	gvnc->keycode_map = x_keycode_to_pc_keycode_map();
 }
 
+static void gvnc_stream_event(struct gvnc *gvnc, int deleted)
+{
+	int n_name = gvnc_read_u32(gvnc);
+	char *name;
+	if (gvnc->has_error)
+		return;
+	if (n_name > 4096) {
+		gvnc->has_error = TRUE;
+		return;
+	}
+	name = g_new(char, n_name + 1);
+	gvnc_read(gvnc, name, n_name);
+	name[n_name] = '\0';
+
+	fprintf(stderr, "Got stream %s '%s'\n", deleted ? "del" : "add", name);
+
+	if (strcmp(name, "serial0") == 0) {
+		gvnc_write_u8(gvnc, 255);
+		gvnc_write_u8(gvnc, 2);
+		if (deleted) {
+			gvnc_write_u8(gvnc, 1);
+			gvnc_write_u8(gvnc, 1); /* XXX id */
+		} else {
+			gvnc_write_u8(gvnc, 0);
+			gvnc_write_u32(gvnc, strlen(name));
+			gvnc_write(gvnc, name, strlen(name));
+		}
+		gvnc_flush(gvnc);
+	}
+
+	free(name);
+}
+
 static void gvnc_framebuffer_update(struct gvnc *gvnc, int32_t etype,
 				    uint16_t x, uint16_t y,
 				    uint16_t width, uint16_t height)
@@ -2195,6 +2228,10 @@ static void gvnc_framebuffer_update(struct gvnc *gvnc, int32_t etype,
 	case GVNC_ENCODING_EXT_KEY_EVENT:
 		gvnc_ext_key_event(gvnc);
 		break;
+	case GVNC_ENCODING_STREAM_EVENT:
+		fprintf(stderr, "Stream event\n");
+		gvnc_stream_event(gvnc, x);
+		break;
 	default:
 		GVNC_DEBUG("Received an unknown encoding type: %d\n", etype);
 		gvnc->has_error = TRUE;
@@ -2295,6 +2332,58 @@ gboolean gvnc_server_message(struct gvnc *gvnc)
 		gvnc_server_cut_text(gvnc, data, n_text);
 		g_free(data);
 	}	break;
+	case 255: {
+		int subtype;
+		subtype = gvnc_read_u8(gvnc);
+		if (subtype != 2) {
+			GVNC_DEBUG("Unknown aliguori message type %d\n", subtype);
+			gvnc->has_error = TRUE;
+			break;
+		}
+		switch (gvnc_read_u8(gvnc)) {
+		case 0: {
+			int id;
+			int n_name;
+			char *name;
+			id = gvnc_read_u32(gvnc);
+			n_name = gvnc_read_u32(gvnc);
+			if (n_name > 4096) {
+				GVNC_DEBUG("Stream name too long %d", n_name);
+				gvnc->has_error = TRUE;
+				break;
+			}
+			name = g_new(char, n_name + 1);
+			gvnc_read(gvnc, name, n_name);
+			name[n_name] = 0;
+			fprintf(stderr, "Stream on %d %s\n", id, name);
+			g_free(name);
+		}	break;
+
+		case 1: {
+			int id;
+			id = gvnc_read_u32(gvnc);
+			fprintf(stderr, "Stream off %d\n", id);
+		}	break;
+
+		case 2: {
+			int id;
+			int n_data;
+			char *data;
+			id = gvnc_read_u32(gvnc);
+			n_data = gvnc_read_u32(gvnc);
+			if (n_data > 4096) {
+				GVNC_DEBUG("Stream data too long %d", n_data);
+				gvnc->has_error = TRUE;
+				break;
+			}
+			data = g_new(char, n_data + 1);
+			gvnc_read(gvnc, data, n_data);
+			data[n_data] = 0;
+			fprintf(stderr, "%s", data);
+			g_free(data);
+		}	break;
+		}
+	}       break;
 	default:
 		GVNC_DEBUG("Received an unknown message: %u\n", msg);
 		gvnc->has_error = TRUE;
diff --git a/src/gvnc.h b/src/gvnc.h
index f748de0..4c3d214 100644
--- a/src/gvnc.h
+++ b/src/gvnc.h
@@ -117,6 +117,7 @@ typedef enum {
 
 	GVNC_ENCODING_POINTER_CHANGE = -257,
 	GVNC_ENCODING_EXT_KEY_EVENT = -258,
+	GVNC_ENCODING_STREAM_EVENT = -260,
 } gvnc_encoding;
 
 typedef enum {
diff --git a/src/vncdisplay.c b/src/vncdisplay.c
index 6c7b662..da42434 100644
--- a/src/vncdisplay.c
+++ b/src/vncdisplay.c
@@ -1306,6 +1306,7 @@ static void *vnc_coroutine(void *opaque)
 	/* this order is extremely important! */
 	int32_t encodings[] = {	GVNC_ENCODING_TIGHT_JPEG5,
 				GVNC_ENCODING_TIGHT,
+				GVNC_ENCODING_STREAM_EVENT,
 				GVNC_ENCODING_EXT_KEY_EVENT,
 				GVNC_ENCODING_DESKTOP_RESIZE,
                                 GVNC_ENCODING_WMVi,


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Gerd Hoffmann]

   Hi,

 > The key requirement here is that it should not
> be neccessary to specifically configure each character device to make
> it available via VNC.  The admin should be able to configure the char
> devices with all current available backends (file, pty, null, tcp, udp,
> unix, etc), and regardless of this config be able to snoop on data from
> any active VNC client on demand.

Hmm.  What is the reason to handle vnc different from 
unix/tcp/pty/whatever?  I would expect something like '-serial 
vnc,name=foo' here ...

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

On Wed, Jul 01, 2009 at 06:42:31PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > The key requirement here is that it should not
> >be neccessary to specifically configure each character device to make
> >it available via VNC.  The admin should be able to configure the char
> >devices with all current available backends (file, pty, null, tcp, udp,
> >unix, etc), and regardless of this config be able to snoop on data from
> >any active VNC client on demand.
> 
> Hmm.  What is the reason to handle vnc different from 
> unix/tcp/pty/whatever?  I would expect something like '-serial 
> vnc,name=foo' here ...

As I tried to explain, I want to be able to configure the serial devices 
using the existing backends *AND* also have the ability to occassionally 
access the data using a VNC client. VNC is not intended to be the main
character device backend here - it is just a way to snooping on the data
being sent to another backend. For the same reason, audio capture over
VNC does not impact the way you configure sound cards to use SDL/pulseaudio
alsa/esd/etc, it just gives you the means to capture the audio data  when
connected with VNC.

You could also envision QEMU getting the ability to add/remove display
backends like SDL/VNC on the fly. Thus VNC might not even be configured
when starting up the guest and specifying character device backend
configs. The capture approach deals with that problem trivially.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Gerd Hoffmann]

On 07/01/09 18:50, Daniel P. Berrange wrote:
> On Wed, Jul 01, 2009 at 06:42:31PM +0200, Gerd Hoffmann wrote:
>>    Hi,
>>
>>> The key requirement here is that it should not
>>> be neccessary to specifically configure each character device to make
>>> it available via VNC.  The admin should be able to configure the char
>>> devices with all current available backends (file, pty, null, tcp, udp,
>>> unix, etc), and regardless of this config be able to snoop on data from
>>> any active VNC client on demand.
>> Hmm.  What is the reason to handle vnc different from
>> unix/tcp/pty/whatever?  I would expect something like '-serial
>> vnc,name=foo' here ...
>
> As I tried to explain, I want to be able to configure the serial devices
> using the existing backends *AND* also have the ability to occassionally
> access the data using a VNC client.

That doesn't answer the question why vnc should be different.

I *do* see the point of being able to have more than one way to access a 
chardev at the same time.  But when building up that infrastructure: 
Why stop half-way through and make it a special hack for vnc?  I think 
it would be *far* more useful to make chardevs in qemu work that way all 
the time.

Each serial-type driver gets a name tag (either user-specified or 
autogenerated).  chardev backends connect by name.  Advantages:

   (1) It is a step forward to a clear split of guest and host state.
   (2) Better test coverage because everybody uses the same code paths.
   (3) You can connect multiple backends to the chardev.  For example
       you can log the console to a file and work interactively via
       pty at the same time.
   (4) We can allow multiple connections for unix/tcp sockets (pretty
       much like the vnc server handles multiple clients today).

Bonus points for making that configurable via monitor, so you can start 
console logging on the fly.  serial console via vnc data stream then 
will be just the sugar on top ;)

> VNC is not intended to be the main
> character device backend here

I'd like to question the concept of a "main chardev backend".

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

On Wed, Jul 01, 2009 at 07:30:23PM +0200, Gerd Hoffmann wrote:
> On 07/01/09 18:50, Daniel P. Berrange wrote:
> >On Wed, Jul 01, 2009 at 06:42:31PM +0200, Gerd Hoffmann wrote:
> >>   Hi,
> >>
> >>>The key requirement here is that it should not
> >>>be neccessary to specifically configure each character device to make
> >>>it available via VNC.  The admin should be able to configure the char
> >>>devices with all current available backends (file, pty, null, tcp, udp,
> >>>unix, etc), and regardless of this config be able to snoop on data from
> >>>any active VNC client on demand.
> >>Hmm.  What is the reason to handle vnc different from
> >>unix/tcp/pty/whatever?  I would expect something like '-serial
> >>vnc,name=foo' here ...
> >
> >As I tried to explain, I want to be able to configure the serial devices
> >using the existing backends *AND* also have the ability to occassionally
> >access the data using a VNC client.
> 
> That doesn't answer the question why vnc should be different.
> 
> I *do* see the point of being able to have more than one way to access a 
> chardev at the same time.  But when building up that infrastructure: 
> Why stop half-way through and make it a special hack for vnc?  I think 
> it would be *far* more useful to make chardevs in qemu work that way all 
> the time.

Ok, if i'm understanding correctly, you're suggesting we change the way
character devices are configured on the CLI, to be split like the NIC 
device args

eg, instead of

  -serial file:/tmp/foo.log,name=serial0

we'd do

  -serial name=serial0  -chardev name=serial0,file:/tmp/foo.log

And allow  an 1-m mapping for serial -> chardev args 

This could perhaps be done by having a 'CharDriverState' implementation
that was a multiplexor, and allow further CharDriverState instances to
be added/removed to this multiplexor on the fly. 

Thus the CharCaptureOps/State bit of my patch would just be another
CharDriverState impl, that gets added/removed on the fly. The
CharMonitorOps/State bit would still be needed to allow monitoring
of device creation/deletion.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Gerd Hoffmann]

On 07/01/09 20:50, Daniel P. Berrange wrote:
> On Wed, Jul 01, 2009 at 07:30:23PM +0200, Gerd Hoffmann wrote:
>> I *do* see the point of being able to have more than one way to access a
>> chardev at the same time.  But when building up that infrastructure:
>> Why stop half-way through and make it a special hack for vnc?  I think
>> it would be *far* more useful to make chardevs in qemu work that way all
>> the time.
>
> Ok, if i'm understanding correctly, you're suggesting we change the way
> character devices are configured on the CLI, to be split like the NIC
> device args
>
> eg, instead of
>
>    -serial file:/tmp/foo.log,name=serial0
>
> we'd do
>
>    -serial name=serial0  -chardev name=serial0,file:/tmp/foo.log
>
> And allow  an 1-m mapping for serial ->  chardev args

Yes.

> This could perhaps be done by having a 'CharDriverState' implementation
> that was a multiplexor, and allow further CharDriverState instances to
> be added/removed to this multiplexor on the fly.

Hmm, don't like that idea.

> Thus the CharCaptureOps/State bit of my patch would just be another
> CharDriverState impl, that gets added/removed on the fly.

I'd keep the CharCaptureOps/State and CharDriverState separation as it 
is now.  CharCapture would probably better named CharBackend or simliar. 
  CharDriverState would simply dispatch to all CharBackends connected.

We can keep the old code path working for a while.  So we can switch 
users one by one to the new way.  When done zap the old one and make 
CharDiverState a pure dispatcher for the Backends.

> The
> CharMonitorOps/State bit would still be needed to allow monitoring
> of device creation/deletion.

Yes.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Anthony Liguori]

Gerd Hoffmann wrote:
> On 07/01/09 18:50, Daniel P. Berrange wrote:
>> On Wed, Jul 01, 2009 at 06:42:31PM +0200, Gerd Hoffmann wrote:
>>>    Hi,
>>>
>>>> The key requirement here is that it should not
>>>> be neccessary to specifically configure each character device to make
>>>> it available via VNC.  The admin should be able to configure the char
>>>> devices with all current available backends (file, pty, null, tcp, 
>>>> udp,
>>>> unix, etc), and regardless of this config be able to snoop on data 
>>>> from
>>>> any active VNC client on demand.
>>> Hmm.  What is the reason to handle vnc different from
>>> unix/tcp/pty/whatever?  I would expect something like '-serial
>>> vnc,name=foo' here ...
>>
>> As I tried to explain, I want to be able to configure the serial devices
>> using the existing backends *AND* also have the ability to occassionally
>> access the data using a VNC client.
>
> That doesn't answer the question why vnc should be different.
>
> I *do* see the point of being able to have more than one way to access 
> a chardev at the same time.  But when building up that infrastructure: 
> Why stop half-way through and make it a special hack for vnc?  I think 
> it would be *far* more useful to make chardevs in qemu work that way 
> all the time.
>
> Each serial-type driver gets a name tag (either user-specified or 
> autogenerated).  chardev backends connect by name.  Advantages:

Dynamically changing the character devices backends should be a not too 
terrible thing to do today and I agree this is the right approach.

Allowing multiple readers/writers for character devices is much more 
complex to get right though.  The current chardev API does not have good 
semantics for just handling one client that connects and disconnects.  
It certainly wouldn't gracefully handle multiple client states at once.

You could do something like a mirrored mode but things get really ugly 
when dealing with multiple read-write clients.  Think of how that would 
behave with the monitor for instance.

I agree that for something like -monitor tcp::1025,server,nowait, you 
want to be able to connect multiple times and get multiple monitor 
sessions.  We would need a serious overhaul of the character device 
infrastructure for that (and that's certainly long overdue).  So IMHO, 
this sort of refactoring shouldn't be a requirement for this patch series.

That doesn't mean we want to take a VNC specific hack though.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Gerd Hoffmann]

On 07/01/09 20:51, Anthony Liguori wrote:
> Allowing multiple readers/writers for character devices is much more
> complex to get right though. The current chardev API does not have good
> semantics for just handling one client that connects and disconnects. It
> certainly wouldn't gracefully handle multiple client states at once.
>
> You could do something like a mirrored mode but things get really ugly
> when dealing with multiple read-write clients.

I'm thinking about mirrored mode of course.  Everything else doesn't 
make sense for guest devices.  And you have that issue with Dan's 
current code already: -serial unix:, then connect via socket and via 
vnc.  Voila, two read-write clients.

> Think of how that would
> behave with the monitor for instance.

> I agree that for something like -monitor tcp::1025,server,nowait, you
> want to be able to connect multiple times and get multiple monitor
> sessions.

Monitor is a special case.  Multiple connections to the same session are 
not very useful there.  Multiple sessions are a different (albeit 
related) problem.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Anthony Liguori]

Gerd Hoffmann wrote:
> Monitor is a special case.  Multiple connections to the same session 
> are not very useful there.  Multiple sessions are a different (albeit 
> related) problem.

Serial is the same.  Imagine a bash shell running on the serial port 
with two VNC client connected and stdio connected.  Utter chaos.  You 
really want to use one or the other, never both at the same time.  If 
that's the case, then just switch the character device on the fly.  
Don't pretend that we can arbitrate multiple clients at once.

Regards,

Anthony Liguori
> cheers,
>   Gerd
>

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Gerd Hoffmann]

On 07/01/09 21:59, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>> Monitor is a special case. Multiple connections to the same session
>> are not very useful there. Multiple sessions are a different (albeit
>> related) problem.
>
> Serial is the same. Imagine a bash shell running on the serial port with
> two VNC client connected and stdio connected. Utter chaos. You really
> want to use one or the other, never both at the same time.

Typing to both at the same time isn't going to work well indeed.  IMHO 
that isn't a reason to enforce one connection only though.  It is still 
a useful feature.  Use case:

vm running at your workstation in the office, with -serial tcp.  You are 
heading home, leaving telnet connected.  At home you'll find you want to 
check something in your vm via vpn.

With current qemu:  You have to zap the telnet session somehow to be 
able to connect.

With switching:  You have to talk to the monitor to reconfigure things.

With multiple connections and multiplexing:  You'll just connect, type a 
few commands, disconnect, done.  You'll even see what you have done when 
you come back to the office the next day.

Also note that the vnc server accepts input from multiple clients as 
well, which can lead to simliar problems.  Nobody wants to kill support 
for multiple clients just because of that, because in practice it isn't 
a issue.


Monitor is different for two reasons:

First, we could actually open a new session.  That wouldn't work for 
serial as we can't hotplug a serial line into the guest on connect.

Second, if the monitor is used by libvirt or some other management app a 
second connection to the same session is seriously harmful.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Anthony Liguori]

Gerd Hoffmann wrote:

> With multiple connections and multiplexing:  You'll just connect, type 
> a few commands, disconnect, done.  You'll even see what you have done 
> when you come back to the office the next day.
>
> Also note that the vnc server accepts input from multiple clients as 
> well, which can lead to simliar problems.  Nobody wants to kill 
> support for multiple clients just because of that, because in practice 
> it isn't a issue.

I don't disagree with your use-case.  What I'm pointing out, is that we 
need to do some major surgery to get there.

The current chardev protocol has no real notion of flow control and 
assumes a static bidirectional channel between the front-end and 
back-end.  Flow control policy is inconsistent and those decisions are 
made in the back-end (i.e. tcp:).

We really need to push flow-control to be a proper part of the protocol 
though so that the front-end can participate.  For instance, 
virtio-console/serial is fully capable of implementing flow control.  
Punting buffering policy to the guest is IMHO the best possible solution 
to the general problem.

Having multiple connections within a chardev really also needs to be 
part of the monitor protocol because it's fundamental to flow-control.  
We want the front-end to have enough information to know how many 
clients are connected and which ones are ready to receive.

When dealing with a single I/O source, it should be front-end policy to 
determine what to do.  Certainly, there are some front-ends (like the 
monitor) that can support multiple I/O sources.  Another example is the 
VNC server which really should be using chardevs.

I agree that mirroring is a pretty reasonable policy to map a single I/O 
source to multiple clients for some devices.  That should be a front-end 
decision though.

In this model, I still don't see having two different back-ends 
connected to a single front-end.  I don't know if I agree there's a 
whole lot of value in that although the implementation is straight 
forward (a special back-end that unifies multiple front-ends as sessions).

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Gerd Hoffmann]

On 07/01/09 23:32, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
>
>> With multiple connections and multiplexing: You'll just connect, type
>> a few commands, disconnect, done. You'll even see what you have done
>> when you come back to the office the next day.
>>
>> Also note that the vnc server accepts input from multiple clients as
>> well, which can lead to simliar problems. Nobody wants to kill support
>> for multiple clients just because of that, because in practice it
>> isn't a issue.
>
> I don't disagree with your use-case. What I'm pointing out, is that we
> need to do some major surgery to get there.

I didn't say it is easy ;)

But I want to make sure we don't miss the big picture when we start 
multiplexing for vnc, so others can join the party.

> [ flow control issues ]

Tricky indeed.  I think Dan's current patches don't care about flow 
control at all.

> I agree that mirroring is a pretty reasonable policy to map a single I/O
> source to multiple clients for some devices. That should be a front-end
> decision though.

Dan's patches can do mirroring only.

I *think* one could build a tcp backend based on Dan's patches which can 
handle multiple connections at the same time.  It would basically do the 
same vnc data streams do:  Listen for connects, when a new connection 
comes in create a new CharCaptureState instance and link it up.  When 
the connection drops teardown.

> In this model, I still don't see having two different back-ends
> connected to a single front-end.

The frontend shouldn't have to care at all about who owns the 
CharCaptureState instances it feeds.

> I don't know if I agree there's a whole
> lot of value in that

Having a file backend writing logs and some other backend for 
interactive work is a very reasonable thing IMHO.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Jamie Lokier]

Gerd Hoffmann wrote:
> vm running at your workstation in the office, with -serial tcp.  You are 
> heading home, leaving telnet connected.  At home you'll find you want to 
> check something in your vm via vpn.
> 
> With current qemu:  You have to zap the telnet session somehow to be 
> able to connect.
> 
> With switching:  You have to talk to the monitor to reconfigure things.
> 
> With multiple connections and multiplexing:  You'll just connect, type a 
> few commands, disconnect, done.  You'll even see what you have done when 
> you come back to the office the next day.

You should have run telnet inside GNU screen :-)

Or since you have VNC, VNC to your desktop instead of the guest :-)

> Monitor is different for two reasons:
> 
> First, we could actually open a new session.  That wouldn't work for 
> serial as we can't hotplug a serial line into the guest on connect.

virtio-serial/vmchannel...  It could handle that if we wanted to.  If
nothing else, as a hotplug PCI serial port, and let the guest's udev
attach a getty.  I doubt anyone would bother, but perhaps if it's
useful for last-resort guest recovery.

> Second, if the monitor is used by libvirt or some other management app a 
> second connection to the same session is seriously harmful.

Quite so!  Right now, I deal with this (with a different management
app) by the exciting method of a qemu monitor proxy written in Perl,
which multiplexes multiple monitor client sessions onto QEMU's single
session.  It works quite well.

-- Jamie

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

On Wed, Jul 01, 2009 at 02:59:23PM -0500, Anthony Liguori wrote:
> Gerd Hoffmann wrote:
> >Monitor is a special case.  Multiple connections to the same session 
> >are not very useful there.  Multiple sessions are a different (albeit 
> >related) problem.
> 
> Serial is the same.  Imagine a bash shell running on the serial port 
> with two VNC client connected and stdio connected.  Utter chaos.  You 
> really want to use one or the other, never both at the same time.  If 
> that's the case, then just switch the character device on the fly.  
> Don't pretend that we can arbitrate multiple clients at once.

This isn't really any different to have 2 VNC clients connected at the
same time and using the same Xorg/gnome-terminal instance concurrently.
There's no QEMU level technical problem with mirroring updates to multiple 
clients. For it to be useful though, it does require that the people/thing 
using each client is 'intelligent', otherwise they'll just end up fighting
with each other. So I think this is more of a policy decision - for both
VNC clients on the graphical console, and multiple char device clients -
for each device you could designate 'exclusive' (only one backend at a time),
"exclusive read/write+shared readonly (one backend can read+write, others 
are forced read only), or 'shared read/write' (every backend can read+write).
This kind of policy is done by many traditional VNC servers, and we could
add it to QEMU without too much trouble I reckon.

For a backend that's intended to be used by some program/service (eg RPC
message bus),  then exlusive would clearly be required. For a backend 
that's intended to be used interactively by a real person, then  any of
the policies could be practical.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Anthony Liguori]

Daniel P. Berrange wrote:
> The following two patches make it possible to tunnel character devices
> over VNC, using a new VNC extension. This is motivated by the existing
> QEMU support for tunnelling audio streams over VNC, and the code follows
> a very similar design.  The key requirement here is that it should not
> be neccessary to specifically configure each character device to make
> it available via VNC. The admin should be able to configure the char
> devices with all current available backends (file, pty, null, tcp, udp,
> unix, etc), and regardless of this config be able to snoop on data from
> any active VNC client on demand.
>   

Shouldn't it just be the character devices put on vc's?

Does snooping imply read-only?  That would be pretty unfortunate.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

On Wed, Jul 01, 2009 at 01:36:23PM -0500, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >The following two patches make it possible to tunnel character devices
> >over VNC, using a new VNC extension. This is motivated by the existing
> >QEMU support for tunnelling audio streams over VNC, and the code follows
> >a very similar design.  The key requirement here is that it should not
> >be neccessary to specifically configure each character device to make
> >it available via VNC. The admin should be able to configure the char
> >devices with all current available backends (file, pty, null, tcp, udp,
> >unix, etc), and regardless of this config be able to snoop on data from
> >any active VNC client on demand.
> >  
> 
> Shouldn't it just be the character devices put on vc's?

The 'vc' concept is a stateful one, requiring the user to switch betweeen
channels statically and is opaque to VNC clients - all they see is a
framebuffer with no idea that QEMU has this magic sequence to change
what the framebuffer displays, nor what vc's are available. The idea of
this extension is to make data streams a protocol level concept so they 
can be more intelligently handled by the client, with the VNC protocol 
extension not being QEMU specific, so our VNC servers could make use of 
this in ways that suits.

> Does snooping imply read-only?  That would be pretty unfortunate.

No, its fully read-write. My gtk-vnc client so far only does reads,
but the QEMU server allows read+write - see the comments about the
protocol messages in the 2nd patch for details.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Wed, Jul 01, 2009 at 01:36:23PM -0500, Anthony Liguori wrote:
>   
>> Daniel P. Berrange wrote:
>>     
>>> The following two patches make it possible to tunnel character devices
>>> over VNC, using a new VNC extension. This is motivated by the existing
>>> QEMU support for tunnelling audio streams over VNC, and the code follows
>>> a very similar design.  The key requirement here is that it should not
>>> be neccessary to specifically configure each character device to make
>>> it available via VNC. The admin should be able to configure the char
>>> devices with all current available backends (file, pty, null, tcp, udp,
>>> unix, etc), and regardless of this config be able to snoop on data from
>>> any active VNC client on demand.
>>>  
>>>       
>> Shouldn't it just be the character devices put on vc's?
>>     
>
> The 'vc' concept is a stateful one, requiring the user to switch betweeen
> channels statically and is opaque to VNC clients - all they see is a
> framebuffer with no idea that QEMU has this magic sequence to change
> what the framebuffer displays, nor what vc's are available.

I understand what you're suggesting, but I think the right way to handle 
this use-case is to allow character devices to be redirected after 
initial open making them truly dynamic.

This would be useful outside of VNC too.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

On Wed, Jul 01, 2009 at 01:47:12PM -0500, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >On Wed, Jul 01, 2009 at 01:36:23PM -0500, Anthony Liguori wrote:
> >  
> >>Daniel P. Berrange wrote:
> >>    
> >>>The following two patches make it possible to tunnel character devices
> >>>over VNC, using a new VNC extension. This is motivated by the existing
> >>>QEMU support for tunnelling audio streams over VNC, and the code follows
> >>>a very similar design.  The key requirement here is that it should not
> >>>be neccessary to specifically configure each character device to make
> >>>it available via VNC. The admin should be able to configure the char
> >>>devices with all current available backends (file, pty, null, tcp, udp,
> >>>unix, etc), and regardless of this config be able to snoop on data from
> >>>any active VNC client on demand.
> >>> 
> >>>      
> >>Shouldn't it just be the character devices put on vc's?
> >>    
> >
> >The 'vc' concept is a stateful one, requiring the user to switch betweeen
> >channels statically and is opaque to VNC clients - all they see is a
> >framebuffer with no idea that QEMU has this magic sequence to change
> >what the framebuffer displays, nor what vc's are available.
> 
> I understand what you're suggesting, but I think the right way to handle 
> this use-case is to allow character devices to be redirected after 
> initial open making them truly dynamic.

The 'vc' reference was puzzelling to me there, but I think you're
basically suggesting the same thing as Gerd is ? To fully de-couple
the device specification vs backend configuration

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Wed, Jul 01, 2009 at 01:47:12PM -0500, Anthony Liguori wrote:
>   
>> Daniel P. Berrange wrote:
>>     
>>> On Wed, Jul 01, 2009 at 01:36:23PM -0500, Anthony Liguori wrote:
>>>  
>>>       
>>>> Daniel P. Berrange wrote:
>>>>    
>>>>         
>>>>> The following two patches make it possible to tunnel character devices
>>>>> over VNC, using a new VNC extension. This is motivated by the existing
>>>>> QEMU support for tunnelling audio streams over VNC, and the code follows
>>>>> a very similar design.  The key requirement here is that it should not
>>>>> be neccessary to specifically configure each character device to make
>>>>> it available via VNC. The admin should be able to configure the char
>>>>> devices with all current available backends (file, pty, null, tcp, udp,
>>>>> unix, etc), and regardless of this config be able to snoop on data from
>>>>> any active VNC client on demand.
>>>>>
>>>>>      
>>>>>           
>>>> Shouldn't it just be the character devices put on vc's?
>>>>    
>>>>         
>>> The 'vc' concept is a stateful one, requiring the user to switch betweeen
>>> channels statically and is opaque to VNC clients - all they see is a
>>> framebuffer with no idea that QEMU has this magic sequence to change
>>> what the framebuffer displays, nor what vc's are available.
>>>       
>> I understand what you're suggesting, but I think the right way to handle 
>> this use-case is to allow character devices to be redirected after 
>> initial open making them truly dynamic.
>>     
>
> The 'vc' reference was puzzelling to me there, but I think you're
> basically suggesting the same thing as Gerd is ? To fully de-couple
> the device specification vs backend configuration
>   

Well, I'm saying, a character device should be exposed via VNC IIF it's 
connected to a 'vc' backend device.  In order to satisfy your use-case, 
I'd also suggest decoupling the monitor front-end's from back-ends so 
that you can reconnect a character device to a 'vc' if you want to.

The easiest way (albeit a little ugly) to do this would be to create a 
pluggable character device that proxied to another character device.  
Change qemu_chr_open() to return qemu_chr_open_proxy(chr, label).

You can then enumerate proxies by label and retarget the proxy to a 
different device.

Regards,

Anthony Liguori

> Daniel
>   

Re: [Qemu-devel] [PATCH 0/2] Tunnel character device data over VNC (v1)

[Daniel P. Berrange]

On Wed, Jul 01, 2009 at 02:11:23PM -0500, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >On Wed, Jul 01, 2009 at 01:47:12PM -0500, Anthony Liguori wrote:
> >  
> >>Daniel P. Berrange wrote:
> >>    
> >>>On Wed, Jul 01, 2009 at 01:36:23PM -0500, Anthony Liguori wrote:
> >>> 
> >>>      
> >>>>Daniel P. Berrange wrote:
> >>>>   
> >>>>        
> >>>>>The following two patches make it possible to tunnel character devices
> >>>>>over VNC, using a new VNC extension. This is motivated by the existing
> >>>>>QEMU support for tunnelling audio streams over VNC, and the code 
> >>>>>follows
> >>>>>a very similar design.  The key requirement here is that it should not
> >>>>>be neccessary to specifically configure each character device to make
> >>>>>it available via VNC. The admin should be able to configure the char
> >>>>>devices with all current available backends (file, pty, null, tcp, udp,
> >>>>>unix, etc), and regardless of this config be able to snoop on data from
> >>>>>any active VNC client on demand.
> >>>>>
> >>>>>     
> >>>>>          
> >>>>Shouldn't it just be the character devices put on vc's?
> >>>>   
> >>>>        
> >>>The 'vc' concept is a stateful one, requiring the user to switch betweeen
> >>>channels statically and is opaque to VNC clients - all they see is a
> >>>framebuffer with no idea that QEMU has this magic sequence to change
> >>>what the framebuffer displays, nor what vc's are available.
> >>>      
> >>I understand what you're suggesting, but I think the right way to handle 
> >>this use-case is to allow character devices to be redirected after 
> >>initial open making them truly dynamic.
> >>    
> >
> >The 'vc' reference was puzzelling to me there, but I think you're
> >basically suggesting the same thing as Gerd is ? To fully de-couple
> >the device specification vs backend configuration
> >  
> 
> Well, I'm saying, a character device should be exposed via VNC IIF it's 
> connected to a 'vc' backend device.  In order to satisfy your use-case, 
> I'd also suggest decoupling the monitor front-end's from back-ends so 
> that you can reconnect a character device to a 'vc' if you want to.

Ah, now I see what you mean :-) FYI, the two main deployment scenarios 
I am thinking of are

 - Serial port connected to a /dev/pts in host (eg, so host admin can
   locally run minicom / virsh console to interact with guest,
   eg QEMU is launched with -serial pty

 - Serial port connected to a file to log all data that is transmitted
   eg QEMU is launched with -serial file:/var/log/qemu/foo.log

Dynamically, rebinding backend to the 'vc' in order to access it from
a VNC client would certainly be doable in the first scenario, as there
is no benefit in keeping it accessible via the pty, while its used over 
VNC.

I'm not sure if dynamically re-binding is sufficiently flexible for the
second case, since you'd get an interruption in data logging to the file.
Similarly if using the 'udp' char driver to log to a netconsole server.
Perhaps we could just live with that restriction for the short term
though, and worry about full concurrent usage at a later date.

> The easiest way (albeit a little ugly) to do this would be to create a 
> pluggable character device that proxied to another character device.  
> Change qemu_chr_open() to return qemu_chr_open_proxy(chr, label).
> 
> You can then enumerate proxies by label and retarget the proxy to a 
> different device.

That sounds doable to me - I'll have a go at changing it to work that
way.


Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|