Re: [Qemu-devel] [PATCH v4 17/24] memory-device: add class function get_device_id()

[David Hildenbrand]

On 30/09/2018 18:19, Auger Eric wrote:
> 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).

Both parts slightly reworded.

>>
>> Reviewed-by: David Gibson <address@hidden>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  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"?

Yes, I can do that.

Thanks!


-- 

Thanks,

David / dhildenb