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 <a...@ozlabs.ru>
> Reviewed-by: Mike Day <ncm...@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
>>
>>
>>
>>
--
Alexey
