Re: [Qemu-devel] [PATCH] qdev: unparent device when fails to set properties

[Amos Kong]

On Tue, Dec 31, 2013 at 05:09:36PM +0800, Hu Tao wrote:
> On Tue, Dec 31, 2013 at 04:06:57PM +0800, Amos Kong wrote:
> > Test steps:
> >   (qemu) device_add e1000,addr=adsf
> >   Property 'e1000.addr' doesn't take value 'adsf'
> >   (qemu) info qtree
> > Then qemu crashed.
> > 
> > When it fails to set properties, qdev's parent is already set, but the
> > object hasn't been added to parent object, object_unparent() won't
> > unparent the device. This patch unparents device in the mediacy.
> > 
> > Signed-off-by: Amos Kong <address@hidden>
> > ---
> >  qdev-monitor.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > index dc37a43..3d8b4f4 100644
> > --- a/qdev-monitor.c
> > +++ b/qdev-monitor.c
> > @@ -527,7 +527,9 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> >          dev->id = id;
> >      }
> >      if (qemu_opt_foreach(opts, set_property, dev, 1) != 0) {
> > -        object_unparent(OBJECT(dev));
> > +        if (OBJECT(dev)->class->unparent) {
> > +            (OBJECT(dev)->class->unparent)(OBJECT(dev));
> > +        }
> 

Hi Tao,

> This means object_unparent()(or device_unparent()) doesn't handle
> incompletely initialized object correctly. How about fix it in
> object_unparent()/device_unparent()?

We can't fix object_unparent() to cleanup this immature object.
device_unparent() is used to clean device, but it's not called.

OBJECT(dev)->class->unparent is initialized to device_unparent().
So my patch just called OBJECT(dev)->class->unparent().
 
> BTW, it must be commit e0a83fc2c1582dc8 introdues the problem.

Yes.

-- 
                        Amos.