Re: [qemu-s390x] [PATCH v1 2/8] spapr: no need to verify the node

[David Hildenbrand]

On 08.06.2018 09:34, Greg Kurz wrote:
> On Thu,  7 Jun 2018 18:52:12 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> The node property can always be queried and the value has already been
>> verified in pc_dimm_realize().
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>>  hw/ppc/spapr.c | 9 +--------
>>  1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 2375cbee12..d038f3243e 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3578,14 +3578,7 @@ static void spapr_machine_device_plug(HotplugHandler 
>> *hotplug_dev,
>>              error_setg(errp, "Memory hotplug not supported for this 
>> machine");
>>              return;
>>          }
>> -        node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, 
>> errp);
>> -        if (*errp) {
> 
> Good riddance :)
> 
>> -            return;
>> -        }
>> -        if (node < 0 || node >= MAX_NODES) {
>> -            error_setg(errp, "Invaild node %d", node);
>> -            return;
>> -        }
>> +        node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, 
>> NULL);
> 
> Maybe pass &error_abort ?

I'm using the same access scheme as in hw/acpi/memory_hotplug.c

("error ignored" vs. "error leads to an abort") - but this will actually
never fail. But I can use error_abort here, does not matter.

Thanks!

> 
>>  
>>          spapr_memory_plug(hotplug_dev, dev, node, errp);
>>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> 


-- 

Thanks,

David / dhildenb