Re: [Qemu-devel] [PATCH v2] qdev: don't add typename to fw_dev_path when get_fw_dev_path isn't implemented

[Paolo Bonzini <pbonzini@redhat.com>]

Il 28/05/2013 13:00, Amos Kong ha scritto:
> Recent virtio refactoring in QEMU made virtio-bus become the parent bus
> of scsi-bus, and virtio-bus doesn't have get_fw_dev_path implementation,
> so redundant typename will be added to fw_dev_path. It causes that
> bootindex parameter of scsi device doesn't work.
> 
> This patch changes qdev_get_fw_dev_path_helper() to add nothing if
> implemented get_fw_dev_path() returns NULL.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> --
> v2: only add nothing to fw_dev_path when get_fw_dev_path() is
>     implemented and return NULL. then it will not effect other devices
>     don't have get_fw_dev_path() implementation.

To achieve the same result, you can simply ensure that all classes
implement the method.  The default implementation is just
g_strdup(object_get_typename(OBJECT(dev))).

Remember that if you do not specify a method in a BusClass's class_init
function, it is inherited from the superclass.

Paolo

> ---
>  hw/core/qdev.c         | 5 +++++
>  hw/virtio/virtio-bus.c | 6 ++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6985ad8..2a08368 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -509,11 +509,16 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
>  
>      if (dev && dev->parent_bus) {
>          char *d;
> +        BusClass *parent_bc = BUS_GET_CLASS(dev->parent_bus);
> +
>          l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
>          d = bus_get_fw_dev_path(dev->parent_bus, dev);
> +
>          if (d) {
>              l += snprintf(p + l, size - l, "%s", d);
>              g_free(d);
> +        } else if (parent_bc->get_fw_dev_path) {
> +            return l;
>          } else {
>              l += snprintf(p + l, size - l, "%s", object_get_typename(OBJECT(dev)));
>          }
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index ea2e11a..6849a01 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -161,10 +161,16 @@ static char *virtio_bus_get_dev_path(DeviceState *dev)
>      return qdev_get_dev_path(proxy);
>  }
>  
> +static char *virtio_bus_get_fw_dev_path(DeviceState *dev)
> +{
> +    return NULL;
> +}
> +
>  static void virtio_bus_class_init(ObjectClass *klass, void *data)
>  {
>      BusClass *bus_class = BUS_CLASS(klass);
>      bus_class->get_dev_path = virtio_bus_get_dev_path;
> +    bus_class->get_fw_dev_path = virtio_bus_get_fw_dev_path;

This hunk is correct.

>  }
>  
>  static const TypeInfo virtio_bus_info = {
>