mbox series

[v2,0/8] qom: Use qlit to represent property defaults

Message ID 20201116224143.1284278-1-ehabkost@redhat.com (mailing list archive)
Headers show
Series qom: Use qlit to represent property defaults | expand

Message

Eduardo Habkost Nov. 16, 2020, 10:41 p.m. UTC
Based-on: 20201104160021.2342108-1-ehabkost@redhat.com
Git branch: https://gitlab.com/ehabkost/qemu/-/commits/work/qdev-qlit-defaults

This extend qlit.h to support all QNum types (signed int,
unsigned int, and double), and use QLitObject to represent field
property defaults.

It allows us to get rid of most type-specific .set_default_value
functions for QOM property types.

Changes v1 -> v2:
* Rebase to latest version of field properties series
* Fix unit test failure
* Coding style changes

Eduardo Habkost (8):
  qobject: Include API docs in docs/devel/qobject.html
  qnum: Make qnum_get_double() get const pointer
  qnum: QNumValue type for QNum value literals
  qnum: qnum_value_is_equal() function
  qlit: Support all types of QNums
  qlit: qlit_type() function
  qom: Make object_property_set_default() public
  qom: Use qlit to represent property defaults

 docs/devel/index.rst                  |   1 +
 docs/devel/qobject.rst                |  11 +++
 include/hw/qdev-properties-system.h   |   2 +-
 include/qapi/qmp/qlit.h               |  16 +++-
 include/qapi/qmp/qnum.h               |  47 ++++++++++-
 include/qapi/qmp/qobject.h            |  48 +++++++----
 include/qom/field-property-internal.h |   4 -
 include/qom/field-property.h          |  26 +++---
 include/qom/object.h                  |  11 +++
 include/qom/property-types.h          |  19 ++---
 hw/core/qdev-properties-system.c      |   8 --
 qobject/qlit.c                        |   5 +-
 qobject/qnum.c                        | 116 +++++++++++++++-----------
 qom/field-property.c                  |  27 ++++--
 qom/object.c                          |   2 +-
 qom/property-types.c                  |  36 ++------
 tests/check-qjson.c                   |  72 ++++++++++++++--
 tests/check-qnum.c                    |  14 ++--
 18 files changed, 301 insertions(+), 164 deletions(-)
 create mode 100644 docs/devel/qobject.rst

Comments

Markus Armbruster Nov. 19, 2020, 12:39 p.m. UTC | #1
Eduardo Habkost <ehabkost@redhat.com> writes:

> Based-on: 20201104160021.2342108-1-ehabkost@redhat.com
> Git branch: https://gitlab.com/ehabkost/qemu/-/commits/work/qdev-qlit-defaults
>
> This extend qlit.h to support all QNum types (signed int,
> unsigned int, and double), and use QLitObject to represent field
> property defaults.
>
> It allows us to get rid of most type-specific .set_default_value
> functions for QOM property types.

What's left?

I'm asking because if you create a new way to get rid of most of an old
way, you're still left with two ways, which may or may not be an
improvement.

Moving defaults from code to data sounds attractive to me.  Data is
easier to reason about than code.  For QAPI, we've been talking about
defining defaults in the schema for a long time, but nobody has gotten
around to finish an implementation.
Eduardo Habkost Nov. 19, 2020, 5:13 p.m. UTC | #2
On Thu, Nov 19, 2020 at 01:39:50PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Based-on: 20201104160021.2342108-1-ehabkost@redhat.com
> > Git branch: https://gitlab.com/ehabkost/qemu/-/commits/work/qdev-qlit-defaults
> >
> > This extend qlit.h to support all QNum types (signed int,
> > unsigned int, and double), and use QLitObject to represent field
> > property defaults.
> >
> > It allows us to get rid of most type-specific .set_default_value
> > functions for QOM property types.
> 
> What's left?

Enums.  Enums properties are a mess to implement, and I plan to
tackle them later.

On all other cases, the external representation of the property
value is similar to the internal representation.  In the case of
enums, the external representation is a string, but the internal
representation is an integer.

> 
> I'm asking because if you create a new way to get rid of most of an old
> way, you're still left with two ways, which may or may not be an
> improvement.

I don't think this is an accurate description.  We had five
different ways of doing this[1].  I replaced four of them with
the new qlit-based mechanism, and now we have two.


[1] The five different ways were:
    qdev_propinfo_set_default_value_enum,
    qdev_propinfo_set_default_value_int,
    qdev_propinfo_set_default_value_uint,
    set_default_uuid_auto, and
    set_default_value_bool.

> 
> Moving defaults from code to data sounds attractive to me.  Data is
> easier to reason about than code.  For QAPI, we've been talking about
> defining defaults in the schema for a long time, but nobody has gotten
> around to finish an implementation.
Paolo Bonzini Nov. 19, 2020, 5:23 p.m. UTC | #3
On 19/11/20 18:13, Eduardo Habkost wrote:
>> What's left?
> Enums.  Enums properties are a mess to implement, and I plan to
> tackle them later.
> 
> On all other cases, the external representation of the property
> value is similar to the internal representation.  In the case of
> enums, the external representation is a string, but the internal
> representation is an integer.
> 

I would have expected a string QLit to work with enums, is there a 
reason why it doesn't?

Paolo
Eduardo Habkost Nov. 19, 2020, 5:55 p.m. UTC | #4
On Thu, Nov 19, 2020 at 06:23:30PM +0100, Paolo Bonzini wrote:
> On 19/11/20 18:13, Eduardo Habkost wrote:
> > > What's left?
> > Enums.  Enums properties are a mess to implement, and I plan to
> > tackle them later.
> > 
> > On all other cases, the external representation of the property
> > value is similar to the internal representation.  In the case of
> > enums, the external representation is a string, but the internal
> > representation is an integer.
> > 
> 
> I would have expected a string QLit to work with enums, is there a reason
> why it doesn't?

It would work, but it would be more inconvenient for callers.
Right now they use the C enum constant instead of a string.
Paolo Bonzini Nov. 19, 2020, 6:25 p.m. UTC | #5
On 19/11/20 18:55, Eduardo Habkost wrote:
> On Thu, Nov 19, 2020 at 06:23:30PM +0100, Paolo Bonzini wrote:
>> On 19/11/20 18:13, Eduardo Habkost wrote:
>>>> What's left?
>>> Enums.  Enums properties are a mess to implement, and I plan to
>>> tackle them later.
>>>
>>> On all other cases, the external representation of the property
>>> value is similar to the internal representation.  In the case of
>>> enums, the external representation is a string, but the internal
>>> representation is an integer.
>>>
>>
>> I would have expected a string QLit to work with enums, is there a reason
>> why it doesn't?
> 
> It would work, but it would be more inconvenient for callers.
> Right now they use the C enum constant instead of a string.

It matches what you have to do already for compat props, so it's not a 
big deal.  I would say just use strings.

Paolo
Eduardo Habkost Nov. 19, 2020, 7:05 p.m. UTC | #6
On Thu, Nov 19, 2020 at 07:25:15PM +0100, Paolo Bonzini wrote:
> On 19/11/20 18:55, Eduardo Habkost wrote:
> > On Thu, Nov 19, 2020 at 06:23:30PM +0100, Paolo Bonzini wrote:
> > > On 19/11/20 18:13, Eduardo Habkost wrote:
> > > > > What's left?
> > > > Enums.  Enums properties are a mess to implement, and I plan to
> > > > tackle them later.
> > > > 
> > > > On all other cases, the external representation of the property
> > > > value is similar to the internal representation.  In the case of
> > > > enums, the external representation is a string, but the internal
> > > > representation is an integer.
> > > > 
> > > 
> > > I would have expected a string QLit to work with enums, is there a reason
> > > why it doesn't?
> > 
> > It would work, but it would be more inconvenient for callers.
> > Right now they use the C enum constant instead of a string.
> 
> It matches what you have to do already for compat props, so it's not a big
> deal.  I would say just use strings.

No problem to me.  I'll do.