Re: [Qemu-devel] [PATCH arm-devs v3 1/9] qom/object: Make uintXX added properties writable

[Andreas Färber]

Am 15.12.2013 06:59, schrieb Peter Crosthwaite:
> Ping!
> 
> I'm trying to figure out what way I want to go here.
> 
> On Sat, Dec 7, 2013 at 12:49 AM, Peter Maydell <address@hidden> wrote:
>> On 3 December 2013 13:19, Andreas Färber <address@hidden> wrote:
>>> Am 03.12.2013 07:59, schrieb Peter Crosthwaite:
>>>> Currently the uintXX property adders make a read only property. This
>>>> is not useful for devices that want to create board (or container)
>>>> configurable dynamic device properties. Fix by trivially adding property
>>>> setters to object_property_add_uintXX.
>>>>
>>>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>>> ---
>>>> changed since v2:
>>>> msg typo: "trivially"
>>>
>>> Not sure if I've asked already, but these functions were added by mst
>>> (so let's CC him) for accessing read-only constants in ACPI code. Your
>>> change seems to make them writable - can anything go wrong when the
>>> setters are used via QMP?
> 
> Maybe. But that should be an ACPI problem.

No, it means that if you change it you need to touch ACPI code as well -
or to design your change in a way that avoids exactly that, e.g. by
adding a new API reusing the existing getters rather than changing the
semantics of the existing API used by ACPI.

> It seems that the semantics
> of these qom/object.c APIs has been set by the lead example. Maybe
> just an extra arg for RD/WR flags would do the trick however?

If you can get the extra arg passed through as opaque then sure, that
would be an option, passing false for all existing users.

>>> I fear we may need two separate sets of
>>> functions, one read-only, one read-write.
>>
>> We don't want a generically writable property for CBAR either, though:
>> we want the standard qdev property semantics of "writable until
>> realize, readonly thereafter".
>>
> 
> Well, with a bit of replumbing I spose we could make qdev property
> adder framework accessible to post_init to have access to
> setter/getter fns that implement these semantics.

Sorry, I don't get how that is related to post_init? All that's needed
is a check of DeviceState::realized in your setter and to error_setg()
out if true.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg