| Message ID | 1385006938-32515-2-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
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 > > > >
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; > } Seems like these exit clauses should return an integer here and below. > 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 > > > >
On 03/13/2014 12:56 AM, Mike Day wrote: > 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; >> } > > Seems like these exit clauses should return an integer here and below. Nope. Bad initfn (which returns int) became good realizefn (which does not return anything and uses Error** instead) :) Thanks for review! > >> 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 >> >> >> >>
Am 21.11.2013 05:08, schrieb Alexey Kardashevskiy: > 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> > --- > 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" > [snip] Thanks, applied to ppc-next without the above hunk: https://github.com/afaerber/qemu-cpu/commits/ppc-next Andreas
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; } 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;
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> --- Changes: v5: * finish_finalize() moved to a separate patch --- hw/ppc/spapr_pci.c | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-)