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

[Peter Crosthwaite]

On Mon, Dec 16, 2013 at 3:56 AM, Andreas Färber <address@hidden> 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.
>

So the whole point of this patch and the next is to avoid having to
write setters and getters for every property. Its going to be super
verbose, is every vendor configurable CPU arm property needs the qdev
setter and getter boilerplate. And the vast majority of these
properties are going to be family or device conditional in some way so
the dynamic property approach is looking like the rule rather than the
exception.

So I guess this problem is why qdev was invented. Since last night,
i've figured out that whole series can actually be done quite cleanly
using qdevs setters and getters. Please see v5 for illustration. Ive
dropped these two patches, so this series is now pure ARM (and
hopefully makes life easier for PMM).

Regards,
Peter

> 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
>