Re: Difference between 'current_machine' vs MACHINE(qdev_get_machine())

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> On 09/01/20 12:23, Philippe Mathieu-Daudé wrote:
>> 
>> 
>>     current_machine =
>> MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
>>     object_property_add_child(object_get_root(), "machine",
>>                               OBJECT(current_machine), &error_abort);
>> 
>> The bigger user of 'current_machine' is the accel/KVM code.
>> 
>> Recently in a0628599f..cc7d44c2e0 "Replace global smp variables with
>> machine smp properties" we started to use MACHINE(qdev_get_machine()).
>> 
>> qdev_get_machine() resolves the machine in the QOM composition tree.
>> I am confused by this comment:
>> 
>>   /* qdev_get_machine() can return something that's not TYPE_MACHINE
>>    * if this is one of the user-only emulators; in that case there's
>>    * no need to check the ignore_memory_transaction_failures board flag.
>>    */
>> 
>> Following a0628599f..cc7d44c2e0, a5e0b33119 use 'current_machine' again.
>> 
>> What are the differences between both form, when should we use one or
>> another (or can we use a single one?). Can this break user-only mode?
>
> I would always use MACHINE(qdev_get_machine()), espeecially outside
> vl.c.

The way it works now, I wouldn't.  If you call qdev_get_machine() before
main() creates "/machine", it "helpfully" creates "/machine" as a
container object.  When main() tries to put the actual machine there,
it'll abort with "attempt to add duplicate property 'machine' to object
(type 'container')".  Trap for the unwary.  See also commit e2fb3fbbf9c
and 1a3ec8c1564.

Now ready your barf bags: We actually rely on this.  The user emulators
call qdev_get_machine() even though they don't have a real machine.
They get a container dummy, which is good enough for them.  The comment
that confused Philippe tries to explain exactly that.

>        Ideally, current_machine would be static within vl.c or even
> unused outside the object_property_add_child() that you quote above.

Before we spread use of qdev_get_machine() even more widely, we should
eliminate its side effect.  Programs that need a "/machine" should
create it explicitly.

> Most of the times, I noticed from a quick grep, we actually want to
> access the accelerator, not the machine, so we could add a
> qemu_get_accelerator() wrapper that does
> MACHINE(qdev_get_machine())->accelerator.

A convenience function to get the accelerator is unobjectionable.  It
having a side effect is objectionable.