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

[Greg Kurz <groug@kaod.org>]

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.

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)) {  
> >   
> 
>