Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback

[Andreas Färber]

Am 03.12.2013 15:00, schrieb Paolo Bonzini:
> Il 03/12/2013 14:44, Andreas Färber ha scritto:
>>>>
>>>> You can check "if (current_machine &&
>>>> current_machine->get_fw_dev_path)", and move current_machine from vl.c
>>>> to hw/qdev/core.c.
>> Please don't encourage moving random stuff into "core" device code.
>>
>> If needed, we can easily add a machine.c, but that should remain
>> softmmu-only.
> 
> Another solution would be to:
> 
> (1) add an interface which contains "get_fw_dev_path".  When
> qdev_get_fw_dev_path is called, walk the QOM tree until an object that
> implements the interface is found.  If none is found, call the bus
> implementation as usual.
> 
> (2) in vl.c, add a way for current_machine to override the /machine
> object.  A 100%-QOMified machine indeed could put a SOC-like Device there.
> 
> (3) for spapr, define the machine object to something that implements
> said interface.
> 
> It seemed a bit complicated for this particular problem, but I cannot
> say it's overengineered.
> 
> More aspects of the configuration could be moved to the new interface
> over time, for example compat properties.

I remember Hervé running into problems with interfaces and ppc -cpu ?
(or -cpu host?), which does an object_class_foreach(), in the middle of
which QOM interfaces tried registering new types, leading to a GLib
assertion. Anthony never replied to that patch despite repeated pings,
so the issue is likely still unresolved.

http://patchwork.ozlabs.org/patch/241072/ (proposed workaround)
http://patchwork.ozlabs.org/patch/241504/ (test case)
http://patchwork.ozlabs.org/patch/241073/ (actual interface attempt)

I've thus been avoiding interfaces.

They are also kind of odd wrt memory management since even when we
object_initialize() previously allocated memory, interfaces of that type
are not part of the object size and are still dynamically allocated
IIRC. That may well be fixable of course by allocating space for
interfaces after the instance_size and bumping its copy in
TypeImpl::instance_size accordingly or calculate it in my proposed
type_get_instance_size().
http://patchwork.ozlabs.org/patch/269591/
http://patchwork.ozlabs.org/patch/269595/

(Which is BTW where I long wanted to introduce a static error_oom
similar to the error_abort discussed in a different thread now.)

Andreas

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