On 05/09/2017 12:35 PM, Marc-André Lureau wrote: > Remove dependency on qapi qtype, replace a field by a few helper > functions to determine the default value type (introduced in commit > 4f2d3d7). > > Signed-off-by: Marc-André Lureau <[email protected]> > --- > include/hw/qdev-core.h | 1 - > include/hw/qdev-properties.h | 5 ----- > hw/core/qdev.c | 32 ++++++++++++++++++++++++++------ > 3 files changed, 26 insertions(+), 12 deletions(-) >
> +++ b/hw/core/qdev.c
> @@ -755,6 +755,30 @@ static void qdev_property_add_legacy(DeviceState *dev,
> Property *prop,
> g_free(name);
> }
>
> +static bool prop_info_is_bool(const PropertyInfo *info)
> +{
> + return info == &qdev_prop_bit
> + || info == &qdev_prop_bit64
> + || info == &qdev_prop_bool;
> +}
I guess we can expand these helpers if we add more property types later.
> +
> +static bool prop_info_is_int(const PropertyInfo *info)
> +{
> + return info == &qdev_prop_uint8
> + || info == &qdev_prop_uint16
> + || info == &qdev_prop_uint32
> + || info == &qdev_prop_int32
> + || info == &qdev_prop_uint64
Interesting dissimilarity between existing types, but not the fault of
your patch.
> + || info == &qdev_prop_size
> + || info == &qdev_prop_pci_devfn
Okay so far.
> + || info == &qdev_prop_on_off_auto
> + || info == &qdev_prop_losttickpolicy
> + || info == &qdev_prop_blockdev_on_error
> + || info == &qdev_prop_bios_chs_trans
Aren't these four enums rather than ints?
> + || info == &qdev_prop_blocksize
> + || info == &qdev_prop_arraylen;
> +}
> +
> @@ -794,16 +818,12 @@ void qdev_property_add_static(DeviceState *dev,
> Property *prop,
> prop->info->description,
> &error_abort);
>
> - if (prop->qtype == QTYPE_NONE) {
> - return;
> - }
> -
> - if (prop->qtype == QTYPE_QBOOL) {
> + if (prop_info_is_bool(prop->info)) {
> object_property_set_bool(obj, prop->defval, prop->name,
> &error_abort);
> } else if (prop->info->enum_table) {
> object_property_set_str(obj, prop->info->enum_table[prop->defval],
> prop->name, &error_abort);
> - } else if (prop->qtype == QTYPE_QINT) {
> + } else if (prop_info_is_int(prop->info)) {
> object_property_set_int(obj, prop->defval, prop->name, &error_abort);
So enum_table has priority over prop_info_is_int(), in which case, the
four types I pointed out as being enums will still use set_str() rather
than set_int().
I'm not necessarily sold on this patch - previously, the type was local
to the declaration of the struct (you could tell by reading
qdev_prop_bit that it was a boolean type); now the type is remote (you
have to hunt elsewhere to see how the property is categorized). I'm not
rejecting it (I see how getting rid of a dependency on QType makes it
easier for the rest of the series to change QType underpinnings), but
wonder if that is a strong enough justification.
But if we DO keep it, you'll want a v2 that cleans up the
prop_info_is_int() impedance mismatch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
