[Qemu-devel] [PATCH v6 00/18] PULL: virtio-serial fixes

[Qemu-devel] [PATCH v6 00/18] PULL: virtio-serial fixes

[Amit Shah]

Hi Anthony,

The following changes since commit 14a6063a91083c9cbe1bc502ee58fc7ca146bc1a:
  Richard Henderson (1):
        Implement cpu_get_real_ticks for Alpha.

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/amit/vs-qemu-kvm.git for-anthony

This series updates the virtio-serial ABI to announce ports via
control messages instead of config space updates. Allows for stable
port enumeration with hot-plug/unplug and migrations.

There are also several fixes rolled in.

Juan and Gerd have acked the previous series.

v6:
- Handle guest writes to ports that aren't active (don't segfault)
  - changes patches 16 and 17
- Marcelo's patch to wake up iothread on guest read notification
  - adds patch 18

Overall:
- Users can set the port id they wish to instantiate ports at by using
  the ,nr= parameter to 'virtserialport' and 'virtconsole' devices

- Migration fixes: refuse migration when:
  - number of active ports is different between the src and destination
  - max_nr_ports a device can support on the src is more

- If a qemu chardev connection to a port is closed on the dest while
  it was open on the src, inform the guest about this. (Also do the
  same for port closed on src but open on dest.)

- Use control messages for relaying new port information instead of
  config space (changes abi)

- Propagate error message from guest in instantiating a port or a
  device to the user.

- Handle scatter/gather for control output and data output from the
  guest

- Fix abuse of virtio api in the virtqueue_push() function

- Add an API for the ports for flow control: ports can signal when
  they're ready to accept data / stop sending data.

- Wake up iothread on read notifications: send out data to guest as
  soon as we know there are empty buffers to fill data in.

Amit Shah (17):
      virtio-serial: save/load: Ensure target has enough ports
      virtio-serial: save/load: Ensure nr_ports on src and dest are same.
      virtio-serial: save/load: Ensure we have hot-plugged ports instantiated
      virtio-serial: save/load: Send target host connection status if different
      virtio-serial: Use control messages to notify guest of new ports
      virtio-serial: whitespace: match surrounding code
      virtio-serial: Remove redundant check for 0-sized write request
      virtio-serial: Update copyright year to 2010
      virtio-serial: Propagate errors in initialising ports / devices in guest
      virtio-serial: Send out guest data to ports only if port is opened
      iov: Introduce a new file for helpers around iovs, add iov_from_buf()
      iov: Add iov_to_buf and iov_size helpers
      virtio-serial: Handle scatter-gather buffers for control messages
      virtio-serial: Handle scatter/gather input from the guest
      virtio-serial: Apps should consume all data that guest sends out / Fix virtio api abuse
      virtio-serial: Discard data that guest sends us when ports aren't connected
      virtio-serial: Implement flow control for individual ports

Marcelo Tosatti (1):
      virtio-serial-bus: wake up iothread upon guest read notification

 Makefile               |    2 +
 Makefile.objs          |    1 +
 hw/iov.c               |   70 ++++++++++
 hw/iov.h               |   19 +++
 hw/virtio-balloon.c    |   35 +-----
 hw/virtio-console.c    |   11 +-
 hw/virtio-net.c        |   20 +---
 hw/virtio-serial-bus.c |  342 ++++++++++++++++++++++++++++++++++++------------
 hw/virtio-serial.h     |   34 ++++--
 9 files changed, 386 insertions(+), 148 deletions(-)
 create mode 100644 hw/iov.c
 create mode 100644 hw/iov.h

[Qemu-devel] [PATCH v6 01/18] virtio-serial: save/load: Ensure target has enough ports

[Amit Shah]

The target could be started with max_nr_ports for a virtio-serial device
lesser than what was available on the source machine. Fail the migration
in such a case.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
Reported-by: Juan Quintela <quintela@redhat.com>
---
 hw/virtio-serial-bus.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 17c1ec1..9a7f0c1 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -374,10 +374,13 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
 
     /* Items in struct VirtIOSerial */
 
+    qemu_put_be32s(f, &s->bus->max_nr_ports);
+
     /* Do this because we might have hot-unplugged some ports */
     nr_active_ports = 0;
-    QTAILQ_FOREACH(port, &s->ports, next)
+    QTAILQ_FOREACH(port, &s->ports, next) {
         nr_active_ports++;
+    }
 
     qemu_put_be32s(f, &nr_active_ports);
 
@@ -399,7 +402,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIOSerial *s = opaque;
     VirtIOSerialPort *port;
-    uint32_t nr_active_ports;
+    uint32_t max_nr_ports, nr_active_ports;
     unsigned int i;
 
     if (version_id > 2) {
@@ -420,6 +423,12 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 
     /* Items in struct VirtIOSerial */
 
+    qemu_get_be32s(f, &max_nr_ports);
+    if (max_nr_ports > s->bus->max_nr_ports) {
+        /* Source could have more ports than us. Fail migration. */
+        return -EINVAL;
+    }
+
     qemu_get_be32s(f, &nr_active_ports);
 
     /* Items in struct VirtIOSerialPort */
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 02/18] virtio-serial: save/load: Ensure nr_ports on src and dest are same.

[Amit Shah]

The number of ports on the source as well as the destination machines
should match. If they don't, it means some ports that got hotplugged on
the source aren't instantiated on the destination. Or that ports that
were hot-unplugged on the source are created on the destination.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
Reported-by: Juan Quintela <quintela@redhat.com>
---
 hw/virtio-serial-bus.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 9a7f0c1..d31e62d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -402,7 +402,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIOSerial *s = opaque;
     VirtIOSerialPort *port;
-    uint32_t max_nr_ports, nr_active_ports;
+    uint32_t max_nr_ports, nr_active_ports, nr_ports;
     unsigned int i;
 
     if (version_id > 2) {
@@ -419,7 +419,21 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
     /* The config space */
     qemu_get_be16s(f, &s->config.cols);
     qemu_get_be16s(f, &s->config.rows);
-    s->config.nr_ports = qemu_get_be32(f);
+    nr_ports = qemu_get_be32(f);
+
+    if (nr_ports != s->config.nr_ports) {
+        /*
+         * Source hot-plugged/unplugged ports and we don't have all of
+         * them here.
+         *
+         * Note: This condition cannot check for all hotplug/unplug
+         * events: eg, if one port was hot-plugged and one was
+         * unplugged, the nr_ports remains the same but the port id's
+         * would have changed and we won't catch it here. A later
+         * check for !find_port_by_id() will confirm if this happened.
+         */
+        return -EINVAL;
+    }
 
     /* Items in struct VirtIOSerial */
 
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 03/18] virtio-serial: save/load: Ensure we have hot-plugged ports instantiated

[Amit Shah]

If some ports that were hot-plugged on the source are not available on
the destination, fail migration instead of trying to deref a NULL
pointer.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
Reported-by: Juan Quintela <quintela@redhat.com>
---
 hw/virtio-serial-bus.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index d31e62d..5316ef6 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -451,6 +451,13 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 
         id = qemu_get_be32(f);
         port = find_port_by_id(s, id);
+        if (!port) {
+            /*
+             * The requested port was hot-plugged on the source but we
+             * don't have it
+             */
+            return -EINVAL;
+        }
 
         port->guest_connected = qemu_get_byte(f);
     }
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 04/18] virtio-serial: save/load: Send target host connection status if different

[Amit Shah]

If the host connection to a port is closed on the destination machine
after migration, whereas the connection was open on the source, the
guest has to be informed of that.

Similar for a host connection open on the destination.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 5316ef6..484dc94 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -395,6 +395,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
          */
         qemu_put_be32s(f, &port->id);
         qemu_put_byte(f, port->guest_connected);
+        qemu_put_byte(f, port->host_connected);
     }
 }
 
@@ -448,6 +449,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
     /* Items in struct VirtIOSerialPort */
     for (i = 0; i < nr_active_ports; i++) {
         uint32_t id;
+        bool host_connected;
 
         id = qemu_get_be32(f);
         port = find_port_by_id(s, id);
@@ -460,6 +462,15 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
         }
 
         port->guest_connected = qemu_get_byte(f);
+        host_connected = qemu_get_byte(f);
+        if (host_connected != port->host_connected) {
+            /*
+             * We have to let the guest know of the host connection
+             * status change
+             */
+            send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN,
+                               port->host_connected);
+        }
     }
 
     return 0;
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 05/18] virtio-serial: Use control messages to notify guest of new ports

[Amit Shah]

Allow the port 'id's to be set by a user on the command line. This is
needed by management apps that will want a stable port numbering scheme
for hot-plug/unplug and migration.

Since the port numbers are shared with the guest (to identify ports in
control messages), we just send a control message to the guest
indicating addition of new ports (hot-plug) or notifying the guest of
the available ports when the guest sends us a DEVICE_READY control
message.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c    |    2 +
 hw/virtio-serial-bus.c |  184 +++++++++++++++++++++++++++++++----------------
 hw/virtio-serial.h     |   17 +++--
 3 files changed, 133 insertions(+), 70 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index bd44ec6..17b221d 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -99,6 +99,7 @@ static VirtIOSerialPortInfo virtconsole_info = {
     .exit          = virtconsole_exitfn,
     .qdev.props = (Property[]) {
         DEFINE_PROP_UINT8("is_console", VirtConsole, port.is_console, 1),
+        DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
         DEFINE_PROP_CHR("chardev", VirtConsole, chr),
         DEFINE_PROP_STRING("name", VirtConsole, port.name),
         DEFINE_PROP_END_OF_LIST(),
@@ -133,6 +134,7 @@ static VirtIOSerialPortInfo virtserialport_info = {
     .init          = virtserialport_initfn,
     .exit          = virtconsole_exitfn,
     .qdev.props = (Property[]) {
+        DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
         DEFINE_PROP_CHR("chardev", VirtConsole, chr),
         DEFINE_PROP_STRING("name", VirtConsole, port.name),
         DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 484dc94..bb11a9b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -41,6 +41,10 @@ struct VirtIOSerial {
     VirtIOSerialBus *bus;
 
     QTAILQ_HEAD(, VirtIOSerialPort) ports;
+
+    /* bitmap for identifying active ports */
+    uint32_t *ports_map;
+
     struct virtio_console_config config;
 };
 
@@ -48,6 +52,10 @@ static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
 {
     VirtIOSerialPort *port;
 
+    if (id == VIRTIO_CONSOLE_BAD_ID) {
+        return NULL;
+    }
+
     QTAILQ_FOREACH(port, &vser->ports, next) {
         if (port->id == id)
             return port;
@@ -208,14 +216,25 @@ static void handle_control_message(VirtIOSerial *vser, void *buf)
     size_t buffer_len;
 
     gcpkt = buf;
-    port = find_port_by_id(vser, ldl_p(&gcpkt->id));
-    if (!port)
-        return;
 
     cpkt.event = lduw_p(&gcpkt->event);
     cpkt.value = lduw_p(&gcpkt->value);
 
+    port = find_port_by_id(vser, ldl_p(&gcpkt->id));
+    if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY)
+        return;
+
     switch(cpkt.event) {
+    case VIRTIO_CONSOLE_DEVICE_READY:
+        /*
+         * The device is up, we can now tell the device about all the
+         * ports we have here.
+         */
+        QTAILQ_FOREACH(port, &vser->ports, next) {
+            send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1);
+        }
+        break;
+
     case VIRTIO_CONSOLE_PORT_READY:
         /*
          * Now that we know the guest asked for the port name, we're
@@ -363,6 +382,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
     VirtIOSerial *s = opaque;
     VirtIOSerialPort *port;
     uint32_t nr_active_ports;
+    unsigned int i;
 
     /* The virtio device */
     virtio_save(&s->vdev, f);
@@ -370,13 +390,17 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
     /* The config space */
     qemu_put_be16s(f, &s->config.cols);
     qemu_put_be16s(f, &s->config.rows);
-    qemu_put_be32s(f, &s->config.nr_ports);
 
-    /* Items in struct VirtIOSerial */
+    qemu_put_be32s(f, &s->config.max_nr_ports);
+
+    /* The ports map */
+
+    for (i = 0; i < (s->config.max_nr_ports + 31) / 32; i++) {
+        qemu_put_be32s(f, &s->ports_map[i]);
+    }
 
-    qemu_put_be32s(f, &s->bus->max_nr_ports);
+    /* Ports */
 
-    /* Do this because we might have hot-unplugged some ports */
     nr_active_ports = 0;
     QTAILQ_FOREACH(port, &s->ports, next) {
         nr_active_ports++;
@@ -388,11 +412,6 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
      * Items in struct VirtIOSerialPort.
      */
     QTAILQ_FOREACH(port, &s->ports, next) {
-        /*
-         * We put the port number because we may not have an active
-         * port at id 0 that's reserved for a console port, or in case
-         * of ports that might have gotten unplugged
-         */
         qemu_put_be32s(f, &port->id);
         qemu_put_byte(f, port->guest_connected);
         qemu_put_byte(f, port->host_connected);
@@ -403,7 +422,8 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIOSerial *s = opaque;
     VirtIOSerialPort *port;
-    uint32_t max_nr_ports, nr_active_ports, nr_ports;
+    size_t ports_map_size;
+    uint32_t max_nr_ports, nr_active_ports, *ports_map;
     unsigned int i;
 
     if (version_id > 2) {
@@ -420,29 +440,29 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
     /* The config space */
     qemu_get_be16s(f, &s->config.cols);
     qemu_get_be16s(f, &s->config.rows);
-    nr_ports = qemu_get_be32(f);
 
-    if (nr_ports != s->config.nr_ports) {
-        /*
-         * Source hot-plugged/unplugged ports and we don't have all of
-         * them here.
-         *
-         * Note: This condition cannot check for all hotplug/unplug
-         * events: eg, if one port was hot-plugged and one was
-         * unplugged, the nr_ports remains the same but the port id's
-         * would have changed and we won't catch it here. A later
-         * check for !find_port_by_id() will confirm if this happened.
-         */
+    qemu_get_be32s(f, &max_nr_ports);
+    if (max_nr_ports > s->config.max_nr_ports) {
+        /* Source could have had more ports than us. Fail migration. */
         return -EINVAL;
     }
 
-    /* Items in struct VirtIOSerial */
+    ports_map_size = sizeof(uint32_t) * (max_nr_ports + 31) / 32;
+    ports_map = qemu_malloc(ports_map_size);
 
-    qemu_get_be32s(f, &max_nr_ports);
-    if (max_nr_ports > s->bus->max_nr_ports) {
-        /* Source could have more ports than us. Fail migration. */
-        return -EINVAL;
+    for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
+        qemu_get_be32s(f, &ports_map[i]);
+
+        if (ports_map[i] != s->ports_map[i]) {
+            /*
+             * Ports active on source and destination don't
+             * match. Fail migration.
+             */
+            qemu_free(ports_map);
+            return -EINVAL;
+        }
     }
+    qemu_free(ports_map);
 
     qemu_get_be32s(f, &nr_active_ports);
 
@@ -453,13 +473,6 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 
         id = qemu_get_be32(f);
         port = find_port_by_id(s, id);
-        if (!port) {
-            /*
-             * The requested port was hot-plugged on the source but we
-             * don't have it
-             */
-            return -EINVAL;
-        }
 
         port->guest_connected = qemu_get_byte(f);
         host_connected = qemu_get_byte(f);
@@ -472,7 +485,6 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
                                port->host_connected);
         }
     }
-
     return 0;
 }
 
@@ -507,6 +519,50 @@ static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
                    indent, "", port->host_connected);
 }
 
+/* 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;
+
+    for (i = 0; i < (vser->config.max_nr_ports + 31) / 32; i++) {
+        uint32_t map, bit;
+
+        map = vser->ports_map[i];
+        bit = ffs(~map);
+        if (bit) {
+            return (bit - 1) + i * 32;
+        }
+    }
+    return VIRTIO_CONSOLE_BAD_ID;
+}
+
+static void mark_port_added(VirtIOSerial *vser, uint32_t port_id)
+{
+    unsigned int i;
+
+    i = port_id / 32;
+    vser->ports_map[i] |= 1U << (port_id % 32);
+}
+
+static void add_port(VirtIOSerial *vser, uint32_t port_id)
+{
+    mark_port_added(vser, port_id);
+
+    send_control_event(find_port_by_id(vser, port_id),
+                       VIRTIO_CONSOLE_PORT_ADD, 1);
+}
+
+static void remove_port(VirtIOSerial *vser, uint32_t port_id)
+{
+    unsigned int i;
+
+    i = port_id / 32;
+    vser->ports_map[i] &= ~(1U << (port_id % 32));
+
+    send_control_event(find_port_by_id(vser, port_id),
+                       VIRTIO_CONSOLE_PORT_REMOVE, 1);
+}
+
 static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
 {
     VirtIOSerialDevice *dev = DO_UPCAST(VirtIOSerialDevice, qdev, qdev);
@@ -525,19 +581,36 @@ static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
      */
     plugging_port0 = port->is_console && !find_port_by_id(port->vser, 0);
 
-    if (port->vser->config.nr_ports == bus->max_nr_ports && !plugging_port0) {
-        error_report("virtio-serial-bus: Maximum device limit reached");
+    if (find_port_by_id(port->vser, port->id)) {
+        error_report("virtio-serial-bus: A port already exists at id %u\n",
+                     port->id);
         return -1;
     }
-    dev->info = info;
 
+    if (port->id == VIRTIO_CONSOLE_BAD_ID) {
+        if (plugging_port0) {
+            port->id = 0;
+        } else {
+            port->id = find_free_port_id(port->vser);
+            if (port->id == VIRTIO_CONSOLE_BAD_ID) {
+                error_report("virtio-serial-bus: Maximum port limit for this device reached\n");
+                return -1;
+            }
+        }
+    }
+
+    if (port->id >= port->vser->config.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);
+        return -1;
+    }
+
+    dev->info = info;
     ret = info->init(dev);
     if (ret) {
         return ret;
     }
 
-    port->id = plugging_port0 ? 0 : port->vser->config.nr_ports++;
-
     if (!use_multiport(port->vser)) {
         /*
          * Allow writes to guest in this case; we have no way of
@@ -550,6 +623,8 @@ static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
     port->ivq = port->vser->ivqs[port->id];
     port->ovq = port->vser->ovqs[port->id];
 
+    add_port(port->vser, port->id);
+
     /* Send an update to the guest about this new port added */
     virtio_notify_config(&port->vser->vdev);
 
@@ -562,26 +637,8 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
     VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev);
     VirtIOSerial *vser = port->vser;
 
-    send_control_event(port, VIRTIO_CONSOLE_PORT_REMOVE, 1);
+    remove_port(port->vser, port->id);
 
-    /*
-     * Don't decrement nr_ports here; thus we keep a linearly
-     * increasing port id. Not utilising an id again saves us a couple
-     * of complications:
-     *
-     * - Not having to bother about sending the port id to the guest
-     *   kernel on hotplug or on addition of new ports; the guest can
-     *   also linearly increment the port number. This is preferable
-     *   because the config space won't have the need to store a
-     *   ports_map.
-     *
-     * - Extra state to be stored for all the "holes" that got created
-     *   so that we keep filling in the ids from the least available
-     *   index.
-     *
-     * When such a functionality is desired, a control message to add
-     * a port can be introduced.
-     */
     QTAILQ_REMOVE(&vser->ports, port, next);
 
     if (port->info->exit)
@@ -641,11 +698,12 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports)
     }
 
     vser->config.max_nr_ports = max_nr_ports;
+    vser->ports_map = qemu_mallocz((max_nr_ports + 31) / 32);
     /*
      * Reserve location 0 for a console port for backward compat
      * (old kernel, new qemu)
      */
-    vser->config.nr_ports = 1;
+    mark_port_added(vser, 0);
 
     vser->vdev.get_features = get_features;
     vser->vdev.get_config = get_config;
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index f297b00..0548689 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -27,6 +27,8 @@
 /* Features supported */
 #define VIRTIO_CONSOLE_F_MULTIPORT	1
 
+#define VIRTIO_CONSOLE_BAD_ID           (~(uint32_t)0)
+
 struct virtio_console_config {
     /*
      * These two fields are used by VIRTIO_CONSOLE_F_SIZE which
@@ -36,7 +38,6 @@ struct virtio_console_config {
     uint16_t rows;
 
     uint32_t max_nr_ports;
-    uint32_t nr_ports;
 } __attribute__((packed));
 
 struct virtio_console_control {
@@ -46,12 +47,14 @@ struct virtio_console_control {
 };
 
 /* Some events for the internal messages (control packets) */
-#define VIRTIO_CONSOLE_PORT_READY	0
-#define VIRTIO_CONSOLE_CONSOLE_PORT	1
-#define VIRTIO_CONSOLE_RESIZE		2
-#define VIRTIO_CONSOLE_PORT_OPEN	3
-#define VIRTIO_CONSOLE_PORT_NAME	4
-#define VIRTIO_CONSOLE_PORT_REMOVE	5
+#define VIRTIO_CONSOLE_DEVICE_READY	0
+#define VIRTIO_CONSOLE_PORT_ADD		1
+#define VIRTIO_CONSOLE_PORT_REMOVE	2
+#define VIRTIO_CONSOLE_PORT_READY	3
+#define VIRTIO_CONSOLE_CONSOLE_PORT	4
+#define VIRTIO_CONSOLE_RESIZE		5
+#define VIRTIO_CONSOLE_PORT_OPEN	6
+#define VIRTIO_CONSOLE_PORT_NAME	7
 
 /* == In-qemu interface == */
 
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 06/18] virtio-serial: whitespace: match surrounding code

[Amit Shah]

The virtio-serial code doesn't mix declarations and definitions, so
separate them out on different lines.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index bb11a9b..8efba0b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -354,7 +354,10 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
 
 static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
 {
-    VirtIOSerial *vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+    VirtIOSerial *vser;
+
+    vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+
     if (vser->bus->max_nr_ports > 1) {
         features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT);
     }
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 07/18] virtio-serial: Remove redundant check for 0-sized write request

[Amit Shah]

The check for a 0-sized write request to a guest port is not necessary;
the while loop below won't be executed in this case and all will be
fine.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 8efba0b..c1d9851 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -91,9 +91,6 @@ static size_t write_to_port(VirtIOSerialPort *port,
     if (!virtio_queue_ready(vq)) {
         return 0;
     }
-    if (!size) {
-        return 0;
-    }
 
     while (offset < size) {
         int i;
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 08/18] virtio-serial: Update copyright year to 2010

[Amit Shah]

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c    |    2 +-
 hw/virtio-serial-bus.c |    2 +-
 hw/virtio-serial.h     |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 17b221d..bbbb6b8 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -1,7 +1,7 @@
 /*
  * Virtio Console and Generic Serial Port Devices
  *
- * Copyright Red Hat, Inc. 2009
+ * Copyright Red Hat, Inc. 2009, 2010
  *
  * Authors:
  *  Amit Shah <amit.shah@redhat.com>
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index c1d9851..c77ea4f 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -1,7 +1,7 @@
 /*
  * A bus for connecting virtio serial and console ports
  *
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009, 2010 Red Hat, Inc.
  *
  * Author(s):
  *  Amit Shah <amit.shah@redhat.com>
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 0548689..f023873 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -2,7 +2,7 @@
  * Virtio Serial / Console Support
  *
  * Copyright IBM, Corp. 2008
- * Copyright Red Hat, Inc. 2009
+ * Copyright Red Hat, Inc. 2009, 2010
  *
  * Authors:
  *  Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 09/18] virtio-serial: Propagate errors in initialising ports / devices in guest

[Amit Shah]

If adding of ports or devices in the guest fails we can send out a QMP
event so that management software can deal with it.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index c77ea4f..3a09f0d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -223,6 +223,11 @@ static void handle_control_message(VirtIOSerial *vser, void *buf)
 
     switch(cpkt.event) {
     case VIRTIO_CONSOLE_DEVICE_READY:
+        if (!cpkt.value) {
+            error_report("virtio-serial-bus: Guest failure in adding device %s\n",
+                         vser->bus->qbus.name);
+            break;
+        }
         /*
          * The device is up, we can now tell the device about all the
          * ports we have here.
@@ -233,6 +238,11 @@ static void handle_control_message(VirtIOSerial *vser, void *buf)
         break;
 
     case VIRTIO_CONSOLE_PORT_READY:
+        if (!cpkt.value) {
+            error_report("virtio-serial-bus: Guest failure in adding port %u for device %s\n",
+                         port->id, vser->bus->qbus.name);
+            break;
+        }
         /*
          * Now that we know the guest asked for the port name, we're
          * sure the guest has initialised whatever state is necessary
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 10/18] virtio-serial: Send out guest data to ports only if port is opened

[Amit Shah]

Data should be written only when ports are open.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 3a09f0d..6befd4d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -335,6 +335,11 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
             goto next_buf;
         }
 
+	if (!port->host_connected) {
+            ret = 0;
+            goto next_buf;
+        }
+
         /*
          * A port may not have any handler registered for consuming the
          * data that the guest sends or it may not have a chardev associated
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 11/18] iov: Introduce a new file for helpers around iovs, add iov_from_buf()

[Amit Shah]

The virtio-net code uses iov_fill() which fills an iov from a linear
buffer. The virtio-serial-bus code does something similar in an
open-coded function.

Create a new iov.c file that has iov_from_buf().

Convert virtio-net and virtio-serial-bus over to use this functionality.
virtio-net used ints to hold sizes, the new function is going to use
size_t types.

Later commits will add the opposite functionality -- going from an iov
to a linear buffer.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 Makefile               |    2 ++
 Makefile.objs          |    1 +
 hw/iov.c               |   33 +++++++++++++++++++++++++++++++++
 hw/iov.h               |   16 ++++++++++++++++
 hw/virtio-net.c        |   20 +++-----------------
 hw/virtio-serial-bus.c |   15 +++++++--------
 6 files changed, 62 insertions(+), 25 deletions(-)
 create mode 100644 hw/iov.c
 create mode 100644 hw/iov.h

diff --git a/Makefile b/Makefile
index a404fda..18e7368 100644
--- a/Makefile
+++ b/Makefile
@@ -124,6 +124,8 @@ curses.o: curses.c keymaps.h curses_keys.h
 
 bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
 
+iov.o: iov.c iov.h
+
 ######################################################################
 
 qemu-img.o: qemu-img-cmds.h
diff --git a/Makefile.objs b/Makefile.objs
index 69d6879..1c7c64b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -100,6 +100,7 @@ common-obj-y += keymaps.o
 common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
 common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-y += vnc.o acl.o d3des.o
+common-obj-y += iov.o
 common-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
 common-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/hw/iov.c b/hw/iov.c
new file mode 100644
index 0000000..07bd499
--- /dev/null
+++ b/hw/iov.c
@@ -0,0 +1,33 @@
+/*
+ * Helpers for getting linearized buffers from iov / filling buffers into iovs
+ *
+ * Copyright IBM, Corp. 2007, 2008
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * Author(s):
+ *  Anthony Liguori <aliguori@us.ibm.com>
+ *  Amit Shah <amit.shah@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "iov.h"
+
+size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
+                    const void *buf, size_t size)
+{
+    size_t offset;
+    unsigned int i;
+
+    offset = 0;
+    for (i = 0; offset < size && i < iovcnt; i++) {
+        size_t len;
+
+        len = MIN(iov[i].iov_len, size - offset);
+
+        memcpy(iov[i].iov_base, buf + offset, len);
+        offset += len;
+    }
+    return offset;
+}
diff --git a/hw/iov.h b/hw/iov.h
new file mode 100644
index 0000000..5e3e541
--- /dev/null
+++ b/hw/iov.h
@@ -0,0 +1,16 @@
+/*
+ * Helpers for getting linearized buffers from iov / filling buffers into iovs
+ *
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * Author(s):
+ *  Amit Shah <amit.shah@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu-common.h"
+
+size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
+                    const void *buf, size_t size);
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index acb3cec..d602c56 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include "iov.h"
 #include "virtio.h"
 #include "net.h"
 #include "net/checksum.h"
@@ -445,21 +446,6 @@ static void work_around_broken_dhclient(struct virtio_net_hdr *hdr,
     }
 }
 
-static int iov_fill(struct iovec *iov, int iovcnt, const void *buf, int count)
-{
-    int offset, i;
-
-    offset = i = 0;
-    while (offset < count && i < iovcnt) {
-        int len = MIN(iov[i].iov_len, count - offset);
-        memcpy(iov[i].iov_base, buf + offset, len);
-        offset += len;
-        i++;
-    }
-
-    return offset;
-}
-
 static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,
                           const void *buf, size_t size, size_t hdr_len)
 {
@@ -595,8 +581,8 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_
         }
 
         /* copy in packet.  ugh */
-        len = iov_fill(sg, elem.in_num,
-                       buf + offset, size - offset);
+        len = iov_from_buf(sg, elem.in_num,
+                           buf + offset, size - offset);
         total += len;
 
         /* signal other side */
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 6befd4d..a72b6b5 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -15,6 +15,7 @@
  * the COPYING file in the top-level directory.
  */
 
+#include "iov.h"
 #include "monitor.h"
 #include "qemu-queue.h"
 #include "sysbus.h"
@@ -84,27 +85,25 @@ static size_t write_to_port(VirtIOSerialPort *port,
 {
     VirtQueueElement elem;
     VirtQueue *vq;
-    size_t offset = 0;
-    size_t len = 0;
+    size_t offset;
 
     vq = port->ivq;
     if (!virtio_queue_ready(vq)) {
         return 0;
     }
 
+    offset = 0;
     while (offset < size) {
-        int i;
+        size_t len;
 
         if (!virtqueue_pop(vq, &elem)) {
             break;
         }
 
-        for (i = 0; offset < size && i < elem.in_num; i++) {
-            len = MIN(elem.in_sg[i].iov_len, size - offset);
+        len = iov_from_buf(elem.in_sg, elem.in_num,
+                           buf + offset, size - offset);
+        offset += len;
 
-            memcpy(elem.in_sg[i].iov_base, buf + offset, len);
-            offset += len;
-        }
         virtqueue_push(vq, &elem, len);
     }
 
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 12/18] iov: Add iov_to_buf and iov_size helpers

[Amit Shah]

iov_to_buf() puts the buffer contents in the iov in a linearized buffer.

iov_size() gets the length of the contents in the iov.

The iov_to_buf() function is the memcpy_to_iovec() function that was
used in virtio-ballon.c.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/iov.c            |   37 +++++++++++++++++++++++++++++++++++++
 hw/iov.h            |    3 +++
 hw/virtio-balloon.c |   35 ++++-------------------------------
 3 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/hw/iov.c b/hw/iov.c
index 07bd499..588cd04 100644
--- a/hw/iov.c
+++ b/hw/iov.c
@@ -31,3 +31,40 @@ size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
     }
     return offset;
 }
+
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
+                  void *buf, size_t offset, size_t size)
+{
+    uint8_t *ptr;
+    size_t iov_off, buf_off;
+    unsigned int i;
+
+    ptr = buf;
+    iov_off = 0;
+    buf_off = 0;
+    for (i = 0; i < iovcnt && size; i++) {
+        if (offset < (iov_off + iov[i].iov_len)) {
+            size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
+
+            memcpy(ptr + buf_off, iov[i].iov_base + (offset - iov_off), len);
+
+            buf_off += len;
+            offset += len;
+            size -= len;
+        }
+        iov_off += iov[i].iov_len;
+    }
+    return buf_off;
+}
+
+size_t iov_size(const struct iovec *iov, const unsigned int iovcnt)
+{
+    size_t len;
+    unsigned int i;
+
+    len = 0;
+    for (i = 0; i < iovcnt; i++) {
+        len += iov[i].iov_len;
+    }
+    return len;
+}
diff --git a/hw/iov.h b/hw/iov.h
index 5e3e541..60a8547 100644
--- a/hw/iov.h
+++ b/hw/iov.h
@@ -14,3 +14,6 @@
 
 size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
                     const void *buf, size_t size);
+size_t iov_to_buf(const struct iovec *iov, const unsigned int iovcnt,
+                  void *buf, size_t offset, size_t size);
+size_t iov_size(const struct iovec *iov, const unsigned int iovcnt);
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index f55f7ec..152af80 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include "iov.h"
 #include "qemu-common.h"
 #include "virtio.h"
 #include "pc.h"
@@ -92,33 +93,6 @@ static QObject *get_stats_qobject(VirtIOBalloon *dev)
     return QOBJECT(dict);
 }
 
-/* FIXME: once we do a virtio refactoring, this will get subsumed into common
- * code */
-static size_t memcpy_from_iovector(void *data, size_t offset, size_t size,
-                                   struct iovec *iov, int iovlen)
-{
-    int i;
-    uint8_t *ptr = data;
-    size_t iov_off = 0;
-    size_t data_off = 0;
-
-    for (i = 0; i < iovlen && size; i++) {
-        if (offset < (iov_off + iov[i].iov_len)) {
-            size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
-
-            memcpy(ptr + data_off, iov[i].iov_base + (offset - iov_off), len);
-
-            data_off += len;
-            offset += len;
-            size -= len;
-        }
-
-        iov_off += iov[i].iov_len;
-    }
-
-    return data_off;
-}
-
 static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOBalloon *s = to_virtio_balloon(vdev);
@@ -128,8 +102,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
         size_t offset = 0;
         uint32_t pfn;
 
-        while (memcpy_from_iovector(&pfn, offset, 4,
-                                    elem.out_sg, elem.out_num) == 4) {
+        while (iov_to_buf(elem.out_sg, elem.out_num, &pfn, offset, 4) == 4) {
             ram_addr_t pa;
             ram_addr_t addr;
 
@@ -181,8 +154,8 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
      */
     reset_stats(s);
 
-    while (memcpy_from_iovector(&stat, offset, sizeof(stat), elem->out_sg,
-                                elem->out_num) == sizeof(stat)) {
+    while (iov_to_buf(elem->out_sg, elem->out_num, &stat, offset, sizeof(stat))
+           == sizeof(stat)) {
         uint16_t tag = tswap16(stat.tag);
         uint64_t val = tswap64(stat.val);
 
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 13/18] virtio-serial: Handle scatter-gather buffers for control messages

[Amit Shah]

Current control messages are small enough to not be split into multiple
buffers but we could run into such a situation in the future or a
malicious guest could cause such a situation.

So handle the entire iov request for control messages.

Also ensure the size of the control request is >= what we expect
otherwise we risk accessing memory that we don't own.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
CC: Avi Kivity <avi@redhat.com>
Reported-by: Avi Kivity <avi@redhat.com>
---
 hw/virtio-serial-bus.c |   31 ++++++++++++++++++++++++++++---
 1 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index a72b6b5..b8410c3 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -204,7 +204,7 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
 }
 
 /* Guest wants to notify us of some event */
-static void handle_control_message(VirtIOSerial *vser, void *buf)
+static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
 {
     struct VirtIOSerialPort *port;
     struct virtio_console_control cpkt, *gcpkt;
@@ -213,6 +213,11 @@ static void handle_control_message(VirtIOSerial *vser, void *buf)
 
     gcpkt = buf;
 
+    if (len < sizeof(cpkt)) {
+        /* The guest sent an invalid control packet */
+        return;
+    }
+
     cpkt.event = lduw_p(&gcpkt->event);
     cpkt.value = lduw_p(&gcpkt->value);
 
@@ -306,13 +311,33 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtQueueElement elem;
     VirtIOSerial *vser;
+    uint8_t *buf;
+    size_t len;
 
     vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
 
+    len = 0;
+    buf = NULL;
     while (virtqueue_pop(vq, &elem)) {
-        handle_control_message(vser, elem.out_sg[0].iov_base);
-        virtqueue_push(vq, &elem, elem.out_sg[0].iov_len);
+        size_t cur_len, copied;
+
+        cur_len = iov_size(elem.out_sg, elem.out_num);
+        /*
+         * Allocate a new buf only if we didn't have one previously or
+         * if the size of the buf differs
+         */
+        if (cur_len > len) {
+            qemu_free(buf);
+
+            buf = qemu_malloc(cur_len);
+            len = cur_len;
+        }
+        copied = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, len);
+
+        handle_control_message(vser, buf, copied);
+        virtqueue_push(vq, &elem, copied);
     }
+    qemu_free(buf);
     virtio_notify(vdev, vq);
 }
 
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 14/18] virtio-serial: Handle scatter/gather input from the guest

[Amit Shah]

Current guests don't send more than one iov but it can change later.
Ensure we handle that case.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
CC: Avi Kivity <avi@redhat.com>
---
 hw/virtio-serial-bus.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index b8410c3..3053a35 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -351,7 +351,8 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
 
     while (virtqueue_pop(vq, &elem)) {
         VirtIOSerialPort *port;
-        size_t ret;
+        uint8_t *buf;
+        size_t ret, buf_size;
 
         port = find_port_by_vq(vser, vq);
         if (!port) {
@@ -374,9 +375,12 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
             goto next_buf;
         }
 
-        /* The guest always sends only one sg */
-        ret = port->info->have_data(port, elem.out_sg[0].iov_base,
-                                    elem.out_sg[0].iov_len);
+        buf_size = iov_size(elem.out_sg, elem.out_num);
+        buf = qemu_malloc(buf_size);
+        ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
+
+        ret = port->info->have_data(port, buf, ret);
+        qemu_free(buf);
 
     next_buf:
         virtqueue_push(vq, &elem, ret);
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 15/18] virtio-serial: Apps should consume all data that guest sends out / Fix virtio api abuse

[Amit Shah]

We cannot indicate to the guest how much data was consumed by an app for
out_bufs.  So we just have to assume the apps will consume all the data
that are handed over to them.

Fix the virtio api abuse in control_out() and handle_output().

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c    |    7 ++-----
 hw/virtio-serial-bus.c |    6 +++---
 hw/virtio-serial.h     |    6 +++---
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index bbbb6b8..caea11f 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -20,14 +20,11 @@ typedef struct VirtConsole {
 
 
 /* Callback function that's called when the guest sends us data */
-static size_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
+static void flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
 {
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
-    ssize_t ret;
 
-    ret = qemu_chr_write(vcon->chr, buf, len);
-
-    return ret < 0 ? 0 : ret;
+    qemu_chr_write(vcon->chr, buf, len);
 }
 
 /* Readiness of the guest to accept data on a port */
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 3053a35..ad44127 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -335,7 +335,7 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
         copied = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, len);
 
         handle_control_message(vser, buf, copied);
-        virtqueue_push(vq, &elem, copied);
+        virtqueue_push(vq, &elem, 0);
     }
     qemu_free(buf);
     virtio_notify(vdev, vq);
@@ -379,11 +379,11 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
         buf = qemu_malloc(buf_size);
         ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
 
-        ret = port->info->have_data(port, buf, ret);
+        port->info->have_data(port, buf, ret);
         qemu_free(buf);
 
     next_buf:
-        virtqueue_push(vq, &elem, ret);
+        virtqueue_push(vq, &elem, 0);
     }
     virtio_notify(vdev, vq);
 }
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index f023873..62d76a2 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -136,10 +136,10 @@ struct VirtIOSerialPortInfo {
 
     /*
      * Guest wrote some data to the port. This data is handed over to
-     * the app via this callback. The app should return the number of
-     * bytes it successfully consumed.
+     * the app via this callback.  The app is supposed to consume all
+     * the data that is presented to it.
      */
-    size_t (*have_data)(VirtIOSerialPort *port, const uint8_t *buf, size_t len);
+    void (*have_data)(VirtIOSerialPort *port, const uint8_t *buf, size_t len);
 };
 
 /* Interface to the virtio-serial bus */
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 16/18] virtio-serial: Discard data that guest sends us when ports aren't connected

[Amit Shah]

Before the earlier patch, we relied on incorrect virtio api usage to
signal to the guest that a particular buffer wasn't consumed by the
host.

After fixing that, we now just discard the data the guest sends us while
a host port is disconnected or doesn't have a handler registered for
consuming data.

This commit really doesn't change anything from the current behaviour,
just makes the code slightly better by spinning off data handling to
ports in another function.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |   69 ++++++++++++++++++++++-------------------------
 1 files changed, 32 insertions(+), 37 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index ad44127..8d07152 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -111,6 +111,30 @@ static size_t write_to_port(VirtIOSerialPort *port,
     return offset;
 }
 
+static void flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
+                              VirtIODevice *vdev, bool discard)
+{
+    VirtQueueElement elem;
+
+    assert(port || discard);
+
+    while (virtqueue_pop(vq, &elem)) {
+        uint8_t *buf;
+        size_t ret, buf_size;
+
+        if (!discard) {
+            buf_size = iov_size(elem.out_sg, elem.out_num);
+            buf = qemu_malloc(buf_size);
+            ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
+
+            port->info->have_data(port, buf, ret);
+            qemu_free(buf);
+        }
+        virtqueue_push(vq, &elem, 0);
+    }
+    virtio_notify(vdev, vq);
+}
+
 static size_t send_control_msg(VirtIOSerialPort *port, void *buf, size_t len)
 {
     VirtQueueElement elem;
@@ -345,47 +369,18 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
 static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOSerial *vser;
-    VirtQueueElement elem;
+    VirtIOSerialPort *port;
+    bool discard;
 
     vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+    port = find_port_by_vq(vser, vq);
 
-    while (virtqueue_pop(vq, &elem)) {
-        VirtIOSerialPort *port;
-        uint8_t *buf;
-        size_t ret, buf_size;
-
-        port = find_port_by_vq(vser, vq);
-        if (!port) {
-            ret = 0;
-            goto next_buf;
-        }
-
-	if (!port->host_connected) {
-            ret = 0;
-            goto next_buf;
-        }
-
-        /*
-         * A port may not have any handler registered for consuming the
-         * data that the guest sends or it may not have a chardev associated
-         * with it. Just ignore the data in that case.
-         */
-        if (!port->info->have_data) {
-            ret = 0;
-            goto next_buf;
-        }
-
-        buf_size = iov_size(elem.out_sg, elem.out_num);
-        buf = qemu_malloc(buf_size);
-        ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
-
-        port->info->have_data(port, buf, ret);
-        qemu_free(buf);
-
-    next_buf:
-        virtqueue_push(vq, &elem, 0);
+    discard = false;
+    if (!port || !port->host_connected || !port->info->have_data) {
+        discard = true;
     }
-    virtio_notify(vdev, vq);
+
+    flush_queued_data(port, vq, vdev, discard);
 }
 
 static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 17/18] virtio-serial: Implement flow control for individual ports

[Amit Shah]

Individual ports can now signal to the virtio-serial core to stop
sending data if the ports cannot immediately handle new data.  When a
port later unthrottles, any data queued up in the virtqueue are sent to
the port.

Disable throttling once a port is closed (and we discard all the
unconsumed buffers in the vq).

The guest kernel can reclaim the buffers when it receives the port close
event or when a port is being removed. Ensure we free up the buffers
before we send out any events to the guest.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |   50 ++++++++++++++++++++++++++++++++++++++++++-----
 hw/virtio-serial.h     |    9 ++++++++
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 8d07152..97694d5 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -111,14 +111,14 @@ static size_t write_to_port(VirtIOSerialPort *port,
     return offset;
 }
 
-static void flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
-                              VirtIODevice *vdev, bool discard)
+static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
+                                 VirtIODevice *vdev, bool discard)
 {
     VirtQueueElement elem;
 
     assert(port || discard);
 
-    while (virtqueue_pop(vq, &elem)) {
+    while ((discard || !port->throttled) && virtqueue_pop(vq, &elem)) {
         uint8_t *buf;
         size_t ret, buf_size;
 
@@ -135,6 +135,13 @@ static void flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
     virtio_notify(vdev, vq);
 }
 
+static void flush_queued_data(VirtIOSerialPort *port, bool discard)
+{
+    assert(port || discard);
+
+    do_flush_queued_data(port, port->ovq, &port->vser->vdev, discard);
+}
+
 static size_t send_control_msg(VirtIOSerialPort *port, void *buf, size_t len)
 {
     VirtQueueElement elem;
@@ -186,6 +193,13 @@ int virtio_serial_open(VirtIOSerialPort *port)
 int virtio_serial_close(VirtIOSerialPort *port)
 {
     port->host_connected = false;
+    /*
+     * If there's any data the guest sent which the app didn't
+     * consume, reset the throttling flag and discard the data.
+     */
+    port->throttled = false;
+    flush_queued_data(port, true);
+
     send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
 
     return 0;
@@ -227,6 +241,20 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
     return 0;
 }
 
+void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
+{
+    if (!port) {
+        return;
+    }
+
+    port->throttled = throttle;
+    if (throttle) {
+        return;
+    }
+
+    flush_queued_data(port, false);
+}
+
 /* Guest wants to notify us of some event */
 static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
 {
@@ -380,7 +408,11 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
         discard = true;
     }
 
-    flush_queued_data(port, vq, vdev, discard);
+    if (!discard && port->throttled) {
+        return;
+    }
+
+    do_flush_queued_data(port, vq, vdev, discard);
 }
 
 static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
@@ -555,6 +587,8 @@ static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
                    indent, "", port->guest_connected);
     monitor_printf(mon, "%*s dev-prop-int: host_connected: %d\n",
                    indent, "", port->host_connected);
+    monitor_printf(mon, "%*s dev-prop-int: throttled: %d\n",
+                   indent, "", port->throttled);
 }
 
 /* This function is only used if a port id is not provided by the user */
@@ -592,13 +626,17 @@ static void add_port(VirtIOSerial *vser, uint32_t port_id)
 
 static void remove_port(VirtIOSerial *vser, uint32_t port_id)
 {
+    VirtIOSerialPort *port;
     unsigned int i;
 
     i = port_id / 32;
     vser->ports_map[i] &= ~(1U << (port_id % 32));
 
-    send_control_event(find_port_by_id(vser, port_id),
-                       VIRTIO_CONSOLE_PORT_REMOVE, 1);
+    port = find_port_by_id(vser, port_id);
+    /* Flush out any unconsumed buffers first */
+    flush_queued_data(port, true);
+
+    send_control_event(port, VIRTIO_CONSOLE_PORT_REMOVE, 1);
 }
 
 static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 62d76a2..a93b545 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -110,6 +110,8 @@ struct VirtIOSerialPort {
     bool guest_connected;
     /* Is this device open for IO on the host? */
     bool host_connected;
+    /* Do apps not want to receive data? */
+    bool throttled;
 };
 
 struct VirtIOSerialPortInfo {
@@ -173,4 +175,11 @@ ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
  */
 size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
 
+/*
+ * Flow control: Ports can signal to the virtio-serial core to stop
+ * sending data or re-start sending data, depending on the 'throttle'
+ * value here.
+ */
+void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
+
 #endif
-- 
1.6.2.5

[Qemu-devel] [PATCH v6 18/18] virtio-serial-bus: wake up iothread upon guest read notification

[Amit Shah]

From: Marcelo Tosatti <mtosatti@redhat.com>

Wake up iothread when buffers are consumed.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 97694d5..ee868e9 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -417,6 +417,7 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
 
 static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
 {
+    qemu_notify_event();
 }
 
 static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH v6 05/18] virtio-serial: Use control messages to notify guest of new ports

[Anthony Liguori]

On 04/27/2010 07:33 AM, Amit Shah wrote:
> Allow the port 'id's to be set by a user on the command line. This is
> needed by management apps that will want a stable port numbering scheme
> for hot-plug/unplug and migration.
>
> Since the port numbers are shared with the guest (to identify ports in
> control messages), we just send a control message to the guest
> indicating addition of new ports (hot-plug) or notifying the guest of
> the available ports when the guest sends us a DEVICE_READY control
> message.
>
> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>    

If you're introducing a new message type, don't you need to negotiate 
that feature with the guest?

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH v6 18/18] virtio-serial-bus: wake up iothread upon guest read notification

[Anthony Liguori]

On 04/27/2010 07:34 AM, Amit Shah wrote:
> From: Marcelo Tosatti<mtosatti@redhat.com>
>
> Wake up iothread when buffers are consumed.
>
> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>    

What's the race here?  This looks very odd to me.

Regards,

Anthony Liguori

> ---
>   hw/virtio-serial-bus.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 97694d5..ee868e9 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -417,6 +417,7 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
>
>   static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
>   {
> +    qemu_notify_event();
>   }
>
>   static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
>    

[Qemu-devel] Re: [PATCH v6 18/18] virtio-serial-bus: wake up iothread upon guest read notification

[Marcelo Tosatti]

On Tue, Apr 27, 2010 at 12:41:27PM -0500, Anthony Liguori wrote:
> On 04/27/2010 07:34 AM, Amit Shah wrote:
> >From: Marcelo Tosatti<mtosatti@redhat.com>
> >
> >Wake up iothread when buffers are consumed.
> >
> >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> >Signed-off-by: Amit Shah<amit.shah@redhat.com>
> 
> What's the race here?  This looks very odd to me.

We discussed this on the following thread:

http://www.mail-archive.com/kvm@vger.kernel.org/msg29249.html

[Qemu-devel] Re: [PATCH v6 18/18] virtio-serial-bus: wake up iothread upon guest read notification

[Anthony Liguori]

On 04/27/2010 12:58 PM, Marcelo Tosatti wrote:
> On Tue, Apr 27, 2010 at 12:41:27PM -0500, Anthony Liguori wrote:
>    
>> On 04/27/2010 07:34 AM, Amit Shah wrote:
>>      
>>> From: Marcelo Tosatti<mtosatti@redhat.com>
>>>
>>> Wake up iothread when buffers are consumed.
>>>
>>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>>        
>> What's the race here?  This looks very odd to me.
>>      
> We discussed this on the following thread:
>
> http://www.mail-archive.com/kvm@vger.kernel.org/msg29249.html
>    

I don't think there was ever an adequate explanation of exactly what was 
happening.

My suspicion is that the real problem is due to the can_read() handlers 
whereas it's necessary to invoke the main loop for can_read() to be 
executed again to cause the event to be polled.  The better solution 
though is to not use the can_read() handler and instead explicitly 
register and deregister the read callbacks.

qemu_set_fd_handler() would require a notify call but that makes sense.

I think this is a good justification for removing uses of 
qemu_set_fd_handler2().

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH v6 05/18] virtio-serial: Use control messages to notify guest of new ports

[Amit Shah]

On (Tue) Apr 27 2010 [12:37:00], Anthony Liguori wrote:
> On 04/27/2010 07:33 AM, Amit Shah wrote:
>> Allow the port 'id's to be set by a user on the command line. This is
>> needed by management apps that will want a stable port numbering scheme
>> for hot-plug/unplug and migration.
>>
>> Since the port numbers are shared with the guest (to identify ports in
>> control messages), we just send a control message to the guest
>> indicating addition of new ports (hot-plug) or notifying the guest of
>> the available ports when the guest sends us a DEVICE_READY control
>> message.
>>
>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>    
>
> If you're introducing a new message type, don't you need to negotiate  
> that feature with the guest?

Since we didn't have any kernel or qemu released with the older ABI,
changing the numbers isn't a bad thing (we disabled multiport support
for 2.6.34 because of this).

		Amit

[Qemu-devel] Re: [PATCH v6 18/18] virtio-serial-bus: wake up iothread upon guest read notification

[Amit Shah]

On (Tue) Apr 27 2010 [12:41:27], Anthony Liguori wrote:
> On 04/27/2010 07:34 AM, Amit Shah wrote:
>> From: Marcelo Tosatti<mtosatti@redhat.com>
>>
>> Wake up iothread when buffers are consumed.
>>
>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>    
>
> What's the race here?  This looks very odd to me.

When the guest indicates it has added buffers to the vq, the iothread
can then start consuming them. Without this notification, the iothread
only polls for free buffers when it times out or gets woken up
otherwise.

Other virtio devices do it similarly as well.

		Amit

[Qemu-devel] Re: [PATCH v6 18/18] virtio-serial-bus: wake up iothread upon guest read notification

[Anthony Liguori]

On 04/28/2010 02:29 AM, Amit Shah wrote:
> On (Tue) Apr 27 2010 [12:41:27], Anthony Liguori wrote:
>    
>> On 04/27/2010 07:34 AM, Amit Shah wrote:
>>      
>>> From: Marcelo Tosatti<mtosatti@redhat.com>
>>>
>>> Wake up iothread when buffers are consumed.
>>>
>>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>>
>>>        
>> What's the race here?  This looks very odd to me.
>>      
> When the guest indicates it has added buffers to the vq, the iothread
> can then start consuming them. Without this notification, the iothread
> only polls for free buffers when it times out or gets woken up
> otherwise.
>    

When you say, polls for free buffers, what do you mean by that?

You mean, there's a can_read() somewhere that checks for free buffers?

I think switching to qemu_set_fd_handler() would be a better solution.

Regards,

Anthony Liguori

> Other virtio devices do it similarly as well.
>
> 		Amit
>    

[Qemu-devel] Re: [PATCH v6 05/18] virtio-serial: Use control messages to notify guest of new ports

[Anthony Liguori]

On 04/27/2010 11:27 PM, Amit Shah wrote:
> On (Tue) Apr 27 2010 [12:37:00], Anthony Liguori wrote:
>    
>> On 04/27/2010 07:33 AM, Amit Shah wrote:
>>      
>>> Allow the port 'id's to be set by a user on the command line. This is
>>> needed by management apps that will want a stable port numbering scheme
>>> for hot-plug/unplug and migration.
>>>
>>> Since the port numbers are shared with the guest (to identify ports in
>>> control messages), we just send a control message to the guest
>>> indicating addition of new ports (hot-plug) or notifying the guest of
>>> the available ports when the guest sends us a DEVICE_READY control
>>> message.
>>>
>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>>
>>>        
>> If you're introducing a new message type, don't you need to negotiate
>> that feature with the guest?
>>      
> Since we didn't have any kernel or qemu released with the older ABI,
> changing the numbers isn't a bad thing (we disabled multiport support
> for 2.6.34 because of this).
>    

Ok.

Regards,

Anthony Liguori

> 		Amit
>    

[Qemu-devel] Re: [PATCH v6 18/18] virtio-serial-bus: wake up iothread upon guest read notification

[Amit Shah]

On (Wed) Apr 28 2010 [08:25:59], Anthony Liguori wrote:
> On 04/28/2010 02:29 AM, Amit Shah wrote:
>> On (Tue) Apr 27 2010 [12:41:27], Anthony Liguori wrote:
>>    
>>> On 04/27/2010 07:34 AM, Amit Shah wrote:
>>>      
>>>> From: Marcelo Tosatti<mtosatti@redhat.com>
>>>>
>>>> Wake up iothread when buffers are consumed.
>>>>
>>>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
>>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>>>
>>>>        
>>> What's the race here?  This looks very odd to me.
>>>      
>> When the guest indicates it has added buffers to the vq, the iothread
>> can then start consuming them. Without this notification, the iothread
>> only polls for free buffers when it times out or gets woken up
>> otherwise.
>>    
>
> When you say, polls for free buffers, what do you mean by that?
>
> You mean, there's a can_read() somewhere that checks for free buffers?

Not just can_read(), in-qemu applications too can query when writes to
guests will go through.

> I think switching to qemu_set_fd_handler() would be a better solution.

Hm, there's no fd here. How to signal to apps (ports) when a guest
becomes writable?

		Amit

[Qemu-devel] Re: [PATCH v6 01/18] virtio-serial: save/load: Ensure target has enough ports

[Anthony Liguori]

Applied 1-17.  Thanks.

Regards,

Anthony Liguori

On 04/27/2010 07:33 AM, Amit Shah wrote:
> The target could be started with max_nr_ports for a virtio-serial device
> lesser than what was available on the source machine. Fail the migration
> in such a case.
>
> Signed-off-by: Amit Shah<amit.shah@redhat.com>
> Reported-by: Juan Quintela<quintela@redhat.com>
> ---
>   hw/virtio-serial-bus.c |   13 +++++++++++--
>   1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 17c1ec1..9a7f0c1 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -374,10 +374,13 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
>
>       /* Items in struct VirtIOSerial */
>
> +    qemu_put_be32s(f,&s->bus->max_nr_ports);
> +
>       /* Do this because we might have hot-unplugged some ports */
>       nr_active_ports = 0;
> -    QTAILQ_FOREACH(port,&s->ports, next)
> +    QTAILQ_FOREACH(port,&s->ports, next) {
>           nr_active_ports++;
> +    }
>
>       qemu_put_be32s(f,&nr_active_ports);
>
> @@ -399,7 +402,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>   {
>       VirtIOSerial *s = opaque;
>       VirtIOSerialPort *port;
> -    uint32_t nr_active_ports;
> +    uint32_t max_nr_ports, nr_active_ports;
>       unsigned int i;
>
>       if (version_id>  2) {
> @@ -420,6 +423,12 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>
>       /* Items in struct VirtIOSerial */
>
> +    qemu_get_be32s(f,&max_nr_ports);
> +    if (max_nr_ports>  s->bus->max_nr_ports) {
> +        /* Source could have more ports than us. Fail migration. */
> +        return -EINVAL;
> +    }
> +
>       qemu_get_be32s(f,&nr_active_ports);
>
>       /* Items in struct VirtIOSerialPort */
>