Hi,
Am 04.11.2013 07:41, schrieb 赵小强:
于 09/27/2013 01:30 PM, xiaoqiang zhao 写道:
From: "xiaoqiang.zhao" <address@hidden>
Change apic and kvm/apic to use QOM interface.
Includes:
1. APICCommonState now use QOM realizefn
2. Remove DO_UPCAST() for APICCommonState
3. Use type constant
4. Change DeviceState pointers from 'd' to 'dev', sounds better?
Signed-off-by: xiaoqiang zhao <address@hidden>
---
hw/i386/kvm/apic.c | 40 ++++++++++++++++++----
hw/intc/apic.c | 70
+++++++++++++++++++++++++--------------
hw/intc/apic_common.c | 70
+++++++++++++++++++--------------------
include/hw/i386/apic_internal.h | 1 -
4 files changed, 113 insertions(+), 68 deletions(-)
[...]
ping.
patch link:
http://patchwork.ozlabs.org/patch/278481
http://patchwork.ozlabs.org/patch/278476
Thanks for reminding. A couple of issues with these two patches:
When sending more than one patch, please include a small cover letter.
That gives a better overview and gives you a central place to ping or us
to provide review comments covering both patches.
The DO_UPCAST() is a good cleanup, please put it in a separate patch to
decouple it from the rest.
As for d vs. dev, I've usually only changed it for the init/realize
functions I touched or when there was a new name conflict between
generic and specific device. But I'm fine with all your d changes. Just
please put them in a cleanup patch together with DO_UPCAST(), same for
the register_types prefix, then the actual "dangerous" patch becomes
smaller and thus easier to review for us. (Instead of 2 you'd have 4.)
The main issue though is that Anthony has requested to change the
pattern for overriding virtual methods: Can you please update the patch
so that void (*init)(APICCommonState *dev); simply gets replaced by
DeviceRealize realize;? I.e. drop the two new ...Class'es and
parent_realize, leave the call logic as it is now and just update the
function signature. Same for the ioapic.
For the second patch, I'd appreciate a second reviewer for the static
variable being refactored.
Note that you can pull the DEVICE(s) -> dev conversion into a preceding
cleanup patch by introducing a local DeviceState *dev = DEVICE(s).
Also there's some minor whitespace issues in ioapic_common_realize()
that we'll need to fix.
Thanks in advance and sorry for the delay,
Andreas