Re: [Qemu-ppc] [Qemu-devel] [PATCH] spapr: fix memory hotplug error path

[Greg Kurz]

On Mon, 3 Jul 2017 15:43:26 +0200
Igor Mammedov <address@hidden> wrote:

> On Mon, 03 Jul 2017 14:21:31 +0200
> Greg Kurz <address@hidden> wrote:
> 
> > QEMU shouldn't abort if spapr_add_lmbs()->spapr_drc_attach() fails.
> > Let's propagate the error instead, like it is done everywhere else
> > where spapr_drc_attach() is called.
> > 
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> >  hw/ppc/spapr.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 70b3fd374e2b..e103be500189 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2601,6 +2601,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t 
> > addr_start, uint64_t size,
> >      int i, fdt_offset, fdt_size;
> >      void *fdt;
> >      uint64_t addr = addr_start;
> > +    Error *local_err = NULL;
> >  
> >      for (i = 0; i < nr_lmbs; i++) {
> >          drc = spapr_drc_by_id(TYPE_SPAPR_DRC_LMB,
> > @@ -2611,7 +2612,12 @@ static void spapr_add_lmbs(DeviceState *dev, 
> > uint64_t addr_start, uint64_t size,
> >          fdt_offset = spapr_populate_memory_node(fdt, node, addr,
> >                                                  SPAPR_MEMORY_BLOCK_SIZE);
> >  
> > -        spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
> > +        spapr_drc_attach(drc, dev, fdt, fdt_offset, &local_err);
> > +        if (local_err) {
> > +            g_free(fdt);
> > +            error_propagate(errp, local_err);
> > +            return;
> > +        }
> >          addr += SPAPR_MEMORY_BLOCK_SIZE;
> >      }
> >      /* send hotplug notification to the
> > @@ -2657,7 +2663,7 @@ static void spapr_memory_plug(HotplugHandler 
> > *hotplug_dev, DeviceState *dev,
> >  
> >      spapr_add_lmbs(dev, addr, size, node,
> >                     spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT),
> > -                   &error_abort);
> > +                   &local_err);  
> where is code that handles error condition?
>  i.e. undo what pc_dimm_memory_plug() has done
> 

Oops, you're right and I also need to add some rollback in
spapr_add_lmbs() as well... :-\

> >  
> >  out:
> >      error_propagate(errp, local_err);
> > 
> >   
>

pgpeklk1R6wv4.pgp
Description: OpenPGP digital signature