From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55902) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aidYY-0002AO-Pl for qemu-devel@nongnu.org; Wed, 23 Mar 2016 03:50:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aidYV-0000ZB-JS for qemu-devel@nongnu.org; Wed, 23 Mar 2016 03:50:18 -0400 Received: from [59.151.112.132] (port=14216 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aidYU-0000Tm-Sn for qemu-devel@nongnu.org; Wed, 23 Mar 2016 03:50:15 -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: Cao jin Message-ID: <56F24B2A.6000407@cn.fujitsu.com> Date: Wed, 23 Mar 2016 15:52:10 +0800 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: marcel@redhat.com, armbru@redhat.com On 03/23/2016 03:26 PM, Wei Jiangang wrote: > > -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; > the preferred style I think, is /*err/ > 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; since we have local /*err/ to avoid null /**errp/ venture, I guess it should be used here too. > } > > 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; > -- Yours Sincerely, Cao jin