From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51318) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZeIYD-0007IW-W2 for qemu-devel@nongnu.org; Tue, 22 Sep 2015 04:03:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZeIYB-0001jR-85 for qemu-devel@nongnu.org; Tue, 22 Sep 2015 04:03:45 -0400 Received: from e06smtp15.uk.ibm.com ([195.75.94.111]:35342) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZeIYA-0001j7-Vf for qemu-devel@nongnu.org; Tue, 22 Sep 2015 04:03:43 -0400 Received: from /spool/local by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 22 Sep 2015 09:03:40 +0100 Date: Tue, 22 Sep 2015 10:02:17 +0200 From: David Hildenbrand Message-ID: <20150922100217.41db925e@thinkpad-w530> In-Reply-To: <20150921153842.GF4004@thinpad.lan.raisama.net> References: <1442577640-11612-1-git-send-email-armbru@redhat.com> <1442577640-11612-6-git-send-email-armbru@redhat.com> <55FC05B7.9080902@de.ibm.com> <20150921103050.02ca68f8@thinkpad-w530> <20150921153842.GF4004@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/7] qdev: Protect device-list-properties against broken devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Peter Maydell , Peter Crosthwaite , qemu-devel@nongnu.org, qemu-stable@nongnu.org, Alexander Graf , Markus Armbruster , Christian Borntraeger , qemu-ppc@nongnu.org, Antony Pavlov , stefanha@redhat.com, Cornelia Huck , Paolo Bonzini , Alistair Francis , afaerber@suse.de, Li Guang , Richard Henderson > No knowledge should be required for object_new(). Classes' instance_init > functions should have no side-effects outside the object itself. While this should theoretically be true, I can guarantee to you that this is not the case for all devices :) (especially as there are too many unwritten laws related to that whole qmp/qom thing flying around). E.g. we can save ourselves from the creation of other devices (init + realize) by setting a device to !hotpluggable in the device class. This code simply bypasses such checks and assumes that QEMUs internal structure can deal with that. We saw that this doesn't work this far. Other interfaces (object_add) seem to rely on TYPE_USER_CREATABLE, so we can't trigger "everything" from outside QEMU. One can argue that we simply have to fix the devices - nevertheless I think this is the wrong approach to the problem. > > > > > Feels a little like hacking an interface to a java program, which allows to > > create any object of a special kind dynamically (constructor arguments?), > > reading some member variable (names) via reflections and then throwing that > > object away. Which sounds very wrong to me. > > I wouldn't call it wrong. Confusing, maybe. We use a mix of class-based > and prototype-based approaches to inheritance and property > introspection. > > > > > Wonder if it wouldn't make more sense to query only the static properties of a > > device. I mean if the dynamic properties are dynamic by definition (and can > > change during runtime). So looking up the static properties via the typename > > feels more KIS-style to me. Of course, the relevant properties would have to be > > defined statically then, which shouldn't be a problem. > > It depends on your definition of "shouldn't be a problem". :) Well, it might require some internal rework of that property thingy, hehe ;) > > The static property system is more limited than the QOM dynamic property > system, today. We already depend on introspection of dynamic properties > registered by instance_init functions, so we would need to move > everything to a better static property system if we want to stop doing > object_new() during class introspection without regressions. > > > > > Dynamic properties that are actually created could depend on almost > > everything in the system - why fake something that we don't know. > > Properties registered on instance_init shouldn't depend on anything else > on the system. Properties registered later in the object lifetime (e.g. > on realize) can. > Okay, so if the properties in instance_init should depend on nothing else in the system, they are always the same for one QEMU version + device. So maybe it would make sense to define all these "fixed properties" on a device class level (e.g. via dc->props) and fill them with life afterwards (if we can't completely specify them at that point). Of course this would need some rework/careful thinking (e.g. concept of uninitialized properties). Looking at the errors we get with that approach tells me that it will easily break again in the future (we need tests for all devices - do we already have it?) and makes me wonder if a definition on a device class level wouldn't be the right thing to do - a clean interface description that is valid for each device instance. David