From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60733) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akCVV-0004MU-Ld for qemu-devel@nongnu.org; Sun, 27 Mar 2016 11:21:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1akCVR-0006sy-6z for qemu-devel@nongnu.org; Sun, 27 Mar 2016 11:21:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38908) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akCVQ-0006su-Us for qemu-devel@nongnu.org; Sun, 27 Mar 2016 11:21:33 -0400 References: <1458717980-26686-1-git-send-email-weijg.fnst@cn.fujitsu.com> <1458717980-26686-3-git-send-email-weijg.fnst@cn.fujitsu.com> From: Marcel Apfelbaum Message-ID: <56F7FA75.8070901@redhat.com> Date: Sun, 27 Mar 2016 18:21:25 +0300 MIME-Version: 1.0 In-Reply-To: <1458717980-26686-3-git-send-email-weijg.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/2] hw/pci-bridge: Convert pxb initialization functions to Error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wei Jiangang , qemu-devel@nongnu.org Cc: Cao jin , armbru@redhat.com On 03/23/2016 09:26 AM, Wei Jiangang wrote: > Firstly, convert pxb_dev_init_common() to Error and rename > it to pxb_dev_realize_common(). > Actually, pxb_register_bus() is converted as well. > > And then, > convert pxb_dev_initfn() and pxb_pcie_dev_initfn() to Error, > rename them to pxb_dev_realize() and pxb_pcie_dev_realize() > respectively. > > Signed-off-by: Wei Jiangang > Signed-off-by: Cao jin > --- > hw/pci-bridge/pci_expander_bridge.c | 51 +++++++++++++++++-------------------- > 1 file changed, 24 insertions(+), 27 deletions(-) > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > index ba320bd..efb8863 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,17 +208,18 @@ 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; > PCIBus *bus; > const char *dev_name = NULL; > + Error *local_err = NULL; > > 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,7 +244,9 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) > > PCI_HOST_BRIDGE(ds)->bus = bus; > > - if (pxb_register_bus(dev, bus)) { > + pxb_register_bus(dev, bus, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > goto err_register_bus; > } > > @@ -262,23 +260,22 @@ 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)); > - return -EINVAL; > } > > -static int pxb_dev_initfn(PCIDevice *dev) > +static void pxb_dev_realize(PCIDevice *dev, Error **errp) > { > if (pci_bus_is_express(dev->bus)) { > - error_report("pxb devices cannot reside on a PCIe bus!"); > - return -EINVAL; > + error_setg(errp, "pxb devices cannot reside on a PCIe bus"); > + return; > } > > - return pxb_dev_init_common(dev, false); > + pxb_dev_realize_common(dev, false, errp); > } > > static void pxb_dev_exitfn(PCIDevice *pci_dev) > @@ -300,7 +297,7 @@ static void pxb_dev_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > - k->init = pxb_dev_initfn; > + k->realize = pxb_dev_realize; > k->exit = pxb_dev_exitfn; > k->vendor_id = PCI_VENDOR_ID_REDHAT; > k->device_id = PCI_DEVICE_ID_REDHAT_PXB; > @@ -318,14 +315,14 @@ static const TypeInfo pxb_dev_info = { > .class_init = pxb_dev_class_init, > }; > > -static int pxb_pcie_dev_initfn(PCIDevice *dev) > +static void pxb_pcie_dev_realize(PCIDevice *dev, Error **errp) > { > if (!pci_bus_is_express(dev->bus)) { > - error_report("pxb-pcie devices cannot reside on a PCI bus!"); > - return -EINVAL; > + error_setg(errp, "pxb-pcie devices cannot reside on a PCI bus"); > + return; > } > > - return pxb_dev_init_common(dev, true); > + pxb_dev_realize_common(dev, true, errp); > } > > static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data) > @@ -333,7 +330,7 @@ static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > - k->init = pxb_pcie_dev_initfn; > + k->realize = pxb_pcie_dev_realize; > k->exit = pxb_dev_exitfn; > k->vendor_id = PCI_VENDOR_ID_REDHAT; > k->device_id = PCI_DEVICE_ID_REDHAT_PXB_PCIE; > Looks good to me. Reviewed-by: Marcel Apfelbaum Thanks, Marcel