Hi David,
On 9/26/18 11:42 AM, 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. Provide a default function that can be overridden.
>
> 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.
s/set// ?
>
> If we want to make 2. a MemoryDevice but report errors/information to the
to be reworded?
> user, we always have to report the id of 1. (because that's the device the
> user instantiated and configured).
>
> Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
> ---
> hw/mem/memory-device.c | 19 +++++++++++++++++--
> include/hw/mem/memory-device.h | 5 +++++
> 2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
> index cf85199a72..6dc40e8764 100644
> --- a/hw/mem/memory-device.c
> +++ b/hw/mem/memory-device.c
> @@ -174,8 +174,10 @@ 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(md);
> +
> + error_setg(errp, "address range conflicts with '%s'",
address range conflicts with memory device id='%s"?
Thanks
Eric
> + id ? id : "(unnamed)")> goto out;
> }
> new_addr = QEMU_ALIGN_UP(md_addr + md_size, align);
> @@ -328,10 +330,23 @@ uint64_t memory_device_get_region_size(const
> MemoryDeviceState *md,
> return memory_region_size(mr);
> }
>
> +static const char *memory_device_get_device_id(const MemoryDeviceState *md)
> +{
> + return DEVICE(md)->id;
> +}
> +
> +static void memory_device_class_init(ObjectClass *oc, void *data)
> +{
> + MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
> +
> + mdc->get_device_id = memory_device_get_device_id;
> +}
> +
> static const TypeInfo memory_device_info = {
> .name = TYPE_MEMORY_DEVICE,
> .parent = TYPE_INTERFACE,
> .class_size = sizeof(MemoryDeviceClass),
> + .class_init = memory_device_class_init,
> };
>
> static void memory_device_register_types(void)
> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index 97de693bca..52fc117ce2 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -45,6 +45,10 @@ typedef struct MemoryDeviceState {
> * 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: 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. The default
> + * implementation (unless overridden) will return the ordinary device id.
> */
> typedef struct MemoryDeviceClass {
> /* private */
> @@ -57,6 +61,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);
>
