From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58695) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtEpo-00058H-Kx for qemu-devel@nongnu.org; Sat, 07 Jun 2014 07:31:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WtEpd-0007gN-6h for qemu-devel@nongnu.org; Sat, 07 Jun 2014 07:30:52 -0400 Received: from mail-wg0-x22c.google.com ([2a00:1450:400c:c00::22c]:54924) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtEpc-0007g4-CF for qemu-devel@nongnu.org; Sat, 07 Jun 2014 07:30:40 -0400 Received: by mail-wg0-f44.google.com with SMTP id x13so1156149wgg.15 for ; Sat, 07 Jun 2014 04:30:39 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5392F7DC.2040905@redhat.com> Date: Sat, 07 Jun 2014 13:30:36 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <53918B9C.4080408@redhat.com> <5391D5AB.8030508@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH memory v4 00/10] Memory Region QOMification List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , "qemu-devel@nongnu.org Developers" , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Il 07/06/2014 01:50, Peter Crosthwaite ha scritto: > On Sat, Jun 7, 2014 at 12:52 AM, Paolo Bonzini 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