qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] spice: add chardev (v3)


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] spice: add chardev (v3)
Date: Fri, 17 Dec 2010 09:01:31 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Lightning/1.0b1 Thunderbird/3.0.10

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},
address@hidden
address@hidden
address@hidden
  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)
address@hidden -chardev spicevmc ,address@hidden ,address@hidden, address@hidden
+
address@hidden debug level for spicevmc
+
address@hidden 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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]