Re: [Qemu-devel] [PATCH v2 05/45] qdev: remove PropertyInfo.qtype field

[Peter Maydell]

On 1 June 2017 at 12:19, Markus Armbruster <address@hidden> wrote:
> Lovely cleanup.
>
> The interesting part is the move of the bits controlling use of ->defval
> from Property member .qtype (set only by DEFINE_PROP_foo() macros) to
> its PropertyInfo method .info->set_default_value().  No functional
> change if (1) we preserve existing use of ->defval and (2) we don't add
> new uses of ->defval, or at least not uses that matter.
>
> Direction (1) is obvious for DEFINE_PROP_BIT(), DEFINE_PROP_BIT64(),
> DEFINE_PROP_BOOL().  They set .info to &qdev_prop_bit, &qdev_prop_bit64,
> &qdev_prop_bool, respectively.  These all get .set_default_value =
> set_default_value_bool, which preserves the effect of .qtype =
> QTYPE_BOOL.  Good.
>
> DEFINE_PROP_DEFAULT() sets .info to point to its argument _prop.  The
> following arguments occur:
>
> * In include/hw/qdev-properties.h: qdev_prop_uint8, qdev_prop_uint16,
>   qdev_prop_uint32, qdev_prop_int32, qdev_prop_uint64, qdev_prop_size,
>   qdev_prop_pci_devfn, qdev_prop_on_off_auto, qdev_prop_losttickpolicy,
>   qdev_prop_blockdev_on_error, qdev_prop_bios_chs_trans,
>   qdev_prop_blocksize
>
> * In hw/block/fdc.c: qdev_prop_fdc_drive_type
>
> * In hw/net/e1000e.c: local copies of qdev_prop_uint8, qdev_prop_uint16.
>
> To preserve the effect of .qtype = QTYPE_QINT, these PropertyInfo need
> .set_default_value = set_default_value_int or .set_default_value =
> set_default_value_enum, depending on .enum_table.  They get it.  Good.
>
> DEFINE_PROP_ARRAY() sets .info to &qdev_prop_arraylen.  Needs and gets
> .set_default_value = set_default_value_int.  Good.
>
> DEFINE_PROP_ARRAY() also takes a PropertyInfo argument, but it's not
> relevant here, as the .qtype you remove doesn't apply to it.  I'm
> mentioning this, because it confused me briefly.
>
> Direction (2) isn't as visible in the patch.  For each PropertyInfo you
> change, we need to find and check stores into .info not wrapped in the
> DEFINE_PROP_foo() macros you change.
>
> I can't find any.  Good.

I think this is going to break a patch I was halfway through
writing :-(

What I want is "DEFINE_PROP_UINT32, but don't set the default value"
(because the default value in this case (a) isn't constant and
(b) is already in the struct field that the property modifies).
My plan for doing this was:
 static Property arm_cpu_pmsav7_dregion_property =
    DEFINE_PROP("pmsav7-dregion", ARMCPU, pmsav7_dregion,
                qdev_prop_uint32, uint32_t);

This won't work any more if qdev_property_add_static
assumes that "qdev_prop_uint32" implies "always set the default value".
So how should I obtain those semantics with this cleanup in place ?

thanks
-- PMM