Re: [Qemu-devel] [PATCH v2 05/20] memory-device: convert get_region_size() to get_memory_region()

[David Hildenbrand]

Am 13.09.18 um 14:20 schrieb Igor Mammedov:
> On Wed, 29 Aug 2018 17:36:09 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> To factor out plugging and unplugging of memory device we need access to
>> the memory region. So let's replace get_region_size() by
>> get_memory_region().
>>
>> If any memory device will in the future have multiple memory regions
>> that make up the total region size, it can simply use a wrapping memory
>> region to embed the other ones.
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  hw/mem/memory-device.c         | 10 ++++++----
>>  hw/mem/pc-dimm.c               | 19 ++++++++++++++-----
>>  include/hw/mem/memory-device.h |  2 +-
>>  3 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index d87599c280..3bd30d98bb 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -56,11 +56,13 @@ static int memory_device_used_region_size(Object *obj, 
>> void *opaque)
>>  
>>      if (object_dynamic_cast(obj, TYPE_MEMORY_DEVICE)) {
>>          const DeviceState *dev = DEVICE(obj);
>> -        const MemoryDeviceState *md = MEMORY_DEVICE(obj);
>> +        MemoryDeviceState *md = MEMORY_DEVICE(obj);
> it's a getter, why do we loose 'const' here?

The MemoryRegion pointer that is returned is not const, so this getter
can never take a const pointer (as the MemoryRegion is e.g. part of the
MemoryDeviceState for NVDIMM) - unless we cast away the const
internally, which also sounds bad.

We could keep get_region_size() (along with the const pointer) and
add/factor out get_memory_region() in addition. What do you prefer?

Thanks!

-- 

Thanks,

David / dhildenb