qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH qom v1 1/1] qom: object: remove parent pointer w


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH qom v1 1/1] qom: object: remove parent pointer when unparenting
Date: Wed, 11 Jun 2014 13:33:12 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0

Il 11/06/2014 13:28, Andreas Färber ha scritto:
Am 11.06.2014 12:19, schrieb Peter Crosthwaite:
On Wed, Jun 11, 2014 at 6:13 PM, Andreas Färber <address@hidden> wrote:
Am 27.05.2014 02:39, schrieb Peter Crosthwaite:
Certain parts of the QOM framework test this pointer to determine if
an object is parented. Nuke it when the object is unparented to allow
for reuse of an object after unparenting.

Signed-off-by: Peter Crosthwaite <address@hidden>
---

 qom/object.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qom/object.c b/qom/object.c
index e42b254..8319e89 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -402,6 +402,7 @@ void object_unparent(Object *obj)
     if (obj->parent) {
         object_property_del_child(obj->parent, obj, NULL);
     }
+    obj->parent = NULL;
     object_unref(obj);
 }


This looks okay to me, and it might also help the segfault on hot-unplug
Stefan and Kevin reported before I went on travels.


Welcome back.

Any objection to moving this one line up into the if?


No problem. Will respin.

I've done so myself, but now I wonder why we are checking obj->parent at
all there after we already return if !obj->parent? Is this to guard
against ObjectClass::unparent() changing Object::parent? Either way, the
two variants you posted and I suggested should be fine.

Yes, in case unparent might already end up removing the property.

I have a patch that moves the unparent call to object_finalize_child_property and only removes the property here. The patch would apply anyway, so I'm okay with it.

Paolo




reply via email to

[Prev in Thread] Current Thread [Next in Thread]