From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38308) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TqPcd-0008TE-09 for qemu-devel@nongnu.org; Wed, 02 Jan 2013 09:48:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TqPcS-0006yo-Ud for qemu-devel@nongnu.org; Wed, 02 Jan 2013 09:48:46 -0500 Received: from mail-ie0-f176.google.com ([209.85.223.176]:64116) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TqPcS-0006yk-O6 for qemu-devel@nongnu.org; Wed, 02 Jan 2013 09:48:36 -0500 Received: by mail-ie0-f176.google.com with SMTP id 13so16597200iea.7 for ; Wed, 02 Jan 2013 06:48:35 -0800 (PST) From: Anthony Liguori In-Reply-To: <1353888766-6951-1-git-send-email-afaerber@suse.de> References: <1353888766-6951-1-git-send-email-afaerber@suse.de> Date: Wed, 02 Jan 2013 08:48:31 -0600 Message-ID: <877gnvd4jk.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC 00/34] QOM realize, device-only plus ISA conversion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= , qemu-devel@nongnu.org Cc: Kevin Wolf , Peter Maydell , Eduardo Habkost , Markus Armbruster , Julien Grall , Stefan Hajnoczi , Igor Mammedov , Cornelia Huck , Paolo Bonzini , Frederic Konrad 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=C3=A4rber writes: > Hello Anthony and Paolo, > > As announced at KVM Forum, I have been preparing a new approach to increm= entally > get us Anthony's QOM realizefn concept. A previous attempt by Paolo and m= e had > been turned down for making this available at Object-level and over quest= ions > whether BlockDriverState may need its own three-stage realization model. > > So here's an all-new patchset doing it at DeviceState-level only, adaptin= g the > signature to void realize(DeviceState *, Error **). > CPUState is on a good way to get derived from DeviceState, so in the futu= re > 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 m= ethods > in object-oriented programming languages, it becomes the derived method's > responsibility to decide when and whether to call the parent class' metho= d. 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 i= nto > 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 dev= ices). > > It starts by preparing the "realized" property, wrapping qdev's initfn (0= 4). > 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 reali= zefns > have been reviewed not to create new devices that would mess with recursi= ve > child realization. (In the previous series we had recursive realization b= ut > on my request dropped the hook-up of qdev due to the aforementioned quirk= s.) > > At that point there is a coexistence of QOM device realizefn and qdev ini= tfn. > For the first time now I have set out to actually eliminate some qdev ini= tfns, > 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 cleanu= ps > 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 so= urce > files for, e.g., vl.c is left for a follow-up, but for instance I was uns= ure > about TYPE_ISA_FDC (should this be TYPE_ISA_FLOPPY_DRIVE_CONTROLLER as wi= th > 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 > Cc: Paolo Bonzini > > Cc: Julien Grall > Cc: Frederic Konrad > Cc: Peter Maydell > Cc: Cornelia Huck > Cc: Kevin Wolf > Cc: Stefan Hajnoczi > Cc: Markus Armbruster > Cc: Eduardo Habkost > Cc: Igor Mammedov > > Andreas F=C3=A4rber (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=C3=A4ndert, 692 Zeilen hinzugef=C3=BCgt(+), 481 Zeilen ent= fernt(-) > > --=20 > 1.7.10.4