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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

On Fri, Jun 08, 2018 at 09:46:57AM +0200, 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.

Actually, I missed this - error_abort is preferable.  That's general
the right choice for things that shouldn't ever fail.  This way if
they *do* fail we get a clear error immediately.

> Reviewed-by: Greg Kurz <groug@kaod.org>


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]