diff mbox

[3/8] virtio-serial-bus: Maintain guest and host port open/close state

Message ID 1261597948-24293-4-git-send-email-amit.shah@redhat.com
State New
Headers show

Commit Message

Amit Shah Dec. 23, 2009, 7:52 p.m. UTC 
Via control channel messages, the guest can tell us whether a port got
opened or closed. Similarly, we can also indicate to the guest of host
port open/close events.

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

Comments

Anthony Liguori Dec. 23, 2009, 11:14 p.m. UTC | #1 
On 12/23/2009 01:52 PM, Amit Shah wrote:
> Via control channel messages, the guest can tell us whether a port got
> opened or closed. Similarly, we can also indicate to the guest of host
> port open/close events.
>
> Signed-off-by: Amit Shah<amit.shah@redhat.com>
> ---
>   hw/virtio-serial-bus.c |   73 ++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/virtio-serial.c     |    6 ++++
>   hw/virtio-serial.h     |    6 ++++
>   3 files changed, 85 insertions(+), 0 deletions(-)
>

> @@ -347,6 +378,8 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data)
>   static void virtio_serial_save(QEMUFile *f, void *opaque)
>   {
>       VirtIOSerial *s = opaque;
> +    VirtIOSerialPort *port;
> +    uint32_t nr_active_ports;
>
>       /* The virtio device */
>       virtio_save(&s->vdev, f);
> @@ -357,11 +390,35 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
>
>       /* Items in struct VirtIOSerial */
>       qemu_put_be32s(f,&s->guest_features);
> +
> +    /* Do this because we might have hot-unplugged some ports */
> +    nr_active_ports = 0;
> +    QTAILQ_FOREACH(port,&s->ports, next)
> +        nr_active_ports++;
> +
> +    qemu_put_be32s(f,&nr_active_ports);
> +
> +    /*
> +     * 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);
> +
> +    }
>   }


I imagine this sort of thing is going to give Juan quite a head-ache 
when it comes time to VMState conversion.

Are there not separate qdev devices for each port?  Can't the state be 
stored in that?

Regards,

Anthony Liguori
Amit Shah Dec. 24, 2009, 5:27 a.m. UTC | #2 
On (Wed) Dec 23 2009 [17:14:28], Anthony Liguori wrote:
>
>> +    /*
>> +     * 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);
>> +
>> +    }
>>   }
>
>
> I imagine this sort of thing is going to give Juan quite a head-ache  
> when it comes time to VMState conversion.

Juan has gone through this and he thinks it's fine; conversion to
vmstate shouldn't be a problem. Unless things have changed since then..

> Are there not separate qdev devices for each port?  Can't the state be  
> stored in that?

All the state here is common to all the ports so it makes sense to do it
here, in one place.

For any extra state the ports might have, they can do their own
save/restore.

		Amit
diff mbox

Patch

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 9eb60c6..27120da 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -170,11 +170,22 @@  static size_t send_control_event(VirtIOSerialPort *port, uint16_t event,
 /* Functions for use inside qemu to open and read from/write to ports */
 int virtio_serial_open(VirtIOSerialPort *port)
 {
+    /* Don't allow opening an already-open port */
+    if (port->host_connected) {
+        return 0;
+    }
+    /* Send port open notification to the guest */
+    port->host_connected = true;
+    send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+
     return 0;
 }
 
 int virtio_serial_close(VirtIOSerialPort *port)
 {
+    port->host_connected = false;
+    send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
+
     return 0;
 }
 
@@ -182,6 +193,9 @@  int virtio_serial_close(VirtIOSerialPort *port)
 ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
                             size_t size)
 {
+    if (!port || !port->host_connected || !port->guest_connected) {
+        return 0;
+    }
     return write_to_port(port, buf, size);
 }
 
@@ -202,6 +216,9 @@  size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
         virtio_queue_empty(vq)) {
         return 0;
     }
+    if (use_multiport(port->vser) && !port->guest_connected) {
+        return 0;
+    }
 
     size = 4096;
     if (virtqueue_avail_bytes(vq, size, 0)) {
@@ -237,6 +254,9 @@  static void handle_control_message(VirtIOSerial *vser, void *buf)
         if (port->is_console) {
             send_control_event(port, VIRTIO_CONSOLE_CONSOLE_PORT, 1);
         }
+        if (port->host_connected) {
+            send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+        }
         /*
          * When the guest has asked us for this information it means
          * the guest is all setup and has its virtqueues
@@ -247,6 +267,17 @@  static void handle_control_message(VirtIOSerial *vser, void *buf)
             port->info->guest_ready(port);
         }
         break;
+    case VIRTIO_CONSOLE_PORT_OPEN:
+        port->guest_connected = cpkt->value;
+        if (cpkt->value && port->info->guest_open) {
+            /* Send the guest opened notification if an app is interested */
+            port->info->guest_open(port);
+        }
+        if (!cpkt->value && port->info->guest_close) {
+            /* Send the guest closed notification if an app is interested */
+            port->info->guest_close(port);
+        }
+        break;
     }
 }
 
@@ -347,6 +378,8 @@  static void set_config(VirtIODevice *vdev, const uint8_t *config_data)
 static void virtio_serial_save(QEMUFile *f, void *opaque)
 {
     VirtIOSerial *s = opaque;
+    VirtIOSerialPort *port;
+    uint32_t nr_active_ports;
 
     /* The virtio device */
     virtio_save(&s->vdev, f);
@@ -357,11 +390,35 @@  static void virtio_serial_save(QEMUFile *f, void *opaque)
 
     /* Items in struct VirtIOSerial */
     qemu_put_be32s(f, &s->guest_features);
+
+    /* Do this because we might have hot-unplugged some ports */
+    nr_active_ports = 0;
+    QTAILQ_FOREACH(port, &s->ports, next)
+        nr_active_ports++;
+
+    qemu_put_be32s(f, &nr_active_ports);
+
+    /*
+     * 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);
+
+    }
 }
 
 static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIOSerial *s = opaque;
+    VirtIOSerialPort *port;
+    uint32_t nr_active_ports;
+    unsigned int i;
 
     if (version_id > 2) {
         return -EINVAL;
@@ -380,6 +437,18 @@  static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
     /* Items in struct VirtIOSerial */
     qemu_get_be32s(f, &s->guest_features);
 
+    qemu_get_be32s(f, &nr_active_ports);
+
+    /* Items in struct VirtIOSerialPort */
+    for (i = 0; i < nr_active_ports; i++) {
+        uint32_t id;
+
+        id = qemu_get_be32(f);
+        port = find_port_by_id(s, id);
+
+        port->guest_connected = qemu_get_byte(f);
+
+    }
     return 0;
 }
 
@@ -410,6 +479,10 @@  static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
                    indent, "", port->id);
     monitor_printf(mon, "%*s dev-prop-int: is_console: %d\n",
                    indent, "", port->is_console);
+    monitor_printf(mon, "%*s dev-prop-int: guest_connected: %d\n",
+                   indent, "", port->guest_connected);
+    monitor_printf(mon, "%*s dev-prop-int: host_connected: %d\n",
+                   indent, "", port->host_connected);
 }
 
 static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
diff --git a/hw/virtio-serial.c b/hw/virtio-serial.c
index a52ede8..654a259 100644
--- a/hw/virtio-serial.c
+++ b/hw/virtio-serial.c
@@ -69,6 +69,12 @@  static int virtconsole_initfn(VirtIOSerialDevice *dev)
     /* Tell the guest we're a console so it attaches us to an hvc console */
     port->is_console = true;
 
+    /*
+     * For console ports, just assume the guest is ready to accept our
+     * data.
+     */
+    port->guest_connected = true;
+
     if (vcon->chr) {
         qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
                               vcon);
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index e604abc..b855375 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -53,6 +53,7 @@  struct virtio_console_header {
 #define VIRTIO_CONSOLE_PORT_READY	0
 #define VIRTIO_CONSOLE_CONSOLE_PORT	1
 #define VIRTIO_CONSOLE_RESIZE		2
+#define VIRTIO_CONSOLE_PORT_OPEN	3
 
 /* == In-qemu interface == */
 
@@ -96,6 +97,11 @@  struct VirtIOSerialPort {
 
     /* Identify if this is a port that binds with hvc in the guest */
     uint8_t is_console;
+
+    /* Is the corresponding guest device open? */
+    bool guest_connected;
+    /* Is this device open for IO on the host? */
+    bool host_connected;
 };
 
 struct VirtIOSerialPortInfo {