| Message ID | 1402489176-19738-4-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 11, 2014 at 10:19 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > This ensures that the unparent callback is called automatically > when the parent object is finalized. > > Note that there's no need to keep a reference neither in > object_unparent nor in object_finalize_child_property. The > reference held by the child property itself will do. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > qom/object.c | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 01087e5..69a95d6 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -386,19 +386,9 @@ static void object_property_del_child(Object *obj, Object *child, Error **errp) > > void object_unparent(Object *obj) > { > - if (!obj->parent) { > - return; > - } > - > - object_ref(obj); > - if (obj->class->unparent) { > - (obj->class->unparent)(obj); > - } This changes this publicly visible API significantly, in that unparent from the childs point-of-view is now invisible in cases where you are doing a standalone object_unparent() call. I'm not sure why parent finalisation is the only occasion on which a genuine and complete unparenting is allowed? I think this API should remain complete should a QOM user wish to re-arrange child relationships seperate from any finalisations. > if (obj->parent) { > object_property_del_child(obj->parent, obj, NULL); > - obj->parent = NULL; > } > - object_unref(obj); > } > > static void object_deinit(Object *obj, TypeImpl *type) > @@ -1017,6 +1007,10 @@ static void object_finalize_child_property(Object *obj, const char *name, > { > Object *child = opaque; > > + if (child->class->unparent) { > + (child->class->unparent)(child); > + } > + child->parent = NULL; Can you just leave object_unparent() mostly as-is and call it from here? It's already guarded against repeated calls should there be users calling it manually before parent finalisations. Regards, Peter > object_unref(child); > } > > -- > 1.8.3.1 > > >
Il 11/06/2014 15:35, Peter Crosthwaite ha scritto: > On Wed, Jun 11, 2014 at 10:19 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> This ensures that the unparent callback is called automatically >> when the parent object is finalized. >> >> Note that there's no need to keep a reference neither in >> object_unparent nor in object_finalize_child_property. The >> reference held by the child property itself will do. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> qom/object.c | 14 ++++---------- >> 1 file changed, 4 insertions(+), 10 deletions(-) >> >> diff --git a/qom/object.c b/qom/object.c >> index 01087e5..69a95d6 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -386,19 +386,9 @@ static void object_property_del_child(Object *obj, Object *child, Error **errp) >> >> void object_unparent(Object *obj) >> { >> - if (!obj->parent) { >> - return; >> - } >> - >> - object_ref(obj); >> - if (obj->class->unparent) { >> - (obj->class->unparent)(obj); >> - } > > This changes this publicly visible API significantly, in that unparent > from the childs point-of-view is now invisible in cases where you are > doing a standalone object_unparent() call. > > I'm not sure why parent finalisation is the only occasion on which a > genuine and complete unparenting is allowed? I think this API should > remain complete should a QOM user wish to re-arrange child > relationships seperate from any finalisations. No, this is moved to object_finalize_child_property, i.e. the release callback of child<> properties. object_property_del_child calls it. The change is that, before, the unparent callback was only invoked if you explicitly called object_unparent. Now it's always called as part of finalize, even if you unparent the outer object but not the inner ones. I find it more intuitive. Note that this is not yet needed in these 13 patches. Paolo >> if (obj->parent) { >> object_property_del_child(obj->parent, obj, NULL); >> - obj->parent = NULL; >> } >> - object_unref(obj); >> } >> >> static void object_deinit(Object *obj, TypeImpl *type) >> @@ -1017,6 +1007,10 @@ static void object_finalize_child_property(Object *obj, const char *name, >> { >> Object *child = opaque; >> >> + if (child->class->unparent) { >> + (child->class->unparent)(child); >> + } >> + child->parent = NULL; > > Can you just leave object_unparent() mostly as-is and call it from > here? It's already guarded against repeated calls should there be > users calling it manually before parent finalisations. > > Regards, > Peter > >> object_unref(child); >> } >> >> -- >> 1.8.3.1 >> >> >>
On Thu, Jun 12, 2014 at 12:01 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 11/06/2014 15:35, Peter Crosthwaite ha scritto: > >> On Wed, Jun 11, 2014 at 10:19 PM, Paolo Bonzini <pbonzini@redhat.com> >> wrote: >>> >>> This ensures that the unparent callback is called automatically >>> when the parent object is finalized. >>> >>> Note that there's no need to keep a reference neither in >>> object_unparent nor in object_finalize_child_property. The >>> reference held by the child property itself will do. >>> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> --- >>> qom/object.c | 14 ++++---------- >>> 1 file changed, 4 insertions(+), 10 deletions(-) >>> >>> diff --git a/qom/object.c b/qom/object.c >>> index 01087e5..69a95d6 100644 >>> --- a/qom/object.c >>> +++ b/qom/object.c >>> @@ -386,19 +386,9 @@ static void object_property_del_child(Object *obj, >>> Object *child, Error **errp) >>> >>> void object_unparent(Object *obj) >>> { >>> - if (!obj->parent) { >>> - return; >>> - } >>> - >>> - object_ref(obj); >>> - if (obj->class->unparent) { >>> - (obj->class->unparent)(obj); >>> - } >> >> >> This changes this publicly visible API significantly, in that unparent >> from the childs point-of-view is now invisible in cases where you are >> doing a standalone object_unparent() call. >> >> I'm not sure why parent finalisation is the only occasion on which a >> genuine and complete unparenting is allowed? I think this API should >> remain complete should a QOM user wish to re-arrange child >> relationships seperate from any finalisations. > > > No, this is moved to object_finalize_child_property, i.e. the release > callback of child<> properties. object_property_del_child calls it. > Ok it's adding up for me now. Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> Regards, Peter > The change is that, before, the unparent callback was only invoked if you > explicitly called object_unparent. Now it's always called as part of > finalize, even if you unparent the outer object but not the inner ones. I > find it more intuitive. > > Note that this is not yet needed in these 13 patches. > > Paolo > > >>> if (obj->parent) { >>> object_property_del_child(obj->parent, obj, NULL); >>> - obj->parent = NULL; >>> } >>> - object_unref(obj); >>> } >>> >>> static void object_deinit(Object *obj, TypeImpl *type) >>> @@ -1017,6 +1007,10 @@ static void object_finalize_child_property(Object >>> *obj, const char *name, >>> { >>> Object *child = opaque; >>> >>> + if (child->class->unparent) { >>> + (child->class->unparent)(child); >>> + } >>> + child->parent = NULL; >> >> >> Can you just leave object_unparent() mostly as-is and call it from >> here? It's already guarded against repeated calls should there be >> users calling it manually before parent finalisations. >> >> Regards, >> Peter >> >>> object_unref(child); >>> } >>> >>> -- >>> 1.8.3.1 >>> >>> >>> > >
diff --git a/qom/object.c b/qom/object.c index 01087e5..69a95d6 100644 --- a/qom/object.c +++ b/qom/object.c @@ -386,19 +386,9 @@ static void object_property_del_child(Object *obj, Object *child, Error **errp) void object_unparent(Object *obj) { - if (!obj->parent) { - return; - } - - object_ref(obj); - if (obj->class->unparent) { - (obj->class->unparent)(obj); - } if (obj->parent) { object_property_del_child(obj->parent, obj, NULL); - obj->parent = NULL; } - object_unref(obj); } static void object_deinit(Object *obj, TypeImpl *type) @@ -1017,6 +1007,10 @@ static void object_finalize_child_property(Object *obj, const char *name, { Object *child = opaque; + if (child->class->unparent) { + (child->class->unparent)(child); + } + child->parent = NULL; object_unref(child); }
This ensures that the unparent callback is called automatically when the parent object is finalized. Note that there's no need to keep a reference neither in object_unparent nor in object_finalize_child_property. The reference held by the child property itself will do. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- qom/object.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)