Re: [PATCH 2/2] qom: Manage references to embedded child objects

On 2026/06/15 22:42, Mark Cave-Ayland wrote:
> On 15/06/2026 10:28, Peter Maydell wrote:
> 
>> On Mon, 15 Jun 2026 at 09:35, Daniel P. Berrangé <[email protected]> 
>> wrote:
>>>
>>> On Mon, Jun 15, 2026 at 09:31:36AM +0100, Peter Maydell wrote:
>>>> On Mon, 15 Jun 2026 at 09:13, Daniel P. Berrangé 
>>>> <[email protected]> wrote:
>>>>> Do we know how many examples we have of embedding objects inside
>>>>> another ?
>>>>
>>>> It is an extremely common pattern for device and SoC model
>>>> implementations; we've been recommending it for years.
>>>>
>>>>> I would much prefer if we forbid the embedding of objects. It is
>>>>> horrible design practice to have some QOM objects which can be
>>>>> freed via reference count and some which cannot.
>>>>
>>>> That would be a very large amount of code to rewrite to the
>>>> new paradigm. I don't object inherently ("you have pointers to
>>>> your child objects" works better when they might be implemented
>>>> in Rust and might play better with being able to create
>>>> machines and wire them up on the command line); I'm just
>>>> noting how much work it would be if you wanted to make
>>>> embedding forbidden.
>>>
>>> Would it be a more tractable problem to "fix" object structure only
>>> incrementally as they gain a need to be managed/reference from
>>> Rust code, or does the Rust usage already implicitly extend too
>>> broadly
>>
>> At the moment we have exactly 2 Rust devices. You can see the
>> workaround we ended up with for the PL011 in commits 5b87a07e768
>> and cc3d262aa93a4, where we pad out the C device struct and
>> assert on the Rust side that its version isn't any bigger.
>>
>> For that particular problem we could say "you need to refactor
>> all users of device X to not embed it before creating the Rust
>> version". There is I think only one embedded use of the PL011
>> (I was actually expecting more). This does have the awkward
>> effect that anybody wanting to do a "rewrite device in rust"
>> is now forced into "do a big refactor of some old C code they
>> don't care about". We would also need to actually document and
>> provide some good examples of "this is how we think we should
>> be writing device models that have child objects now".
> 
> I think given the legacy of the QEMU codebase then this will always be 
> an issue, but we've solved this in the past with the introduction of 
> MemoryRegions so there's no reason we couldn't do it again.
> 
> This is a similar problem to the use of non-class object properties I 
> was discussing in the thread with Daniel: it seems the general consensus 
> is that they shouldn't exist, but this hasn't been formalised anywhere. 
> If we can formalise the decision not to use embedded QOM objects, then 
> we can start by ensuring that new submissions don't use them and go from 
> there.
> 
> Question: who currently should make these decisions? Is it restricted to 
> the QOM maintainers?
> 
>> But embedded MemoryRegions are enormously common, and devices
>> that embed other devices that embed MemoryRegions are also
>> pretty common. So I don't know how much of a gradual approach
>> you could take for the issue descibed in this patch.
> 
> Zoltan did post a patch that allowed for adding externally referenced 
> MemoryRegions a while back: my reason for not supporting the approach 
> was because it added yet another way of adding MemoryRegions to work 
> around a particular legacy use case. However if there is agreement that 
> this is the way we should do things, I suspect that the series will have 
> solved most of the problems in this area.

Personally, I do not have a strong preference between supporting 
references to embedded objects and deprecating object embedding 
entirely, so this patch takes the smaller step needed to address the 
immediate issue. I would not object to deprecating embedding if that is 
the direction we want to take.

Regards,
Akihiko Odaki