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

[Peter Crosthwaite]

On Mon, Dec 16, 2013 at 4:07 AM, Michael S. Tsirkin <address@hidden> wrote:
> On Sun, Dec 15, 2013 at 06:56:48PM +0100, Andreas Färber wrote:
>> 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
>
> I think ability to add read only properties is reasonable though.
> ACPI wants these since we calculate the value ourselves.
>

So i'm starting to wonder whether specific semantic rules such as this
as the responsibility of the QOM core or the layers above. Is this
just a more restrictive variant of qdev properties and should be
implemented on that layer as qdev extension?

Regards,
Peter

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