From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39513)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <akong@redhat.com>) id 1Vxw01-0007eF-LP
	for qemu-devel@nongnu.org; Tue, 31 Dec 2013 04:52:42 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <akong@redhat.com>) id 1Vxvzv-0001aG-LE
	for qemu-devel@nongnu.org; Tue, 31 Dec 2013 04:52:33 -0500
Received: from mx1.redhat.com ([209.132.183.28]:61575)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <akong@redhat.com>) id 1Vxvzv-0001aC-Be
	for qemu-devel@nongnu.org; Tue, 31 Dec 2013 04:52:27 -0500
Date: Tue, 31 Dec 2013 17:52:19 +0800
From: Amos Kong <akong@redhat.com>
Message-ID: <20131231095219.GA22565@amosk.info>
References: <1388477217-23491-1-git-send-email-akong@redhat.com>
	<20131231090936.GA30667@G08FNSTD100614.fnst.cn.fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20131231090936.GA30667@G08FNSTD100614.fnst.cn.fujitsu.com>
Subject: Re: [Qemu-devel] [PATCH] qdev: unparent device when fails to set
 properties
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Hu Tao <hutao@cn.fujitsu.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, aliguori@amazon.com, afaerber@suse.de

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 <akong@redhat.com>
> > ---
> >  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.