[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/20] memory-device: convert get_region_size
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/20] memory-device: convert get_region_size() to get_memory_region() |
Date: |
Mon, 17 Sep 2018 12:46:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
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