On 03/26/2012 02:30 PM, Jan Kiszka wrote:
On 2012-03-26 19:33, Anthony Liguori wrote:
On 03/26/2012 07:20 AM, Jan Kiszka wrote:
On 2012-03-26 04:06, Wanpeng Li wrote:
From: Anthony Liguori<aligu...@us.ibm.com>
This series aggressively refactors the PC machine initialization to be more
modelled and less ad-hoc. The highlights of this series are:
1) Things like -m and -bios-name are now device model properties
2) The i440fx and piix3 are now modelled in a thorough fashion
3) Most of the chipset features of the piix3 are modelled through composition
4) i440fx_init is trivialized to creating devices and setting properties
5) convert MemoryRegion to QOM
6) convert PCI host bridge to QOM
The point (4) is the most important one. As we refactor in this fashion,
we should quickly get to the point where machine->init disappears completely in
favor of just creating a handful of devices.
The two stage initialization of QOM is important here. instance_init() is when
composed devices are created which means that after you've created a device, all
of its children are visible in the device model. This lets you set properties
of the parent and its children.
realize() (which is still called DeviceState::init today) will be called right
before the guest starts up for the first time.
While I see the value of the overall direction, I still disagree on
making internal data structures of HPET, RTC and 8254 publicly
available. That's a wrong step back. I'm sure there are smarter
solutions, alse as there were some proposals back then in the original
thread.
I'm not fully decided myself. A couple things are clear to me though:
1) We must expose type proper types in header files. We need there to be a
globally accessible RTCState type and functions that operate on it.
I'm not sure what "proper type" means in this context, but I'm quite
sure that there should be no need for poking into the internal of the
class outside of mc146818rtc.c.
It needs to be at least a forward reference. So we can avoid stuff like:
int apic_accept_pic_intr(DeviceState *s);
It should be:
int apic_accept_pic_intr(APICState *s);
So we can make use of the lovely type checking provided by the compiler to us.
We even abstracted the specifics of the
RTC away when it is embedded into a super-IO and interacts with an HPET.
If QOM requires such poking, then that requirement should be reassessed.
There are a couple of ways to make types private while still having forward
declarations. None of them are straight forward. That's why I suggest we save
this for another day.
2) We can simplify memory management by knowing the size of the type in the
header files too.
Is this more than a malloc-free pair?
Since this is an easily refactorable thing to look at later, I think we should
start with extracting the types.
My worry is that those three refactorings set bad examples for others.
So I'd like to avoid such back and forth if possible.
I'm not really worried about it. It's so easier to refactor this later. Why
rush it now?
Regards,
Anthony Liguori
Jan