Re: [Qemu-devel] [PATCH v3 00/14] qemu: generate acpi tables for the guest

[Michael S. Tsirkin]

On Thu, Jul 25, 2013 at 05:50:55PM +0200, Andreas Färber 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
> > 
> > Please review, and consider for 1.6.
> 
> 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:
> 
> 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.
> 
> However here I am still seeing *functions* added in device code to check
> 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
> 
> I'm not sure if there was a misunderstanding or whether the PC QOM model
> 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 = 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.
> 
> User-added -devices will show up in /machine/peripheral or
> /machine/peripheral-anon depending on whether id= 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.
> 
> 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.
> 
> Will take a closer look at the implementation later.
> 
> Regards,
> Andreas
> 

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.





> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg