From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36150) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QkG9r-0004aT-Gr for qemu-devel@nongnu.org; Fri, 22 Jul 2011 09:52:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QkG9q-0007Si-F7 for qemu-devel@nongnu.org; Fri, 22 Jul 2011 09:52:51 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:60813) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QkG9q-0007Se-AS for qemu-devel@nongnu.org; Fri, 22 Jul 2011 09:52:50 -0400 Received: by yia25 with SMTP id 25so1405805yia.4 for ; Fri, 22 Jul 2011 06:52:49 -0700 (PDT) Message-ID: <4E2980AF.5070601@codemonkey.ws> Date: Fri, 22 Jul 2011 08:52:47 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <4E259F6E.8000204@us.ibm.com> <4E2824D2.2050401@redhat.com> <4E2827A2.6010603@us.ibm.com> <4E282BE3.1050404@redhat.com> <4E283C90.8010806@us.ibm.com> <4E283FFE.6090201@redhat.com> <4E28497C.5010801@us.ibm.com> <4E284C74.2010708@redhat.com> <4E285492.1070006@codemonkey.ws> <4E292AC9.5080200@redhat.com> <4E296FBF.1090308@codemonkey.ws> <4E297805.5040607@redhat.com> In-Reply-To: <4E297805.5040607@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC] QEMU Object Model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Avi Kivity , qemu-devel , Markus Armbruster On 07/22/2011 08:15 AM, Kevin Wolf wrote: > Am 22.07.2011 14:40, schrieb Anthony Liguori: >> On 07/22/2011 02:46 AM, Kevin Wolf wrote: >>> Am 21.07.2011 18:32, schrieb Anthony Liguori: >>>> Just as we're now realizing that we need to do dramatic things in the >>>> block layer to make -blockdev work, I'm sure we're going to realize that >>>> we want to do PCI hotplug of virtio-serial and therefore we need to do >>>> dynamic creation/destruction of character devices. >>> >>> Just to have it said: -blockdev isn't the reason for doing something >>> like this. The reason is that you want it. (Not saying anything about >>> whether it is a good or a bad thing to want) >>> >>> What -blockdev really needs is an additional parameter to bdrv_open that >>> carries the options. Could be as simple as a QDict, though that might >>> not align well with your QAPI which wants everything to be C. But in any >>> case, generalising the qdev properties mechanism to be usable in other >>> contexts should be enough for anything. >> >> That's what QOM is. qdev generalized. > > Hm, yes, quite possible. > > Then maybe it's just its usage that I'm not happy with. You're turning > the block layer upside down to change BlockDriverStates and BDRV*State > to fit this API, which at the first sight looks pretty invasive. The changes are pretty systematic and boil down to: 1) Instead of BlockDriverState having an opaque pointer that points to BDRVQcowState, BDRVQcowState contains a BlockDriverState and you use the equivalent of container_of() to access it. 2) Instead of creating an empty BDS and then creating the substate and setting opaque, you create the substate through a factory. 3) Instead of passing a single const char * to bdrv_open(), each driver registers the options it supports. There is still an open() but the open takes no parameters because the parameters are set before open() is called. 4) Instead of having a fixed backing_file and file BDS * in BDS, each driver can register as many named BDS * as they want. It shouldn't change much code at all in the image formats. The only place that it will really change is block.c but not any of the code that actually matters. > I was > thinking of an object not for the whole BDS, but just for the same > purpose as the QDict would have been used, that is something that can be > passed to bdrv_open. Passing a QDict to bdrv_open() would allow you to set more properties, but there are other things that need to be address: a) how do you describe which properties are valid for a given image format? b) how do you specify how a backing_file gets initialized? c) how do support composing a BDS from multiple BDS's (to support VMDK's fully)? I think once you come up with a mechanism to do all of this, you get 90% of what QOM. By doing the extra 10%, we don't have to reinvent this for every other subsystem. netdev and qdev have already done this, but in different ways. Doing this in a third way would be pretty bad IMHO. > > On the other hand, it's really hard to quickly see in your git tree what > changes QOM really means. Real patches to the existing code would be > much more useful than an additional modified copy of the file. I'll have the first RFC out next week. Just trying to get some input as early as possible. Regards, Anthony Liguori > > Kevin >