All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Fabiano Rosas" <farosas@suse.de>,
	"Juraj Marcin" <jmarcin@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Cédric Le Goater" <clg@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH 5/5] qom: Make container_get() strict to always walk or return container
Date: Tue, 19 Nov 2024 15:25:10 -0500	[thread overview]
Message-ID: <Zzz0Jt_BbkJyzbzB@x1n> (raw)
In-Reply-To: <ZzxianGKK71p0yA1@redhat.com>

On Tue, Nov 19, 2024 at 10:03:22AM +0000, Daniel P. Berrangé wrote:
> The docs are a welcome addition, but at the same time the docs won't get
> read most of the time.
> 
> With this in mind, IMHO, it is a conceptually *terrible* design for us to
> have a method called "get" which magically *creates* stuff as a side-effect
> of its calling. We'd be well served by fixing that design problem.
> 
> If I look in the code at what calls we have to container_get, and more
> specifically what "path" values we pass, there are not actually that many:
> 
>   /objects
>   /chardevs
>   /unattached
>   /machine
>   /peripheral
>   /peripheral-anon
>   /backend
>   /dr-connector
>   
> 
> Ignoring the last one, those other 7 containers are things we expect
> to exist in *every* system emulator.
> 
> Second, every single one of them is a single level deep. IOW, the for()
> loop in container_get is effectively pointless.
> 
> We can fix this by having a single method:
> 
>  void container_create_builtin(Object *root)
> 
> which creates the 7 built-in standard containers we expect
> everywhere, with open coded object_new + add_child calls. 
> 
> Then all current users of container_get() can switch over
> to object_resolve_path, and container_get() can be eliminated.
> 
> The 'dr-connector' creation can just be open-coded using
> object_new() in the spapr code.

Yes I think this could make sense, also after I noticed that the assert I
added may not always work..  Please ignore this series then, I'll prepare
something else soon.

-- 
Peter Xu



      reply	other threads:[~2024-11-19 20:26 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-18 22:13 [PATCH 0/5] QOM: Enforce container_get() to operate on containers only Peter Xu
2024-11-18 22:13 ` [PATCH 1/5] qom: Add TYPE_CONTAINER macro Peter Xu
2024-11-19  9:42   ` Daniel P. Berrangé
2024-11-19 19:52     ` Peter Xu
2024-11-18 22:13 ` [PATCH 2/5] ppc/e500: Avoid abuse of container_get() Peter Xu
2024-11-18 22:13 ` [PATCH 3/5] qdev: Make device_set_realized() always safe in tests Peter Xu
2024-11-19  9:46   ` Daniel P. Berrangé
2024-11-19 20:14     ` Peter Xu
2024-11-18 22:13 ` [PATCH 4/5] qdev: Make qdev_get_machine() not use container_get() Peter Xu
2024-11-18 22:13 ` [PATCH 5/5] qom: Make container_get() strict to always walk or return container Peter Xu
2024-11-18 23:06   ` Peter Xu
2024-11-19  8:09     ` Paolo Bonzini
2024-11-19 20:06       ` Peter Xu
2024-11-19 20:30         ` Paolo Bonzini
2024-11-19 21:43           ` Peter Xu
2024-11-20 11:45             ` Paolo Bonzini
2024-11-20 16:24               ` Peter Xu
2024-11-19 10:03   ` Daniel P. Berrangé
2024-11-19 20:25     ` Peter Xu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zzz0Jt_BbkJyzbzB@x1n \
    --to=peterx@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=clg@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=farosas@suse.de \
    --cc=jmarcin@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.