Peter Maydell <peter.mayd...@linaro.org> writes:
> On 1 June 2017 at 12:19, Markus Armbruster <arm...@redhat.com> 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".
After this patch, qdev_property_add_static() delegates assigning a
default value to prop->info->set_default_value(obj, prop).
qdev_prop_uint32.set_default_value() assigns prop->defval.
> So how should I obtain those semantics with this cleanup in place ?
Two ways come to mind:
* Define a PropertyInfo like qdev_prop_uint32 with a null
set_default_value(), and use that.
* Add a flag to Property that makes qdev_property_add_static() skip
prop->info->set_default_value(), set it for your property.
Actually, I'd probably do it the other way: call ->set_default_value()
only when the flag is set. No need to check it's non-null then.
Setting the flag when it's null is a programming error.
Could one of these two work for you?
