[Qemu-devel] [PATCH v2 00/26] use realizefn for SysBusDevice, part 1

[Qemu-devel] [PATCH v2 00/26] use realizefn for SysBusDevice, part 1

[Hu Tao]

This series updates part of devices inheriting from SysbusDevice
to use DeviceState::realize, and QOM'ify them.

These devices are default to x86_64-softmmu. I'm planning to
make patches in the same manner, that is, each series is for
devices default to each target. After all devices are converted
to realizefn, SysBusDeviceClass::init can be removed.

v2 is based on qom-next at https://github.com/afaerber/qemu-cpu/tree/qom-next

Changes:

  v2:  1. swap the order of some patches to first use type-cast macro
          then convert to realizefn.
       2. document SysBusDeviceClass::init
       3. don't touch VMStateDescription::name
       4. drop patch for ehci as the part's already been done by Andreas

Cc: Andreas Färber <afaerber@suse.de>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>

Hu Tao (26):
  sysbus: document SysBusDeviceClass about @init
  ohci: QOM'ify some more
  ohci: use realize for ohci
  i440fx-pcihost: use realize for i440fx-pcihost
  i440fx: use type-safe cast instead of directly access of parent dev
  q35: use type-safe cast instead of directly access of parent dev
  q35: use realize for q35 host
  fdc: QOM'ify some more
  fdc: use realize for fdc.
  pflash-cfi01: QOM'ify some more
  pflash_cfi01: use realize for pflash_cfi01
  pflash-cfi02: QOM'ify some more
  pflash_cfi02: use realize for pflash_cfi02
  ahci: QOM'ify some more
  ahci: use realize for ahci
  fwcfg: QOM'ify some more
  fwcfg: use realize for fwcfg
  scsi esp: QOM'ify some more
  scsi esp: use realize for scsi esp
  hpet: QOM'ify some more
  hpet: use realize for hpet
  kvmclock: QOM'ify some more
  kvmclock: use realize for kvmclock
  kvmvapic realize
  ioapic: use realize for ioapic
  isa bus: remove isabus_bridge_init since it does nothing

 hw/block/fdc.c          | 87 +++++++++++++++++++++++++++++++------------------
 hw/block/pflash_cfi01.c | 28 ++++++++--------
 hw/block/pflash_cfi02.c | 31 +++++++++---------
 hw/i386/kvm/clock.c     | 15 +++++----
 hw/i386/kvmvapic.c      | 12 +++----
 hw/ide/ahci.c           | 21 ++++++------
 hw/intc/ioapic_common.c | 12 +++----
 hw/isa/isa-bus.c        |  8 -----
 hw/nvram/fw_cfg.c       | 36 +++++++++++---------
 hw/pci-host/piix.c      | 36 ++++++++++++--------
 hw/pci-host/q35.c       | 60 +++++++++++++++++-----------------
 hw/scsi/esp.c           | 46 ++++++++++++++++----------
 hw/timer/hpet.c         | 44 +++++++++++++++----------
 hw/usb/hcd-ohci.c       | 32 +++++++++---------
 include/hw/sysbus.h     | 10 ++++++
 15 files changed, 268 insertions(+), 210 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 01/26] sysbus: document SysBusDeviceClass about @init

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 include/hw/sysbus.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 7c2e316..9614758 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -23,6 +23,16 @@ typedef struct SysBusDevice SysBusDevice;
 #define SYS_BUS_DEVICE_GET_CLASS(obj) \
      OBJECT_GET_CLASS(SysBusDeviceClass, (obj), TYPE_SYS_BUS_DEVICE)
 
+/*
+ * SysBusDeviceClass:
+ * @parent_class: This is private
+ * @init: Callback function invoked when the #DeviceState::realized property
+ * is changed to %true. Deprecated, new types inheriting directly from
+ * TYPE_SYS_BUS_DEVICE should use #DeviceClass::realize instead, new leaf
+ * types should consult their respective parent type. SysBusDeviceClass is
+ * not implementing #DeviceClass::realize, so deriving classes can simply
+ * ignore it.
+ */
 typedef struct SysBusDeviceClass {
     DeviceClass parent_class;
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 01/26] sysbus: document SysBusDeviceClass about @init

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  include/hw/sysbus.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index 7c2e316..9614758 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -23,6 +23,16 @@ typedef struct SysBusDevice SysBusDevice;
>  #define SYS_BUS_DEVICE_GET_CLASS(obj) \
>       OBJECT_GET_CLASS(SysBusDeviceClass, (obj), TYPE_SYS_BUS_DEVICE)
>  
> +/*
> + * SysBusDeviceClass:
> + * @parent_class: This is private

Because it is private, the usual way is to suppress its documentation
below via private/public markers.

> + * @init: Callback function invoked when the #DeviceState::realized property
> + * is changed to %true. Deprecated, new types inheriting directly from
> + * TYPE_SYS_BUS_DEVICE should use #DeviceClass::realize instead, new leaf
> + * types should consult their respective parent type. SysBusDeviceClass is
> + * not implementing #DeviceClass::realize, so deriving classes can simply
> + * ignore it.
> + */
>  typedef struct SysBusDeviceClass {
>      DeviceClass parent_class;
>  

Thanks for documenting this. I'm queuing it with the following gtk-doc
syntax and contential modifications, please let me know if you wish for
any wording changes.

Andreas

diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 9614758..8c17165 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -23,18 +23,20 @@ typedef struct SysBusDevice SysBusDevice;
 #define SYS_BUS_DEVICE_GET_CLASS(obj) \
      OBJECT_GET_CLASS(SysBusDeviceClass, (obj), TYPE_SYS_BUS_DEVICE)

-/*
+/**
  * SysBusDeviceClass:
- * @parent_class: This is private
- * @init: Callback function invoked when the #DeviceState::realized
property
+ * @init: Callback function invoked when the #DeviceState.realized property
  * is changed to %true. Deprecated, new types inheriting directly from
- * TYPE_SYS_BUS_DEVICE should use #DeviceClass::realize instead, new leaf
- * types should consult their respective parent type. SysBusDeviceClass is
- * not implementing #DeviceClass::realize, so deriving classes can simply
- * ignore it.
+ * TYPE_SYS_BUS_DEVICE should use #DeviceClass.realize instead, new leaf
+ * types should consult their respective parent type.
+ *
+ * SysBusDeviceClass is not overriding #DeviceClass.realize, so derived
+ * classes overriding it are not required to invoke its implementation.
  */
 typedef struct SysBusDeviceClass {
+    /*< private >*/
     DeviceClass parent_class;
+    /*< public >*/

     int (*init)(SysBusDevice *dev);
 } SysBusDeviceClass;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 01/26] sysbus: document SysBusDeviceClass about @init

[Hu Tao]

On Wed, Jul 03, 2013 at 03:19:53AM +0200, Andreas Färber wrote:
> Am 01.07.2013 12:18, schrieb Hu Tao:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  include/hw/sysbus.h | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> > index 7c2e316..9614758 100644
> > --- a/include/hw/sysbus.h
> > +++ b/include/hw/sysbus.h
> > @@ -23,6 +23,16 @@ typedef struct SysBusDevice SysBusDevice;
> >  #define SYS_BUS_DEVICE_GET_CLASS(obj) \
> >       OBJECT_GET_CLASS(SysBusDeviceClass, (obj), TYPE_SYS_BUS_DEVICE)
> >  
> > +/*
> > + * SysBusDeviceClass:
> > + * @parent_class: This is private
> 
> Because it is private, the usual way is to suppress its documentation
> below via private/public markers.
> 
> > + * @init: Callback function invoked when the #DeviceState::realized property
> > + * is changed to %true. Deprecated, new types inheriting directly from
> > + * TYPE_SYS_BUS_DEVICE should use #DeviceClass::realize instead, new leaf
> > + * types should consult their respective parent type. SysBusDeviceClass is
> > + * not implementing #DeviceClass::realize, so deriving classes can simply
> > + * ignore it.
> > + */
> >  typedef struct SysBusDeviceClass {
> >      DeviceClass parent_class;
> >  
> 
> Thanks for documenting this. I'm queuing it with the following gtk-doc
> syntax and contential modifications, please let me know if you wish for
> any wording changes.

Thanks for the changes, looks OK to me.

> 
> Andreas
> 
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index 9614758..8c17165 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -23,18 +23,20 @@ typedef struct SysBusDevice SysBusDevice;
>  #define SYS_BUS_DEVICE_GET_CLASS(obj) \
>       OBJECT_GET_CLASS(SysBusDeviceClass, (obj), TYPE_SYS_BUS_DEVICE)
> 
> -/*
> +/**
>   * SysBusDeviceClass:
> - * @parent_class: This is private
> - * @init: Callback function invoked when the #DeviceState::realized
> property
> + * @init: Callback function invoked when the #DeviceState.realized property
>   * is changed to %true. Deprecated, new types inheriting directly from
> - * TYPE_SYS_BUS_DEVICE should use #DeviceClass::realize instead, new leaf
> - * types should consult their respective parent type. SysBusDeviceClass is
> - * not implementing #DeviceClass::realize, so deriving classes can simply
> - * ignore it.
> + * TYPE_SYS_BUS_DEVICE should use #DeviceClass.realize instead, new leaf
> + * types should consult their respective parent type.
> + *
> + * SysBusDeviceClass is not overriding #DeviceClass.realize, so derived
> + * classes overriding it are not required to invoke its implementation.
>   */
>  typedef struct SysBusDeviceClass {
> +    /*< private >*/
>      DeviceClass parent_class;
> +    /*< public >*/
> 
>      int (*init)(SysBusDevice *dev);
>  } SysBusDeviceClass;
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 02/26] ohci: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/usb/hcd-ohci.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 5513924..912255d 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1842,6 +1842,8 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
     return 0;
 }
 
+#define TYPE_PCI_OHCI "pci-ohci"
+#define PCI_OHCI(obj) OBJECT_CHECK(OHCIPCIState, (obj), TYPE_PCI_OHCI)
 typedef struct {
     PCIDevice pci_dev;
     OHCIState state;
@@ -1852,23 +1854,25 @@ typedef struct {
 
 static int usb_ohci_initfn_pci(struct PCIDevice *dev)
 {
-    OHCIPCIState *ohci = DO_UPCAST(OHCIPCIState, pci_dev, dev);
+    OHCIPCIState *ohci = PCI_OHCI(dev);
 
     ohci->pci_dev.config[PCI_CLASS_PROG] = 0x10; /* OHCI */
     ohci->pci_dev.config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
 
-    if (usb_ohci_init(&ohci->state, &dev->qdev, ohci->num_ports, 0,
+    if (usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
                       ohci->masterbus, ohci->firstport,
                       pci_get_address_space(dev)) != 0) {
         return -1;
     }
     ohci->state.irq = ohci->pci_dev.irq[0];
 
-    /* TODO: avoid cast below by using dev */
-    pci_register_bar(&ohci->pci_dev, 0, 0, &ohci->state.mem);
+    pci_register_bar(dev, 0, 0, &ohci->state.mem);
     return 0;
 }
 
+#define TYPE_SYSBUS_OHCI "sysbus-ohci"
+#define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
+
 typedef struct {
     SysBusDevice busdev;
     OHCIState ohci;
@@ -1878,10 +1882,10 @@ typedef struct {
 
 static int ohci_init_pxa(SysBusDevice *dev)
 {
-    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
+    OHCISysBusState *s = SYSBUS_OHCI(dev);
 
     /* Cannot fail as we pass NULL for masterbus */
-    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
+    usb_ohci_init(&s->ohci, DEVICE(dev), s->num_ports, s->dma_offset, NULL, 0,
                   &address_space_memory);
     sysbus_init_irq(dev, &s->ohci.irq);
     sysbus_init_mmio(dev, &s->ohci.mem);
@@ -1911,7 +1915,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ohci_pci_info = {
-    .name          = "pci-ohci",
+    .name          = TYPE_PCI_OHCI,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(OHCIPCIState),
     .class_init    = ohci_pci_class_init,
@@ -1934,7 +1938,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ohci_sysbus_info = {
-    .name          = "sysbus-ohci",
+    .name          = TYPE_SYSBUS_OHCI,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(OHCISysBusState),
     .class_init    = ohci_sysbus_class_init,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 02/26] ohci: QOM'ify some more

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Introduce type constant and avoid DO_UPCAST().
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/usb/hcd-ohci.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 5513924..912255d 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -1842,6 +1842,8 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
>      return 0;
>  }
>  
> +#define TYPE_PCI_OHCI "pci-ohci"
> +#define PCI_OHCI(obj) OBJECT_CHECK(OHCIPCIState, (obj), TYPE_PCI_OHCI)

I added a while line here and converted the last three remaining uses of
pci_dev in the initfn so that both fields could be renamed to parent_obj
as proof of complete conversion.

Thanks, queued this and the following one on qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

There was an overlap with Peter's usb/* PCIDevice patch, I believe this
one is a superset of the ohci part.

Andreas

>  typedef struct {
>      PCIDevice pci_dev;
>      OHCIState state;
> @@ -1852,23 +1854,25 @@ typedef struct {
>  
>  static int usb_ohci_initfn_pci(struct PCIDevice *dev)
>  {
> -    OHCIPCIState *ohci = DO_UPCAST(OHCIPCIState, pci_dev, dev);
> +    OHCIPCIState *ohci = PCI_OHCI(dev);
>  
>      ohci->pci_dev.config[PCI_CLASS_PROG] = 0x10; /* OHCI */
>      ohci->pci_dev.config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
>  
> -    if (usb_ohci_init(&ohci->state, &dev->qdev, ohci->num_ports, 0,
> +    if (usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
>                        ohci->masterbus, ohci->firstport,
>                        pci_get_address_space(dev)) != 0) {
>          return -1;
>      }
>      ohci->state.irq = ohci->pci_dev.irq[0];
>  
> -    /* TODO: avoid cast below by using dev */
> -    pci_register_bar(&ohci->pci_dev, 0, 0, &ohci->state.mem);
> +    pci_register_bar(dev, 0, 0, &ohci->state.mem);
>      return 0;
>  }
>  
> +#define TYPE_SYSBUS_OHCI "sysbus-ohci"
> +#define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
> +
>  typedef struct {
>      SysBusDevice busdev;
>      OHCIState ohci;
> @@ -1878,10 +1882,10 @@ typedef struct {
>  
>  static int ohci_init_pxa(SysBusDevice *dev)
>  {
> -    OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
> +    OHCISysBusState *s = SYSBUS_OHCI(dev);
>  
>      /* Cannot fail as we pass NULL for masterbus */
> -    usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
> +    usb_ohci_init(&s->ohci, DEVICE(dev), s->num_ports, s->dma_offset, NULL, 0,
>                    &address_space_memory);
>      sysbus_init_irq(dev, &s->ohci.irq);
>      sysbus_init_mmio(dev, &s->ohci.mem);
> @@ -1911,7 +1915,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo ohci_pci_info = {
> -    .name          = "pci-ohci",
> +    .name          = TYPE_PCI_OHCI,
>      .parent        = TYPE_PCI_DEVICE,
>      .instance_size = sizeof(OHCIPCIState),
>      .class_init    = ohci_pci_class_init,
> @@ -1934,7 +1938,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo ohci_sysbus_info = {
> -    .name          = "sysbus-ohci",
> +    .name          = TYPE_SYSBUS_OHCI,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(OHCISysBusState),
>      .class_init    = ohci_sysbus_class_init,
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 03/26] ohci: use realize for ohci

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/usb/hcd-ohci.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 912255d..ee824ee 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1880,17 +1880,16 @@ typedef struct {
     dma_addr_t dma_offset;
 } OHCISysBusState;
 
-static int ohci_init_pxa(SysBusDevice *dev)
+static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 {
     OHCISysBusState *s = SYSBUS_OHCI(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
 
     /* Cannot fail as we pass NULL for masterbus */
-    usb_ohci_init(&s->ohci, DEVICE(dev), s->num_ports, s->dma_offset, NULL, 0,
+    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
                   &address_space_memory);
-    sysbus_init_irq(dev, &s->ohci.irq);
-    sysbus_init_mmio(dev, &s->ohci.mem);
-
-    return 0;
+    sysbus_init_irq(b, &s->ohci.irq);
+    sysbus_init_mmio(b, &s->ohci.mem);
 }
 
 static Property ohci_pci_properties[] = {
@@ -1930,9 +1929,8 @@ static Property ohci_sysbus_properties[] = {
 static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
 
-    sbc->init = ohci_init_pxa;
+    dc->realize = ohci_realize_pxa;
     dc->desc = "OHCI USB Controller";
     dc->props = ohci_sysbus_properties;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 03/26] ohci: use realize for ohci

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/usb/hcd-ohci.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index 912255d..ee824ee 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -1880,17 +1880,16 @@ typedef struct {
>      dma_addr_t dma_offset;
>  } OHCISysBusState;
>  
> -static int ohci_init_pxa(SysBusDevice *dev)
> +static void ohci_realize_pxa(DeviceState *dev, Error **errp)
>  {
>      OHCISysBusState *s = SYSBUS_OHCI(dev);
> +    SysBusDevice *b = SYS_BUS_DEVICE(dev);

I stumbled over b not being very descriptive for SysBusDevice (I
would've expected d for ...Device). Seeing that PCIDevice *pd was used
elsewhere, I will adopt sbd here, as suggested by Peter C.

https://github.com/afaerber/qemu-cpu/commits/qom-next

Regards,
Andreas

>  
>      /* Cannot fail as we pass NULL for masterbus */
> -    usb_ohci_init(&s->ohci, DEVICE(dev), s->num_ports, s->dma_offset, NULL, 0,
> +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset, NULL, 0,
>                    &address_space_memory);
> -    sysbus_init_irq(dev, &s->ohci.irq);
> -    sysbus_init_mmio(dev, &s->ohci.mem);
> -
> -    return 0;
> +    sysbus_init_irq(b, &s->ohci.irq);
> +    sysbus_init_mmio(b, &s->ohci.mem);
>  }
>  
>  static Property ohci_pci_properties[] = {
> @@ -1930,9 +1929,8 @@ static Property ohci_sysbus_properties[] = {
>  static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    sbc->init = ohci_init_pxa;
> +    dc->realize = ohci_realize_pxa;
>      dc->desc = "OHCI USB Controller";
>      dc->props = ohci_sysbus_properties;
>  }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 04/26] i440fx-pcihost: use realize for i440fx-pcihost

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/piix.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9e68c3..801341a 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -197,21 +197,26 @@ static const VMStateDescription vmstate_i440fx = {
     }
 };
 
-static int i440fx_pcihost_initfn(SysBusDevice *dev)
+static void i440fx_pcihost_initfn(Object *obj)
 {
-    PCIHostState *s = PCI_HOST_BRIDGE(dev);
+    PCIHostState *s = PCI_HOST_BRIDGE(obj);
 
     memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s,
                           "pci-conf-idx", 4);
-    sysbus_add_io(dev, 0xcf8, &s->conf_mem);
-    sysbus_init_ioports(&s->busdev, 0xcf8, 4);
-
     memory_region_init_io(&s->data_mem, &pci_host_data_le_ops, s,
                           "pci-conf-data", 4);
-    sysbus_add_io(dev, 0xcfc, &s->data_mem);
-    sysbus_init_ioports(&s->busdev, 0xcfc, 4);
+}
 
-    return 0;
+static void i440fx_pcihost_realize(DeviceState *dev, Error **errp)
+{
+    PCIHostState *s = PCI_HOST_BRIDGE(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
+
+    sysbus_add_io(b, 0xcf8, &s->conf_mem);
+    sysbus_init_ioports(b, 0xcf8, 4);
+
+    sysbus_add_io(b, 0xcfc, &s->data_mem);
+    sysbus_init_ioports(b, 0xcfc, 4);
 }
 
 static int i440fx_initfn(PCIDevice *dev)
@@ -632,9 +637,8 @@ static const TypeInfo i440fx_info = {
 static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = i440fx_pcihost_initfn;
+    dc->realize = i440fx_pcihost_realize;
     dc->fw_name = "pci";
     dc->no_user = 1;
 }
@@ -643,6 +647,7 @@ static const TypeInfo i440fx_pcihost_info = {
     .name          = "i440FX-pcihost",
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(I440FXState),
+    .instance_init = i440fx_pcihost_initfn,
     .class_init    = i440fx_pcihost_class_init,
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 04/26] i440fx-pcihost: use realize for i440fx-pcihost

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/pci-host/piix.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)

Thanks, rebased and applied to qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 05/26] i440fx: use type-safe cast instead of directly access of parent dev

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/piix.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 801341a..e2437df 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -121,22 +121,24 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 static void i440fx_update_memory_mappings(PCII440FXState *d)
 {
     int i;
+    PCIDevice *pd = PCI_DEVICE(d);
 
     memory_region_transaction_begin();
     for (i = 0; i < 13; i++) {
         pam_update(&d->pam_regions[i], i,
-                   d->dev.config[I440FX_PAM + ((i + 1) / 2)]);
+                   pd->config[I440FX_PAM + ((i + 1) / 2)]);
     }
-    smram_update(&d->smram_region, d->dev.config[I440FX_SMRAM], d->smm_enabled);
+    smram_update(&d->smram_region, pd->config[I440FX_SMRAM], d->smm_enabled);
     memory_region_transaction_commit();
 }
 
 static void i440fx_set_smm(int val, void *arg)
 {
     PCII440FXState *d = arg;
+    PCIDevice *pd = PCI_DEVICE(d);
 
     memory_region_transaction_begin();
-    smram_set_smm(&d->smm_enabled, val, d->dev.config[I440FX_SMRAM],
+    smram_set_smm(&d->smm_enabled, val, pd->config[I440FX_SMRAM],
                   &d->smram_region);
     memory_region_transaction_commit();
 }
@@ -158,9 +160,10 @@ static void i440fx_write_config(PCIDevice *dev,
 static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
 {
     PCII440FXState *d = opaque;
+    PCIDevice *pd = PCI_DEVICE(d);
     int ret, i;
 
-    ret = pci_device_load(&d->dev, f);
+    ret = pci_device_load(pd, f);
     if (ret < 0)
         return ret;
     i440fx_update_memory_mappings(d);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 05/26] i440fx: use type-safe cast instead of directly access of parent dev

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/pci-host/piix.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Thanks, converting the remaining dev uses on top and renaming.
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

@@ -194,7 +197,7 @@ static const VMStateDescription vmstate_i440fx = {
     .load_state_old = i440fx_load_old,
     .post_load = i440fx_post_load,
     .fields      = (VMStateField []) {
-        VMSTATE_PCI_DEVICE(dev, PCII440FXState),
+        VMSTATE_PCI_DEVICE(parent_obj, PCII440FXState),
         VMSTATE_UINT8(smm_enabled, PCII440FXState),
         VMSTATE_END_OF_LIST()
     }
@@ -221,7 +224,7 @@ static int i440fx_initfn(PCIDevice *dev)
 {
     PCII440FXState *d = I440FX_PCI_DEVICE(dev);

-    d->dev.config[I440FX_SMRAM] = 0x02;
+    dev->config[I440FX_SMRAM] = 0x02;

     cpu_smm_register(&i440fx_set_smm, d);
     return 0;
@@ -308,9 +311,10 @@ static PCIBus *i440fx_common_init(const char
*device_name,
     *piix3_devfn = piix3->dev.devfn;

     ram_size = ram_size / 8 / 1024 / 1024;
-    if (ram_size > 255)
+    if (ram_size > 255) {
         ram_size = 255;
-    (*pi440fx_state)->dev.config[0x57]=ram_size;
+    }
+    d->config[0x57] = ram_size;

     i440fx_update_memory_mappings(f);



-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 06/26] q35: use type-safe cast instead of directly access of parent dev

[Hu Tao]

And remove variables if possible.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/q35.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 24df6b5..61b525f 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -36,28 +36,26 @@
 
 static int q35_host_init(SysBusDevice *dev)
 {
-    PCIBus *b;
-    PCIHostState *pci = FROM_SYSBUS(PCIHostState, dev);
-    Q35PCIHost *s = Q35_HOST_DEVICE(&dev->qdev);
+    PCIHostState *pci = PCI_HOST_BRIDGE(dev);
+    Q35PCIHost *s = Q35_HOST_DEVICE(dev);
 
     memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci,
                           "pci-conf-idx", 4);
     sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
-    sysbus_init_ioports(&pci->busdev, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
+    sysbus_init_ioports(dev, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
 
     memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci,
                           "pci-conf-data", 4);
     sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
-    sysbus_init_ioports(&pci->busdev, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
+    sysbus_init_ioports(dev, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
-    if (pcie_host_init(&s->host) < 0) {
+    if (pcie_host_init(PCIE_HOST_BRIDGE(s)) < 0) {
         return -1;
     }
-    b = pci_bus_new(&s->host.pci.busdev.qdev, "pcie.0",
-                    s->mch.pci_address_space, s->mch.address_space_io,
-                    0, TYPE_PCIE_BUS);
-    s->host.pci.bus = b;
-    qdev_set_parent_bus(DEVICE(&s->mch), BUS(b));
+    pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
+                           s->mch.pci_address_space, s->mch.address_space_io,
+                           0, TYPE_PCIE_BUS);
+    qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->mch));
 
     return 0;
@@ -105,9 +103,8 @@ static const TypeInfo q35_host_info = {
 static void mch_update_pciexbar(MCHPCIState *mch)
 {
     PCIDevice *pci_dev = &mch->d;
-    BusState *bus = qdev_get_parent_bus(&pci_dev->qdev);
+    BusState *bus = qdev_get_parent_bus(DEVICE(pci_dev));
     DeviceState *qdev = bus->parent;
-    Q35PCIHost *s = Q35_HOST_DEVICE(qdev);
 
     uint64_t pciexbar;
     int enable;
@@ -139,18 +136,19 @@ static void mch_update_pciexbar(MCHPCIState *mch)
         break;
     }
     addr = pciexbar & addr_mask;
-    pcie_host_mmcfg_update(&s->host, enable, addr, length);
+    pcie_host_mmcfg_update(PCIE_HOST_BRIDGE(qdev), enable, addr, length);
 }
 
 /* PAM */
 static void mch_update_pam(MCHPCIState *mch)
 {
+    PCIDevice *pd = PCI_DEVICE(mch);
     int i;
 
     memory_region_transaction_begin();
     for (i = 0; i < 13; i++) {
         pam_update(&mch->pam_regions[i], i,
-                   mch->d.config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
+                   pd->config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
     }
     memory_region_transaction_commit();
 }
@@ -158,8 +156,10 @@ static void mch_update_pam(MCHPCIState *mch)
 /* SMRAM */
 static void mch_update_smram(MCHPCIState *mch)
 {
+    PCIDevice *pd = PCI_DEVICE(mch);
+
     memory_region_transaction_begin();
-    smram_update(&mch->smram_region, mch->d.config[MCH_HOST_BRDIGE_SMRAM],
+    smram_update(&mch->smram_region, pd->config[MCH_HOST_BRDIGE_SMRAM],
                     mch->smm_enabled);
     memory_region_transaction_commit();
 }
@@ -167,9 +167,10 @@ static void mch_update_smram(MCHPCIState *mch)
 static void mch_set_smm(int smm, void *arg)
 {
     MCHPCIState *mch = arg;
+    PCIDevice *pd = PCI_DEVICE(mch);
 
     memory_region_transaction_begin();
-    smram_set_smm(&mch->smm_enabled, smm, mch->d.config[MCH_HOST_BRDIGE_SMRAM],
+    smram_set_smm(&mch->smm_enabled, smm, pd->config[MCH_HOST_BRDIGE_SMRAM],
                     &mch->smram_region);
     memory_region_transaction_commit();
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 06/26] q35: use type-safe cast instead of directly access of parent dev

[Andreas Färber]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

Am 01.07.2013 12:18, schrieb Hu Tao:
> And remove variables if possible.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/pci-host/q35.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)

Thanks, converted the remaining ones as attached and queued on qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Do note that we will need to later touch the PCIBus again to use
pci_bus_new_inplace().

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[-- Attachment #2: q35.diff --]
[-- Type: text/x-patch, Size: 3138 bytes --]

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e4cde04..480d981 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -59,6 +59,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     const char *boot_device = args->boot_device;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
     Q35PCIHost *q35_host;
+    PCIDevice *q35_pci;
     PCIBus *host_bus;
     PCIDevice *lpc;
     BusState *idebus[MAX_SATA_PORTS];
@@ -133,7 +134,8 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     q35_host->mch.above_4g_mem_size = above_4g_mem_size;
     /* pci */
     qdev_init_nofail(DEVICE(q35_host));
-    host_bus = q35_host->host.pci.bus;
+    q35_pci = PCI_DEVICE(q35_host);
+    host_bus = q35_pci->bus;
     /* create ISA bus */
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
                                           ICH9_LPC_FUNC), true,
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index f92f661..beda3e6 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -62,7 +62,7 @@ static int q35_host_init(SysBusDevice *dev)
 }
 
 static Property mch_props[] = {
-    DEFINE_PROP_UINT64("MCFG", Q35PCIHost, host.base_addr,
+    DEFINE_PROP_UINT64("MCFG", Q35PCIHost, parent_obj.base_addr,
                         MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT),
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -102,9 +102,9 @@ static const TypeInfo q35_host_info = {
 /* PCIe MMCFG */
 static void mch_update_pciexbar(MCHPCIState *mch)
 {
-    PCIDevice *pci_dev = &mch->d;
-    BusState *bus = qdev_get_parent_bus(DEVICE(pci_dev));
-    DeviceState *qdev = bus->parent;
+    PCIDevice *pci_dev = PCI_DEVICE(mch);
+    BusState *bus = qdev_get_parent_bus(DEVICE(mch));
+    PCIExpressHost *pehb = PCIE_HOST_BRIDGE(bus->parent);
 
     uint64_t pciexbar;
     int enable;
@@ -136,7 +136,7 @@ static void mch_update_pciexbar(MCHPCIState *mch)
         break;
     }
     addr = pciexbar & addr_mask;
-    pcie_host_mmcfg_update(PCIE_HOST_BRIDGE(qdev), enable, addr, length);
+    pcie_host_mmcfg_update(pehb, enable, addr, length);
 }
 
 /* PAM */
@@ -220,7 +220,7 @@ static const VMStateDescription vmstate_mch = {
     .minimum_version_id_old = 1,
     .post_load = mch_post_load,
     .fields = (VMStateField []) {
-        VMSTATE_PCI_DEVICE(d, MCHPCIState),
+        VMSTATE_PCI_DEVICE(parent_obj, MCHPCIState),
         VMSTATE_UINT8(smm_enabled, MCHPCIState),
         VMSTATE_END_OF_LIST()
     }
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index e182c82..1a8ff83 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -43,7 +43,10 @@
      OBJECT_CHECK(MCHPCIState, (obj), TYPE_MCH_PCI_DEVICE)
 
 typedef struct MCHPCIState {
-    PCIDevice d;
+    /*< private >*/
+    PCIDevice parent_obj;
+    /*< public >*/
+
     MemoryRegion *ram_memory;
     MemoryRegion *pci_address_space;
     MemoryRegion *system_memory;
@@ -58,7 +61,10 @@ typedef struct MCHPCIState {
 } MCHPCIState;
 
 typedef struct Q35PCIHost {
-    PCIExpressHost host;
+    /*< private >*/
+    PCIExpressHost parent_obj;
+    /*< public >*/
+
     MCHPCIState mch;
 } Q35PCIHost;
 

[-- Attachment #3: q35a.diff --]
[-- Type: text/x-patch, Size: 941 bytes --]

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 480d981..bdac09e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -59,7 +59,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     const char *boot_device = args->boot_device;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
     Q35PCIHost *q35_host;
-    PCIDevice *q35_pci;
+    PCIHostState *phb;
     PCIBus *host_bus;
     PCIDevice *lpc;
     BusState *idebus[MAX_SATA_PORTS];
@@ -134,8 +134,8 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     q35_host->mch.above_4g_mem_size = above_4g_mem_size;
     /* pci */
     qdev_init_nofail(DEVICE(q35_host));
-    q35_pci = PCI_DEVICE(q35_host);
-    host_bus = q35_pci->bus;
+    phb = PCI_HOST_BRIDGE(q35_host);
+    host_bus = phb->bus;
     /* create ISA bus */
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
                                           ICH9_LPC_FUNC), true,

[Qemu-devel] [PATCH v2 07/26] q35: use realize for q35 host

[Hu Tao]

and split off memory region initialization into instance_init.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/pci-host/q35.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 61b525f..244e2f5 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -34,31 +34,27 @@
  * Q35 host
  */
 
-static int q35_host_init(SysBusDevice *dev)
+static void q35_host_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *pci = PCI_HOST_BRIDGE(dev);
     Q35PCIHost *s = Q35_HOST_DEVICE(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
 
-    memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci,
-                          "pci-conf-idx", 4);
-    sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
-    sysbus_init_ioports(dev, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
+    sysbus_add_io(b, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
+    sysbus_init_ioports(b, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
 
-    memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci,
-                          "pci-conf-data", 4);
-    sysbus_add_io(dev, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
-    sysbus_init_ioports(dev, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
+    sysbus_add_io(b, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
+    sysbus_init_ioports(b, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
     if (pcie_host_init(PCIE_HOST_BRIDGE(s)) < 0) {
-        return -1;
+        error_setg(errp, "failed to initialize pcie host");
+        return;
     }
     pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
                            s->mch.pci_address_space, s->mch.address_space_io,
                            0, TYPE_PCIE_BUS);
     qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->mch));
-
-    return 0;
 }
 
 static Property mch_props[] = {
@@ -70,9 +66,8 @@ static Property mch_props[] = {
 static void q35_host_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = q35_host_init;
+    dc->realize = q35_host_realize;
     dc->props = mch_props;
     dc->fw_name = "pci";
 }
@@ -80,6 +75,12 @@ static void q35_host_class_init(ObjectClass *klass, void *data)
 static void q35_host_initfn(Object *obj)
 {
     Q35PCIHost *s = Q35_HOST_DEVICE(obj);
+    PCIHostState *pci = PCI_HOST_BRIDGE(obj);
+
+    memory_region_init_io(&pci->conf_mem, &pci_host_conf_le_ops, pci,
+                          "pci-conf-idx", 4);
+    memory_region_init_io(&pci->data_mem, &pci_host_data_le_ops, pci,
+                          "pci-conf-data", 4);
 
     object_initialize(&s->mch, TYPE_MCH_PCI_DEVICE);
     object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 07/26] q35: use realize for q35 host

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> and split off memory region initialization into instance_init.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/pci-host/q35.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)

Thanks, rebased and applied to qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

As mentioned, pci_bus_new() in realize should be replaced with
pci_bus_new_inplace() in instance_init. However investigating that, I
noticed that either function registers VMState, which is supposed to be
done at realize time. This being a system chipset device, I think we can
get away with that (same as for PReP), but I'll do it in a follow-up so
that the blame goes on me.

I think the long-term solution would be extending our realize/unrealize
mechanisms to BusClass.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 08/26] fdc: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST(), container_of(),
and use DEVICE() to avoid accessing parent qdev directly.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/fdc.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 930f191..f8270cb 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -544,6 +544,9 @@ struct FDCtrl {
     uint8_t timer1;
 };
 
+#define TYPE_SYSBUS_FDC "sysbus-fdc"
+#define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
+
 typedef struct FDCtrlSysBus {
     SysBusDevice busdev;
     struct FDCtrl state;
@@ -773,10 +776,9 @@ static const VMStateDescription vmstate_fdc = {
 
 static void fdctrl_external_reset_sysbus(DeviceState *d)
 {
-    FDCtrlSysBus *sys = container_of(d, FDCtrlSysBus, busdev.qdev);
-    FDCtrl *s = &sys->state;
+    FDCtrlSysBus *sys = SYSBUS_FDC(d);
 
-    fdctrl_reset(s, 0);
+    fdctrl_reset(&sys->state, 0);
 }
 
 static void fdctrl_external_reset_isa(DeviceState *d)
@@ -2049,8 +2051,8 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
     DeviceState *dev;
     FDCtrlSysBus *sys;
 
-    dev = qdev_create(NULL, "sysbus-fdc");
-    sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
+    dev = qdev_create(NULL, TYPE_SYSBUS_FDC);
+    sys = SYSBUS_FDC(dev);
     fdctrl = &sys->state;
     fdctrl->dma_chann = dma_chann; /* FIXME */
     if (fds[0]) {
@@ -2060,8 +2062,8 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
         qdev_prop_set_drive_nofail(dev, "driveB", fds[1]->bdrv);
     }
     qdev_init_nofail(dev);
-    sysbus_connect_irq(&sys->busdev, 0, irq);
-    sysbus_mmio_map(&sys->busdev, 0, mmio_base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq);
+    sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, mmio_base);
 }
 
 void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
@@ -2075,9 +2077,9 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
         qdev_prop_set_drive_nofail(dev, "drive", fds[0]->bdrv);
     }
     qdev_init_nofail(dev);
-    sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
-    sysbus_connect_irq(&sys->busdev, 0, irq);
-    sysbus_mmio_map(&sys->busdev, 0, io_base);
+    sys = SYSBUS_FDC(dev);
+    sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq);
+    sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, io_base);
     *fdc_tc = qdev_get_gpio_in(dev, 0);
 }
 
@@ -2145,17 +2147,17 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
 
 static int sysbus_fdc_init1(SysBusDevice *dev)
 {
-    FDCtrlSysBus *sys = DO_UPCAST(FDCtrlSysBus, busdev, dev);
+    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
     FDCtrl *fdctrl = &sys->state;
     int ret;
 
     memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_ops, fdctrl, "fdc", 0x08);
     sysbus_init_mmio(dev, &fdctrl->iomem);
     sysbus_init_irq(dev, &fdctrl->irq);
-    qdev_init_gpio_in(&dev->qdev, fdctrl_handle_tc, 1);
+    qdev_init_gpio_in(DEVICE(dev), fdctrl_handle_tc, 1);
     fdctrl->dma_chann = -1;
 
-    qdev_set_legacy_instance_id(&dev->qdev, 0 /* io */, 2); /* FIXME */
+    qdev_set_legacy_instance_id(DEVICE(dev), 0 /* io */, 2); /* FIXME */
     ret = fdctrl_init_common(fdctrl);
 
     return ret;
@@ -2163,16 +2165,17 @@ static int sysbus_fdc_init1(SysBusDevice *dev)
 
 static int sun4m_fdc_init1(SysBusDevice *dev)
 {
-    FDCtrl *fdctrl = &(FROM_SYSBUS(FDCtrlSysBus, dev)->state);
+    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
+    FDCtrl *fdctrl = &sys->state;
 
     memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_strict_ops, fdctrl,
                           "fdctrl", 0x08);
     sysbus_init_mmio(dev, &fdctrl->iomem);
     sysbus_init_irq(dev, &fdctrl->irq);
-    qdev_init_gpio_in(&dev->qdev, fdctrl_handle_tc, 1);
+    qdev_init_gpio_in(DEVICE(dev), fdctrl_handle_tc, 1);
 
     fdctrl->sun4m = 1;
-    qdev_set_legacy_instance_id(&dev->qdev, 0 /* io */, 2); /* FIXME */
+    qdev_set_legacy_instance_id(DEVICE(dev), 0 /* io */, 2); /* FIXME */
     return fdctrl_init_common(fdctrl);
 }
 
@@ -2253,7 +2256,7 @@ static void sysbus_fdc_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo sysbus_fdc_info = {
-    .name          = "sysbus-fdc",
+    .name          = TYPE_SYSBUS_FDC,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FDCtrlSysBus),
     .class_init    = sysbus_fdc_class_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 09/26] fdc: use realize for fdc.

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/fdc.c | 62 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f8270cb..0fe0cf9 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2145,38 +2145,58 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
     add_boot_device_path(isa->bootindexB, dev, "/floppy@1");
 }
 
-static int sysbus_fdc_init1(SysBusDevice *dev)
+static void sysbus_fdc_initfn(Object *obj)
 {
-    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
+    FDCtrlSysBus *sys = SYSBUS_FDC(obj);
     FDCtrl *fdctrl = &sys->state;
-    int ret;
 
     memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_ops, fdctrl, "fdc", 0x08);
-    sysbus_init_mmio(dev, &fdctrl->iomem);
-    sysbus_init_irq(dev, &fdctrl->irq);
-    qdev_init_gpio_in(DEVICE(dev), fdctrl_handle_tc, 1);
-    fdctrl->dma_chann = -1;
+}
 
-    qdev_set_legacy_instance_id(DEVICE(dev), 0 /* io */, 2); /* FIXME */
-    ret = fdctrl_init_common(fdctrl);
+static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
+{
+    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
+    FDCtrl *fdctrl = &sys->state;
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
 
-    return ret;
+
+    sysbus_init_mmio(b, &fdctrl->iomem);
+    sysbus_init_irq(b, &fdctrl->irq);
+    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
+    fdctrl->dma_chann = -1;
+
+    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
+    if (fdctrl_init_common(fdctrl) < 0) {
+        error_setg(errp, "Floppy init failed.");
+        return;
+    }
 }
 
-static int sun4m_fdc_init1(SysBusDevice *dev)
+static void sun4m_fdc_initfn(Object *obj)
 {
-    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
+    FDCtrlSysBus *sys = SYSBUS_FDC(obj);
     FDCtrl *fdctrl = &sys->state;
 
     memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_strict_ops, fdctrl,
                           "fdctrl", 0x08);
-    sysbus_init_mmio(dev, &fdctrl->iomem);
-    sysbus_init_irq(dev, &fdctrl->irq);
-    qdev_init_gpio_in(DEVICE(dev), fdctrl_handle_tc, 1);
+}
+
+static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
+{
+    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
+    FDCtrl *fdctrl = &sys->state;
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
+
+    sysbus_init_mmio(b, &fdctrl->iomem);
+    sysbus_init_irq(b, &fdctrl->irq);
+    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
 
     fdctrl->sun4m = 1;
-    qdev_set_legacy_instance_id(DEVICE(dev), 0 /* io */, 2); /* FIXME */
-    return fdctrl_init_common(fdctrl);
+    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
+    if (fdctrl_init_common(fdctrl) < 0) {
+        error_setg(errp, "Floppy init failed.");
+        return;
+    }
 }
 
 FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
@@ -2247,9 +2267,8 @@ static Property sysbus_fdc_properties[] = {
 static void sysbus_fdc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = sysbus_fdc_init1;
+    dc->realize = sysbus_fdc_realize;
     dc->reset = fdctrl_external_reset_sysbus;
     dc->vmsd = &vmstate_sysbus_fdc;
     dc->props = sysbus_fdc_properties;
@@ -2259,6 +2278,7 @@ static const TypeInfo sysbus_fdc_info = {
     .name          = TYPE_SYSBUS_FDC,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FDCtrlSysBus),
+    .instance_init = sysbus_fdc_initfn,
     .class_init    = sysbus_fdc_class_init,
 };
 
@@ -2270,9 +2290,8 @@ static Property sun4m_fdc_properties[] = {
 static void sun4m_fdc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = sun4m_fdc_init1;
+    dc->realize = sun4m_fdc_realize;
     dc->reset = fdctrl_external_reset_sysbus;
     dc->vmsd = &vmstate_sysbus_fdc;
     dc->props = sun4m_fdc_properties;
@@ -2282,6 +2301,7 @@ static const TypeInfo sun4m_fdc_info = {
     .name          = "SUNW,fdtwo",
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FDCtrlSysBus),
+    .instance_init = sun4m_fdc_initfn,
     .class_init    = sun4m_fdc_class_init,
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 09/26] fdc: use realize for fdc.

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/block/fdc.c | 62 ++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 41 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index f8270cb..0fe0cf9 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2145,38 +2145,58 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
>      add_boot_device_path(isa->bootindexB, dev, "/floppy@1");
>  }
>  
> -static int sysbus_fdc_init1(SysBusDevice *dev)
> +static void sysbus_fdc_initfn(Object *obj)
>  {
> -    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
> +    FDCtrlSysBus *sys = SYSBUS_FDC(obj);
>      FDCtrl *fdctrl = &sys->state;
> -    int ret;
>  
>      memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_ops, fdctrl, "fdc", 0x08);
> -    sysbus_init_mmio(dev, &fdctrl->iomem);
> -    sysbus_init_irq(dev, &fdctrl->irq);
> -    qdev_init_gpio_in(DEVICE(dev), fdctrl_handle_tc, 1);
> -    fdctrl->dma_chann = -1;
> +}
>  
> -    qdev_set_legacy_instance_id(DEVICE(dev), 0 /* io */, 2); /* FIXME */
> -    ret = fdctrl_init_common(fdctrl);
> +static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
> +{
> +    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
> +    FDCtrl *fdctrl = &sys->state;
> +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
>  
> -    return ret;
> +
> +    sysbus_init_mmio(b, &fdctrl->iomem);
> +    sysbus_init_irq(b, &fdctrl->irq);
> +    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
> +    fdctrl->dma_chann = -1;
> +
> +    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
> +    if (fdctrl_init_common(fdctrl) < 0) {
> +        error_setg(errp, "Floppy init failed.");
> +        return;
> +    }
>  }
>  
> -static int sun4m_fdc_init1(SysBusDevice *dev)
> +static void sun4m_fdc_initfn(Object *obj)
>  {
> -    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
> +    FDCtrlSysBus *sys = SYSBUS_FDC(obj);
>      FDCtrl *fdctrl = &sys->state;
>  
>      memory_region_init_io(&fdctrl->iomem, &fdctrl_mem_strict_ops, fdctrl,
>                            "fdctrl", 0x08);
> -    sysbus_init_mmio(dev, &fdctrl->iomem);
> -    sysbus_init_irq(dev, &fdctrl->irq);
> -    qdev_init_gpio_in(DEVICE(dev), fdctrl_handle_tc, 1);
> +}
> +
> +static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
> +{
> +    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
> +    FDCtrl *fdctrl = &sys->state;
> +    SysBusDevice *b = SYS_BUS_DEVICE(dev);

I've renamed this variable to Peter's suggested sbd and posted a
follow-up to rename fdctrl_init_common() to fdctrl_realize_common() and
to propagate Error** from fdctrl_connect_drives() directly into the
realizefn.

Andreas

> +
> +    sysbus_init_mmio(b, &fdctrl->iomem);
> +    sysbus_init_irq(b, &fdctrl->irq);
> +    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
>  
>      fdctrl->sun4m = 1;
> -    qdev_set_legacy_instance_id(DEVICE(dev), 0 /* io */, 2); /* FIXME */
> -    return fdctrl_init_common(fdctrl);
> +    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
> +    if (fdctrl_init_common(fdctrl) < 0) {
> +        error_setg(errp, "Floppy init failed.");
> +        return;
> +    }
>  }
>  
>  FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
> @@ -2247,9 +2267,8 @@ static Property sysbus_fdc_properties[] = {
>  static void sysbus_fdc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = sysbus_fdc_init1;
> +    dc->realize = sysbus_fdc_realize;
>      dc->reset = fdctrl_external_reset_sysbus;
>      dc->vmsd = &vmstate_sysbus_fdc;
>      dc->props = sysbus_fdc_properties;
> @@ -2259,6 +2278,7 @@ static const TypeInfo sysbus_fdc_info = {
>      .name          = TYPE_SYSBUS_FDC,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(FDCtrlSysBus),
> +    .instance_init = sysbus_fdc_initfn,
>      .class_init    = sysbus_fdc_class_init,
>  };
>  
> @@ -2270,9 +2290,8 @@ static Property sun4m_fdc_properties[] = {
>  static void sun4m_fdc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = sun4m_fdc_init1;
> +    dc->realize = sun4m_fdc_realize;
>      dc->reset = fdctrl_external_reset_sysbus;
>      dc->vmsd = &vmstate_sysbus_fdc;
>      dc->props = sun4m_fdc_properties;
> @@ -2282,6 +2301,7 @@ static const TypeInfo sun4m_fdc_info = {
>      .name          = "SUNW,fdtwo",
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(FDCtrlSysBus),
> +    .instance_init = sun4m_fdc_initfn,
>      .class_init    = sun4m_fdc_class_init,
>  };
>  
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH qom-next] fdc: Improve error propagation for QOM realize

[Andreas Färber]

Rename fdctrl_init_common() to fdctrl_realize_common() and let
fdctrl_connect_drives() propagate an Error through it.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/block/fdc.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 9a983fb..d32f6ba 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1997,7 +1997,7 @@ static const BlockDevOps fdctrl_block_ops = {
 };
 
 /* Init functions */
-static int fdctrl_connect_drives(FDCtrl *fdctrl)
+static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp)
 {
     unsigned int i;
     FDrive *drive;
@@ -2008,12 +2008,12 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
 
         if (drive->bs) {
             if (bdrv_get_on_error(drive->bs, 0) != BLOCKDEV_ON_ERROR_ENOSPC) {
-                error_report("fdc doesn't support drive option werror");
-                return -1;
+                error_setg(errp, "fdc doesn't support drive option werror");
+                return;
             }
             if (bdrv_get_on_error(drive->bs, 1) != BLOCKDEV_ON_ERROR_REPORT) {
-                error_report("fdc doesn't support drive option rerror");
-                return -1;
+                error_setg(errp, "fdc doesn't support drive option rerror");
+                return;
             }
         }
 
@@ -2023,7 +2023,6 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
             bdrv_set_dev_ops(drive->bs, &fdctrl_block_ops, drive);
         }
     }
-    return 0;
 }
 
 ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
@@ -2089,7 +2088,7 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
     *fdc_tc = qdev_get_gpio_in(dev, 0);
 }
 
-static int fdctrl_init_common(FDCtrl *fdctrl)
+static void fdctrl_realize_common(FDCtrl *fdctrl, Error **errp)
 {
     int i, j;
     static int command_tables_inited = 0;
@@ -2110,15 +2109,16 @@ static int fdctrl_init_common(FDCtrl *fdctrl)
     fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);
     fdctrl->fifo_size = 512;
     fdctrl->result_timer = qemu_new_timer_ns(vm_clock,
-                                          fdctrl_result_timer, fdctrl);
+                                             fdctrl_result_timer, fdctrl);
 
     fdctrl->version = 0x90; /* Intel 82078 controller */
     fdctrl->config = FD_CONFIG_EIS | FD_CONFIG_EFIFO; /* Implicit seek, polling & FIFO enabled */
     fdctrl->num_floppies = MAX_FD;
 
-    if (fdctrl->dma_chann != -1)
+    if (fdctrl->dma_chann != -1) {
         DMA_register_channel(fdctrl->dma_chann, &fdctrl_transfer_handler, fdctrl);
-    return fdctrl_connect_drives(fdctrl);
+    }
+    fdctrl_connect_drives(fdctrl, errp);
 }
 
 static const MemoryRegionPortio fdc_portio_list[] = {
@@ -2132,7 +2132,7 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
     ISADevice *isadev = ISA_DEVICE(dev);
     FDCtrlISABus *isa = ISA_FDC(dev);
     FDCtrl *fdctrl = &isa->state;
-    int ret;
+    Error *err = NULL;
 
     isa_register_portio_list(isadev, isa->iobase, fdc_portio_list, fdctrl,
                              "fdc");
@@ -2141,9 +2141,9 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
     fdctrl->dma_chann = isa->dma;
 
     qdev_set_legacy_instance_id(dev, isa->iobase, 2);
-    ret = fdctrl_init_common(fdctrl);
-    if (ret < 0) {
-        error_setg(errp, "Floppy init failed.");
+    fdctrl_realize_common(fdctrl, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
         return;
     }
 
@@ -2165,6 +2165,7 @@ static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
     FDCtrlSysBus *sys = SYSBUS_FDC(dev);
     FDCtrl *fdctrl = &sys->state;
     SysBusDevice *b = SYS_BUS_DEVICE(dev);
+    Error *err = NULL;
 
     sysbus_init_mmio(b, &fdctrl->iomem);
     sysbus_init_irq(b, &fdctrl->irq);
@@ -2172,8 +2173,9 @@ static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
     fdctrl->dma_chann = -1;
 
     qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
-    if (fdctrl_init_common(fdctrl) < 0) {
-        error_setg(errp, "Floppy init failed.");
+    fdctrl_realize_common(fdctrl, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
         return;
     }
 }
@@ -2192,6 +2194,7 @@ static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
     FDCtrlSysBus *sys = SYSBUS_FDC(dev);
     FDCtrl *fdctrl = &sys->state;
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    Error *err = NULL;
 
     sysbus_init_mmio(sbd, &fdctrl->iomem);
     sysbus_init_irq(sbd, &fdctrl->irq);
@@ -2199,8 +2202,9 @@ static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
 
     fdctrl->sun4m = 1;
     qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
-    if (fdctrl_init_common(fdctrl) < 0) {
-        error_setg(errp, "Floppy init failed.");
+    fdctrl_realize_common(fdctrl, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
         return;
     }
 }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-next] fdc: Improve error propagation for QOM realize

[Hu Tao]

On Sun, Jul 21, 2013 at 11:31:57AM +0200, Andreas Färber wrote:
> Rename fdctrl_init_common() to fdctrl_realize_common() and let
> fdctrl_connect_drives() propagate an Error through it.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/block/fdc.c | 40 ++++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 9a983fb..d32f6ba 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -1997,7 +1997,7 @@ static const BlockDevOps fdctrl_block_ops = {
>  };
>  
>  /* Init functions */
> -static int fdctrl_connect_drives(FDCtrl *fdctrl)
> +static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp)
>  {
>      unsigned int i;
>      FDrive *drive;
> @@ -2008,12 +2008,12 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
>  
>          if (drive->bs) {
>              if (bdrv_get_on_error(drive->bs, 0) != BLOCKDEV_ON_ERROR_ENOSPC) {
> -                error_report("fdc doesn't support drive option werror");
> -                return -1;
> +                error_setg(errp, "fdc doesn't support drive option werror");
> +                return;
>              }
>              if (bdrv_get_on_error(drive->bs, 1) != BLOCKDEV_ON_ERROR_REPORT) {
> -                error_report("fdc doesn't support drive option rerror");
> -                return -1;
> +                error_setg(errp, "fdc doesn't support drive option rerror");
> +                return;
>              }
>          }
>  
> @@ -2023,7 +2023,6 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
>              bdrv_set_dev_ops(drive->bs, &fdctrl_block_ops, drive);
>          }
>      }
> -    return 0;
>  }
>  
>  ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
> @@ -2089,7 +2088,7 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
>      *fdc_tc = qdev_get_gpio_in(dev, 0);
>  }
>  
> -static int fdctrl_init_common(FDCtrl *fdctrl)
> +static void fdctrl_realize_common(FDCtrl *fdctrl, Error **errp)
>  {
>      int i, j;
>      static int command_tables_inited = 0;
> @@ -2110,15 +2109,16 @@ static int fdctrl_init_common(FDCtrl *fdctrl)
>      fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);
>      fdctrl->fifo_size = 512;
>      fdctrl->result_timer = qemu_new_timer_ns(vm_clock,
> -                                          fdctrl_result_timer, fdctrl);
> +                                             fdctrl_result_timer, fdctrl);
>  
>      fdctrl->version = 0x90; /* Intel 82078 controller */
>      fdctrl->config = FD_CONFIG_EIS | FD_CONFIG_EFIFO; /* Implicit seek, polling & FIFO enabled */
>      fdctrl->num_floppies = MAX_FD;
>  
> -    if (fdctrl->dma_chann != -1)
> +    if (fdctrl->dma_chann != -1) {
>          DMA_register_channel(fdctrl->dma_chann, &fdctrl_transfer_handler, fdctrl);
> -    return fdctrl_connect_drives(fdctrl);
> +    }
> +    fdctrl_connect_drives(fdctrl, errp);
>  }
>  
>  static const MemoryRegionPortio fdc_portio_list[] = {
> @@ -2132,7 +2132,7 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
>      ISADevice *isadev = ISA_DEVICE(dev);
>      FDCtrlISABus *isa = ISA_FDC(dev);
>      FDCtrl *fdctrl = &isa->state;
> -    int ret;
> +    Error *err = NULL;
>  
>      isa_register_portio_list(isadev, isa->iobase, fdc_portio_list, fdctrl,
>                               "fdc");
> @@ -2141,9 +2141,9 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
>      fdctrl->dma_chann = isa->dma;
>  
>      qdev_set_legacy_instance_id(dev, isa->iobase, 2);
> -    ret = fdctrl_init_common(fdctrl);
> -    if (ret < 0) {
> -        error_setg(errp, "Floppy init failed.");
> +    fdctrl_realize_common(fdctrl, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
>          return;
>      }
>  
> @@ -2165,6 +2165,7 @@ static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
>      FDCtrlSysBus *sys = SYSBUS_FDC(dev);
>      FDCtrl *fdctrl = &sys->state;
>      SysBusDevice *b = SYS_BUS_DEVICE(dev);
> +    Error *err = NULL;
>  
>      sysbus_init_mmio(b, &fdctrl->iomem);
>      sysbus_init_irq(b, &fdctrl->irq);
> @@ -2172,8 +2173,9 @@ static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
>      fdctrl->dma_chann = -1;
>  
>      qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
> -    if (fdctrl_init_common(fdctrl) < 0) {
> -        error_setg(errp, "Floppy init failed.");
> +    fdctrl_realize_common(fdctrl, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
>          return;
>      }
>  }
> @@ -2192,6 +2194,7 @@ static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
>      FDCtrlSysBus *sys = SYSBUS_FDC(dev);
>      FDCtrl *fdctrl = &sys->state;
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    Error *err = NULL;
>  
>      sysbus_init_mmio(sbd, &fdctrl->iomem);
>      sysbus_init_irq(sbd, &fdctrl->irq);
> @@ -2199,8 +2202,9 @@ static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
>  
>      fdctrl->sun4m = 1;
>      qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
> -    if (fdctrl_init_common(fdctrl) < 0) {
> -        error_setg(errp, "Floppy init failed.");
> +    fdctrl_realize_common(fdctrl, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
>          return;
>      }
>  }
> -- 
> 1.8.1.4
> 

Reviewed-by: Hu Tao <hutao@cn.fujitsu.com>

Re: [Qemu-devel] [PATCH qom-next] fdc: Improve error propagation for QOM realize

[Stefan Hajnoczi]

On Sun, Jul 21, 2013 at 11:31:57AM +0200, Andreas Färber wrote:
> Rename fdctrl_init_common() to fdctrl_realize_common() and let
> fdctrl_connect_drives() propagate an Error through it.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/block/fdc.c | 40 ++++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[Qemu-devel] [PATCH v2 10/26] pflash-cfi01: QOM'ify some more

[Hu Tao]

Introduce type constant and replace FROM_SYSBUS().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi01.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 63d7c99..7f7af06 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -60,6 +60,9 @@ do {                                                        \
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
+#define TYPE_CFI_PFLASH01 "cfi.pflash01"
+#define CFI_PFLASH01(obj) OBJECT_CHECK(pflash_t, (obj), TYPE_CFI_PFLASH01)
+
 struct pflash_t {
     SysBusDevice busdev;
     BlockDriverState *bs;
@@ -565,7 +568,7 @@ static const MemoryRegionOps pflash_cfi01_ops_le = {
 
 static int pflash_cfi01_init(SysBusDevice *dev)
 {
-    pflash_t *pfl = FROM_SYSBUS(typeof(*pfl), dev);
+    pflash_t *pfl = CFI_PFLASH01(dev);
     uint64_t total_len;
     int ret;
 
@@ -719,7 +722,7 @@ static void pflash_cfi01_class_init(ObjectClass *klass, void *data)
 
 
 static const TypeInfo pflash_cfi01_info = {
-    .name           = "cfi.pflash01",
+    .name           = TYPE_CFI_PFLASH01,
     .parent         = TYPE_SYS_BUS_DEVICE,
     .instance_size  = sizeof(struct pflash_t),
     .class_init     = pflash_cfi01_class_init,
@@ -740,10 +743,7 @@ pflash_t *pflash_cfi01_register(hwaddr base,
                                 uint16_t id0, uint16_t id1,
                                 uint16_t id2, uint16_t id3, int be)
 {
-    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
-    SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
-    pflash_t *pfl = (pflash_t *)object_dynamic_cast(OBJECT(dev),
-                                                    "cfi.pflash01");
+    DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
 
     if (bs && qdev_prop_set_drive(dev, "drive", bs)) {
         abort();
@@ -759,8 +759,8 @@ pflash_t *pflash_cfi01_register(hwaddr base,
     qdev_prop_set_string(dev, "name", name);
     qdev_init_nofail(dev);
 
-    sysbus_mmio_map(busdev, 0, base);
-    return pfl;
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    return CFI_PFLASH01(dev);
 }
 
 MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 11/26] pflash_cfi01: use realize for pflash_cfi01

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi01.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 7f7af06..06c045b 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -566,7 +566,7 @@ static const MemoryRegionOps pflash_cfi01_ops_le = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int pflash_cfi01_init(SysBusDevice *dev)
+static void pflash_cfi01_realize(DeviceState *dev, Error **err)
 {
     pflash_t *pfl = CFI_PFLASH01(dev);
     uint64_t total_len;
@@ -586,7 +586,7 @@ static int pflash_cfi01_init(SysBusDevice *dev)
         pfl->name, total_len);
     vmstate_register_ram(&pfl->mem, DEVICE(pfl));
     pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
-    sysbus_init_mmio(dev, &pfl->mem);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
 
     if (pfl->bs) {
         /* read the initial flash content */
@@ -595,7 +595,8 @@ static int pflash_cfi01_init(SysBusDevice *dev)
         if (ret < 0) {
             vmstate_unregister_ram(&pfl->mem, DEVICE(pfl));
             memory_region_destroy(&pfl->mem);
-            return 1;
+            error_setg(err, "failed to read the initial flash content");
+            return;
         }
     }
 
@@ -692,8 +693,6 @@ static int pflash_cfi01_init(SysBusDevice *dev)
     pfl->cfi_table[0x3c] = 0x00;
 
     pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
-
-    return 0;
 }
 
 static Property pflash_cfi01_properties[] = {
@@ -713,9 +712,8 @@ static Property pflash_cfi01_properties[] = {
 static void pflash_cfi01_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = pflash_cfi01_init;
+    dc->realize = pflash_cfi01_realize;
     dc->props = pflash_cfi01_properties;
     dc->vmsd = &vmstate_pflash;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 12/26] pflash-cfi02: QOM'ify some more

[Hu Tao]

Introduce type constant and replace FROM_SYSBUS().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi02.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 5f25246..f1e76f0 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -55,6 +55,9 @@ do {                                                       \
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
+#define TYPE_CFI_PFLASH02 "cfi.pflash02"
+#define CFI_PFLASH02(obj) OBJECT_CHECK(pflash_t, (obj), TYPE_CFI_PFLASH02)
+
 struct pflash_t {
     SysBusDevice busdev;
     BlockDriverState *bs;
@@ -588,7 +591,7 @@ static const MemoryRegionOps pflash_cfi02_ops_le = {
 
 static int pflash_cfi02_init(SysBusDevice *dev)
 {
-    pflash_t *pfl = FROM_SYSBUS(typeof(*pfl), dev);
+    pflash_t *pfl = CFI_PFLASH02(dev);
     uint32_t chip_len;
     int ret;
 
@@ -737,7 +740,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo pflash_cfi02_info = {
-    .name           = "cfi.pflash02",
+    .name           = TYPE_CFI_PFLASH02,
     .parent         = TYPE_SYS_BUS_DEVICE,
     .instance_size  = sizeof(struct pflash_t),
     .class_init     = pflash_cfi02_class_init,
@@ -760,10 +763,7 @@ pflash_t *pflash_cfi02_register(hwaddr base,
                                 uint16_t unlock_addr0, uint16_t unlock_addr1,
                                 int be)
 {
-    DeviceState *dev = qdev_create(NULL, "cfi.pflash02");
-    SysBusDevice *busdev = SYS_BUS_DEVICE(dev);
-    pflash_t *pfl = (pflash_t *)object_dynamic_cast(OBJECT(dev),
-                                                    "cfi.pflash02");
+    DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH02);
 
     if (bs && qdev_prop_set_drive(dev, "drive", bs)) {
         abort();
@@ -782,6 +782,6 @@ pflash_t *pflash_cfi02_register(hwaddr base,
     qdev_prop_set_string(dev, "name", name);
     qdev_init_nofail(dev);
 
-    sysbus_mmio_map(busdev, 0, base);
-    return pfl;
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    return CFI_PFLASH02(dev);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 13/26] pflash_cfi02: use realize for pflash_cfi02

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/block/pflash_cfi02.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f1e76f0..a31fd25 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -589,7 +589,7 @@ static const MemoryRegionOps pflash_cfi02_ops_le = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int pflash_cfi02_init(SysBusDevice *dev)
+static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 {
     pflash_t *pfl = CFI_PFLASH02(dev);
     uint32_t chip_len;
@@ -613,14 +613,16 @@ static int pflash_cfi02_init(SysBusDevice *dev)
         /* read the initial flash content */
         ret = bdrv_read(pfl->bs, 0, pfl->storage, chip_len >> 9);
         if (ret < 0) {
-            g_free(pfl);
-            return 1;
+            vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl));
+            memory_region_destroy(&pfl->orig_mem);
+            error_setg(errp, "failed to read the initial flash content");
+            return;
         }
     }
 
     pflash_setup_mappings(pfl);
     pfl->rom_mode = 1;
-    sysbus_init_mmio(dev, &pfl->mem);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
 
     if (pfl->bs) {
         pfl->ro = bdrv_is_read_only(pfl->bs);
@@ -709,8 +711,6 @@ static int pflash_cfi02_init(SysBusDevice *dev)
 
     pfl->cfi_table[0x3b] = 0x00;
     pfl->cfi_table[0x3c] = 0x00;
-
-    return 0;
 }
 
 static Property pflash_cfi02_properties[] = {
@@ -733,9 +733,8 @@ static Property pflash_cfi02_properties[] = {
 static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = pflash_cfi02_init;
+    dc->realize = pflash_cfi02_realize;
     dc->props = pflash_cfi02_properties;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 14/26] ahci: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/ide/ahci.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 3c86b3a..5dcd662 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1275,6 +1275,9 @@ const VMStateDescription vmstate_ahci = {
     },
 };
 
+#define TYPE_SYSBUS_AHCI "sysbus-ahci"
+#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
+
 typedef struct SysbusAHCIState {
     SysBusDevice busdev;
     AHCIState ahci;
@@ -1292,15 +1295,15 @@ static const VMStateDescription vmstate_sysbus_ahci = {
 
 static void sysbus_ahci_reset(DeviceState *dev)
 {
-    SysbusAHCIState *s = DO_UPCAST(SysbusAHCIState, busdev.qdev, dev);
+    SysbusAHCIState *s = SYSBUS_AHCI(dev);
 
     ahci_reset(&s->ahci);
 }
 
 static int sysbus_ahci_init(SysBusDevice *dev)
 {
-    SysbusAHCIState *s = FROM_SYSBUS(SysbusAHCIState, dev);
-    ahci_init(&s->ahci, &dev->qdev, NULL, s->num_ports);
+    SysbusAHCIState *s = SYSBUS_AHCI(dev);
+    ahci_init(&s->ahci, DEVICE(dev), NULL, s->num_ports);
 
     sysbus_init_mmio(dev, &s->ahci.mem);
     sysbus_init_irq(dev, &s->ahci.irq);
@@ -1324,7 +1327,7 @@ static void sysbus_ahci_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo sysbus_ahci_info = {
-    .name          = "sysbus-ahci",
+    .name          = TYPE_SYSBUS_AHCI,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysbusAHCIState),
     .class_init    = sysbus_ahci_class_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 15/26] ahci: use realize for ahci

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/ide/ahci.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 5dcd662..d92239b 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1300,14 +1300,13 @@ static void sysbus_ahci_reset(DeviceState *dev)
     ahci_reset(&s->ahci);
 }
 
-static int sysbus_ahci_init(SysBusDevice *dev)
+static void sysbus_ahci_realize(DeviceState *dev, Error **errp)
 {
     SysbusAHCIState *s = SYSBUS_AHCI(dev);
-    ahci_init(&s->ahci, DEVICE(dev), NULL, s->num_ports);
+    ahci_init(&s->ahci, dev, NULL, s->num_ports);
 
-    sysbus_init_mmio(dev, &s->ahci.mem);
-    sysbus_init_irq(dev, &s->ahci.irq);
-    return 0;
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->ahci.mem);
+    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->ahci.irq);
 }
 
 static Property sysbus_ahci_properties[] = {
@@ -1317,10 +1316,9 @@ static Property sysbus_ahci_properties[] = {
 
 static void sysbus_ahci_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    sbc->init = sysbus_ahci_init;
+    dc->realize = sysbus_ahci_realize;
     dc->vmsd = &vmstate_sysbus_ahci;
     dc->props = sysbus_ahci_properties;
     dc->reset = sysbus_ahci_reset;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 15/26] ahci: use realize for ahci

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/ide/ahci.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 5dcd662..d92239b 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1300,14 +1300,13 @@ static void sysbus_ahci_reset(DeviceState *dev)
>      ahci_reset(&s->ahci);
>  }
>  
> -static int sysbus_ahci_init(SysBusDevice *dev)
> +static void sysbus_ahci_realize(DeviceState *dev, Error **errp)
>  {
>      SysbusAHCIState *s = SYSBUS_AHCI(dev);
> -    ahci_init(&s->ahci, DEVICE(dev), NULL, s->num_ports);
> +    ahci_init(&s->ahci, dev, NULL, s->num_ports);

I prepared a local patch to rename ahci_init() to ahci_realize(), but
didn't find the time to investigate which parts could safely stay in an
ahci_init() called from instance_init, so postponing that.

Andreas

>  
> -    sysbus_init_mmio(dev, &s->ahci.mem);
> -    sysbus_init_irq(dev, &s->ahci.irq);
> -    return 0;
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->ahci.mem);
> +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->ahci.irq);
>  }
>  
>  static Property sysbus_ahci_properties[] = {
> @@ -1317,10 +1316,9 @@ static Property sysbus_ahci_properties[] = {
>  
>  static void sysbus_ahci_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    sbc->init = sysbus_ahci_init;
> +    dc->realize = sysbus_ahci_realize;
>      dc->vmsd = &vmstate_sysbus_ahci;
>      dc->props = sysbus_ahci_properties;
>      dc->reset = sysbus_ahci_reset;
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 16/26] fwcfg: QOM'ify some more

[Hu Tao]

Use type constant if possible and avoid DO_UPCAST().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/nvram/fw_cfg.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 3c255ce..c8722c2 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -35,6 +35,7 @@
 #define TYPE_FW_CFG "fw_cfg"
 #define FW_CFG_NAME "fw_cfg"
 #define FW_CFG_PATH "/machine/" FW_CFG_NAME
+#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG)
 
 typedef struct FWCfgEntry {
     uint32_t len;
@@ -326,7 +327,7 @@ static const MemoryRegionOps fw_cfg_comb_mem_ops = {
 
 static void fw_cfg_reset(DeviceState *d)
 {
-    FWCfgState *s = DO_UPCAST(FWCfgState, busdev.qdev, d);
+    FWCfgState *s = FW_CFG(d);
 
     fw_cfg_select(s, 0);
 }
@@ -489,12 +490,12 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     SysBusDevice *d;
     FWCfgState *s;
 
-    dev = qdev_create(NULL, "fw_cfg");
+    dev = qdev_create(NULL, TYPE_FW_CFG);
     qdev_prop_set_uint32(dev, "ctl_iobase", ctl_port);
     qdev_prop_set_uint32(dev, "data_iobase", data_port);
     d = SYS_BUS_DEVICE(dev);
 
-    s = DO_UPCAST(FWCfgState, busdev.qdev, dev);
+    s = FW_CFG(dev);
 
     assert(!object_resolve_path(FW_CFG_PATH, NULL));
 
@@ -524,7 +525,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
 
 static int fw_cfg_init1(SysBusDevice *dev)
 {
-    FWCfgState *s = FROM_SYSBUS(FWCfgState, dev);
+    FWCfgState *s = FW_CFG(dev);
 
     memory_region_init_io(&s->ctl_iomem, &fw_cfg_ctl_mem_ops, s,
                           "fwcfg.ctl", FW_CFG_SIZE);
@@ -557,8 +558,7 @@ static Property fw_cfg_properties[] = {
 
 FWCfgState *fw_cfg_find(void)
 {
-    return OBJECT_CHECK(FWCfgState, object_resolve_path(FW_CFG_PATH, NULL),
-                        TYPE_FW_CFG);
+    return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
 }
 
 static void fw_cfg_class_init(ObjectClass *klass, void *data)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 17/26] fwcfg: use realize for fwcfg

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/nvram/fw_cfg.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index c8722c2..656ffcc 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -523,31 +523,37 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
     return s;
 }
 
-static int fw_cfg_init1(SysBusDevice *dev)
+static void fw_cfg_initfn(Object *obj)
 {
-    FWCfgState *s = FW_CFG(dev);
+    FWCfgState *s = FW_CFG(obj);
 
     memory_region_init_io(&s->ctl_iomem, &fw_cfg_ctl_mem_ops, s,
                           "fwcfg.ctl", FW_CFG_SIZE);
-    sysbus_init_mmio(dev, &s->ctl_iomem);
     memory_region_init_io(&s->data_iomem, &fw_cfg_data_mem_ops, s,
                           "fwcfg.data", FW_CFG_DATA_SIZE);
-    sysbus_init_mmio(dev, &s->data_iomem);
     /* In case ctl and data overlap: */
     memory_region_init_io(&s->comb_iomem, &fw_cfg_comb_mem_ops, s,
                           "fwcfg", FW_CFG_SIZE);
+}
+
+static void fw_cfg_realize(DeviceState *dev, Error **errp)
+{
+    FWCfgState *s = FW_CFG(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
+
+    sysbus_init_mmio(b, &s->ctl_iomem);
+    sysbus_init_mmio(b, &s->data_iomem);
 
     if (s->ctl_iobase + 1 == s->data_iobase) {
-        sysbus_add_io(dev, s->ctl_iobase, &s->comb_iomem);
+        sysbus_add_io(b, s->ctl_iobase, &s->comb_iomem);
     } else {
         if (s->ctl_iobase) {
-            sysbus_add_io(dev, s->ctl_iobase, &s->ctl_iomem);
+            sysbus_add_io(b, s->ctl_iobase, &s->ctl_iomem);
         }
         if (s->data_iobase) {
-            sysbus_add_io(dev, s->data_iobase, &s->data_iomem);
+            sysbus_add_io(b, s->data_iobase, &s->data_iomem);
         }
     }
-    return 0;
 }
 
 static Property fw_cfg_properties[] = {
@@ -564,9 +570,8 @@ FWCfgState *fw_cfg_find(void)
 static void fw_cfg_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = fw_cfg_init1;
+    dc->realize = fw_cfg_realize;
     dc->no_user = 1;
     dc->reset = fw_cfg_reset;
     dc->vmsd = &vmstate_fw_cfg;
@@ -577,6 +582,7 @@ static const TypeInfo fw_cfg_info = {
     .name          = TYPE_FW_CFG,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(FWCfgState),
+    .instance_init = fw_cfg_initfn,
     .class_init    = fw_cfg_class_init,
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 17/26] fwcfg: use realize for fwcfg

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/nvram/fw_cfg.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index c8722c2..656ffcc 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -523,31 +523,37 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>      return s;
>  }
>  
> -static int fw_cfg_init1(SysBusDevice *dev)
> +static void fw_cfg_initfn(Object *obj)
>  {
> -    FWCfgState *s = FW_CFG(dev);
> +    FWCfgState *s = FW_CFG(obj);
>  
>      memory_region_init_io(&s->ctl_iomem, &fw_cfg_ctl_mem_ops, s,
>                            "fwcfg.ctl", FW_CFG_SIZE);
> -    sysbus_init_mmio(dev, &s->ctl_iomem);
>      memory_region_init_io(&s->data_iomem, &fw_cfg_data_mem_ops, s,
>                            "fwcfg.data", FW_CFG_DATA_SIZE);
> -    sysbus_init_mmio(dev, &s->data_iomem);
>      /* In case ctl and data overlap: */
>      memory_region_init_io(&s->comb_iomem, &fw_cfg_comb_mem_ops, s,
>                            "fwcfg", FW_CFG_SIZE);
> +}

There's no reason not to have sysbus_init_mmio() in instance_init,
changed that.

Andreas

> +
> +static void fw_cfg_realize(DeviceState *dev, Error **errp)
> +{
> +    FWCfgState *s = FW_CFG(dev);
> +    SysBusDevice *b = SYS_BUS_DEVICE(dev);
> +
> +    sysbus_init_mmio(b, &s->ctl_iomem);
> +    sysbus_init_mmio(b, &s->data_iomem);
>  
>      if (s->ctl_iobase + 1 == s->data_iobase) {
> -        sysbus_add_io(dev, s->ctl_iobase, &s->comb_iomem);
> +        sysbus_add_io(b, s->ctl_iobase, &s->comb_iomem);
>      } else {
>          if (s->ctl_iobase) {
> -            sysbus_add_io(dev, s->ctl_iobase, &s->ctl_iomem);
> +            sysbus_add_io(b, s->ctl_iobase, &s->ctl_iomem);
>          }
>          if (s->data_iobase) {
> -            sysbus_add_io(dev, s->data_iobase, &s->data_iomem);
> +            sysbus_add_io(b, s->data_iobase, &s->data_iomem);
>          }
>      }
> -    return 0;
>  }
>  
>  static Property fw_cfg_properties[] = {
> @@ -564,9 +570,8 @@ FWCfgState *fw_cfg_find(void)
>  static void fw_cfg_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = fw_cfg_init1;
> +    dc->realize = fw_cfg_realize;
>      dc->no_user = 1;
>      dc->reset = fw_cfg_reset;
>      dc->vmsd = &vmstate_fw_cfg;
> @@ -577,6 +582,7 @@ static const TypeInfo fw_cfg_info = {
>      .name          = TYPE_FW_CFG,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(FWCfgState),
> +    .instance_init = fw_cfg_initfn,
>      .class_init    = fw_cfg_class_init,
>  };
>  
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 17/26] fwcfg: use realize for fwcfg

[Hu Tao]

On Sun, Jul 21, 2013 at 11:35:55AM +0200, Andreas Färber wrote:
> Am 01.07.2013 12:18, schrieb Hu Tao:
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/nvram/fw_cfg.c | 26 ++++++++++++++++----------
> >  1 file changed, 16 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> > index c8722c2..656ffcc 100644
> > --- a/hw/nvram/fw_cfg.c
> > +++ b/hw/nvram/fw_cfg.c
> > @@ -523,31 +523,37 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
> >      return s;
> >  }
> >  
> > -static int fw_cfg_init1(SysBusDevice *dev)
> > +static void fw_cfg_initfn(Object *obj)
> >  {
> > -    FWCfgState *s = FW_CFG(dev);
> > +    FWCfgState *s = FW_CFG(obj);
> >  
> >      memory_region_init_io(&s->ctl_iomem, &fw_cfg_ctl_mem_ops, s,
> >                            "fwcfg.ctl", FW_CFG_SIZE);
> > -    sysbus_init_mmio(dev, &s->ctl_iomem);
> >      memory_region_init_io(&s->data_iomem, &fw_cfg_data_mem_ops, s,
> >                            "fwcfg.data", FW_CFG_DATA_SIZE);
> > -    sysbus_init_mmio(dev, &s->data_iomem);
> >      /* In case ctl and data overlap: */
> >      memory_region_init_io(&s->comb_iomem, &fw_cfg_comb_mem_ops, s,
> >                            "fwcfg", FW_CFG_SIZE);
> > +}
> 
> There's no reason not to have sysbus_init_mmio() in instance_init,
> changed that.

Thanks.

[Qemu-devel] [PATCH v2 18/26] scsi esp: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid DO_UPCAST().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/scsi/esp.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 0c81a50..f7d6389 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -578,6 +578,9 @@ const VMStateDescription vmstate_esp = {
     }
 };
 
+#define TYPE_ESP "esp"
+#define ESP(obj) OBJECT_CHECK(SysBusESPState, (obj), TYPE_ESP)
+
 typedef struct {
     SysBusDevice busdev;
     MemoryRegion iomem;
@@ -623,8 +626,8 @@ void esp_init(hwaddr espaddr, int it_shift,
     SysBusESPState *sysbus;
     ESPState *esp;
 
-    dev = qdev_create(NULL, "esp");
-    sysbus = DO_UPCAST(SysBusESPState, busdev.qdev, dev);
+    dev = qdev_create(NULL, TYPE_ESP);
+    sysbus = ESP(dev);
     esp = &sysbus->esp;
     esp->dma_memory_read = dma_memory_read;
     esp->dma_memory_write = dma_memory_write;
@@ -652,8 +655,7 @@ static const struct SCSIBusInfo esp_scsi_info = {
 
 static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
 {
-    DeviceState *d = opaque;
-    SysBusESPState *sysbus = container_of(d, SysBusESPState, busdev.qdev);
+    SysBusESPState *sysbus = ESP(opaque);
     ESPState *s = &sysbus->esp;
 
     switch (irq) {
@@ -668,7 +670,7 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
 
 static int sysbus_esp_init(SysBusDevice *dev)
 {
-    SysBusESPState *sysbus = FROM_SYSBUS(SysBusESPState, dev);
+    SysBusESPState *sysbus = ESP(dev);
     ESPState *s = &sysbus->esp;
 
     sysbus_init_irq(dev, &s->irq);
@@ -687,7 +689,7 @@ static int sysbus_esp_init(SysBusDevice *dev)
 
 static void sysbus_esp_hard_reset(DeviceState *dev)
 {
-    SysBusESPState *sysbus = DO_UPCAST(SysBusESPState, busdev.qdev, dev);
+    SysBusESPState *sysbus = ESP(dev);
     esp_hard_reset(&sysbus->esp);
 }
 
@@ -713,7 +715,7 @@ static void sysbus_esp_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo sysbus_esp_info = {
-    .name          = "esp",
+    .name          = TYPE_ESP,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusESPState),
     .class_init    = sysbus_esp_class_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 19/26] scsi esp: use realize for scsi esp

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/scsi/esp.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f7d6389..714d2fd 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -668,23 +668,33 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
     }
 }
 
-static int sysbus_esp_init(SysBusDevice *dev)
+static void sysbus_esp_init(Object *obj)
+{
+    SysBusESPState *sysbus = ESP(obj);
+
+    memory_region_init_io(&sysbus->iomem, &sysbus_esp_mem_ops, sysbus,
+                          "esp", ESP_REGS << sysbus->it_shift);
+}
+
+static void sysbus_esp_realize(DeviceState *dev, Error **errp)
 {
     SysBusESPState *sysbus = ESP(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
     ESPState *s = &sysbus->esp;
 
-    sysbus_init_irq(dev, &s->irq);
+    sysbus_init_irq(b, &s->irq);
     assert(sysbus->it_shift != -1);
 
     s->chip_id = TCHI_FAS100A;
-    memory_region_init_io(&sysbus->iomem, &sysbus_esp_mem_ops, sysbus,
-                          "esp", ESP_REGS << sysbus->it_shift);
-    sysbus_init_mmio(dev, &sysbus->iomem);
+    sysbus_init_mmio(b, &sysbus->iomem);
 
-    qdev_init_gpio_in(&dev->qdev, sysbus_esp_gpio_demux, 2);
+    qdev_init_gpio_in(dev, sysbus_esp_gpio_demux, 2);
 
-    scsi_bus_new(&s->bus, &dev->qdev, &esp_scsi_info, NULL);
-    return scsi_bus_legacy_handle_cmdline(&s->bus);
+    scsi_bus_new(&s->bus, dev, &esp_scsi_info, NULL);
+    if (scsi_bus_legacy_handle_cmdline(&s->bus) < 0) {
+        error_setg(errp, "handling scsi bus failed");
+        return;
+    }
 }
 
 static void sysbus_esp_hard_reset(DeviceState *dev)
@@ -707,9 +717,8 @@ static const VMStateDescription vmstate_sysbus_esp_scsi = {
 static void sysbus_esp_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = sysbus_esp_init;
+    dc->realize = sysbus_esp_realize;
     dc->reset = sysbus_esp_hard_reset;
     dc->vmsd = &vmstate_sysbus_esp_scsi;
 }
@@ -718,6 +727,7 @@ static const TypeInfo sysbus_esp_info = {
     .name          = TYPE_ESP,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SysBusESPState),
+    .instance_init = sysbus_esp_init,
     .class_init    = sysbus_esp_class_init,
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 19/26] scsi esp: use realize for scsi esp

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/scsi/esp.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index f7d6389..714d2fd 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -668,23 +668,33 @@ static void sysbus_esp_gpio_demux(void *opaque, int irq, int level)
>      }
>  }
>  
> -static int sysbus_esp_init(SysBusDevice *dev)
> +static void sysbus_esp_init(Object *obj)
> +{
> +    SysBusESPState *sysbus = ESP(obj);
> +
> +    memory_region_init_io(&sysbus->iomem, &sysbus_esp_mem_ops, sysbus,
> +                          "esp", ESP_REGS << sysbus->it_shift);

it_shift is a field set after initialization, so moving this back into
realizefn.

> +}
> +
> +static void sysbus_esp_realize(DeviceState *dev, Error **errp)
>  {
>      SysBusESPState *sysbus = ESP(dev);
> +    SysBusDevice *b = SYS_BUS_DEVICE(dev);

Please try to order variables from DeviceState to specific type, also
renaming to sbd for clarity.

>      ESPState *s = &sysbus->esp;
>  
> -    sysbus_init_irq(dev, &s->irq);
> +    sysbus_init_irq(b, &s->irq);
>      assert(sysbus->it_shift != -1);
>  
>      s->chip_id = TCHI_FAS100A;
> -    memory_region_init_io(&sysbus->iomem, &sysbus_esp_mem_ops, sysbus,
> -                          "esp", ESP_REGS << sysbus->it_shift);
> -    sysbus_init_mmio(dev, &sysbus->iomem);
> +    sysbus_init_mmio(b, &sysbus->iomem);
>  
> -    qdev_init_gpio_in(&dev->qdev, sysbus_esp_gpio_demux, 2);
> +    qdev_init_gpio_in(dev, sysbus_esp_gpio_demux, 2);
>  
> -    scsi_bus_new(&s->bus, &dev->qdev, &esp_scsi_info, NULL);
> -    return scsi_bus_legacy_handle_cmdline(&s->bus);
> +    scsi_bus_new(&s->bus, dev, &esp_scsi_info, NULL);
> +    if (scsi_bus_legacy_handle_cmdline(&s->bus) < 0) {
> +        error_setg(errp, "handling scsi bus failed");
> +        return;
> +    }

scsi_bus_legacy_handle_cmdline() is calling for better error
propagation, will post a follow-up.

Andreas

>  }
>  
>  static void sysbus_esp_hard_reset(DeviceState *dev)
> @@ -707,9 +717,8 @@ static const VMStateDescription vmstate_sysbus_esp_scsi = {
>  static void sysbus_esp_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>  
> -    k->init = sysbus_esp_init;
> +    dc->realize = sysbus_esp_realize;
>      dc->reset = sysbus_esp_hard_reset;
>      dc->vmsd = &vmstate_sysbus_esp_scsi;
>  }
> @@ -718,6 +727,7 @@ static const TypeInfo sysbus_esp_info = {
>      .name          = TYPE_ESP,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(SysBusESPState),
> +    .instance_init = sysbus_esp_init,
>      .class_init    = sysbus_esp_class_init,
>  };
>  
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH qom-next] scsi: Improve error propagation for scsi_bus_legacy_handle_cmdline()

[Andreas Färber]

Let scsi_bus_legacy_add_drive() and scsi_bus_legacy_handle_cmdline()
return an Error**. Prepare qdev initfns for QOM realize error model.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/pci/pci-hotplug-old.c |  2 +-
 hw/scsi/esp-pci.c        |  7 ++++++-
 hw/scsi/esp.c            |  6 ++++--
 hw/scsi/lsi53c895a.c     |  7 ++++++-
 hw/scsi/megasas.c        |  7 ++++++-
 hw/scsi/scsi-bus.c       | 22 +++++++++++++++-------
 hw/scsi/spapr_vscsi.c    |  7 ++++++-
 hw/scsi/virtio-scsi.c    |  7 ++++++-
 hw/usb/dev-storage.c     |  4 +++-
 include/hw/scsi/scsi.h   |  4 ++--
 10 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
index 8077289..619fe47 100644
--- a/hw/pci/pci-hotplug-old.c
+++ b/hw/pci/pci-hotplug-old.c
@@ -127,7 +127,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
     dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
     dinfo->bus = scsibus->busnr;
     scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
-                                        false, -1, NULL);
+                                        false, -1, NULL, NULL);
     if (!scsidev) {
         return -1;
     }
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 6cdfd53..2ac21d4 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -346,6 +346,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
     DeviceState *d = DEVICE(dev);
     ESPState *s = &pci->esp;
     uint8_t *pci_conf;
+    Error *err = NULL;
 
     pci_conf = dev->config;
 
@@ -364,7 +365,11 @@ static int esp_pci_scsi_init(PCIDevice *dev)
 
     scsi_bus_new(&s->bus, d, &esp_pci_scsi_info, NULL);
     if (!d->hotplugged) {
-        return scsi_bus_legacy_handle_cmdline(&s->bus);
+        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
+        if (err != NULL) {
+            error_free(err);
+            return -1;
+        }
     }
     return 0;
 }
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index c24b632..94639b8 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -676,6 +676,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error **errp)
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     SysBusESPState *sysbus = ESP(dev);
     ESPState *s = &sysbus->esp;
+    Error *err = NULL;
 
     sysbus_init_irq(sbd, &s->irq);
     assert(sysbus->it_shift != -1);
@@ -688,8 +689,9 @@ static void sysbus_esp_realize(DeviceState *dev, Error **errp)
     qdev_init_gpio_in(dev, sysbus_esp_gpio_demux, 2);
 
     scsi_bus_new(&s->bus, dev, &esp_scsi_info, NULL);
-    if (scsi_bus_legacy_handle_cmdline(&s->bus) < 0) {
-        error_setg(errp, "Handling legacy SCSI command line failed");
+    scsi_bus_legacy_handle_cmdline(&s->bus, &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
         return;
     }
 }
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index e11224f..776e31a 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2096,6 +2096,7 @@ static int lsi_scsi_init(PCIDevice *dev)
     LSIState *s = LSI53C895A(dev);
     DeviceState *d = DEVICE(dev);
     uint8_t *pci_conf;
+    Error *err = NULL;
 
     pci_conf = dev->config;
 
@@ -2118,7 +2119,11 @@ static int lsi_scsi_init(PCIDevice *dev)
 
     scsi_bus_new(&s->bus, d, &lsi_scsi_info, NULL);
     if (!d->hotplugged) {
-        return scsi_bus_legacy_handle_cmdline(&s->bus);
+        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
+        if (err != NULL) {
+            error_free(err);
+            return -1;
+        }
     }
     return 0;
 }
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 2ae4346..eb52164 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2102,6 +2102,7 @@ static int megasas_scsi_init(PCIDevice *dev)
     MegasasState *s = MEGASAS(dev);
     uint8_t *pci_conf;
     int i, bar_type;
+    Error *err = NULL;
 
     pci_conf = dev->config;
 
@@ -2172,7 +2173,11 @@ static int megasas_scsi_init(PCIDevice *dev)
 
     scsi_bus_new(&s->bus, DEVICE(dev), &megasas_scsi_info, NULL);
     if (!d->hotplugged) {
-        return scsi_bus_legacy_handle_cmdline(&s->bus);
+        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
+        if (err != NULL) {
+            error_free(err);
+            return -1;
+        }
     }
     return 0;
 }
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index a92b7c1..b5a863a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -209,10 +209,11 @@ static int scsi_qdev_exit(DeviceState *qdev)
 /* handle legacy '-drive if=scsi,...' cmd line args */
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
                                       int unit, bool removable, int bootindex,
-                                      const char *serial)
+                                      const char *serial, Error **errp)
 {
     const char *driver;
     DeviceState *dev;
+    Error *err = NULL;
 
     driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk";
     dev = qdev_create(&bus->qbus, driver);
@@ -227,19 +228,25 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
         qdev_prop_set_string(dev, "serial", serial);
     }
     if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
+        error_setg(errp, "Setting drive property failed");
         qdev_free(dev);
         return NULL;
     }
-    if (qdev_init(dev) < 0)
+    object_property_set_bool(OBJECT(dev), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        qdev_free(dev);
         return NULL;
+    }
     return SCSI_DEVICE(dev);
 }
 
-int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
+void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, Error **errp)
 {
     Location loc;
     DriveInfo *dinfo;
-    int res = 0, unit;
+    int unit;
+    Error *err = NULL;
 
     loc_push_none(&loc);
     for (unit = 0; unit <= bus->info->max_target; unit++) {
@@ -248,13 +255,14 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
             continue;
         }
         qemu_opts_loc_restore(dinfo->opts);
-        if (!scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit, false, -1, NULL)) {
-            res = -1;
+        scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit, false, -1, NULL,
+                                  &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
             break;
         }
     }
     loc_pop(&loc);
-    return res;
 }
 
 static int32_t scsi_invalid_field(SCSIRequest *req, uint8_t *buf)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index e8978bf..55b44b9 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -912,12 +912,17 @@ static void spapr_vscsi_reset(VIOsPAPRDevice *dev)
 static int spapr_vscsi_init(VIOsPAPRDevice *dev)
 {
     VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(dev);
+    Error *err = NULL;
 
     dev->crq.SendFunc = vscsi_do_crq;
 
     scsi_bus_new(&s->bus, &dev->qdev, &vscsi_scsi_info, NULL);
     if (!dev->qdev.hotplugged) {
-        scsi_bus_legacy_handle_cmdline(&s->bus);
+        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
+        if (err != NULL) {
+            error_free(err);
+            return -1;
+        }
     }
 
     return 0;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 712f0ad..42cb73b 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -619,6 +619,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
     VirtIOSCSI *s = VIRTIO_SCSI(vdev);
     static int virtio_scsi_id;
+    Error *err = NULL;
     int ret;
 
     ret = virtio_scsi_common_init(vs);
@@ -629,7 +630,11 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
     scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vdev->bus_name);
 
     if (!qdev->hotplugged) {
-        scsi_bus_legacy_handle_cmdline(&s->bus);
+        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
+        if (err != NULL) {
+            error_free(err);
+            return -1;
+        }
     }
 
     register_savevm(qdev, "virtio-scsi", virtio_scsi_id++, 1,
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index fe914ab..1954811 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -595,6 +595,7 @@ static int usb_msd_initfn_storage(USBDevice *dev)
     MSDState *s = DO_UPCAST(MSDState, dev, dev);
     BlockDriverState *bs = s->conf.bs;
     SCSIDevice *scsi_dev;
+    Error *err = NULL;
 
     if (!bs) {
         error_report("drive property not set");
@@ -619,7 +620,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
     usb_desc_init(dev);
     scsi_bus_new(&s->bus, &s->dev.qdev, &usb_msd_scsi_info_storage, NULL);
     scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
-                                            s->conf.bootindex, dev->serial);
+                                         s->conf.bootindex, dev->serial,
+                                         &err);
     if (!scsi_dev) {
         return -1;
     }
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 9786e00..8786531 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -162,8 +162,8 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
 
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
                                       int unit, bool removable, int bootindex,
-                                      const char *serial);
-int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
+                                      const char *serial, Error **errp);
+void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, Error **errp);
 
 /*
  * Predefined sense codes
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-next] scsi: Improve error propagation for scsi_bus_legacy_handle_cmdline()

[Hu Tao]

On Sun, Jul 21, 2013 at 12:30:54PM +0200, Andreas Färber wrote:
> Let scsi_bus_legacy_add_drive() and scsi_bus_legacy_handle_cmdline()
> return an Error**. Prepare qdev initfns for QOM realize error model.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/pci/pci-hotplug-old.c |  2 +-
>  hw/scsi/esp-pci.c        |  7 ++++++-
>  hw/scsi/esp.c            |  6 ++++--
>  hw/scsi/lsi53c895a.c     |  7 ++++++-
>  hw/scsi/megasas.c        |  7 ++++++-
>  hw/scsi/scsi-bus.c       | 22 +++++++++++++++-------
>  hw/scsi/spapr_vscsi.c    |  7 ++++++-
>  hw/scsi/virtio-scsi.c    |  7 ++++++-
>  hw/usb/dev-storage.c     |  4 +++-
>  include/hw/scsi/scsi.h   |  4 ++--
>  10 files changed, 55 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> index 8077289..619fe47 100644
> --- a/hw/pci/pci-hotplug-old.c
> +++ b/hw/pci/pci-hotplug-old.c
> @@ -127,7 +127,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
>      dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
>      dinfo->bus = scsibus->busnr;
>      scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
> -                                        false, -1, NULL);
> +                                        false, -1, NULL, NULL);
>      if (!scsidev) {
>          return -1;
>      }
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index 6cdfd53..2ac21d4 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -346,6 +346,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
>      DeviceState *d = DEVICE(dev);
>      ESPState *s = &pci->esp;
>      uint8_t *pci_conf;
> +    Error *err = NULL;
>  
>      pci_conf = dev->config;
>  
> @@ -364,7 +365,11 @@ static int esp_pci_scsi_init(PCIDevice *dev)
>  
>      scsi_bus_new(&s->bus, d, &esp_pci_scsi_info, NULL);
>      if (!d->hotplugged) {
> -        return scsi_bus_legacy_handle_cmdline(&s->bus);
> +        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
> +        if (err != NULL) {

           if (error_is_set(&err))

> +            error_free(err);
> +            return -1;
> +        }
>      }
>      return 0;
>  }
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index c24b632..94639b8 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -676,6 +676,7 @@ static void sysbus_esp_realize(DeviceState *dev, Error **errp)
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      SysBusESPState *sysbus = ESP(dev);
>      ESPState *s = &sysbus->esp;
> +    Error *err = NULL;
>  
>      sysbus_init_irq(sbd, &s->irq);
>      assert(sysbus->it_shift != -1);
> @@ -688,8 +689,9 @@ static void sysbus_esp_realize(DeviceState *dev, Error **errp)
>      qdev_init_gpio_in(dev, sysbus_esp_gpio_demux, 2);
>  
>      scsi_bus_new(&s->bus, dev, &esp_scsi_info, NULL);
> -    if (scsi_bus_legacy_handle_cmdline(&s->bus) < 0) {
> -        error_setg(errp, "Handling legacy SCSI command line failed");
> +    scsi_bus_legacy_handle_cmdline(&s->bus, &err);
> +    if (err != NULL) {

       if (error_is_set(&err))

> +        error_propagate(errp, err);
>          return;
>      }
>  }
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index e11224f..776e31a 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -2096,6 +2096,7 @@ static int lsi_scsi_init(PCIDevice *dev)
>      LSIState *s = LSI53C895A(dev);
>      DeviceState *d = DEVICE(dev);
>      uint8_t *pci_conf;
> +    Error *err = NULL;
>  
>      pci_conf = dev->config;
>  
> @@ -2118,7 +2119,11 @@ static int lsi_scsi_init(PCIDevice *dev)
>  
>      scsi_bus_new(&s->bus, d, &lsi_scsi_info, NULL);
>      if (!d->hotplugged) {
> -        return scsi_bus_legacy_handle_cmdline(&s->bus);
> +        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
> +        if (err != NULL) {

           if (error_is_set(&err))

> +            error_free(err);
> +            return -1;
> +        }
>      }
>      return 0;
>  }
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index 2ae4346..eb52164 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2102,6 +2102,7 @@ static int megasas_scsi_init(PCIDevice *dev)
>      MegasasState *s = MEGASAS(dev);
>      uint8_t *pci_conf;
>      int i, bar_type;
> +    Error *err = NULL;
>  
>      pci_conf = dev->config;
>  
> @@ -2172,7 +2173,11 @@ static int megasas_scsi_init(PCIDevice *dev)
>  
>      scsi_bus_new(&s->bus, DEVICE(dev), &megasas_scsi_info, NULL);
>      if (!d->hotplugged) {
> -        return scsi_bus_legacy_handle_cmdline(&s->bus);
> +        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
> +        if (err != NULL) {

           if (error_is_set(&err))

> +            error_free(err);
> +            return -1;
> +        }
>      }
>      return 0;
>  }
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index a92b7c1..b5a863a 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -209,10 +209,11 @@ static int scsi_qdev_exit(DeviceState *qdev)
>  /* handle legacy '-drive if=scsi,...' cmd line args */
>  SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
>                                        int unit, bool removable, int bootindex,
> -                                      const char *serial)
> +                                      const char *serial, Error **errp)
>  {
>      const char *driver;
>      DeviceState *dev;
> +    Error *err = NULL;
>  
>      driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk";
>      dev = qdev_create(&bus->qbus, driver);
> @@ -227,19 +228,25 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
>          qdev_prop_set_string(dev, "serial", serial);
>      }
>      if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
> +        error_setg(errp, "Setting drive property failed");
>          qdev_free(dev);
>          return NULL;
>      }
> -    if (qdev_init(dev) < 0)
> +    object_property_set_bool(OBJECT(dev), true, "realized", &err);
> +    if (err != NULL) {

       if (error_is_set(&err))

> +        error_propagate(errp, err);
> +        qdev_free(dev);
>          return NULL;
> +    }
>      return SCSI_DEVICE(dev);
>  }
>  
> -int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
> +void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, Error **errp)
>  {
>      Location loc;
>      DriveInfo *dinfo;
> -    int res = 0, unit;
> +    int unit;
> +    Error *err = NULL;
>  
>      loc_push_none(&loc);
>      for (unit = 0; unit <= bus->info->max_target; unit++) {
> @@ -248,13 +255,14 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
>              continue;
>          }
>          qemu_opts_loc_restore(dinfo->opts);
> -        if (!scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit, false, -1, NULL)) {
> -            res = -1;
> +        scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit, false, -1, NULL,
> +                                  &err);
> +        if (err != NULL) {

           if (error_is_set(&err))

> +            error_propagate(errp, err);
>              break;
>          }
>      }
>      loc_pop(&loc);
> -    return res;
>  }
>  
>  static int32_t scsi_invalid_field(SCSIRequest *req, uint8_t *buf)
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index e8978bf..55b44b9 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -912,12 +912,17 @@ static void spapr_vscsi_reset(VIOsPAPRDevice *dev)
>  static int spapr_vscsi_init(VIOsPAPRDevice *dev)
>  {
>      VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(dev);
> +    Error *err = NULL;
>  
>      dev->crq.SendFunc = vscsi_do_crq;
>  
>      scsi_bus_new(&s->bus, &dev->qdev, &vscsi_scsi_info, NULL);
>      if (!dev->qdev.hotplugged) {
> -        scsi_bus_legacy_handle_cmdline(&s->bus);
> +        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
> +        if (err != NULL) {

           if (error_is_set(&err))

> +            error_free(err);
> +            return -1;
> +        }
>      }
>  
>      return 0;
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 712f0ad..42cb73b 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -619,6 +619,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
>      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>      VirtIOSCSI *s = VIRTIO_SCSI(vdev);
>      static int virtio_scsi_id;
> +    Error *err = NULL;
>      int ret;
>  
>      ret = virtio_scsi_common_init(vs);
> @@ -629,7 +630,11 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
>      scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vdev->bus_name);
>  
>      if (!qdev->hotplugged) {
> -        scsi_bus_legacy_handle_cmdline(&s->bus);
> +        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
> +        if (err != NULL) {

           if (error_is_set(&err))

> +            error_free(err);
> +            return -1;
> +        }
>      }
>  
>      register_savevm(qdev, "virtio-scsi", virtio_scsi_id++, 1,
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index fe914ab..1954811 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -595,6 +595,7 @@ static int usb_msd_initfn_storage(USBDevice *dev)
>      MSDState *s = DO_UPCAST(MSDState, dev, dev);
>      BlockDriverState *bs = s->conf.bs;
>      SCSIDevice *scsi_dev;
> +    Error *err = NULL;
>  
>      if (!bs) {
>          error_report("drive property not set");
> @@ -619,7 +620,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
>      usb_desc_init(dev);
>      scsi_bus_new(&s->bus, &s->dev.qdev, &usb_msd_scsi_info_storage, NULL);
>      scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
> -                                            s->conf.bootindex, dev->serial);
> +                                         s->conf.bootindex, dev->serial,
> +                                         &err);
>      if (!scsi_dev) {
>          return -1;
>      }
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index 9786e00..8786531 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -162,8 +162,8 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
>  
>  SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
>                                        int unit, bool removable, int bootindex,
> -                                      const char *serial);
> -int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
> +                                      const char *serial, Error **errp);
> +void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, Error **errp);
>  
>  /*
>   * Predefined sense codes
> -- 
> 1.8.1.4
> 
> 

Re: [Qemu-devel] [PATCH qom-next] scsi: Improve error propagation for scsi_bus_legacy_handle_cmdline()

[Andreas Färber]

Am 22.07.2013 11:16, schrieb Hu Tao:
> On Sun, Jul 21, 2013 at 12:30:54PM +0200, Andreas Färber wrote:
>> Let scsi_bus_legacy_add_drive() and scsi_bus_legacy_handle_cmdline()
>> return an Error**. Prepare qdev initfns for QOM realize error model.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  hw/pci/pci-hotplug-old.c |  2 +-
>>  hw/scsi/esp-pci.c        |  7 ++++++-
>>  hw/scsi/esp.c            |  6 ++++--
>>  hw/scsi/lsi53c895a.c     |  7 ++++++-
>>  hw/scsi/megasas.c        |  7 ++++++-
>>  hw/scsi/scsi-bus.c       | 22 +++++++++++++++-------
>>  hw/scsi/spapr_vscsi.c    |  7 ++++++-
>>  hw/scsi/virtio-scsi.c    |  7 ++++++-
>>  hw/usb/dev-storage.c     |  4 +++-
>>  include/hw/scsi/scsi.h   |  4 ++--
>>  10 files changed, 55 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
>> index 8077289..619fe47 100644
>> --- a/hw/pci/pci-hotplug-old.c
>> +++ b/hw/pci/pci-hotplug-old.c
>> @@ -127,7 +127,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
>>      dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
>>      dinfo->bus = scsibus->busnr;
>>      scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
>> -                                        false, -1, NULL);
>> +                                        false, -1, NULL, NULL);
>>      if (!scsidev) {
>>          return -1;
>>      }
>> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
>> index 6cdfd53..2ac21d4 100644
>> --- a/hw/scsi/esp-pci.c
>> +++ b/hw/scsi/esp-pci.c
>> @@ -346,6 +346,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
>>      DeviceState *d = DEVICE(dev);
>>      ESPState *s = &pci->esp;
>>      uint8_t *pci_conf;
>> +    Error *err = NULL;
>>  
>>      pci_conf = dev->config;
>>  
>> @@ -364,7 +365,11 @@ static int esp_pci_scsi_init(PCIDevice *dev)
>>  
>>      scsi_bus_new(&s->bus, d, &esp_pci_scsi_info, NULL);
>>      if (!d->hotplugged) {
>> -        return scsi_bus_legacy_handle_cmdline(&s->bus);
>> +        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
>> +        if (err != NULL) {
> 
>            if (error_is_set(&err))

I was specifically instructed not to use error_is_set() on local err
variables. It is intended for potentially-NULL errp only, which &err
will never be.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 20/26] hpet: QOM'ify some more

[Hu Tao]

Introduce type constant, avoid FROM_SYSBUS().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/timer/hpet.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 95dd01d..af80e63 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -42,6 +42,9 @@
 
 #define HPET_MSI_SUPPORT        0
 
+#define TYPE_HPET "hpet"
+#define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET)
+
 struct HPETState;
 typedef struct HPETTimer {  /* timers */
     uint8_t tn;             /*timer number*/
@@ -634,7 +637,8 @@ static const MemoryRegionOps hpet_ram_ops = {
 
 static void hpet_reset(DeviceState *d)
 {
-    HPETState *s = FROM_SYSBUS(HPETState, SYS_BUS_DEVICE(d));
+    HPETState *s = HPET(d);
+    SysBusDevice *bus = SYS_BUS_DEVICE(d);
     int i;
 
     for (i = 0; i < s->num_timers; i++) {
@@ -657,7 +661,7 @@ static void hpet_reset(DeviceState *d)
     s->hpet_offset = 0ULL;
     s->config = 0ULL;
     hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
-    hpet_cfg.hpet[s->hpet_id].address = SYS_BUS_DEVICE(d)->mmio[0].addr;
+    hpet_cfg.hpet[s->hpet_id].address = bus->mmio[0].addr;
 
     /* to document that the RTC lowers its output on reset as well */
     s->rtc_irq_level = 0;
@@ -665,7 +669,7 @@ static void hpet_reset(DeviceState *d)
 
 static void hpet_handle_legacy_irq(void *opaque, int n, int level)
 {
-    HPETState *s = FROM_SYSBUS(HPETState, opaque);
+    HPETState *s = HPET(opaque);
 
     if (n == HPET_LEGACY_PIT_INT) {
         if (!hpet_in_legacy_mode(s)) {
@@ -681,7 +685,7 @@ static void hpet_handle_legacy_irq(void *opaque, int n, int level)
 
 static int hpet_init(SysBusDevice *dev)
 {
-    HPETState *s = FROM_SYSBUS(HPETState, dev);
+    HPETState *s = HPET(dev);
     int i;
     HPETTimer *timer;
 
@@ -746,7 +750,7 @@ static void hpet_device_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo hpet_device_info = {
-    .name          = "hpet",
+    .name          = TYPE_HPET,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(HPETState),
     .class_init    = hpet_device_class_init,
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 21/26] hpet: use realize for hpet

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/timer/hpet.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index af80e63..731fb92 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -683,7 +683,16 @@ static void hpet_handle_legacy_irq(void *opaque, int n, int level)
     }
 }
 
-static int hpet_init(SysBusDevice *dev)
+static void hpet_init(Object *obj)
+{
+    HPETState *s = HPET(obj);
+
+    /* HPET Area */
+    memory_region_init_io(&s->iomem, &hpet_ram_ops, s, "hpet", 0x400);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static void hpet_realize(DeviceState *dev, Error **errp)
 {
     HPETState *s = HPET(dev);
     int i;
@@ -695,14 +704,14 @@ static int hpet_init(SysBusDevice *dev)
     }
 
     if (hpet_cfg.count == 8) {
-        fprintf(stderr, "Only 8 instances of HPET is allowed\n");
-        return -1;
+        error_setg(errp, "Only 8 instances of HPET is allowed");
+        return;
     }
 
     s->hpet_id = hpet_cfg.count++;
 
     for (i = 0; i < HPET_NUM_IRQ_ROUTES; i++) {
-        sysbus_init_irq(dev, &s->irqs[i]);
+        sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irqs[i]);
     }
 
     if (s->num_timers < HPET_MIN_TIMERS) {
@@ -722,13 +731,8 @@ static int hpet_init(SysBusDevice *dev)
     s->capability |= (s->num_timers - 1) << HPET_ID_NUM_TIM_SHIFT;
     s->capability |= ((HPET_CLK_PERIOD) << 32);
 
-    qdev_init_gpio_in(&dev->qdev, hpet_handle_legacy_irq, 2);
-    qdev_init_gpio_out(&dev->qdev, &s->pit_enabled, 1);
-
-    /* HPET Area */
-    memory_region_init_io(&s->iomem, &hpet_ram_ops, s, "hpet", 0x400);
-    sysbus_init_mmio(dev, &s->iomem);
-    return 0;
+    qdev_init_gpio_in(dev, hpet_handle_legacy_irq, 2);
+    qdev_init_gpio_out(dev, &s->pit_enabled, 1);
 }
 
 static Property hpet_device_properties[] = {
@@ -740,9 +744,8 @@ static Property hpet_device_properties[] = {
 static void hpet_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = hpet_init;
+    dc->realize = hpet_realize;
     dc->no_user = 1;
     dc->reset = hpet_reset;
     dc->vmsd = &vmstate_hpet;
@@ -753,6 +756,7 @@ static const TypeInfo hpet_device_info = {
     .name          = TYPE_HPET,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(HPETState),
+    .instance_init = hpet_init,
     .class_init    = hpet_device_class_init,
 };
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 22/26] kvmclock: QOM'ify some more

[Hu Tao]

Introduce type constant and avoid FROM_SYSBUS().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/kvm/clock.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 87d4d0f..fcdcbca 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -22,6 +22,9 @@
 #include <linux/kvm.h>
 #include <linux/kvm_para.h>
 
+#define TYPE_KVM_CLOCK "kvmclock"
+#define KVM_CLOCK(obj) OBJECT_CHECK(KVMClockState, (obj), TYPE_KVM_CLOCK)
+
 typedef struct KVMClockState {
     SysBusDevice busdev;
     uint64_t clock;
@@ -89,7 +92,7 @@ static void kvmclock_vm_state_change(void *opaque, int running,
 
 static int kvmclock_init(SysBusDevice *dev)
 {
-    KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
+    KVMClockState *s = KVM_CLOCK(dev);
 
     qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
     return 0;
@@ -119,7 +122,7 @@ static void kvmclock_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo kvmclock_info = {
-    .name          = "kvmclock",
+    .name          = TYPE_KVM_CLOCK,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(KVMClockState),
     .class_init    = kvmclock_class_init,
@@ -131,7 +134,7 @@ void kvmclock_create(void)
     if (kvm_enabled() &&
         first_cpu->features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
                                          (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
-        sysbus_create_simple("kvmclock", -1, NULL);
+        sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
     }
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 22/26] kvmclock: QOM'ify some more

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Introduce type constant and avoid FROM_SYSBUS().
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/i386/kvm/clock.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Thanks, by now I've picked up all remaining QOM cast patches (and
renamed and marked up their parent field):
https://github.com/afaerber/qemu-cpu/commits/qom-next

The remaining realizefn patches still need some calm review time.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 23/26] kvmclock: use realize for kvmclock

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/kvm/clock.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index fcdcbca..4f6e6f9 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -90,12 +90,11 @@ static void kvmclock_vm_state_change(void *opaque, int running,
     }
 }
 
-static int kvmclock_init(SysBusDevice *dev)
+static void kvmclock_realize(DeviceState *dev, Error **errp)
 {
     KVMClockState *s = KVM_CLOCK(dev);
 
     qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
-    return 0;
 }
 
 static const VMStateDescription kvmclock_vmsd = {
@@ -114,9 +113,8 @@ static const VMStateDescription kvmclock_vmsd = {
 static void kvmclock_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = kvmclock_init;
+    dc->realize = kvmclock_realize;
     dc->no_user = 1;
     dc->vmsd = &kvmclock_vmsd;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 24/26] kvmvapic realize

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/kvmvapic.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index f93629f..fa85096 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -698,19 +698,18 @@ static const MemoryRegionOps vapic_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int vapic_init(SysBusDevice *dev)
+static void vapic_realize(DeviceState *dev, Error **errp)
 {
     VAPICROMState *s = VAPIC(dev);
+    SysBusDevice *b = SYS_BUS_DEVICE(dev);
 
     memory_region_init_io(&s->io, &vapic_ops, s, "kvmvapic", 2);
-    sysbus_add_io(dev, VAPIC_IO_PORT, &s->io);
-    sysbus_init_ioports(dev, VAPIC_IO_PORT, 2);
+    sysbus_add_io(b, VAPIC_IO_PORT, &s->io);
+    sysbus_init_ioports(b, VAPIC_IO_PORT, 2);
 
     option_rom[nb_option_roms].name = "kvmvapic.bin";
     option_rom[nb_option_roms].bootindex = -1;
     nb_option_roms++;
-
-    return 0;
 }
 
 static void do_vapic_enable(void *data)
@@ -806,13 +805,12 @@ static const VMStateDescription vmstate_vapic = {
 
 static void vapic_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->no_user = 1;
     dc->reset   = vapic_reset;
     dc->vmsd    = &vmstate_vapic;
-    sc->init    = vapic_init;
+    dc->realize = vapic_realize;
 }
 
 static const TypeInfo vapic_type = {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 25/26] ioapic: use realize for ioapic

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/intc/ioapic_common.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 5c5bb3c..5dc1f1e 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -57,23 +57,22 @@ static int ioapic_dispatch_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static int ioapic_init_common(SysBusDevice *dev)
+static void ioapic_common_realize(DeviceState *dev, Error **errp)
 {
     IOAPICCommonState *s = IOAPIC_COMMON(dev);
     IOAPICCommonClass *info;
     static int ioapic_no;
 
     if (ioapic_no >= MAX_IOAPICS) {
-        return -1;
+        error_setg(errp, "invalid ioapic number: %d", ioapic_no);
+        return;
     }
 
     info = IOAPIC_COMMON_GET_CLASS(s);
     info->init(s, ioapic_no);
 
-    sysbus_init_mmio(&s->busdev, &s->io_memory);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->io_memory);
     ioapic_no++;
-
-    return 0;
 }
 
 static const VMStateDescription vmstate_ioapic_common = {
@@ -95,10 +94,9 @@ static const VMStateDescription vmstate_ioapic_common = {
 
 static void ioapic_common_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    sc->init = ioapic_init_common;
+    dc->realize = ioapic_common_realize;
     dc->vmsd = &vmstate_ioapic_common;
     dc->no_user = 1;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 25/26] ioapic: use realize for ioapic

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/intc/ioapic_common.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
> index 5c5bb3c..5dc1f1e 100644
> --- a/hw/intc/ioapic_common.c
> +++ b/hw/intc/ioapic_common.c
> @@ -57,23 +57,22 @@ static int ioapic_dispatch_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> -static int ioapic_init_common(SysBusDevice *dev)
> +static void ioapic_common_realize(DeviceState *dev, Error **errp)
>  {
>      IOAPICCommonState *s = IOAPIC_COMMON(dev);
>      IOAPICCommonClass *info;
>      static int ioapic_no;
>  
>      if (ioapic_no >= MAX_IOAPICS) {
> -        return -1;
> +        error_setg(errp, "invalid ioapic number: %d", ioapic_no);

Since the ioapic number was not chosen by the user, I have changed this
similar to the preceding error message, referring to the maximum instead.

Andreas

> +        return;
>      }
>  
>      info = IOAPIC_COMMON_GET_CLASS(s);
>      info->init(s, ioapic_no);
>  
> -    sysbus_init_mmio(&s->busdev, &s->io_memory);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->io_memory);
>      ioapic_no++;
> -
> -    return 0;
>  }
>  
>  static const VMStateDescription vmstate_ioapic_common = {
> @@ -95,10 +94,9 @@ static const VMStateDescription vmstate_ioapic_common = {
>  
>  static void ioapic_common_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    sc->init = ioapic_init_common;
> +    dc->realize = ioapic_common_realize;
>      dc->vmsd = &vmstate_ioapic_common;
>      dc->no_user = 1;
>  }
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2 26/26] isa bus: remove isabus_bridge_init since it does nothing

[Hu Tao]

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/isa/isa-bus.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 136d17e..6aea97b 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -192,18 +192,10 @@ static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     }
 }
 
-static int isabus_bridge_init(SysBusDevice *dev)
-{
-    /* nothing */
-    return 0;
-}
-
 static void isabus_bridge_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
-    k->init = isabus_bridge_init;
     dc->fw_name = "isa";
     dc->no_user = 1;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 00/26] use realizefn for SysBusDevice, part 1

[Andreas Färber]

Am 01.07.2013 12:18, schrieb Hu Tao:
> Hu Tao (26):
>   sysbus: document SysBusDeviceClass about @init
>   ohci: QOM'ify some more
>   ohci: use realize for ohci
>   i440fx-pcihost: use realize for i440fx-pcihost
>   i440fx: use type-safe cast instead of directly access of parent dev
>   q35: use type-safe cast instead of directly access of parent dev
>   q35: use realize for q35 host
>   fdc: QOM'ify some more
>   fdc: use realize for fdc.
>   pflash-cfi01: QOM'ify some more
>   pflash_cfi01: use realize for pflash_cfi01
>   pflash-cfi02: QOM'ify some more
>   pflash_cfi02: use realize for pflash_cfi02
>   ahci: QOM'ify some more
>   ahci: use realize for ahci
>   fwcfg: QOM'ify some more
>   fwcfg: use realize for fwcfg
>   scsi esp: QOM'ify some more
>   scsi esp: use realize for scsi esp
>   hpet: QOM'ify some more
>   hpet: use realize for hpet
>   kvmclock: QOM'ify some more
>   kvmclock: use realize for kvmclock
>   kvmvapic realize
>   ioapic: use realize for ioapic
>   isa bus: remove isabus_bridge_init since it does nothing

By now I believe I've queued all of these, partially with minor changes
like b or err-vs.-errp variable renames or replacement of repeated
in-place QOM casts with a variable and one bug fix. Most realizefn
patches conflicted due to Memory API changes. In two cases I inserted
error propagation improvements as indicated. Testing appreciated.

git://github.com/afaerber/qemu-cpu.git qom-next
https://github.com/afaerber/qemu-cpu/commits/qom-next

Regards,
Andreas

>  hw/block/fdc.c          | 87 +++++++++++++++++++++++++++++++------------------
>  hw/block/pflash_cfi01.c | 28 ++++++++--------
>  hw/block/pflash_cfi02.c | 31 +++++++++---------
>  hw/i386/kvm/clock.c     | 15 +++++----
>  hw/i386/kvmvapic.c      | 12 +++----
>  hw/ide/ahci.c           | 21 ++++++------
>  hw/intc/ioapic_common.c | 12 +++----
>  hw/isa/isa-bus.c        |  8 -----
>  hw/nvram/fw_cfg.c       | 36 +++++++++++---------
>  hw/pci-host/piix.c      | 36 ++++++++++++--------
>  hw/pci-host/q35.c       | 60 +++++++++++++++++-----------------
>  hw/scsi/esp.c           | 46 ++++++++++++++++----------
>  hw/timer/hpet.c         | 44 +++++++++++++++----------
>  hw/usb/hcd-ohci.c       | 32 +++++++++---------
>  include/hw/sysbus.h     | 10 ++++++
>  15 files changed, 268 insertions(+), 210 deletions(-)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg