[Qemu-devel] [PULL] char, virtio-serial, spice

[Qemu-devel] [PULL] char, virtio-serial, spice

[Amit Shah]

Hello,

These are assorted fixes to the char code, big endian handling of
virtio-serial code  and addition of a guest open/close notification
for spice to use from virtio-console.

(The git mirror might take a while to show up these patches.)

The following changes since commit a7d3970d0635ebce1412736e7aaf11d387919dc8:

  target-arm: Don't update base register on abort in Thumb T1 LDM
  (2011-04-27 20:14:34 +0200)

are available in the git repository at:
  git://git.kernel.org/pub/scm/virt/qemu/amit/char.git for-anthony

Alexey Kardashevskiy (1):
  virtio-serial: Fix endianness bug in the config space

Amit Shah (1):
  char: Detect chardev release by NULL handlers as well as NULL opaque

Hans de Goede (3):
  chardev: Allow frontends to notify backends of guest open / close
  virtio-console: notify backend of guest open / close
  spice-chardev: listen to frontend guest open / close

Kusanagi Kouichi (1):
  char: Allow devices to use a single multiplexed chardev.

 hw/qdev-properties.c   |    4 ++--
 hw/virtio-console.c    |   18 ++++++++++++++++++
 hw/virtio-serial-bus.c |   23 +++++++++++++----------
 qemu-char.c            |   24 ++++++++++++++++++++++--
 qemu-char.h            |    6 +++++-
 spice-qemu-char.c      |   14 ++++++++++++++
 6 files changed, 74 insertions(+), 15 deletions(-)

-- 
1.7.4.4

[Qemu-devel] [PATCH 1/6] chardev: Allow frontends to notify backends of guest open / close

[Amit Shah]

From: Hans de Goede <hdegoede@redhat.com>

Some frontends know when the guest has opened the "channel" and is actively
listening to it, for example virtio-serial. This patch adds 2 new qemu-chardev
functions which can be used by frontends to signal guest open / close, and
allows interested backends to listen to this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu-char.c |   17 +++++++++++++++++
 qemu-char.h |    4 ++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 03858d4..710d98f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -480,6 +480,9 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState *drv)
     chr->chr_write = mux_chr_write;
     chr->chr_update_read_handler = mux_chr_update_read_handler;
     chr->chr_accept_input = mux_chr_accept_input;
+    /* Frontend guest-open / -close notification is not support with muxes */
+    chr->chr_guest_open = NULL;
+    chr->chr_guest_close = NULL;
 
     /* Muxes are always open on creation */
     qemu_chr_generic_open(chr);
@@ -2579,6 +2582,20 @@ void qemu_chr_set_echo(struct CharDriverState *chr, bool echo)
     }
 }
 
+void qemu_chr_guest_open(struct CharDriverState *chr)
+{
+    if (chr->chr_guest_open) {
+        chr->chr_guest_open(chr);
+    }
+}
+
+void qemu_chr_guest_close(struct CharDriverState *chr)
+{
+    if (chr->chr_guest_close) {
+        chr->chr_guest_close(chr);
+    }
+}
+
 void qemu_chr_close(CharDriverState *chr)
 {
     QTAILQ_REMOVE(&chardevs, chr, next);
diff --git a/qemu-char.h b/qemu-char.h
index fb96eef..2f8512e 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -65,6 +65,8 @@ struct CharDriverState {
     void (*chr_close)(struct CharDriverState *chr);
     void (*chr_accept_input)(struct CharDriverState *chr);
     void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
+    void (*chr_guest_open)(struct CharDriverState *chr);
+    void (*chr_guest_close)(struct CharDriverState *chr);
     void *opaque;
     QEMUBH *bh;
     char *label;
@@ -79,6 +81,8 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
                                     void (*init)(struct CharDriverState *s));
 CharDriverState *qemu_chr_open(const char *label, const char *filename, void (*init)(struct CharDriverState *s));
 void qemu_chr_set_echo(struct CharDriverState *chr, bool echo);
+void qemu_chr_guest_open(struct CharDriverState *chr);
+void qemu_chr_guest_close(struct CharDriverState *chr);
 void qemu_chr_close(CharDriverState *chr);
 void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
-- 
1.7.4.4

[Qemu-devel] [PATCH 2/6] virtio-console: notify backend of guest open / close

[Amit Shah]

From: Hans de Goede <hdegoede@redhat.com>

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 6b5237b..de539c4 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -28,6 +28,22 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
     return qemu_chr_write(vcon->chr, buf, len);
 }
 
+/* Callback function that's called when the guest opens the port */
+static void guest_open(VirtIOSerialPort *port)
+{
+    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+    qemu_chr_guest_open(vcon->chr);
+}
+
+/* Callback function that's called when the guest closes the port */
+static void guest_close(VirtIOSerialPort *port)
+{
+    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+    qemu_chr_guest_close(vcon->chr);
+}
+
 /* Readiness of the guest to accept data on a port */
 static int chr_can_read(void *opaque)
 {
@@ -64,6 +80,8 @@ static int generic_port_init(VirtConsole *vcon, VirtIOSerialPort *port)
         qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
                               vcon);
         vcon->port.info->have_data = flush_buf;
+        vcon->port.info->guest_open = guest_open;
+        vcon->port.info->guest_close = guest_close;
     }
     return 0;
 }
-- 
1.7.4.4

[Qemu-devel] [PATCH 3/6] spice-chardev: listen to frontend guest open / close

[Amit Shah]

From: Hans de Goede <hdegoede@redhat.com>

Note the vmc_register_interface() in spice_chr_write is left in place
in case someone uses spice-chardev with a frontend which does not have
guest open / close notification.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 spice-qemu-char.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 517f337..fa15a71 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -131,6 +131,18 @@ static void spice_chr_close(struct CharDriverState *chr)
     qemu_free(s);
 }
 
+static void spice_chr_guest_open(struct CharDriverState *chr)
+{
+    SpiceCharDriver *s = chr->opaque;
+    vmc_register_interface(s);
+}
+
+static void spice_chr_guest_close(struct CharDriverState *chr)
+{
+    SpiceCharDriver *s = chr->opaque;
+    vmc_unregister_interface(s);
+}
+
 static void print_allowed_subtypes(void)
 {
     const char** psubtype;
@@ -183,6 +195,8 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
     chr->opaque = s;
     chr->chr_write = spice_chr_write;
     chr->chr_close = spice_chr_close;
+    chr->chr_guest_open = spice_chr_guest_open;
+    chr->chr_guest_close = spice_chr_guest_close;
 
     qemu_chr_generic_open(chr);
 
-- 
1.7.4.4

[Qemu-devel] [PATCH 4/6] char: Allow devices to use a single multiplexed chardev.

[Amit Shah]

From: Kusanagi Kouichi <slash@ac.auone-net.jp>

This fixes regression caused by commit
2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6
("char: Prevent multiple devices opening same chardev"):

-nodefaults -nographic -chardev stdio,id=stdio,mux=on,signal=off \
 -mon stdio -device virtio-serial-pci \
 -device virtconsole,chardev=stdio -device isa-serial,chardev=stdio

fails with:

qemu-system-x86_64: -device isa-serial,chardev=stdio: Property 'isa-serial.chardev' can't take value 'stdio', it's in use

Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/qdev-properties.c |    4 ++--
 qemu-char.c          |    5 ++++-
 qemu-char.h          |    2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1088a26..eff2d24 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str)
     if (*ptr == NULL) {
         return -ENOENT;
     }
-    if ((*ptr)->assigned) {
+    if ((*ptr)->avail_connections < 1) {
         return -EEXIST;
     }
-    (*ptr)->assigned = 1;
+    --(*ptr)->avail_connections;
     return 0;
 }
 
diff --git a/qemu-char.c b/qemu-char.c
index 710d98f..eaf6571 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -199,7 +199,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
 {
     if (!opaque) {
         /* chr driver being released. */
-        s->assigned = 0;
+        ++s->avail_connections;
     }
     s->chr_can_read = fd_can_read;
     s->chr_read = fd_read;
@@ -2547,7 +2547,10 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
         snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
         chr = qemu_chr_open_mux(base);
         chr->filename = base->filename;
+        chr->avail_connections = MAX_MUX;
         QTAILQ_INSERT_TAIL(&chardevs, chr, next);
+    } else {
+        chr->avail_connections = 1;
     }
     chr->label = qemu_strdup(qemu_opts_id(opts));
     return chr;
diff --git a/qemu-char.h b/qemu-char.h
index 2f8512e..892c6da 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -72,7 +72,7 @@ struct CharDriverState {
     char *label;
     char *filename;
     int opened;
-    int assigned; /* chardev assigned to a device */
+    int avail_connections;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
-- 
1.7.4.4

[Qemu-devel] [PATCH 5/6] char: Detect chardev release by NULL handlers as well as NULL opaque

[Amit Shah]

Juan says he prefers these extra checks to ensure a user of a chardev is
releasing it.

Requested-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu-char.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index eaf6571..5e04a20 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -197,7 +197,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
                            IOEventHandler *fd_event,
                            void *opaque)
 {
-    if (!opaque) {
+    if (!opaque && !fd_can_read && !fd_read && !fd_event) {
         /* chr driver being released. */
         ++s->avail_connections;
     }
-- 
1.7.4.4

[Qemu-devel] [PATCH 6/6] virtio-serial: Fix endianness bug in the config space

[Amit Shah]

From: Alexey Kardashevskiy <aik@ozlabs.ru>

The virtio serial specification requres that the values in the config
space are encoded in native endian of the guest.

The qemu virtio-serial code did not do conversion to the guest endian
format what caused problems when host and guest use different format.

This patch corrects the qemu side, correctly doing host-native <->
guest-native conversions when accessing the config space. This won't
break any setups that aren't already broken, and fixes the case
of different host and guest endianness.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 6227379..f10d48f 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -494,7 +494,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
     VirtIOSerial *s = opaque;
     VirtIOSerialPort *port;
     uint32_t nr_active_ports;
-    unsigned int i;
+    unsigned int i, max_nr_ports;
 
     /* The virtio device */
     virtio_save(&s->vdev, f);
@@ -506,8 +506,8 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
     qemu_put_be32s(f, &s->config.max_nr_ports);
 
     /* The ports map */
-
-    for (i = 0; i < (s->config.max_nr_ports + 31) / 32; i++) {
+    max_nr_ports = tswap32(s->config.max_nr_ports);
+    for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
         qemu_put_be32s(f, &s->ports_map[i]);
     }
 
@@ -568,7 +568,8 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
     qemu_get_be16s(f, &s->config.rows);
 
     qemu_get_be32s(f, &max_nr_ports);
-    if (max_nr_ports > s->config.max_nr_ports) {
+    tswap32s(&max_nr_ports);
+    if (max_nr_ports > tswap32(s->config.max_nr_ports)) {
         /* Source could have had more ports than us. Fail migration. */
         return -EINVAL;
     }
@@ -670,9 +671,10 @@ static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
 /* This function is only used if a port id is not provided by the user */
 static uint32_t find_free_port_id(VirtIOSerial *vser)
 {
-    unsigned int i;
+    unsigned int i, max_nr_ports;
 
-    for (i = 0; i < (vser->config.max_nr_ports + 31) / 32; i++) {
+    max_nr_ports = tswap32(vser->config.max_nr_ports);
+    for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
         uint32_t map, bit;
 
         map = vser->ports_map[i];
@@ -720,7 +722,7 @@ static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
     VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
     VirtIOSerialPortInfo *info = DO_UPCAST(VirtIOSerialPortInfo, qdev, base);
     VirtIOSerialBus *bus = DO_UPCAST(VirtIOSerialBus, qbus, qdev->parent_bus);
-    int ret;
+    int ret, max_nr_ports;
     bool plugging_port0;
 
     port->vser = bus->vser;
@@ -750,9 +752,10 @@ static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
         }
     }
 
-    if (port->id >= port->vser->config.max_nr_ports) {
+    max_nr_ports = tswap32(port->vser->config.max_nr_ports);
+    if (port->id >= max_nr_ports) {
         error_report("virtio-serial-bus: Out-of-range port id specified, max. allowed: %u\n",
-                     port->vser->config.max_nr_ports - 1);
+                     max_nr_ports - 1);
         return -1;
     }
 
@@ -863,7 +866,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
         vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
     }
 
-    vser->config.max_nr_ports = conf->max_virtserial_ports;
+    vser->config.max_nr_ports = tswap32(conf->max_virtserial_ports);
     vser->ports_map = qemu_mallocz(((conf->max_virtserial_ports + 31) / 32)
         * sizeof(vser->ports_map[0]));
     /*
-- 
1.7.4.4

Re: [Qemu-devel] [PULL] char, virtio-serial, spice

[Anthony Liguori]

On 04/28/2011 02:30 AM, Amit Shah wrote:
> Hello,
>
> These are assorted fixes to the char code, big endian handling of
> virtio-serial code  and addition of a guest open/close notification
> for spice to use from virtio-console.
>
> (The git mirror might take a while to show up these patches.)

Pulled.  Thanks.

Regards,

Anthony Liguori

>
> The following changes since commit a7d3970d0635ebce1412736e7aaf11d387919dc8:
>
>    target-arm: Don't update base register on abort in Thumb T1 LDM
>    (2011-04-27 20:14:34 +0200)
>
> are available in the git repository at:
>    git://git.kernel.org/pub/scm/virt/qemu/amit/char.git for-anthony
>
> Alexey Kardashevskiy (1):
>    virtio-serial: Fix endianness bug in the config space
>
> Amit Shah (1):
>    char: Detect chardev release by NULL handlers as well as NULL opaque
>
> Hans de Goede (3):
>    chardev: Allow frontends to notify backends of guest open / close
>    virtio-console: notify backend of guest open / close
>    spice-chardev: listen to frontend guest open / close
>
> Kusanagi Kouichi (1):
>    char: Allow devices to use a single multiplexed chardev.
>
>   hw/qdev-properties.c   |    4 ++--
>   hw/virtio-console.c    |   18 ++++++++++++++++++
>   hw/virtio-serial-bus.c |   23 +++++++++++++----------
>   qemu-char.c            |   24 ++++++++++++++++++++++--
>   qemu-char.h            |    6 +++++-
>   spice-qemu-char.c      |   14 ++++++++++++++
>   6 files changed, 74 insertions(+), 15 deletions(-)
>