From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44026) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2OEs-0001ZW-OK for qemu-devel@nongnu.org; Thu, 25 Jul 2013 12:18:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V2OEp-0000rH-QC for qemu-devel@nongnu.org; Thu, 25 Jul 2013 12:18:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35911) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2OEp-0000r5-Ig for qemu-devel@nongnu.org; Thu, 25 Jul 2013 12:17:59 -0400 Date: Thu, 25 Jul 2013 19:19:17 +0300 From: "Michael S. Tsirkin" Message-ID: <20130725161917.GA7079@redhat.com> References: <1374681580-17439-1-git-send-email-mst@redhat.com> <51F1495F.2010502@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <51F1495F.2010502@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 00/14] qemu: generate acpi tables for the guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: qemu-devel@nongnu.org, Anthony Liguori On Thu, Jul 25, 2013 at 05:50:55PM +0200, Andreas F=E4rber wrote: > Am 24.07.2013 18:01, schrieb Michael S. Tsirkin: > > This code can also be found here: > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi > >=20 > > Please review, and consider for 1.6. >=20 > Quite frankly, this is still not looking the way I imagined it based on > the KVM call discussion and Anthony's comments that I remember: >=20 > I believe Anthony asked to extract the information from the QOM tree, > originally from the SeaBIOS side, then later agreeing to do it on the > QEMU side. >=20 > However here I am still seeing *functions* added in device code to chec= k > device existence and to extract individual fields. I was assuming (and > clearly prefer) such code to live in a central place, be it acpi-build.= c > or something else, and to use QOM *API*s to obtain information when > needed rather than building up lots of new structs duplicating that > data. That would at the same time be a test case for how useful the QOM > tree is >=20 > I'm not sure if there was a misunderstanding or whether the PC QOM mode= l > still sucks^W is incomplete? Anthony and Ping Fang(?) had both posted > patches to improve the composition tree once. If there's properties > missing that you need to access for ACPI, we should simply add them. > For i440fx we have /machine/i440fx. > For q35 I encountered an mch child on q35-pcihost, but what's trivially > missing apparently is to add q35-pcihost as a child to /machine, e.g. > /machine/q35. > Then you'll end up doing > Object *obj =3D object_resolve_path_component(qdev_get_machine(), "q35/= mch"); > object_property_get_int(obj, "foo", &err); > object_property_get_string(obj, "bar", &err); > and so on. No need to do the TYPE_... based search for everything. >=20 > User-added -devices will show up in /machine/peripheral or > /machine/peripheral-anon depending on whether id=3D is used, so there a > type-based search probably makes sense. And there is nothing wrong with > moving the TYPE_* constants to a device header where not yet the case, > to allow that from generic code. >=20 > Similarly, please don't open-code OBJECT_CHECK()s, do a trivial patch > with a macro that we can then reuse elsewhere. I'd be happy to review > such QOM patches and help fast-track them into master. >=20 > Will take a closer look at the implementation later. >=20 > Regards, > Andreas >=20 This is not my understanding of previous comments on list or on KVM call. Basically it sounds like you want to make my work depend on completion of QOM conversion. I think we explicitly agreed full QOM convertion is not a blocker. Meanwhile, I'm adding APIs in particular so you can work on converting things to QOM without bothering with ACPI. If you want to know what is missing, you only need to look at the patches themselves, once they are merged you can rework them internally without need to touch acpi code. As for your suggestion to poke at internal structures in a central place = - generally, I don't see why poking at internal field or properties of all kind of device structures or hard-coding paths in acpi-build is a good idea, surely accessing structures through APIs is the basic idea of data hiding. Using QOM to find devices rather than passing pointers around makes sense to me since this removes the need to depend on initialization order. > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg