Re: [Qemu-devel] [PATCH 1/2] virtio-serial: create a linked list of all active devices

[Markus Armbruster]

Amit Shah <address@hidden> writes:

> To ensure two virtserialports don't get added to the system with the
> same 'name' parameter, we need to access all the ports on all the
> devices added, and compare the names.
>
> We currently don't have a list of all VirtIOSerial devices added to the
> system.  This commit adds a simple linked list in which devices are put
> when they're initialized, and removed when they go away.
>
> Signed-off-by: Amit Shah <address@hidden>
> ---
>  hw/char/virtio-serial-bus.c       | 10 ++++++++++
>  include/hw/virtio/virtio-serial.h |  2 ++
>  2 files changed, 12 insertions(+)
>
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 07bebc0..8c26f4e 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -26,6 +26,10 @@
>  #include "hw/virtio/virtio-serial.h"
>  #include "hw/virtio/virtio-access.h"
>  
> +struct VirtIOSerialDevices {
> +    QLIST_HEAD(, VirtIOSerial) devices;
> +} vserdevices;
> +

Any particular reason for stuffing the list into a struct?

>  static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
>  {
>      VirtIOSerialPort *port;
> @@ -975,6 +979,8 @@ static void virtio_serial_device_realize(DeviceState 
> *dev, Error **errp)
>       */
>      register_savevm(dev, "virtio-console", -1, 3, virtio_serial_save,
>                      virtio_serial_load, vser);
> +
> +    QLIST_INSERT_HEAD(&vserdevices.devices, vser, next);
>  }
>  
>  static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
> @@ -1003,6 +1009,8 @@ static void virtio_serial_device_unrealize(DeviceState 
> *dev, Error **errp)
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VirtIOSerial *vser = VIRTIO_SERIAL(dev);
>  
> +    QLIST_REMOVE(vser, next);
> +
>      unregister_savevm(dev, "virtio-console", vser);
>  
>      g_free(vser->ivqs);
> @@ -1027,6 +1035,8 @@ static void virtio_serial_class_init(ObjectClass 
> *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
>  
> +    QLIST_INIT(&vserdevices.devices);
> +
>      dc->props = virtio_serial_properties;
>      set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>      vdc->realize = virtio_serial_device_realize;
> diff --git a/include/hw/virtio/virtio-serial.h 
> b/include/hw/virtio/virtio-serial.h
> index 4746312..a679e54 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -202,6 +202,8 @@ struct VirtIOSerial {
>  
>      QTAILQ_HEAD(, VirtIOSerialPort) ports;
>  
> +    QLIST_ENTRY(VirtIOSerial) next;
> +
>      /* bitmap for identifying active ports */
>      uint32_t *ports_map;

Patch looks simple & safe to me, but I can't help to wonder whether want
(or already have?) more generic infrastructure offering "for all devices
of a certain kind" functionality, which is what 2/2 needs.  Andreas?