Re: [Qemu-devel] [PATCH v2 04/20] memory-device: get_region_size()/get_plugged_size() might fail

[David Hildenbrand]

Am 03.09.18 um 15:40 schrieb Igor Mammedov:
> On Wed, 29 Aug 2018 17:36:08 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> Let's properly forward the error, so errors from get_region_size() /
>> get_plugged_size(), can be handled.
>>
>> Users right now call both functions after the device has been realized,
>> which is guaranteed to no fail (we'll document this behavior in a
>> follow-up patch).
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  hw/mem/memory-device.c         | 6 +++---
>>  hw/mem/pc-dimm.c               | 5 +++--
>>  include/hw/mem/memory-device.h | 4 ++--
>>  3 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index 0d9892b715..d87599c280 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -60,7 +60,7 @@ static int memory_device_used_region_size(Object *obj, 
>> void *opaque)
>>          const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj);
>>  
>>          if (dev->realized) {
>> -            *size += mdc->get_region_size(md);
>> +            *size += mdc->get_region_size(md, &error_abort);
>>          }
>>      }
>>  
>> @@ -167,7 +167,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, 
>> const uint64_t *hint,
>>          uint64_t md_size, md_addr;
>>  
>>          md_addr = mdc->get_addr(md);
>> -        md_size = mdc->get_region_size(md);
>> +        md_size = mdc->get_region_size(md, &error_abort);
> s/error_abort/errp/, so it won't crash on hotplug?

As we are checking devices that have already been realized (and we
therefore called get_region_size() already a couple of times), this will
never crash.

We could use errp, but then (to do it correctly) we would need a fresh
local error variable, so we are really allowed to check for *errp).

That's why I favor error_abort here.

> 
>>          if (*errp) {
>>              goto out;
> is this ever reachable?

No, looks like a leftover that can be dropped.

Thanks!

-- 

Thanks,

David / dhildenb