spice: add chardev (v3)

Adding a chardev backend for spice, for usage by spice vdagent in
conjunction with a properly named virtio-serial device.

Example usage:
 qemu -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -devic

This is equivalent to the old:
 qemu -device virtio-serial -device spicevmc,subtype=vdagent

longer to write, but generated by libvirt, and requires one less device.

v1->v3 changes: (v2 had a wrong commit message)
 * removed spice-qemu-char.h, folded into ui/qemu-spice.h
 * removed dead IOCTL code
 * removed comment
 * removed ifdef CONFIG_SPICE from qemu-config.c and qemu-options.hx help.
---
 Makefile.objs     |    2 +-
 qemu-char.c       |    4 +
 qemu-config.c     |    6 ++
 qemu-options.hx   |   16 ++++-
 spice-qemu-char.c |  185 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ui/qemu-spice.h   |    3 +
 6 files changed, 214 insertions(+), 2 deletions(-)
 create mode 100644 spice-qemu-char.c

On 12/17/2010 07:39 AM, Alon Levy wrote:
> Adding a chardev backend for spice, for usage by spice vdagent in
> conjunction with a properly named virtio-serial device.
>
> Example usage:
>   qemu -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -devic
>
> This is equivalent to the old:
>   qemu -device virtio-serial -device spicevmc,subtype=vdagent
>
> longer to write, but generated by libvirt, and requires one less device.
>
> v1->v3 changes: (v2 had a wrong commit message)
>   * removed spice-qemu-char.h, folded into ui/qemu-spice.h
>   * removed dead IOCTL code
>   * removed comment
>   * removed ifdef CONFIG_SPICE from qemu-config.c and qemu-options.hx help.
>    

What doe this channel do?

I really don't feel comfortable with this.  This is not connecting QEMU 
to an existing interface that happens to fit our model.

This is clearly a "library" that provides thin wrappers around internal 
QEMU interfaces to implement code that belongs in QEMU outside of QEMU.

It's essentially a static plugin.  It's the same problem with QXL.  I 
don't think we should be in the business of having thin shims to 
external libraries when the only reason to have the external library is 
to keep code out of QEMU.

Regards,

Anthony Liguori

> ---
>   Makefile.objs     |    2 +-
>   qemu-char.c       |    4 +
>   qemu-config.c     |    6 ++
>   qemu-options.hx   |   16 ++++-
>   spice-qemu-char.c |  185 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ui/qemu-spice.h   |    3 +
>   6 files changed, 214 insertions(+), 2 deletions(-)
>   create mode 100644 spice-qemu-char.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index cebb945..320b2a9 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -102,7 +102,7 @@ common-obj-$(CONFIG_BRLAPI) += baum.o
>   common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
>   common-obj-$(CONFIG_WIN32) += version.o
>
> -common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o ui/spice-display.o
> +common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o ui/spice-display.o spice-qemu-char.o
>
>   audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
>   audio-obj-$(CONFIG_SDL) += sdlaudio.o
> diff --git a/qemu-char.c b/qemu-char.c
> index edc9ad6..acc7130 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -97,6 +97,7 @@
>   #endif
>
>   #include "qemu_socket.h"
> +#include "ui/qemu-spice.h"
>
>   #define READ_BUF_LEN 4096
>
> @@ -2495,6 +2496,9 @@ static const struct {
>       || defined(__FreeBSD_kernel__)
>       { .name = "parport",   .open = qemu_chr_open_pp },
>   #endif
> +#ifdef CONFIG_SPICE
> +    { .name = "spicevmc",     .open = qemu_chr_open_spice },
> +#endif
>   };
>
>   CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
> diff --git a/qemu-config.c b/qemu-config.c
> index 965fa46..323d3c2 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -146,6 +146,12 @@ static QemuOptsList qemu_chardev_opts = {
>           },{
>               .name = "signal",
>               .type = QEMU_OPT_BOOL,
> +        },{
> +            .name = "name",
> +            .type = QEMU_OPT_STRING,
> +        },{
> +            .name = "debug",
> +            .type = QEMU_OPT_NUMBER,
>           },
>           { /* end of list */ }
>       },
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 4d99a58..5c13f0f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1357,6 +1357,9 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
>   #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
>       "-chardev parport,id=id,path=path[,mux=on|off]\n"
>   #endif
> +#if defined(CONFIG_SPICE)
> +    "-chardev spicevmc,id=id,debug=debug,name=name\n"
> +#endif
>       , QEMU_ARCH_ALL
>   )
>
> @@ -1381,7 +1384,8 @@ Backend is one of:
>   @option{stdio},
>   @option{braille},
>   @option{tty},
> -@option{parport}.
> +@option{parport}
> +@option{spicevmc}.
>   The specific backend will determine the applicable options.
>
>   All devices must have an id, which can be any string up to 127 characters long.
> @@ -1557,6 +1561,16 @@ Connect to a local parallel port.
>   @option{path} specifies the path to the parallel port device. @option{path} is
>   required.
>
> +#if defined(CONFIG_SPICE)
> +@item -chardev spicevmc ,id=@var{id} ,debug=@var{debug}, name=@var{name}
> +
> +@option{debug} debug level for spicevmc
> +
> +@option{name} name of spice channel to connect to
> +
> +Connect to a spice virtual machine channel, such as vdiport.
> +#endif
> +
>   @end table
>   ETEXI
>
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> new file mode 100644
> index 0000000..0ffa674
> --- /dev/null
> +++ b/spice-qemu-char.c
> @@ -0,0 +1,185 @@
> +#include "config-host.h"
> +#include "ui/qemu-spice.h"
> +#include<spice.h>
> +#include<spice-experimental.h>
> +
> +#include "osdep.h"
> +
> +#define dprintf(_scd, _level, _fmt, ...)                                \
> +    do {                                                                \
> +        static unsigned __dprintf_counter = 0;                          \
> +        if (_scd->debug>= _level) {                                    \
> +            fprintf(stderr, "scd: %3d: " _fmt, ++__dprintf_counter, ## __VA_ARGS__);\
> +        }                                                               \
> +    } while (0)
> +
> +#define VMC_MAX_HOST_WRITE    2048
> +
> +typedef struct SpiceCharDriver {
> +    CharDriverState*      chr;
> +    SpiceCharDeviceInstance     sin;
> +    char                  *subtype;
> +    bool                  active;
> +    uint8_t               *buffer;
> +    uint8_t               *datapos;
> +    ssize_t               bufsize, datalen;
> +    uint32_t              debug;
> +} SpiceCharDriver;
> +
> +static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len)
> +{
> +    SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
> +    ssize_t out = 0;
> +    ssize_t last_out;
> +    uint8_t* p = (uint8_t*)buf;
> +
> +    while (len>  0) {
> +        last_out = MIN(len, VMC_MAX_HOST_WRITE);
> +        qemu_chr_read(scd->chr, p, last_out);
> +        if (last_out>  0) {
> +            out += last_out;
> +            len -= last_out;
> +            p += last_out;
> +        } else {
> +            break;
> +        }
> +    }
> +
> +    dprintf(scd, 3, "%s: %lu/%zd\n", __func__, out, len + out);
> +    return out;
> +}
> +
> +static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t *buf, int len)
> +{
> +    SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
> +    int bytes = MIN(len, scd->datalen);
> +
> +    dprintf(scd, 2, "%s: %p %d/%d/%zd\n", __func__, scd->datapos, len, bytes, scd->datalen);
> +    if (bytes>  0) {
> +        memcpy(buf, scd->datapos, bytes);
> +        scd->datapos += bytes;
> +        scd->datalen -= bytes;
> +        assert(scd->datalen>= 0);
> +        if (scd->datalen == 0) {
> +            scd->datapos = 0;
> +        }
> +    }
> +    return bytes;
> +}
> +
> +static SpiceCharDeviceInterface vmc_interface = {
> +    .base.type          = SPICE_INTERFACE_CHAR_DEVICE,
> +    .base.description   = "spice virtual channel char device",
> +    .base.major_version = SPICE_INTERFACE_CHAR_DEVICE_MAJOR,
> +    .base.minor_version = SPICE_INTERFACE_CHAR_DEVICE_MINOR,
> +    .write              = vmc_write,
> +    .read               = vmc_read,
> +};
> +
> +
> +static void vmc_register_interface(SpiceCharDriver *scd)
> +{
> +    if (scd->active) {
> +        return;
> +    }
> +    dprintf(scd, 1, "%s\n", __func__);
> +    scd->sin.base.sif =&vmc_interface.base;
> +    qemu_spice_add_interface(&scd->sin.base);
> +    scd->active = true;
> +}
> +
> +static void vmc_unregister_interface(SpiceCharDriver *scd)
> +{
> +    if (!scd->active) {
> +        return;
> +    }
> +    dprintf(scd, 1, "%s\n", __func__);
> +    spice_server_remove_interface(&scd->sin.base);
> +    scd->active = false;
> +}
> +
> +
> +static int spice_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
> +{
> +    SpiceCharDriver *s = chr->opaque;
> +
> +    dprintf(s, 2, "%s: %d\n", __func__, len);
> +    vmc_register_interface(s);
> +    assert(s->datalen == 0);
> +    if (s->bufsize<  len) {
> +        s->bufsize = len;
> +        s->buffer = qemu_realloc(s->buffer, s->bufsize);
> +    }
> +    memcpy(s->buffer, buf, len);
> +    s->datapos = s->buffer;
> +    s->datalen = len;
> +    spice_server_char_device_wakeup(&s->sin);
> +    return len;
> +}
> +
> +static void spice_chr_close(struct CharDriverState *chr)
> +{
> +    SpiceCharDriver *s = chr->opaque;
> +
> +    printf("%s\n", __func__);
> +    vmc_unregister_interface(s);
> +    qemu_free(s);
> +}
> +
> +static void print_allowed_subtypes(void)
> +{
> +    const char** psubtype;
> +    int i;
> +
> +    fprintf(stderr, "allowed names: ");
> +    for(i=0, psubtype = spice_server_char_device_recognized_subtypes();
> +        *psubtype != NULL; ++psubtype, ++i) {
> +        if (i == 0) {
> +            fprintf(stderr, "%s", *psubtype);
> +        } else {
> +            fprintf(stderr, ", %s", *psubtype);
> +        }
> +    }
> +    fprintf(stderr, "\n");
> +}
> +
> +CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
> +{
> +    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();
> +    const char *subtype = NULL;
> +
> +    if (name == NULL) {
> +        fprintf(stderr, "spice-qemu-char: missing name parameter\n");
> +        print_allowed_subtypes();
> +        return NULL;
> +    }
> +    for(;*psubtype != NULL; ++psubtype) {
> +        if (strcmp(name, *psubtype) == 0) {
> +            subtype = *psubtype;
> +            break;
> +        }
> +    }
> +    if (subtype == NULL) {
> +        fprintf(stderr, "spice-qemu-char: unsupported name\n");
> +        print_allowed_subtypes();
> +        return NULL;
> +    }
> +
> +    chr = qemu_mallocz(sizeof(CharDriverState));
> +    s = qemu_mallocz(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;
> +
> +    qemu_chr_generic_open(chr);
> +
> +    return chr;
> +}
> diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
> index 0e3ad9b..e06af17 100644
> --- a/ui/qemu-spice.h
> +++ b/ui/qemu-spice.h
> @@ -24,6 +24,7 @@
>
>   #include "qemu-option.h"
>   #include "qemu-config.h"
> +#include "qemu-char.h"
>
>   extern int using_spice;
>
> @@ -33,6 +34,8 @@ void qemu_spice_audio_init(void);
>   void qemu_spice_display_init(DisplayState *ds);
>   int qemu_spice_add_interface(SpiceBaseInstance *sin);
>
> +CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
> +
>   #else  /* CONFIG_SPICE */
>
>   #define using_spice 0
>

On Fri, Dec 17, 2010 at 09:01:31AM -0600, Anthony Liguori wrote:
> On 12/17/2010 07:39 AM, Alon Levy wrote:
> >Adding a chardev backend for spice, for usage by spice vdagent in
> >conjunction with a properly named virtio-serial device.
> >
> >Example usage:
> >  qemu -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -devic
> >
> >This is equivalent to the old:
> >  qemu -device virtio-serial -device spicevmc,subtype=vdagent
> >
> >longer to write, but generated by libvirt, and requires one less device.
> >
> >v1->v3 changes: (v2 had a wrong commit message)
> >  * removed spice-qemu-char.h, folded into ui/qemu-spice.h
> >  * removed dead IOCTL code
> >  * removed comment
> >  * removed ifdef CONFIG_SPICE from qemu-config.c and qemu-options.hx help.
> 
> What doe this channel do?
> 

It adds a chardev that connects libspice-server and a device. It is used with
virtio-serial port for the spice vdagent, and with the outstanding patches for
usb-ccid and ccid-card-passthru for smartcard remoting. I thought the spicevmc
device was upstreamed, my mistake, that's why I gave the old commandline.

> I really don't feel comfortable with this.  This is not connecting
> QEMU to an existing interface that happens to fit our model.
> 

What is your model?

virtio-serial ports have been specifically designed for such uses as the spice
vdagent. Instead of using a tcp socket this is reusing libspice-server.

> This is clearly a "library" that provides thin wrappers around
> internal QEMU interfaces to implement code that belongs in QEMU
> outside of QEMU.
> 
> It's essentially a static plugin.  It's the same problem with QXL.
> I don't think we should be in the business of having thin shims to
> external libraries when the only reason to have the external library
> is to keep code out of QEMU.
> 

There is no interest to keep code out of QEMU. Having a separete library
in the case of spice simply makes sense, as we want to use it for physical
machines as well in the future. Most of the logic, if not all, would stay
the same regardless of updates coming from a in guest QXL device or from
an X server etc.

> Regards,
> 
> Anthony Liguori
> 
> >---
> >  Makefile.objs     |    2 +-
> >  qemu-char.c       |    4 +
> >  qemu-config.c     |    6 ++
> >  qemu-options.hx   |   16 ++++-
> >  spice-qemu-char.c |  185 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  ui/qemu-spice.h   |    3 +
> >  6 files changed, 214 insertions(+), 2 deletions(-)
> >  create mode 100644 spice-qemu-char.c
> >
> >diff --git a/Makefile.objs b/Makefile.objs
> >index cebb945..320b2a9 100644
> >--- a/Makefile.objs
> >+++ b/Makefile.objs
> >@@ -102,7 +102,7 @@ common-obj-$(CONFIG_BRLAPI) += baum.o
> >  common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
> >  common-obj-$(CONFIG_WIN32) += version.o
> >
> >-common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o ui/spice-display.o
> >+common-obj-$(CONFIG_SPICE) += ui/spice-core.o ui/spice-input.o ui/spice-display.o spice-qemu-char.o
> >
> >  audio-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
> >  audio-obj-$(CONFIG_SDL) += sdlaudio.o
> >diff --git a/qemu-char.c b/qemu-char.c
> >index edc9ad6..acc7130 100644
> >--- a/qemu-char.c
> >+++ b/qemu-char.c
> >@@ -97,6 +97,7 @@
> >  #endif
> >
> >  #include "qemu_socket.h"
> >+#include "ui/qemu-spice.h"
> >
> >  #define READ_BUF_LEN 4096
> >
> >@@ -2495,6 +2496,9 @@ static const struct {
> >      || defined(__FreeBSD_kernel__)
> >      { .name = "parport",   .open = qemu_chr_open_pp },
> >  #endif
> >+#ifdef CONFIG_SPICE
> >+    { .name = "spicevmc",     .open = qemu_chr_open_spice },
> >+#endif
> >  };
> >
> >  CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
> >diff --git a/qemu-config.c b/qemu-config.c
> >index 965fa46..323d3c2 100644
> >--- a/qemu-config.c
> >+++ b/qemu-config.c
> >@@ -146,6 +146,12 @@ static QemuOptsList qemu_chardev_opts = {
> >          },{
> >              .name = "signal",
> >              .type = QEMU_OPT_BOOL,
> >+        },{
> >+            .name = "name",
> >+            .type = QEMU_OPT_STRING,
> >+        },{
> >+            .name = "debug",
> >+            .type = QEMU_OPT_NUMBER,
> >          },
> >          { /* end of list */ }
> >      },
> >diff --git a/qemu-options.hx b/qemu-options.hx
> >index 4d99a58..5c13f0f 100644
> >--- a/qemu-options.hx
> >+++ b/qemu-options.hx
> >@@ -1357,6 +1357,9 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> >  #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
> >      "-chardev parport,id=id,path=path[,mux=on|off]\n"
> >  #endif
> >+#if defined(CONFIG_SPICE)
> >+    "-chardev spicevmc,id=id,debug=debug,name=name\n"
> >+#endif
> >      , QEMU_ARCH_ALL
> >  )
> >
> >@@ -1381,7 +1384,8 @@ Backend is one of:
> >  @option{stdio},
> >  @option{braille},
> >  @option{tty},
> >-@option{parport}.
> >+@option{parport}
> >+@option{spicevmc}.
> >  The specific backend will determine the applicable options.
> >
> >  All devices must have an id, which can be any string up to 127 characters long.
> >@@ -1557,6 +1561,16 @@ Connect to a local parallel port.
> >  @option{path} specifies the path to the parallel port device. @option{path} is
> >  required.
> >
> >+#if defined(CONFIG_SPICE)
> >+@item -chardev spicevmc ,id=@var{id} ,debug=@var{debug}, name=@var{name}
> >+
> >+@option{debug} debug level for spicevmc
> >+
> >+@option{name} name of spice channel to connect to
> >+
> >+Connect to a spice virtual machine channel, such as vdiport.
> >+#endif
> >+
> >  @end table
> >  ETEXI
> >
> >diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> >new file mode 100644
> >index 0000000..0ffa674
> >--- /dev/null
> >+++ b/spice-qemu-char.c
> >@@ -0,0 +1,185 @@
> >+#include "config-host.h"
> >+#include "ui/qemu-spice.h"
> >+#include<spice.h>
> >+#include<spice-experimental.h>
> >+
> >+#include "osdep.h"
> >+
> >+#define dprintf(_scd, _level, _fmt, ...)                                \
> >+    do {                                                                \
> >+        static unsigned __dprintf_counter = 0;                          \
> >+        if (_scd->debug>= _level) {                                    \
> >+            fprintf(stderr, "scd: %3d: " _fmt, ++__dprintf_counter, ## __VA_ARGS__);\
> >+        }                                                               \
> >+    } while (0)
> >+
> >+#define VMC_MAX_HOST_WRITE    2048
> >+
> >+typedef struct SpiceCharDriver {
> >+    CharDriverState*      chr;
> >+    SpiceCharDeviceInstance     sin;
> >+    char                  *subtype;
> >+    bool                  active;
> >+    uint8_t               *buffer;
> >+    uint8_t               *datapos;
> >+    ssize_t               bufsize, datalen;
> >+    uint32_t              debug;
> >+} SpiceCharDriver;
> >+
> >+static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len)
> >+{
> >+    SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
> >+    ssize_t out = 0;
> >+    ssize_t last_out;
> >+    uint8_t* p = (uint8_t*)buf;
> >+
> >+    while (len>  0) {
> >+        last_out = MIN(len, VMC_MAX_HOST_WRITE);
> >+        qemu_chr_read(scd->chr, p, last_out);
> >+        if (last_out>  0) {
> >+            out += last_out;
> >+            len -= last_out;
> >+            p += last_out;
> >+        } else {
> >+            break;
> >+        }
> >+    }
> >+
> >+    dprintf(scd, 3, "%s: %lu/%zd\n", __func__, out, len + out);
> >+    return out;
> >+}
> >+
> >+static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t *buf, int len)
> >+{
> >+    SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
> >+    int bytes = MIN(len, scd->datalen);
> >+
> >+    dprintf(scd, 2, "%s: %p %d/%d/%zd\n", __func__, scd->datapos, len, bytes, scd->datalen);
> >+    if (bytes>  0) {
> >+        memcpy(buf, scd->datapos, bytes);
> >+        scd->datapos += bytes;
> >+        scd->datalen -= bytes;
> >+        assert(scd->datalen>= 0);
> >+        if (scd->datalen == 0) {
> >+            scd->datapos = 0;
> >+        }
> >+    }
> >+    return bytes;
> >+}
> >+
> >+static SpiceCharDeviceInterface vmc_interface = {
> >+    .base.type          = SPICE_INTERFACE_CHAR_DEVICE,
> >+    .base.description   = "spice virtual channel char device",
> >+    .base.major_version = SPICE_INTERFACE_CHAR_DEVICE_MAJOR,
> >+    .base.minor_version = SPICE_INTERFACE_CHAR_DEVICE_MINOR,
> >+    .write              = vmc_write,
> >+    .read               = vmc_read,
> >+};
> >+
> >+
> >+static void vmc_register_interface(SpiceCharDriver *scd)
> >+{
> >+    if (scd->active) {
> >+        return;
> >+    }
> >+    dprintf(scd, 1, "%s\n", __func__);
> >+    scd->sin.base.sif =&vmc_interface.base;
> >+    qemu_spice_add_interface(&scd->sin.base);
> >+    scd->active = true;
> >+}
> >+
> >+static void vmc_unregister_interface(SpiceCharDriver *scd)
> >+{
> >+    if (!scd->active) {
> >+        return;
> >+    }
> >+    dprintf(scd, 1, "%s\n", __func__);
> >+    spice_server_remove_interface(&scd->sin.base);
> >+    scd->active = false;
> >+}
> >+
> >+
> >+static int spice_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
> >+{
> >+    SpiceCharDriver *s = chr->opaque;
> >+
> >+    dprintf(s, 2, "%s: %d\n", __func__, len);
> >+    vmc_register_interface(s);
> >+    assert(s->datalen == 0);
> >+    if (s->bufsize<  len) {
> >+        s->bufsize = len;
> >+        s->buffer = qemu_realloc(s->buffer, s->bufsize);
> >+    }
> >+    memcpy(s->buffer, buf, len);
> >+    s->datapos = s->buffer;
> >+    s->datalen = len;
> >+    spice_server_char_device_wakeup(&s->sin);
> >+    return len;
> >+}
> >+
> >+static void spice_chr_close(struct CharDriverState *chr)
> >+{
> >+    SpiceCharDriver *s = chr->opaque;
> >+
> >+    printf("%s\n", __func__);
> >+    vmc_unregister_interface(s);
> >+    qemu_free(s);
> >+}
> >+
> >+static void print_allowed_subtypes(void)
> >+{
> >+    const char** psubtype;
> >+    int i;
> >+
> >+    fprintf(stderr, "allowed names: ");
> >+    for(i=0, psubtype = spice_server_char_device_recognized_subtypes();
> >+        *psubtype != NULL; ++psubtype, ++i) {
> >+        if (i == 0) {
> >+            fprintf(stderr, "%s", *psubtype);
> >+        } else {
> >+            fprintf(stderr, ", %s", *psubtype);
> >+        }
> >+    }
> >+    fprintf(stderr, "\n");
> >+}
> >+
> >+CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
> >+{
> >+    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();
> >+    const char *subtype = NULL;
> >+
> >+    if (name == NULL) {
> >+        fprintf(stderr, "spice-qemu-char: missing name parameter\n");
> >+        print_allowed_subtypes();
> >+        return NULL;
> >+    }
> >+    for(;*psubtype != NULL; ++psubtype) {
> >+        if (strcmp(name, *psubtype) == 0) {
> >+            subtype = *psubtype;
> >+            break;
> >+        }
> >+    }
> >+    if (subtype == NULL) {
> >+        fprintf(stderr, "spice-qemu-char: unsupported name\n");
> >+        print_allowed_subtypes();
> >+        return NULL;
> >+    }
> >+
> >+    chr = qemu_mallocz(sizeof(CharDriverState));
> >+    s = qemu_mallocz(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;
> >+
> >+    qemu_chr_generic_open(chr);
> >+
> >+    return chr;
> >+}
> >diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
> >index 0e3ad9b..e06af17 100644
> >--- a/ui/qemu-spice.h
> >+++ b/ui/qemu-spice.h
> >@@ -24,6 +24,7 @@
> >
> >  #include "qemu-option.h"
> >  #include "qemu-config.h"
> >+#include "qemu-char.h"
> >
> >  extern int using_spice;
> >
> >@@ -33,6 +34,8 @@ void qemu_spice_audio_init(void);
> >  void qemu_spice_display_init(DisplayState *ds);
> >  int qemu_spice_add_interface(SpiceBaseInstance *sin);
> >
> >+CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
> >+
> >  #else  /* CONFIG_SPICE */
> >
> >  #define using_spice 0
> 
>

On 12/17/10 16:01, Anthony Liguori wrote:
> On 12/17/2010 07:39 AM, Alon Levy wrote:
>> Adding a chardev backend for spice, for usage by spice vdagent in
>> conjunction with a properly named virtio-serial device.
>>
>> Example usage:
>> qemu -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent
>> -devic

This example is incomplete btw ...

> What doe this channel do?

It is a communication path between spice client and guest.

There used to be just one, for vdagent (spice guest agent) which sends 
mouse events for example.  There will be more in the future, and using 
chardevs allows us to handle that.

> I really don't feel comfortable with this. This is not connecting QEMU
> to an existing interface that happens to fit our model.
>
> This is clearly a "library" that provides thin wrappers around internal
> QEMU interfaces to implement code that belongs in QEMU outside of QEMU.

No.  This is about tunneling server/client connections through spice, so 
we can reuse the authentication and encryption provided by spice.

Assume you have your VM running on machine A and you are sitting in 
front of machine B.  You want use the smartcard attached to host B in 
your virtual machine.

Without spice you'll use this on machine A:
    qemu -chardev socket,server,host=0.0.0.0,port=2001,id=ccid,nowait \
       -usb -device usb-ccid -device ccid-card-passthru,chardev=ccid

and run "vscclient <machine A> 2001" on machine B.

With spice you'll use this on machine A:
   qemu -chardev spicevmc,id=ccid,name=smartcard \
       -usb -device usb-ccid -device ccid-card-passthru,chardev=ccid

and the spice client on machine B will forward the requests to vscclient 
(well, I think it is actually linked to libcaccard, which is effectively 
the same as vscclient just forwards the requests from the network to 
libcaccard too).

There is no hidden magic.

cheers,
   Gerd

On Thu, Jan 06, 2011 at 01:05:33PM +0100, Gerd Hoffmann wrote:
> On 12/17/10 16:01, Anthony Liguori wrote:
> >On 12/17/2010 07:39 AM, Alon Levy wrote:
> >>Adding a chardev backend for spice, for usage by spice vdagent in
> >>conjunction with a properly named virtio-serial device.
> >>
> >>Example usage:
> >>qemu -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent
> >>-devic
> 
> This example is incomplete btw ...

oops - the complete example is:

-device virtio-serial -device spicevmc,subtype=vdagent
-chardev spicevmc,id=vdagent,name=vdagent
-device virtserialport,chardev=vdagent,name=com.redhat.spice.0

> 
> >What doe this channel do?
> 
> It is a communication path between spice client and guest.
> 
> There used to be just one, for vdagent (spice guest agent) which
> sends mouse events for example.  There will be more in the future,
> and using chardevs allows us to handle that.
> 
> >I really don't feel comfortable with this. This is not connecting QEMU
> >to an existing interface that happens to fit our model.
> >
> >This is clearly a "library" that provides thin wrappers around internal
> >QEMU interfaces to implement code that belongs in QEMU outside of QEMU.
> 
> No.  This is about tunneling server/client connections through
> spice, so we can reuse the authentication and encryption provided by
> spice.
> 
> Assume you have your VM running on machine A and you are sitting in
> front of machine B.  You want use the smartcard attached to host B
> in your virtual machine.
> 
> Without spice you'll use this on machine A:
>    qemu -chardev socket,server,host=0.0.0.0,port=2001,id=ccid,nowait \
>       -usb -device usb-ccid -device ccid-card-passthru,chardev=ccid
> 
> and run "vscclient <machine A> 2001" on machine B.
> 
> With spice you'll use this on machine A:
>   qemu -chardev spicevmc,id=ccid,name=smartcard \
>       -usb -device usb-ccid -device ccid-card-passthru,chardev=ccid
> 
> and the spice client on machine B will forward the requests to
> vscclient (well, I think it is actually linked to libcaccard, which
> is effectively the same as vscclient just forwards the requests from
> the network to libcaccard too).
> 
> There is no hidden magic.
> 
> cheers,
>   Gerd
>