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

[David Hildenbrand]

On 24/09/2018 16:40, Igor Mammedov wrote:
> On Thu, 20 Sep 2018 12:32:37 +0200
> David Hildenbrand <address@hidden> 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>
>> ---
>>  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;
> it could be better if default MemoryDeviceClass::get_device_id() would return
> DEVICE(md)->id and we override it for virtio based devices later.
> Then get_device_id() could be reused direcly elsewhere without a conditional 
> assign.
> 

I wonder if default implementations are possible for interfaces, I will
give it a try.


-- 

Thanks,

David / dhildenb