[Qemu-devel] Re: [PATCH master/0.14 2/2] virtio-serial: Add older machine compat property for flow control

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 2011-02-03 at 11:47 +0530, Amit Shah wrote:
> Add a compat property for older machine types.  When this is used (via
> -M pc-0.13, for example), the new flow control mechanisms will not be
> used.  This is done to keep migration from a machine started with older
> type on a pc-0.14+ qemu to an older machine working.
> 
> The property is named 'flow_control' and defaults to on.
> 
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/pc_piix.c           |   16 ++++++++++
>  hw/virtio-pci.c        |    2 +
>  hw/virtio-serial-bus.c |   77 +++++++++++++++++++++++++++++++++++++++++-------
>  hw/virtio-serial.h     |   12 +++++++
>  4 files changed, 96 insertions(+), 11 deletions(-)

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 7b74473..05b0449 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -243,6 +243,10 @@ static QEMUMachine pc_machine_v0_13 = {
>              .driver   = "PCI",
>              .property = "command_serr_enable",
>              .value    = "off",
> +        },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
>          },
>          { /* end of list */ }
>      },
> @@ -263,6 +267,10 @@ static QEMUMachine pc_machine_v0_12 = {
>              .property = "vectors",
>              .value    = stringify(0),
>          },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
> +        },{
>              .driver   = "VGA",
>              .property = "rombar",
>              .value    = stringify(0),
> @@ -298,6 +306,10 @@ static QEMUMachine pc_machine_v0_11 = {
>              .property = "vectors",
>              .value    = stringify(0),
>          },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
> +        },{
>              .driver   = "ide-drive",
>              .property = "ver",
>              .value    = "0.11",
> @@ -341,6 +353,10 @@ static QEMUMachine pc_machine_v0_10 = {
>              .property = "vectors",
>              .value    = stringify(0),
>          },{
> +            .driver   = "virtio-serial-pci",
> +            .property = "flow_control",
> +            .value    = stringify(0),
> +        },{
>              .driver   = "virtio-net-pci",
>              .property = "vectors",
>              .value    = stringify(0),
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 952b5d2..530ce9d 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -904,6 +904,8 @@ static PCIDeviceInfo virtio_info[] = {
>              DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
>              DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy,
>                                 serial.max_virtserial_ports, 31),
> +            DEFINE_PROP_UINT32("flow_control", VirtIOPCIProxy,
> +                               serial.flow_control, 1),
>              DEFINE_PROP_END_OF_LIST(),
>          },
>          .qdev.reset = virtio_pci_reset,
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 1753785..f681646 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -49,6 +49,8 @@ struct VirtIOSerial {
>      uint32_t *ports_map;
>  
>      struct virtio_console_config config;
> +
> +    bool flow_control;
>  };
>  
>  static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
> @@ -123,12 +125,46 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev)
>      virtio_notify(vdev, vq);
>  }
>  
> +static void do_flush_queued_data_no_flow_control(VirtIOSerialPort *port,
> +                                                 VirtQueue *vq,
> +                                                 VirtIODevice *vdev)
> +{
> +    VirtQueueElement elem;
> +
> +    while (!port->throttled && virtqueue_pop(vq, &elem)) {
> +        uint8_t *buf;
> +        size_t ret, buf_size;
> +
> +        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);
> +
> +        /*
> +         * have_data has been modified to return the number of bytes
> +         * successfully consumed.  We can't act upon that information
> +         * in the no-flow-control implementation, so we'll discard it
> +         * here.  No callers currently use flow control, but they
> +         * should take this into account for backward compatibility.
> +         */
> +        port->info->have_data(port, buf, ret);
> +        qemu_free(buf);
> +
> +        virtqueue_push(vq, &elem, 0);
> +    }
> +    virtio_notify(vdev, vq);
> +}
> +
>  static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
>                                   VirtIODevice *vdev)
>  {
>      assert(port);
>      assert(virtio_queue_ready(vq));
>  
> +    if (!virtio_serial_flow_control_enabled(port)) {
> +        do_flush_queued_data_no_flow_control(port, vq, vdev);
> +        return;
> +    }
> +
>      while (!port->throttled) {
>          unsigned int i;
>  
> @@ -296,6 +332,15 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
>      flush_queued_data(port);
>  }
>  
> +bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port)
> +{
> +    if (!port) {
> +        return false;
> +    }
> +
> +    return port->vser->flow_control;
> +}
> +
>  /* Guest wants to notify us of some event */
>  static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
>  {
> @@ -533,17 +578,19 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
>          qemu_put_byte(f, port->guest_connected);
>          qemu_put_byte(f, port->host_connected);
>  
> -	elem_popped = 0;
> -        if (port->elem.out_num) {
> -            elem_popped = 1;
> -        }
> -        qemu_put_be32s(f, &elem_popped);
> -        if (elem_popped) {
> -            qemu_put_be32s(f, &port->iov_idx);
> -            qemu_put_be64s(f, &port->iov_offset);
> +        if (virtio_serial_flow_control_enabled(port)) {
> +            elem_popped = 0;
> +            if (port->elem.out_num) {
> +                elem_popped = 1;
> +            }
> +            qemu_put_be32s(f, &elem_popped);
> +            if (elem_popped) {
> +                qemu_put_be32s(f, &port->iov_idx);
> +                qemu_put_be64s(f, &port->iov_offset);
>  
> -            qemu_put_buffer(f, (unsigned char *)&port->elem,
> -                            sizeof(port->elem));
> +                qemu_put_buffer(f, (unsigned char *)&port->elem,
> +                                sizeof(port->elem));
> +            }
>          }
>      }
>  }
> @@ -816,6 +863,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
>      VirtIOSerial *vser;
>      VirtIODevice *vdev;
>      uint32_t i, max_supported_ports;
> +    unsigned int savevm_ver;
>  
>      if (!conf->max_virtserial_ports)
>          return NULL;
> @@ -881,11 +929,18 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
>  
>      vser->qdev = dev;
>  
> +    vser->flow_control = true;
> +    savevm_ver = 3;
> +    if (!conf->flow_control) {
> +        vser->flow_control = false;
> +        savevm_ver = 2;
> +    }
> +
>      /*
>       * Register for the savevm section with the virtio-console name
>       * to preserve backward compat
>       */
> -    register_savevm(dev, "virtio-console", -1, 3, virtio_serial_save,
> +    register_savevm(dev, "virtio-console", -1, savevm_ver, virtio_serial_save,
>                      virtio_serial_load, vser);
>  
>      return vdev;
> diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
> index de624c3..57d2dee 100644
> --- a/hw/virtio-serial.h
> +++ b/hw/virtio-serial.h
> @@ -48,6 +48,13 @@ struct virtio_console_control {
>  struct virtio_serial_conf {
>      /* Max. number of ports we can have for a the virtio-serial device */
>      uint32_t max_virtserial_ports;
> +
> +    /*
> +     * Should this device behave the way it did in prior to pc-0.14
> +     * (ie. no flow control)?  This will be necessary to allow
> +     * migrations from a pre-0.14-machine type to older qemu code
> +     */
> +    uint32_t flow_control;
>  };
>  
>  /* Some events for the internal messages (control packets) */
> @@ -204,4 +211,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
>   */
>  void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
>  
> +/*
> + * Does this machine type disable the use of flow control?
> + */
> +bool virtio_serial_flow_control_enabled(VirtIOSerialPort *port);
> +
>  #endif