[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 00/34] QOM realize, device-only plus ISA conversio
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [RFC 00/34] QOM realize, device-only plus ISA conversion |
Date: |
Wed, 02 Jan 2013 08:48:31 -0600 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
So there are 3-4 different series in here all rolled into one. This
makes review a bit tedious.
I'd suggest:
1) Pull out all of the QOMification stuff into a separate series.
2) Pull out the style cleanups into another series
3) Pull out the introduction of realize/unrealize into a series
4) Pull out the ISA use of realize into another series.
Individually, each series looks very easy to merge. The general
approach is right along what I was hoping for.
Regards,
Anthony Liguori
Andreas Färber <address@hidden> writes:
> Hello Anthony and Paolo,
>
> As announced at KVM Forum, I have been preparing a new approach to
> incrementally
> get us Anthony's QOM realizefn concept. A previous attempt by Paolo and me had
> been turned down for making this available at Object-level and over questions
> whether BlockDriverState may need its own three-stage realization model.
>
> So here's an all-new patchset doing it at DeviceState-level only, adapting the
> signature to void realize(DeviceState *, Error **).
> CPUState is on a good way to get derived from DeviceState, so in the future
> will benefit from this approach as well.
>
> I've picked ISADevice as an example to showcase what semantic effects the
> switch to QOM realizefn has (in the hopes the number of devices would be
> small):
> As requested by Anthony for QOM CPUState reset and as seen with virtual
> methods
> in object-oriented programming languages, it becomes the derived method's
> responsibility to decide when and whether to call the parent class' method. In
> lack of real vtables this requires to save the parent's method in case we want
> to call it; classes and matching macros may need to be added for that.
> Another point to note is that we should carefully distinguish what goes into
> the qdev initfn / QOM realizefn and what can already go into a QOM initfn.
>
> This series is rebased onto Julien's ioport cleanups (touching on ISA
> devices).
>
> It starts by preparing the "realized" property, wrapping qdev's initfn (04).
> This means setting realized to true will not yet affect its children, as seen
> in Paolo's previous patches. That can be implemented later when the realizefns
> have been reviewed not to create new devices that would mess with recursive
> child realization. (In the previous series we had recursive realization but
> on my request dropped the hook-up of qdev due to the aforementioned quirks.)
>
> At that point there is a coexistence of QOM device realizefn and qdev initfn.
> For the first time now I have set out to actually eliminate some qdev initfns,
> that's what I chose ISA for. This consists of three parts, introducing
> realizefns for ISADevices (28) and recursively for PITs (31) and for PICs
> (34).
> As seen for the PCI host bridge series, I've extracted general QOM cleanups
> from the main conversion patch to arrive at a clean QOM'ish state while
> hopefully keeping the main patches readable.
>
> This series also highlights an interesting find: Beyond the to-be-solved
> CPUState, there is also a "realize" function for BusState (02), which is not
> derived from DeviceState. :-)
> With the device-centric approach taken here it would still be possible to add
> "realized" properties to other types using their own infrastructure (e.g., a
> hardcoded setter rather than a realizefn hook).
>
> Posted as an RFC to encourage bikeshedding, in particular about the type names
> and macros introduced. Adding new header files to move them out of the source
> files for, e.g., vl.c is left for a follow-up, but for instance I was unsure
> about TYPE_ISA_FDC (should this be TYPE_ISA_FLOPPY_DRIVE_CONTROLLER as with
> PCI_HOST_BRIDGE rather than PHB?), and naming of type names and functions is
> highly inconsistent (e.g., isa_vga vs. vga_isa, or pic vs. i8259).
>
> Available for viewing/testing at:
> https://github.com/afaerber/qemu-cpu/commits/realize-qdev
> git://github.com/afaerber/qemu-cpu.git realize-qdev
>
> Regards,
> Andreas
>
> Cc: Anthony Liguori <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
>
> Cc: Julien Grall <address@hidden>
> Cc: Frederic Konrad <address@hidden>
> Cc: Peter Maydell <address@hidden>
> Cc: Cornelia Huck <address@hidden>
> Cc: Kevin Wolf <address@hidden>
> Cc: Stefan Hajnoczi <address@hidden>
> Cc: Markus Armbruster <address@hidden>
> Cc: Eduardo Habkost <address@hidden>
> Cc: Igor Mammedov <address@hidden>
>
> Andreas Färber (34):
> qdev: Eliminate qdev_free() in favor of QOM
> qbus: QOM'ify qbus_realize()
> qdev: Fold state enum into bool realized
> qdev: Prepare "realized" property
> isa: Split off instance_init for ISADevice
> applesmc: QOM'ify
> cirrus_vga: QOM'ify ISA Cirrus VGA
> debugcon: QOM'ify ISA debug console
> fdc: QOM'ify ISA floppy controller
> i82374: QOM'ify
> i8259: Fix PIC_COMMON() macro
> i8259: QOM cleanups
> ide: QOM'ify ISA IDE
> m48t59: QOM'ify ISA M48T59 NVRAM
> mc146818rtc: QOM'ify
> ne2000-isa: QOM'ify
> parallel: QOM'ify
> pc: QOM'ify port 92
> pckbd: QOM'ify
> pcspk: QOM'ify
> sb16: QOM'ify
> serial: QOM'ify ISA serial
> sga: QOM'ify
> vga-isa: QOM'ify ISA VGA
> vmmouse: QOM'ify
> vmport: QOM'ify
> wdt_ib700: QOM'ify
> isa: Use realizefn for ISADevice
> i8254: QOM'ify
> kvm/i8254: QOM'ify some more
> i8254: Convert PITCommonState to QOM realizefn
> i8259: QOM'ify some more
> kvm/i8259: QOM'ify some more
> i8259: Convert PICCommonState to use QOM realizefn
>
> hw/acpi_piix4.c | 2 +-
> hw/applesmc.c | 45 +++++++++---------
> hw/cirrus_vga.c | 27 ++++++-----
> hw/debugcon.c | 34 +++++++++-----
> hw/fdc.c | 45 ++++++++++--------
> hw/fdc.h | 2 +
> hw/i82374.c | 25 +++++-----
> hw/i8254.c | 26 +++++++---
> hw/i8254.h | 7 ++-
> hw/i8254_common.c | 19 ++------
> hw/i8254_internal.h | 1 -
> hw/i8259.c | 64 +++++++++++++++++--------
> hw/i8259_common.c | 20 +++-----
> hw/i8259_internal.h | 7 +--
> hw/ide/isa.c | 55 +++++++++++++---------
> hw/isa-bus.c | 15 ++----
> hw/isa.h | 1 -
> hw/kvm/i8254.c | 59 +++++++++++++++--------
> hw/kvm/i8259.c | 38 ++++++++++++---
> hw/m48t59.c | 54 +++++++++++----------
> hw/mc146818rtc.c | 56 ++++++++++++----------
> hw/mc146818rtc.h | 2 +
> hw/ne2000-isa.c | 27 ++++++-----
> hw/parallel.c | 42 ++++++++++-------
> hw/pc.c | 26 +++++-----
> hw/pc_piix.c | 2 +-
> hw/pci-hotplug.c | 2 +-
> hw/pci_bridge.c | 2 +-
> hw/pcie.c | 2 +-
> hw/pckbd.c | 34 ++++++++------
> hw/pcspk.c | 21 +++++----
> hw/pcspk.h | 4 +-
> hw/qdev-addr.c | 2 +-
> hw/qdev-core.h | 12 ++---
> hw/qdev-monitor.c | 2 +-
> hw/qdev-properties.c | 28 +++++------
> hw/qdev.c | 128
> ++++++++++++++++++++++++++++++++++----------------
> hw/sb16.c | 35 +++++++++-----
> hw/scsi-bus.c | 4 +-
> hw/serial-isa.c | 33 +++++++------
> hw/serial-pci.c | 4 +-
> hw/serial.c | 6 +--
> hw/serial.h | 3 +-
> hw/sga.c | 19 ++++----
> hw/shpc.c | 2 +-
> hw/usb/bus.c | 7 +--
> hw/usb/dev-storage.c | 2 +-
> hw/usb/host-linux.c | 2 +-
> hw/vga-isa.c | 46 +++++++++---------
> hw/vmmouse.c | 24 +++++-----
> hw/vmport.c | 24 ++++++----
> hw/wdt_ib700.c | 22 +++++----
> hw/xen_platform.c | 2 +-
> 53 Dateien geändert, 692 Zeilen hinzugefügt(+), 481 Zeilen entfernt(-)
>
> --
> 1.7.10.4
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [RFC 00/34] QOM realize, device-only plus ISA conversion,
Anthony Liguori <=