| Message ID | 1457951704-5217-2-git-send-email-weijg.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Wei Jiangang <weijg.fnst@cn.fujitsu.com> writes: > 1) add Error **errp parameter for it, > 2) rename pxb_dev_init_common to pxb_dev_realize_common, > and prepare for pxb/pxb-pcie convert to realize. > 3) modify the callers, > including pxb_dev_initfn and pxb_pcie_dev_initfn. > > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> The first line of your commit message is inexpressive. Suggest hw/pci-bridge: Convert to pxb_dev_init_common() to Error The next commits will convert its users to realize(), so rename it to pxb_dev_realize_common() now. The patch actually converts pxb_register_bus() as well, but that's detail. > --- > hw/pci-bridge/pci_expander_bridge.c | 56 ++++++++++++++++++++++++------------- > 1 file changed, 37 insertions(+), 19 deletions(-) > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > index d23b8da..ce5baf8 100644 > --- a/hw/pci-bridge/pci_expander_bridge.c > +++ b/hw/pci-bridge/pci_expander_bridge.c > @@ -160,30 +160,25 @@ static const TypeInfo pxb_host_info = { > }; > > /* > - * Registers the PXB bus as a child of the i440fx root bus. > - * > - * Returns 0 on successs, -1 if i440fx host was not > - * found or the bus number is already in use. > + * Registers the PXB bus as a child of pci host root bus. > */ > -static int pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus) > +static void pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus, Error **errp) > { > PCIBus *bus = dev->bus; > int pxb_bus_num = pci_bus_num(pxb_bus); > > if (bus->parent_dev) { > - error_report("PXB devices can be attached only to root bus."); > - return -1; > + error_setg(errp, "PXB devices can be attached only to root bus."); While there, drop the period (error messages generally don't end in punctuation). More of the same below. > + return; > } > > QLIST_FOREACH(bus, &bus->child, sibling) { > if (pci_bus_num(bus) == pxb_bus_num) { > - error_report("Bus %d is already in use.", pxb_bus_num); > - return -1; > + error_setg(errp, "Bus %d is already in use.", pxb_bus_num); > + return; > } > } > QLIST_INSERT_HEAD(&dev->bus->child, pxb_bus, sibling); > - > - return 0; > } > > static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin) > @@ -213,7 +208,7 @@ static gint pxb_compare(gconstpointer a, gconstpointer b) > 0; > } > > -static int pxb_dev_init_common(PCIDevice *dev, bool pcie) > +static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) > { > PXBDev *pxb = convert_to_pxb(dev); > DeviceState *ds, *bds = NULL; > @@ -222,8 +217,8 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) > > if (pxb->numa_node != NUMA_NODE_UNASSIGNED && > pxb->numa_node >= nb_numa_nodes) { > - error_report("Illegal numa node %d.", pxb->numa_node); > - return -EINVAL; > + error_setg(errp, "Illegal numa node %d.", pxb->numa_node); > + return; > } > > if (dev->qdev.id && *dev->qdev.id) { > @@ -248,8 +243,9 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) > > PCI_HOST_BRIDGE(ds)->bus = bus; > > - if (pxb_register_bus(dev, bus)) { > - return -EINVAL; > + pxb_register_bus(dev, bus, errp); > + if (*errp) { Breaks when a caller passes a null errp. You have to do pxb_register_bus(dev, bus, &err); if (err) { error_propagate(errp, err); with err previously initialized to null. See also the big comment in error.h. > + goto err_register_bus; > } > > qdev_init_nofail(ds); > @@ -262,17 +258,31 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) > pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_HOST); > > pxb_dev_list = g_list_insert_sorted(pxb_dev_list, pxb, pxb_compare); > - return 0; > + > + return; > + > +err_register_bus: > + object_unref(OBJECT(bds)); > + object_unparent(OBJECT(bus)); > + object_unref(OBJECT(ds)); Cleanup new in this patch. Is this a bug fix? If yes, the commit message should mention it. I'd make it a separate patch, before this one. > } > > static int pxb_dev_initfn(PCIDevice *dev) > { > + Error *err = NULL; > + > if (pci_bus_is_express(dev->bus)) { > error_report("pxb devices cannot reside on a PCIe bus!"); > return -EINVAL; > } > > - return pxb_dev_init_common(dev, false); > + pxb_dev_realize_common(dev, false, &err); > + if (err) { > + error_report_err(err); > + return -EINVAL; > + } > + > + return 0; > } > > static void pxb_dev_exitfn(PCIDevice *pci_dev) > @@ -314,12 +324,20 @@ static const TypeInfo pxb_dev_info = { > > static int pxb_pcie_dev_initfn(PCIDevice *dev) > { > + Error *err = NULL; > + > if (!pci_bus_is_express(dev->bus)) { > error_report("pxb-pcie devices cannot reside on a PCI bus!"); > return -EINVAL; > } > > - return pxb_dev_init_common(dev, true); > + pxb_dev_realize_common(dev, true, &err); > + if (err) { > + error_report_err(err); > + return -EINVAL; > + } > + > + return 0; > } > > static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index d23b8da..ce5baf8 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -160,30 +160,25 @@ static const TypeInfo pxb_host_info = { }; /* - * Registers the PXB bus as a child of the i440fx root bus. - * - * Returns 0 on successs, -1 if i440fx host was not - * found or the bus number is already in use. + * Registers the PXB bus as a child of pci host root bus. */ -static int pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus) +static void pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus, Error **errp) { PCIBus *bus = dev->bus; int pxb_bus_num = pci_bus_num(pxb_bus); if (bus->parent_dev) { - error_report("PXB devices can be attached only to root bus."); - return -1; + error_setg(errp, "PXB devices can be attached only to root bus."); + return; } QLIST_FOREACH(bus, &bus->child, sibling) { if (pci_bus_num(bus) == pxb_bus_num) { - error_report("Bus %d is already in use.", pxb_bus_num); - return -1; + error_setg(errp, "Bus %d is already in use.", pxb_bus_num); + return; } } QLIST_INSERT_HEAD(&dev->bus->child, pxb_bus, sibling); - - return 0; } static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin) @@ -213,7 +208,7 @@ static gint pxb_compare(gconstpointer a, gconstpointer b) 0; } -static int pxb_dev_init_common(PCIDevice *dev, bool pcie) +static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) { PXBDev *pxb = convert_to_pxb(dev); DeviceState *ds, *bds = NULL; @@ -222,8 +217,8 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) if (pxb->numa_node != NUMA_NODE_UNASSIGNED && pxb->numa_node >= nb_numa_nodes) { - error_report("Illegal numa node %d.", pxb->numa_node); - return -EINVAL; + error_setg(errp, "Illegal numa node %d.", pxb->numa_node); + return; } if (dev->qdev.id && *dev->qdev.id) { @@ -248,8 +243,9 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) PCI_HOST_BRIDGE(ds)->bus = bus; - if (pxb_register_bus(dev, bus)) { - return -EINVAL; + pxb_register_bus(dev, bus, errp); + if (*errp) { + goto err_register_bus; } qdev_init_nofail(ds); @@ -262,17 +258,31 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_HOST); pxb_dev_list = g_list_insert_sorted(pxb_dev_list, pxb, pxb_compare); - return 0; + + return; + +err_register_bus: + object_unref(OBJECT(bds)); + object_unparent(OBJECT(bus)); + object_unref(OBJECT(ds)); } static int pxb_dev_initfn(PCIDevice *dev) { + Error *err = NULL; + if (pci_bus_is_express(dev->bus)) { error_report("pxb devices cannot reside on a PCIe bus!"); return -EINVAL; } - return pxb_dev_init_common(dev, false); + pxb_dev_realize_common(dev, false, &err); + if (err) { + error_report_err(err); + return -EINVAL; + } + + return 0; } static void pxb_dev_exitfn(PCIDevice *pci_dev) @@ -314,12 +324,20 @@ static const TypeInfo pxb_dev_info = { static int pxb_pcie_dev_initfn(PCIDevice *dev) { + Error *err = NULL; + if (!pci_bus_is_express(dev->bus)) { error_report("pxb-pcie devices cannot reside on a PCI bus!"); return -EINVAL; } - return pxb_dev_init_common(dev, true); + pxb_dev_realize_common(dev, true, &err); + if (err) { + error_report_err(err); + return -EINVAL; + } + + return 0; } static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data)