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

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

[Philippe Mathieu-Daudé]

Hi,

"hw/boards.h" declare current_machine, and vl.c defines it:

     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?

Thanks,

Phil.

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

[Paolo Bonzini]

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.  Ideally, current_machine would be static within vl.c or even
unused outside the object_property_add_child() that you quote above.

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.

Paolo

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

[Like Xu]

On 2020/1/9 20:01, Paolo Bonzini wrote:
> 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.  Ideally, current_machine would be static within vl.c or even
> unused outside the object_property_add_child() that you quote above.
> 
> 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.
> 
> Paolo
> 

I prefer to use MACHINE(qdev_get_machine()) wherever possible.

However, the qdev_get_machine() would return non TYPE_MACHINE object if:
- call qdev_get_machine() before we do 
"object_property_add_child(object_get_root(), "machine", 
OBJECT(current_machine), &error_abort);" in vl.c;
- or in the context with '#ifdef CONFIG_USER_ONLY';

Thanks,
Like Xu

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

[Philippe Mathieu-Daudé]

On 1/9/20 4:24 PM, Like Xu wrote:
> On 2020/1/9 20:01, Paolo Bonzini wrote:
>> 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.  Ideally, current_machine would be static within vl.c or even
>> unused outside the object_property_add_child() that you quote above.
>>
>> 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.
>>
>> Paolo
>>
> 
> I prefer to use MACHINE(qdev_get_machine()) wherever possible.
> 
> However, the qdev_get_machine() would return non TYPE_MACHINE object if:
> - call qdev_get_machine() before we do 
> "object_property_add_child(object_get_root(), "machine", 
> OBJECT(current_machine), &error_abort);" in vl.c;

OK I'll add an assert() in case.

> - or in the context with '#ifdef CONFIG_USER_ONLY';

Good to know, I can simplify further :)

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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> 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.