Re: [Qemu-ppc] [PATCH v3 16/22] memory-device: add optional function get_device_id()

[David Gibson]

On Thu, Sep 20, 2018 at 12:32:37PM +0200, David Hildenbrand wrote:
> When reporting the id of virtio-based memory devices, we always have to
> take the one of the proxy device (parent), not the one of the memory
> device directly.
> 
> Let's generalize this by allowing memory devices to specify an optional
> "get_device_id" function. This id can then be used to report errors to the
> user from memory-device.c code, without having to special case e.g.
> virtio devices.
> 
> While at it, properly treat id == NULL and report "(unnamed)" instead.
> 
> Details:
> 
> When the user creates a virtio device (e.g.  virtio-balloon-pci), two
> devices are actually created.
> 
> 1. Virtio proxy device (e.g. TYPE_VIRTIO_BALLOON_PCI)
> 2. The "real" virtio device (e.g. TYPE_VIRTIO_BALLOON).
> 
> 1. aliases all properties of 2, so 2. can be properly configured using 1.
> 1. gets the device ID set specified by the user. 2. gets no ID set.
> 
> If we want to make 2. a MemoryDevice but report errors/information to the
> user, we always have to report the id of 1. (because that's the device the
> user instantiated and configured).
> 
> Signed-off-by: David Hildenbrand <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  hw/mem/memory-device.c         | 7 +++++--
>  include/hw/mem/memory-device.h | 4 ++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index 534bd38313..92878fc327 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -174,8 +174,11 @@ static uint64_t memory_device_get_free_addr(MachineState 
> *ms,
>  
>          if (ranges_overlap(md_addr, md_size, new_addr, size)) {
>              if (hint) {
> -                const DeviceState *d = DEVICE(md);
> -                error_setg(errp, "address range conflicts with '%s'", d->id);
> +                const char *id = mdc->get_device_id ? mdc->get_device_id(md) 
> :
> +                                 DEVICE(md)->id;
> +
> +                error_setg(errp, "address range conflicts with '%s'",
> +                           id ? id : "(unnamed)");
>                  goto out;
>              }
>              new_addr = QEMU_ALIGN_UP(md_addr + md_size, align);
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index fd0b43c224..66143cffc6 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -42,6 +42,9 @@ typedef struct MemoryDeviceState {
>   * of multiple successive memory regions, a covering memory region is to
>   * be used. Scattered memory regions are not supported for single devices.
>   * @fill_device_info: Translate current @md state into #MemoryDeviceInfo.
> + * @get_device_id: Optional. Allows memory devices behind proxy devices
> + * (e.g. virtio based) to report the id of the proxy device to the user
> + * instead of the (empty) id of the memory device.
>   */
>  typedef struct MemoryDeviceClass {
>      /* private */
> @@ -54,6 +57,7 @@ typedef struct MemoryDeviceClass {
>      MemoryRegion *(*get_memory_region)(MemoryDeviceState *md, Error **errp);
>      void (*fill_device_info)(const MemoryDeviceState *md,
>                               MemoryDeviceInfo *info);
> +    const char *(*get_device_id)(const MemoryDeviceState *md);
>  } MemoryDeviceClass;
>  
>  MemoryDeviceInfoList *qmp_memory_device_list(void);

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature