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

[David Hildenbrand <david@redhat.com>]

On 08.06.2018 09:46, Greg Kurz wrote:
> On Fri, 8 Jun 2018 09:42:48 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 08.06.2018 09:34, Greg Kurz wrote:
>>> On Thu,  7 Jun 2018 18:52:12 +0200
>>> David Hildenbrand <david@redhat.com> wrote:
>>>   
>>>> The node property can always be queried and the value has already been
>>>> verified in pc_dimm_realize().
>>>>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>  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.
>>
> 
> Heh, /me paranoid but this is David's call and he acked that already
> so I guess it's okay.

NULL makes it fit into a single line :)

Thanks!

> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
>> Thanks!
>>
>>>   
>>>>  
>>>>          spapr_memory_plug(hotplug_dev, dev, node, errp);
>>>>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {  
>>>   
>>
>>
> 


-- 

Thanks,

David / dhildenb