Re: [Qemu-devel] [PATCH memory v4 00/10] Memory Region QOMification

[Paolo Bonzini <pbonzini@redhat.com>]

Il 07/06/2014 01:50, Peter Crosthwaite ha scritto:
> On Sat, Jun 7, 2014 at 12:52 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 06/06/2014 15:36, Peter Crosthwaite ha scritto:
>>
>>>> However, I'd rather have the concept bake for a bit by starting with
>>>> read-only properties.
>>>
>>>
>>> The just a matter of pulling our the setters?
>>
>>
>> Yes.  You can leave in all the preparatory refactoring.
>>
>>
>>> I guess it can be done
>>> but I don't fully understand the motivation.
>>
>>
>> I'm not sure what the interactions should be between the setters and, for
>> example, the "realized" property.
>
> Realized of what? MR being just an OBJECT has no realized itself but I
> suppose you could think about some correlation to the encapsulating
> device (if one even exist - it wont for RAM).

Yes, realized of the owner (and even /machine could be a device).

> In addition, some memory regions (for
>> example PCI BARs) are meant to be moved around by the guest, not the host.
>
> So the intention is the free-for-all nature of the non-DEVICE
> properties solves this.

Free-for-all in what sense?

> 1: Properties may be changed at any-time and run-time code (like
> PCI-BAR control code) sets the QOM properties.
> 2: We preserve the current non-QOM API's (e.g.
> memory_region_add_subregion) for run-time changers.
>
> If we go with option #2 I guess we can lock down the setters post
> realize.

Yes, possibly.  We can also add an API to "lock" memory regions (or more 
generally objects).  Then realize can use it.

>> By comparison, with getters only not many things can go wrong.
>
> True. It's worth nothing however that the setters in the patch do not
> change the underlying MR datastructures in any way and all existing
> APIs are preserved without QOMification. My reasoning for not
> QOMifying the regular MR usage API is pretty much your "let it bake
> for a bit". Generally speaking, the setters are yet-to-be-used by
> anyone except my follow up RFC work.

But also in principle the user could use them via qom-set, with 
interesting effects. :)

>>> Can you elaborate this a little more?
>>
>> Basically the container property acts as a kind of "backdoor" to express
>> many:1 relations (which are a bit ugly to express with the current QOM
>> property model).  If object X is on the 1 side and object Y is on the "many"
>> side, you then write the path to Y to a property of X.
>
> I still maintain that this is valid.  Mandating the many -> 1 unidirectional
> linkage is awkward IMO.

I certainly agree, and in fact would like to use it more!

In any case, even if this series is committed without setters, it's a 
good thing.  It's already a point of no return. :)

One thing that is missing is more documentation of the reference 
counting policies, which are important for unpluggable devices.  IIUC 
you are relying on unrealize (and hence unparent) removing the memory 
region from the address spaces.  Then when the child property for the 
memory region is removed at finalize time, the MR dies.  Is this 
correct?  If so, this might even let us remove memory_region_destroy 
altogether.  It might also lead us to implementing floating references 
like GObject's.

Also, the difference between memory_region_ref(mr) and object_ref(mr) is 
important.  Is there a case where we want the latter except as an 
internal implementation detail?  I think no, but I might be wrong.

>> There are obvious similarity between the container/address/visible
>> properties of a MemoryRegion are very similar to bus/addr/realized that we
>> have for -device.
>
> Are you referring to addr as used in Igor's new DIMM work?

I was thinking of PCI addresses, actually.

Paolo