Re: [Qemu-devel] spapr-pci: convert init() callback to realize()

[Mike Day <ncmike@ncultra.org>]

On 21/11/13 15:08 +1100, Alexey Kardashevskiy wrote:
> This converts the old-style init() callback to a new style realize()
> callback as init() now is supposed to do only trivial initialization.
> 
> As a part of convertion, this replaces fprintf(stderr) with error_setg()
> as realize() does not "return" any value, instead it puts the extended
> error into **errp.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Mike Day <ncmike@ncultra.org>

> ---
> Changes:
> v5:
> * finish_finalize() moved to a separate patch
> ---
>  hw/ppc/spapr_pci.c | 44 ++++++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 7763149..aeb012d 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -32,6 +32,7 @@
>  #include "exec/address-spaces.h"
>  #include <libfdt.h>
>  #include "trace.h"
> +#include "qemu/error-report.h"
>  
>  #include "hw/pci/pci_bus.h"
>  
> @@ -494,9 +495,9 @@ static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>      return &phb->iommu_as;
>  }
>  
> -static int spapr_phb_init(SysBusDevice *s)
> +static void spapr_phb_realize(DeviceState *dev, Error **errp)
>  {
> -    DeviceState *dev = DEVICE(s);
> +    SysBusDevice *s = SYS_BUS_DEVICE(dev);
>      sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
>      PCIHostState *phb = PCI_HOST_BRIDGE(s);
>      const char *busname;
> @@ -510,9 +511,9 @@ static int spapr_phb_init(SysBusDevice *s)
>          if ((sphb->buid != -1) || (sphb->dma_liobn != -1)
>              || (sphb->mem_win_addr != -1)
>              || (sphb->io_win_addr != -1)) {
> -            fprintf(stderr, "Either \"index\" or other parameters must"
> -                    " be specified for PAPR PHB, not both\n");
> -            return -1;
> +            error_setg(errp, "Either \"index\" or other parameters must"
> +                       " be specified for PAPR PHB, not both");
> +            return;
 
shouldn't these return an int? Or if the error is returned by pointer
perhaps the function should have a void return?

>          }
>  
>          sphb->buid = SPAPR_PCI_BASE_BUID + sphb->index;
> @@ -525,28 +526,28 @@ static int spapr_phb_init(SysBusDevice *s)
>      }
>  
>      if (sphb->buid == -1) {
> -        fprintf(stderr, "BUID not specified for PHB\n");
> -        return -1;
> +        error_setg(errp, "BUID not specified for PHB");
> +        return;
>      }
>  
>      if (sphb->dma_liobn == -1) {
> -        fprintf(stderr, "LIOBN not specified for PHB\n");
> -        return -1;
> +        error_setg(errp, "LIOBN not specified for PHB");
> +        return;
>      }
>  
>      if (sphb->mem_win_addr == -1) {
> -        fprintf(stderr, "Memory window address not specified for PHB\n");
> -        return -1;
> +        error_setg(errp, "Memory window address not specified for PHB");
> +        return;
>      }
>  
>      if (sphb->io_win_addr == -1) {
> -        fprintf(stderr, "IO window address not specified for PHB\n");
> -        return -1;
> +        error_setg(errp, "IO window address not specified for PHB");
> +        return;
>      }
>  
>      if (find_phb(spapr, sphb->buid)) {
> -        fprintf(stderr, "PCI host bridges must have unique BUIDs\n");
> -        return -1;
> +        error_setg(errp, "PCI host bridges must have unique BUIDs");
> +        return;
>      }
>  
>      sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
> @@ -613,8 +614,9 @@ static int spapr_phb_init(SysBusDevice *s)
>      sphb->tcet = spapr_tce_new_table(dev, sphb->dma_liobn,
>                                       sphb->dma_window_size);
>      if (!sphb->tcet) {
> -        fprintf(stderr, "Unable to create TCE table for %s\n", sphb->dtbusname);
> -        return -1;
> +        error_setg(errp, "Unable to create TCE table for %s",
> +                   sphb->dtbusname);
> +        return;
>      }
>      address_space_init(&sphb->iommu_as, spapr_tce_get_iommu(sphb->tcet),
>                         sphb->dtbusname);
> @@ -631,13 +633,12 @@ static int spapr_phb_init(SysBusDevice *s)
>  
>          irq = spapr_allocate_lsi(0);
>          if (!irq) {
> -            return -1;
> +            error_setg(errp, "spapr_allocate_lsi failed");
> +            return;
>          }
>  
>          sphb->lsi_table[i].irq = irq;
>      }
> -
> -    return 0;
>  }
>  
>  static void spapr_phb_reset(DeviceState *qdev)
> @@ -720,11 +721,10 @@ static const char *spapr_phb_root_bus_path(PCIHostState *host_bridge,
>  static void spapr_phb_class_init(ObjectClass *klass, void *data)
>  {
>      PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass);
> -    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      hc->root_bus_path = spapr_phb_root_bus_path;
> -    sdc->init = spapr_phb_init;
> +    dc->realize = spapr_phb_realize;
>      dc->props = spapr_phb_properties;
>      dc->reset = spapr_phb_reset;
>      dc->vmsd = &vmstate_spapr_pci;
> -- 
> 1.8.4.rc4
> 
> 
> 
>