Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset

From: Igor Mammedov <imammedo@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"John Snow" <jsnow@redhat.com>
Subject: Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
Date: Fri, 23 Oct 2020 17:33:14 +0200	[thread overview]
Message-ID: <20201023173314.6d46ada9@redhat.com> (raw)
In-Reply-To: <20201021133041.GT5733@habkost.net>

On Wed, 21 Oct 2020 09:30:41 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
> > On Fri,  9 Oct 2020 12:01:13 -0400
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >   
> > > The existing object_class_property_add_uint*_ptr() functions are
> > > not very useful, because they need a pointer to the property
> > > value, which can't really be provided before the object is
> > > created.
> > > 
> > > Replace the pointer parameter in those functions with a
> > > `ptrdiff_t offset` parameter.
> > > 
> > > Include a uint8 class property in check-qom-proplist unit tests,
> > > to ensure the feature is working.  
> > 
> > 
> > Not sure I like approach, it's reinventing qdev pointer properties in QOM form.  
> 
> Yes, and that's on purpose.  If we want to eventually merge the
> two competing APIs into a single one, we need to make them
> converge.
> 
> > I had an impression that Paolo wanted qdev pointer properties be gone
> > and replaced by something like link properties.  
> 
> This is completely unrelated to qdev pointer properties and link
> properties.  The properties that use object_property_add_uint*_ptr()
> today are not qdev pointer properties and will never be link
> properties.  They are just integer properties.

right, _prt confused me for a while.

> 
> > 
> > object_property_add_uintXX_ptr() were introduced as a quick hack,
> > when ACPI code generation was moved from Seabios, to avoid more
> > code shuffling in device models and adding more boiler plate in
> > form of custom setters/getters (the later didn't seem to bother
> > us everywhere else where we use object_[class_]property_add() ).
> > Then it spread little bit to another places.
> > 
> > I'd rather get rid of object_property_add_uintXX_ptr() API altogether
> > in favor of object_[class_]property_add() like it is used in other places
> > to handle intXX properties.
> > Adding helpers similar to object_property_add_bool() for intXX
> > could reduce boiler plate need for converting current instances of
> > _ptr(), and such helpers would also help with reducing boilerplate
> > for the rest of instances where object_[class_]property_add()
> > currently is used for dealing with integers.  
> 
> I find object_property_add_bool() terrible.  It requires too much
> boilerplate.  I actually have plans to introduce
> object*_property_add_bool_ptr() to simplify existing
> object_property_add_bool() callers.

But boiler-plate related to QOM properties set/get methods was considered
tolerable back then.
It was a long time ago, so I don't recall why we decided to abandon
qdev properties API.

> I don't love object*_property_add_*_ptr() either.  I consider the
> qdev property API better.  But we need a reasonable alternative,
> because the qdev API can't be used by non-device objects yet.
> I don't think object*_property_add() and
> object*_property_add_bool() are reasonable alternatives.

I also like old qdev API as it can be introspected (it's just data at
class level), very concise when used and has default values.

Instead of duplicating all that pointer arithmetic from qdev properties
in QOM API, it could be better to fix qdev properties so that they
would work for Object as well.
At least all that thrown away type safety would stay constrained/hidden
inside of qdev property macros, instead of being opencoded (offsets) all
over the place.

How hard it would be make qdev properties to work with Object and what
makes you duplicate ugly part of it in QOM instead of making them to
handle Object strait away?
That would also result in huge removal of boiler plate of current QOM
properties.

That should suit your goal to make (most) properties introspectable
and statically described.

[...]