Re: [PATCH 1/3] qom/object: merge double hash table traversal in object_property_del_child

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Apr 30, 2026 at 07:07:03AM +1000, Richard Henderson wrote:
> On 4/29/26 16:20, Bin Guo wrote:
> > object_property_del_child() previously performed two full iterations
> > over obj->properties: the first to find the matching child property and
> > call its release callback, the second to find the same property again
> > and remove it from the hash table.
> > 
> > Merge the two loops into one: when the matching property is found, call
> > the release callback and immediately remove the entry via
> > g_hash_table_iter_remove(), then break.  This halves the number of hash
> > table operations in the common case and avoids the redundant second scan.
> > 
> > Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> > ---
> >   qom/object.c | 7 -------
> >   1 file changed, 7 deletions(-)
> > 
> > diff --git a/qom/object.c b/qom/object.c
> > index f981e27044..9c0e8dfd02 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -629,13 +629,6 @@ static void object_property_del_child(Object *obj, Object *child)
> >                   prop->release(obj, prop->name, prop->opaque);
> >                   prop->release = NULL;
> >               }
> > -            break;
> > -        }
> > -    }
> > -    g_hash_table_iter_init(&iter, obj->properties);
> > -    while (g_hash_table_iter_next(&iter, &key, &value)) {
> > -        prop = value;
> > -        if (object_property_is_child(prop) && prop->opaque == child) {
> >               g_hash_table_iter_remove(&iter);
> >               break;
> >           }
> 
> Fixes: b604a854e84 ("qom: Replace object property list with GHashTable")

On the contrary, this proposed patch breaks that commit.

The original proposal for that old commit had a single iteration,
but it was discovered that this broke cleanup for certain devices,
as prop->release can have side-effects that trigger assertions in
g_hash_table_iter_remove.  This requires a two phase iteration
to be safe.

A test for that problem was added in 8c4d156c187c84b574d287bd4b9ddf9a6975de7c
and this new patch proposal duly breaks the check-qom-proplist test.

With regards,
Daniel