[PATCH 10/12] qom: Create system containers explicitly

Peter Xu posted 12 patches 2 days, 17 hours ago
There is a newer version of this series
[PATCH 10/12] qom: Create system containers explicitly
Posted by Peter Xu 2 days, 17 hours ago
Always explicitly create QEMU system containers upfront.

Root containers will be created when trying to fetch the root object the
1st time.  Machine sub-containers will be created only until machine is
being initialized.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/core/machine.c | 19 ++++++++++++++++---
 qom/object.c      | 16 +++++++++++++++-
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index a35c4a8fae..a184dbf8f0 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1193,14 +1193,27 @@ static void machine_class_base_init(ObjectClass *oc, void *data)
     }
 }
 
+const char *machine_containers[] = {
+    "unattached",
+    "peripheral",
+    "peripheral-anon"
+};
+
+static void qemu_create_machine_containers(Object *machine)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(machine_containers); i++) {
+        container_create(machine, machine_containers[i]);
+    }
+}
+
 static void machine_initfn(Object *obj)
 {
     MachineState *ms = MACHINE(obj);
     MachineClass *mc = MACHINE_GET_CLASS(obj);
 
-    container_get(obj, "/peripheral");
-    container_get(obj, "/peripheral-anon");
-
+    qemu_create_machine_containers(obj);
     ms->dump_guest_core = true;
     ms->mem_merge = (QEMU_MADV_MERGEABLE != QEMU_MADV_INVALID);
     ms->enable_graphics = true;
diff --git a/qom/object.c b/qom/object.c
index 214d6eb4c1..810e6f2bd9 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
     return prop->type;
 }
 
+static Object *object_root_initialize(void)
+{
+    Object *root = object_new(TYPE_CONTAINER);
+
+    /*
+     * Create all QEMU system containers.  "machine" and its sub-containers
+     * are only created when machine initializes (qemu_create_machine()).
+     */
+    container_create(root, "chardevs");
+    container_create(root, "objects");
+
+    return root;
+}
+
 Object *object_get_root(void)
 {
     static Object *root;
 
     if (!root) {
-        root = object_new(TYPE_CONTAINER);
+        root = object_root_initialize();
     }
 
     return root;
-- 
2.45.0
Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Markus Armbruster 2 days, 1 hour ago
Peter Xu <peterx@redhat.com> writes:

> Always explicitly create QEMU system containers upfront.
>
> Root containers will be created when trying to fetch the root object the
> 1st time.

Which ones are affected?

Not a fan of creating stuff on first use, unless there may not be any
use.  But no worse than before.

>            Machine sub-containers will be created only until machine is
> being initialized.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/core/machine.c | 19 ++++++++++++++++---
>  qom/object.c      | 16 +++++++++++++++-
>  2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index a35c4a8fae..a184dbf8f0 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1193,14 +1193,27 @@ static void machine_class_base_init(ObjectClass *oc, void *data)
>      }
>  }
>  
> +const char *machine_containers[] = {
> +    "unattached",
> +    "peripheral",
> +    "peripheral-anon"
> +};
> +
> +static void qemu_create_machine_containers(Object *machine)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(machine_containers); i++) {
> +        container_create(machine, machine_containers[i]);
> +    }
> +}
> +
>  static void machine_initfn(Object *obj)
>  {
>      MachineState *ms = MACHINE(obj);
>      MachineClass *mc = MACHINE_GET_CLASS(obj);
>  
> -    container_get(obj, "/peripheral");
> -    container_get(obj, "/peripheral-anon");
> -
> +    qemu_create_machine_containers(obj);

We now additionally create "/machine/unattached" here.  

>      ms->dump_guest_core = true;
>      ms->mem_merge = (QEMU_MADV_MERGEABLE != QEMU_MADV_INVALID);
>      ms->enable_graphics = true;
> diff --git a/qom/object.c b/qom/object.c
> index 214d6eb4c1..810e6f2bd9 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
>      return prop->type;
>  }
>  
> +static Object *object_root_initialize(void)
> +{
> +    Object *root = object_new(TYPE_CONTAINER);
> +
> +    /*
> +     * Create all QEMU system containers.  "machine" and its sub-containers
> +     * are only created when machine initializes (qemu_create_machine()).
> +     */
> +    container_create(root, "chardevs");
> +    container_create(root, "objects");
> +
> +    return root;
> +}
> +
>  Object *object_get_root(void)
>  {
>      static Object *root;
>  
>      if (!root) {
> -        root = object_new(TYPE_CONTAINER);
> +        root = object_root_initialize();

We now additonally create "/chardevs" and "/objects" here, not just "/".

>      }
>  
>      return root;
Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Peter Xu 1 day, 21 hours ago
On Thu, Nov 21, 2024 at 02:31:23PM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > Always explicitly create QEMU system containers upfront.
> >
> > Root containers will be created when trying to fetch the root object the
> > 1st time.
> 
> Which ones are affected?

I updated the commit message to this:

    qom: Create system containers explicitly
    
    Always explicitly create QEMU system containers upfront.
    
    Root containers will be created when trying to fetch the root object the
    1st time.  They are:
    
      /objects
      /chardevs
      /backend
    
    Machine sub-containers will be created only until machine is being
    initialized.  They are:
    
      /machine/unattached
      /machine/peripheral
      /machine/peripheral-anon

Note that I also added "backend" per request from Dan, as of now.

> 
> Not a fan of creating stuff on first use, unless there may not be any
> use.  But no worse than before.

True.. since that can be a separate question to answer, I avoided going too
far to dig into which binaries may use containers, which may not.

Thanks,

-- 
Peter Xu
Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Daniel P. Berrangé 2 days, 4 hours ago
On Wed, Nov 20, 2024 at 04:57:01PM -0500, Peter Xu wrote:
> Always explicitly create QEMU system containers upfront.
> 
> Root containers will be created when trying to fetch the root object the
> 1st time.  Machine sub-containers will be created only until machine is
> being initialized.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/core/machine.c | 19 ++++++++++++++++---
>  qom/object.c      | 16 +++++++++++++++-
>  2 files changed, 31 insertions(+), 4 deletions(-)


> diff --git a/qom/object.c b/qom/object.c
> index 214d6eb4c1..810e6f2bd9 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
>      return prop->type;
>  }
>  
> +static Object *object_root_initialize(void)
> +{
> +    Object *root = object_new(TYPE_CONTAINER);
> +
> +    /*
> +     * Create all QEMU system containers.  "machine" and its sub-containers
> +     * are only created when machine initializes (qemu_create_machine()).
> +     */
> +    container_create(root, "chardevs");
> +    container_create(root, "objects");

This is where I would expect 'backend' to have been created
rather than ui/console.c, though you could potentially make
a case to create it from the machine function, snice console
stuff can't be used outside of the machine context, while
chardevs/objects can be used in qemu-img/qemu-nbd, etc

> +
> +    return root;
> +}


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Philippe Mathieu-Daudé 2 days, 2 hours ago
On 21/11/24 11:30, Daniel P. Berrangé wrote:
> On Wed, Nov 20, 2024 at 04:57:01PM -0500, Peter Xu wrote:
>> Always explicitly create QEMU system containers upfront.
>>
>> Root containers will be created when trying to fetch the root object the
>> 1st time.  Machine sub-containers will be created only until machine is
>> being initialized.
>>
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>> ---
>>   hw/core/machine.c | 19 ++++++++++++++++---
>>   qom/object.c      | 16 +++++++++++++++-
>>   2 files changed, 31 insertions(+), 4 deletions(-)
> 
> 
>> diff --git a/qom/object.c b/qom/object.c
>> index 214d6eb4c1..810e6f2bd9 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
>>       return prop->type;
>>   }
>>   
>> +static Object *object_root_initialize(void)
>> +{
>> +    Object *root = object_new(TYPE_CONTAINER);
>> +
>> +    /*
>> +     * Create all QEMU system containers.  "machine" and its sub-containers
>> +     * are only created when machine initializes (qemu_create_machine()).
>> +     */
>> +    container_create(root, "chardevs");
>> +    container_create(root, "objects");
> 
> This is where I would expect 'backend' to have been created
> rather than ui/console.c, though you could potentially make
> a case to create it from the machine function, snice console
> stuff can't be used outside of the machine context, while
> chardevs/objects can be used in qemu-img/qemu-nbd, etc

What about creating "backend" container in qemu_create_machine()?


Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Peter Xu 1 day, 21 hours ago
On Thu, Nov 21, 2024 at 02:01:45PM +0100, Philippe Mathieu-Daudé wrote:
> On 21/11/24 11:30, Daniel P. Berrangé wrote:
> > On Wed, Nov 20, 2024 at 04:57:01PM -0500, Peter Xu wrote:
> > > Always explicitly create QEMU system containers upfront.
> > > 
> > > Root containers will be created when trying to fetch the root object the
> > > 1st time.  Machine sub-containers will be created only until machine is
> > > being initialized.
> > > 
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > >   hw/core/machine.c | 19 ++++++++++++++++---
> > >   qom/object.c      | 16 +++++++++++++++-
> > >   2 files changed, 31 insertions(+), 4 deletions(-)
> > 
> > 
> > > diff --git a/qom/object.c b/qom/object.c
> > > index 214d6eb4c1..810e6f2bd9 100644
> > > --- a/qom/object.c
> > > +++ b/qom/object.c
> > > @@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
> > >       return prop->type;
> > >   }
> > > +static Object *object_root_initialize(void)
> > > +{
> > > +    Object *root = object_new(TYPE_CONTAINER);
> > > +
> > > +    /*
> > > +     * Create all QEMU system containers.  "machine" and its sub-containers
> > > +     * are only created when machine initializes (qemu_create_machine()).
> > > +     */
> > > +    container_create(root, "chardevs");
> > > +    container_create(root, "objects");
> > 
> > This is where I would expect 'backend' to have been created
> > rather than ui/console.c, though you could potentially make
> > a case to create it from the machine function, snice console
> > stuff can't be used outside of the machine context, while
> > chardevs/objects can be used in qemu-img/qemu-nbd, etc

Would it hurt if we do it altogether here even if it won't be used in
qemu-img/qemu-nbd?

IMHO we should either make it simple (assuming empty containers won't hurt
there..), or we should just leave "backend" to ui/ code, so we don't assume
which binary is using the ui code: whoever uses it will create the container.

> 
> What about creating "backend" container in qemu_create_machine()?

I remember I started with that but it didn't work.  IIRC that's because
machine_initfn() (or somewhere around the init code) requires the
containers to present, hence it's too late even if we create the containers
right after this line:

    current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));

-- 
Peter Xu


Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Philippe Mathieu-Daudé 1 day, 21 hours ago
On 21/11/24 18:17, Peter Xu wrote:
> On Thu, Nov 21, 2024 at 02:01:45PM +0100, Philippe Mathieu-Daudé wrote:
>> On 21/11/24 11:30, Daniel P. Berrangé wrote:
>>> On Wed, Nov 20, 2024 at 04:57:01PM -0500, Peter Xu wrote:
>>>> Always explicitly create QEMU system containers upfront.
>>>>
>>>> Root containers will be created when trying to fetch the root object the
>>>> 1st time.  Machine sub-containers will be created only until machine is
>>>> being initialized.
>>>>
>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>> ---
>>>>    hw/core/machine.c | 19 ++++++++++++++++---
>>>>    qom/object.c      | 16 +++++++++++++++-
>>>>    2 files changed, 31 insertions(+), 4 deletions(-)
>>>
>>>
>>>> diff --git a/qom/object.c b/qom/object.c
>>>> index 214d6eb4c1..810e6f2bd9 100644
>>>> --- a/qom/object.c
>>>> +++ b/qom/object.c
>>>> @@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
>>>>        return prop->type;
>>>>    }
>>>> +static Object *object_root_initialize(void)
>>>> +{
>>>> +    Object *root = object_new(TYPE_CONTAINER);
>>>> +
>>>> +    /*
>>>> +     * Create all QEMU system containers.  "machine" and its sub-containers
>>>> +     * are only created when machine initializes (qemu_create_machine()).
>>>> +     */
>>>> +    container_create(root, "chardevs");
>>>> +    container_create(root, "objects");
>>>
>>> This is where I would expect 'backend' to have been created
>>> rather than ui/console.c, though you could potentially make
>>> a case to create it from the machine function, snice console
>>> stuff can't be used outside of the machine context, while
>>> chardevs/objects can be used in qemu-img/qemu-nbd, etc
> 
> Would it hurt if we do it altogether here even if it won't be used in
> qemu-img/qemu-nbd?
> 
> IMHO we should either make it simple (assuming empty containers won't hurt
> there..), or we should just leave "backend" to ui/ code, so we don't assume
> which binary is using the ui code: whoever uses it will create the container.
> 
>>
>> What about creating "backend" container in qemu_create_machine()?
> 
> I remember I started with that but it didn't work.  IIRC that's because
> machine_initfn() (or somewhere around the init code) requires the
> containers to present, hence it's too late even if we create the containers
> right after this line:
> 
>      current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));

So qemu_create_machine_containers() really belongs to 
qemu_create_machine() =)

Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Peter Xu 1 day, 21 hours ago
On Thu, Nov 21, 2024 at 06:29:06PM +0100, Philippe Mathieu-Daudé wrote:
> On 21/11/24 18:17, Peter Xu wrote:
> > On Thu, Nov 21, 2024 at 02:01:45PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 21/11/24 11:30, Daniel P. Berrangé wrote:
> > > > On Wed, Nov 20, 2024 at 04:57:01PM -0500, Peter Xu wrote:
> > > > > Always explicitly create QEMU system containers upfront.
> > > > > 
> > > > > Root containers will be created when trying to fetch the root object the
> > > > > 1st time.  Machine sub-containers will be created only until machine is
> > > > > being initialized.
> > > > > 
> > > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > > ---
> > > > >    hw/core/machine.c | 19 ++++++++++++++++---
> > > > >    qom/object.c      | 16 +++++++++++++++-
> > > > >    2 files changed, 31 insertions(+), 4 deletions(-)
> > > > 
> > > > 
> > > > > diff --git a/qom/object.c b/qom/object.c
> > > > > index 214d6eb4c1..810e6f2bd9 100644
> > > > > --- a/qom/object.c
> > > > > +++ b/qom/object.c
> > > > > @@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
> > > > >        return prop->type;
> > > > >    }
> > > > > +static Object *object_root_initialize(void)
> > > > > +{
> > > > > +    Object *root = object_new(TYPE_CONTAINER);
> > > > > +
> > > > > +    /*
> > > > > +     * Create all QEMU system containers.  "machine" and its sub-containers
> > > > > +     * are only created when machine initializes (qemu_create_machine()).
> > > > > +     */
> > > > > +    container_create(root, "chardevs");
> > > > > +    container_create(root, "objects");
> > > > 
> > > > This is where I would expect 'backend' to have been created
> > > > rather than ui/console.c, though you could potentially make
> > > > a case to create it from the machine function, snice console
> > > > stuff can't be used outside of the machine context, while
> > > > chardevs/objects can be used in qemu-img/qemu-nbd, etc
> > 
> > Would it hurt if we do it altogether here even if it won't be used in
> > qemu-img/qemu-nbd?
> > 
> > IMHO we should either make it simple (assuming empty containers won't hurt
> > there..), or we should just leave "backend" to ui/ code, so we don't assume
> > which binary is using the ui code: whoever uses it will create the container.
> > 
> > > 
> > > What about creating "backend" container in qemu_create_machine()?
> > 
> > I remember I started with that but it didn't work.  IIRC that's because
> > machine_initfn() (or somewhere around the init code) requires the
> > containers to present, hence it's too late even if we create the containers
> > right after this line:
> > 
> >      current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
> 
> So qemu_create_machine_containers() really belongs to qemu_create_machine()
> =)

Frankly, I don't immediately get this line..

But when I was trying again just to check my memory, I can't see anything
crash anymore, moving things over.

So while I'll test some more, I can switch to that if I cannot reproduce
any issue with it.  That's:

===8<===

diff --git a/hw/core/machine.c b/hw/core/machine.c
index ed613ec4cb..a72c001c3d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1193,27 +1193,11 @@ static void machine_class_base_init(ObjectClass *oc, void *data)
     }
 }
 
-static const char *const machine_containers[] = {
-    "unattached",
-    "peripheral",
-    "peripheral-anon"
-};
-
-static void qemu_create_machine_containers(Object *machine)
-{
-    int i;
-
-    for (i = 0; i < ARRAY_SIZE(machine_containers); i++) {
-        object_property_add_new_container(machine, machine_containers[i]);
-    }
-}
-
 static void machine_initfn(Object *obj)
 {
     MachineState *ms = MACHINE(obj);
     MachineClass *mc = MACHINE_GET_CLASS(obj);
 
-    qemu_create_machine_containers(obj);
     ms->dump_guest_core = true;
     ms->mem_merge = (QEMU_MADV_MERGEABLE != QEMU_MADV_INVALID);
     ms->enable_graphics = true;
diff --git a/system/vl.c b/system/vl.c
index 822f7ff656..cdc0b6e10c 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2112,6 +2112,21 @@ static void parse_memory_options(void)
     loc_pop(&loc);
 }
 
+static const char *const machine_containers[] = {
+    "unattached",
+    "peripheral",
+    "peripheral-anon"
+};
+
+static void qemu_create_machine_containers(Object *machine)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(machine_containers); i++) {
+        object_property_add_new_container(machine, machine_containers[i]);
+    }
+}
+
 static void qemu_create_machine(QDict *qdict)
 {
     MachineClass *machine_class = select_machine(qdict, &error_fatal);
@@ -2120,6 +2135,7 @@ static void qemu_create_machine(QDict *qdict)
     current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
     object_property_add_child(object_get_root(), "machine",
                               OBJECT(current_machine));
+    qemu_create_machine_containers(OBJECT(current_machine));
     object_property_add_child(machine_get_container("unattached"),
                               "sysbus", OBJECT(sysbus_get_default()));
 

-- 
Peter Xu


Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Philippe Mathieu-Daudé 1 day, 20 hours ago
On 21/11/24 19:03, Peter Xu wrote:
> On Thu, Nov 21, 2024 at 06:29:06PM +0100, Philippe Mathieu-Daudé wrote:
>> On 21/11/24 18:17, Peter Xu wrote:
>>> On Thu, Nov 21, 2024 at 02:01:45PM +0100, Philippe Mathieu-Daudé wrote:
>>>> On 21/11/24 11:30, Daniel P. Berrangé wrote:
>>>>> On Wed, Nov 20, 2024 at 04:57:01PM -0500, Peter Xu wrote:
>>>>>> Always explicitly create QEMU system containers upfront.
>>>>>>
>>>>>> Root containers will be created when trying to fetch the root object the
>>>>>> 1st time.  Machine sub-containers will be created only until machine is
>>>>>> being initialized.
>>>>>>
>>>>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>>>>> ---
>>>>>>     hw/core/machine.c | 19 ++++++++++++++++---
>>>>>>     qom/object.c      | 16 +++++++++++++++-
>>>>>>     2 files changed, 31 insertions(+), 4 deletions(-)
>>>>>
>>>>>
>>>>>> diff --git a/qom/object.c b/qom/object.c
>>>>>> index 214d6eb4c1..810e6f2bd9 100644
>>>>>> --- a/qom/object.c
>>>>>> +++ b/qom/object.c
>>>>>> @@ -1734,12 +1734,26 @@ const char *object_property_get_type(Object *obj, const char *name, Error **errp
>>>>>>         return prop->type;
>>>>>>     }
>>>>>> +static Object *object_root_initialize(void)
>>>>>> +{
>>>>>> +    Object *root = object_new(TYPE_CONTAINER);
>>>>>> +
>>>>>> +    /*
>>>>>> +     * Create all QEMU system containers.  "machine" and its sub-containers
>>>>>> +     * are only created when machine initializes (qemu_create_machine()).
>>>>>> +     */
>>>>>> +    container_create(root, "chardevs");
>>>>>> +    container_create(root, "objects");
>>>>>
>>>>> This is where I would expect 'backend' to have been created
>>>>> rather than ui/console.c, though you could potentially make
>>>>> a case to create it from the machine function, snice console
>>>>> stuff can't be used outside of the machine context, while
>>>>> chardevs/objects can be used in qemu-img/qemu-nbd, etc
>>>
>>> Would it hurt if we do it altogether here even if it won't be used in
>>> qemu-img/qemu-nbd?
>>>
>>> IMHO we should either make it simple (assuming empty containers won't hurt
>>> there..), or we should just leave "backend" to ui/ code, so we don't assume
>>> which binary is using the ui code: whoever uses it will create the container.
>>>
>>>>
>>>> What about creating "backend" container in qemu_create_machine()?
>>>
>>> I remember I started with that but it didn't work.  IIRC that's because
>>> machine_initfn() (or somewhere around the init code) requires the
>>> containers to present, hence it's too late even if we create the containers
>>> right after this line:
>>>
>>>       current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
>>
>> So qemu_create_machine_containers() really belongs to qemu_create_machine()
>> =)
> 
> Frankly, I don't immediately get this line..

"machine_initfn requires the containers to be present" -> machine_initfn
isn't the place to create them, it has to be before or after. Since it
can't be before, the "after" place is qemu_create_machine_containers().
Sorry for not being very clear :/

> 
> But when I was trying again just to check my memory, I can't see anything
> crash anymore, moving things over.
> 
> So while I'll test some more, I can switch to that if I cannot reproduce
> any issue with it.  That's:
> 
> ===8<===
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index ed613ec4cb..a72c001c3d 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1193,27 +1193,11 @@ static void machine_class_base_init(ObjectClass *oc, void *data)
>       }
>   }
>   
> -static const char *const machine_containers[] = {
> -    "unattached",
> -    "peripheral",
> -    "peripheral-anon"
> -};
> -
> -static void qemu_create_machine_containers(Object *machine)
> -{
> -    int i;
> -
> -    for (i = 0; i < ARRAY_SIZE(machine_containers); i++) {
> -        object_property_add_new_container(machine, machine_containers[i]);
> -    }
> -}
> -
>   static void machine_initfn(Object *obj)
>   {
>       MachineState *ms = MACHINE(obj);
>       MachineClass *mc = MACHINE_GET_CLASS(obj);
>   
> -    qemu_create_machine_containers(obj);
>       ms->dump_guest_core = true;
>       ms->mem_merge = (QEMU_MADV_MERGEABLE != QEMU_MADV_INVALID);
>       ms->enable_graphics = true;
> diff --git a/system/vl.c b/system/vl.c
> index 822f7ff656..cdc0b6e10c 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -2112,6 +2112,21 @@ static void parse_memory_options(void)
>       loc_pop(&loc);
>   }
>   
> +static const char *const machine_containers[] = {
> +    "unattached",
> +    "peripheral",
> +    "peripheral-anon"
> +};
> +
> +static void qemu_create_machine_containers(Object *machine)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(machine_containers); i++) {
> +        object_property_add_new_container(machine, machine_containers[i]);
> +    }
> +}
> +
>   static void qemu_create_machine(QDict *qdict)
>   {
>       MachineClass *machine_class = select_machine(qdict, &error_fatal);
> @@ -2120,6 +2135,7 @@ static void qemu_create_machine(QDict *qdict)
>       current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
>       object_property_add_child(object_get_root(), "machine",
>                                 OBJECT(current_machine));
> +    qemu_create_machine_containers(OBJECT(current_machine));
>       object_property_add_child(machine_get_container("unattached"),
>                                 "sysbus", OBJECT(sysbus_get_default()));
>   
> 

Yes, this is exactly what I was thinking of / expecting :)


Re: [PATCH 10/12] qom: Create system containers explicitly
Posted by Philippe Mathieu-Daudé 2 days, 5 hours ago
On 20/11/24 22:57, Peter Xu wrote:
> Always explicitly create QEMU system containers upfront.
> 
> Root containers will be created when trying to fetch the root object the
> 1st time.  Machine sub-containers will be created only until machine is
> being initialized.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/core/machine.c | 19 ++++++++++++++++---
>   qom/object.c      | 16 +++++++++++++++-
>   2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index a35c4a8fae..a184dbf8f0 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1193,14 +1193,27 @@ static void machine_class_base_init(ObjectClass *oc, void *data)
>       }
>   }
>   
> +const char *machine_containers[] = {

    const char *const machine_containers[] = {

> +    "unattached",
> +    "peripheral",
> +    "peripheral-anon"
> +};

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>