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

[Andreas Färber]

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

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