Re: [Qemu-devel] [PATCH v4 5/8] qdev: Register static properties as class properties

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Nov 04, 2016 at 04:22:40PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Instead of registering qdev static properties on instance_init,
> > register them as class properties, at qdev_class_set_props().
> >
> > qdev_property_add_legacy() was replaced by an equivalent
> > qdev_class_property_add_legacy() function.
> > qdev_property_add_static(), on the other hand, can't be
> > eliminated yet because it is used by arm_cpu_post_init().
> >
> > As class properties don't have a release function called when an
> > object instance is destroyed, the release functions for
> > properties are called manually from device_finalize().
> 
> Ignorant question: should class properties have such a release function?

I don't think they need one (because it is not possible to
destroy classes yet). Even if they should, most of the existing
QOM release functions are not meant to be called every time an
object is destroyed (but still get Object* as argument, which is
confusing).

qdev property release functions, on the other hand, are actually
meant to be called every time an object is destroyed.
(Confusing, uh?)

I have patches to remove the release function and to clarify the
intent of qdev release functions. I just didn't include them in
this series to keep it simpler.

> 
> Hmm, I see object_class_property_add() does take a release().  Is that
> one called at the wrong time?

The function is actually never called. I thought they were meant
to be called when the object is destroyed, but most (but not
all?) of the existing release functions are meant to be called to
release its void* opaque pointer (so they were really supposed to
be called only if the class is being destroyed).

> 
> One typo noted below.
> 
[...]
> > +    /* Note: prop->info->release is called on device_finalize(),
> > +     * because it needs to be called on instaqnce destruction, not on
> 
> instance

Thanks. I will fix it in the next version.

-- 
Eduardo