Re: [Qemu-devel] [PATCH for-2.10 2/3] Revert "qdev: Free QemuOpts when the QOM path goes away"

[Greg Kurz]

On Mon, 31 Jul 2017 11:39:13 -0500
Michael Roth <address@hidden> wrote:

> Quoting Greg Kurz (2017-07-31 10:51:39)
> > On Wed, 26 Jul 2017 20:30:54 -0500
> > Michael Roth <address@hidden> wrote:
> >   
> > > This reverts commit abed886ec60cf239a03515cf0b30fb11fa964c44.
> > > 
> > > This patch originally addressed an issue where a DEVICE_DELETED
> > > event could be emitted (in device_unparent()) before a Device's
> > > QemuOpts were cleaned up (in device_finalize()), leading to a
> > > "duplicate ID" error if management attempted to immediately add
> > > a device with the same ID in response to the DEVICE_DELETED event.
> > > 
> > > An alternative will be implemented in a subsequent patch where we
> > > defer the DEVICE_DELETED event until device_finalize(), which would
> > > also prevent the race, so we revert the original fix in preparation.
> > >   
> > 
> > Do you really need to revert abed886ec60cf239a03515cf0b30fb11fa964c44 ?
> > 
> > IIUC, the purpose of this series is to fix/workaround the VFIO cleanup 
> > issue.
> > Even if it also relates to when we emit DEVICE_DELETED, it isn't exactly the
> > same problem here, is it ?  
> 
> That's true, but since the implementation of the VFIO fix requires
> moving qemu_opts_del() to back to finalize(), and since that
> effectively reverts the change made in abed88, I figured it was
> better to revert the patch directly as opposed as a side-effect
> of patch 3.
> 

Fair enough.

Reviewed-by: Greg Kurz <address@hidden>

> >   
> > > Signed-off-by: Michael Roth <address@hidden>
> > > ---
> > >  hw/core/qdev.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > > index a64b35c..08c4061 100644
> > > --- a/hw/core/qdev.c
> > > +++ b/hw/core/qdev.c
> > > @@ -1067,6 +1067,7 @@ static void device_finalize(Object *obj)
> > >      NamedGPIOList *ngl, *next;
> > >  
> > >      DeviceState *dev = DEVICE(obj);
> > > +    qemu_opts_del(dev->opts);
> > >  
> > >      QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
> > >          QLIST_REMOVE(ngl, node);
> > > @@ -1116,9 +1117,6 @@ static void device_unparent(Object *obj)
> > >          g_free(dev->canonical_path);
> > >          dev->canonical_path = NULL;
> > >      }
> > > -
> > > -    qemu_opts_del(dev->opts);
> > > -    dev->opts = NULL;
> > >  }
> > >  
> > >  static void device_class_init(ObjectClass *class, void *data)  
> >   
>

pgpxgnvhnU4c4.pgp
Description: OpenPGP digital signature