On Tue, 27 Jan 2026, Peter Xu wrote:
On Tue, Jan 27, 2026 at 03:10:28PM +0100, BALATON Zoltan wrote:
This series does not convert everything but opens the way to do that and
simplify objects by only storing things in the state that are really needed
by the object and let QOM manage what it can. I'd really like to avoid
having a list of unused stuff in the state struct just because I can't use
something that works and documented just no API to actually use it. Cf.
sii3112 state after this series:
struct SiI3112PCIState {
PCIIDEState i;
SiI3112Regs regs[2];
};
vs. your standard:
struct SiI3112PCIState {
PCIIDEState i;
MemoryRegion mmio;
MemoryRegion bar0;
MemoryRegion bar1;
MemoryRegion bar2;
MemoryRegion bar3;
MemoryRegion bar4;
SiI3112Regs regs[2];
};
The latter solution is fine at least to me.
I respect your preference on how it can be done, but IMHO it is subjective,
and I tend to agree with Peter that we should try to stick with one way of
solving problems, unless very necessary.
In the last three patches the MRs do not look need dynamic deallocation to
me at all.. Introducing fully dynamic MR allocations only for those is an
overkill to me. I'm not fully convinced we need to merge 600+ new LOCs for
new APIs just for them.
It's not a new API in the sense that it's the same as existing
memory_region_init functions just a variant of it to allow using QOM to
manage the lifetime of the memory region which the current API does not
allow. Also this series does not introduce this dynamic lifetime
management, it's already implemented and documented just there's currently
no way to use it. The actual change is much less, most of the lines are
just the documentation comments. If it's a problem to increase
documentation size I could make it much shorter not duplicating a separate
doc comment for these just mention in the corresponding memory_region_init
doc comment that there's also an analogous memory_region_new variant of
it. That also shows there's no new API just two ways of using the same
API.
We have real but rare users of dynamically allocated MRs, currently AFAIU
Since it's already implemented why not use it if it makes the object state
simpler? Adding additional fields that would already be managed by QOM
seems to be unnecessary duplication and complication to me so I'd like to
avoid that and keep the subclass simple.
Regards,
BALATON Zoltan
we're leaning towards having an object wrapping the MRs needed, when one
wants to provide a complete lifecycle for the MR so the memory backing the
MRs can be freed completely separately from the parent object.
Thanks,
