* [Qemu-devel] [PATCH v3 0/2] Convert pxb initfn to Error @ 2016-03-23 10:08 Wei Jiangang 2016-03-23 10:08 ` [Qemu-devel] [PATCH v3 1/2] hw/pci-bridge: Add missing unref in case register-bus fails Wei Jiangang 2016-03-23 10:08 ` [Qemu-devel] [PATCH v3 2/2] hw/pci-bridge: Convert pxb initialization functions to Error Wei Jiangang 0 siblings, 2 replies; 5+ messages in thread From: Wei Jiangang @ 2016-03-23 10:08 UTC (permalink / raw) To: qemu-devel; +Cc: marcel, caoj.fnst, Wei Jiangang, armbru, mst v2 -> v3: - Pass local *err to error_setg() in pxb_dev_realize_common() v1 -> v2: - Extract a separate bugfix patch - Squash v1 patches 1-3 together - Drop the period and exclamation mark - Modify commit message v1: cover-letter The original patch named '[PATCH v3] PXB: convert to realize()' was raised by Cao jin <caoj.fnst@cn.fujitsu.com>, and reviewed by Marcel Apfelbaum <marcel@redhat.com>. It conflicts with 02b0743 that had been merged, so resend a newer version, including the following, hw/pci-bridge: modify pxb_dev_init_common pxb: convert to realize() pxb-pcie: convert to realize() Wei Jiangang (2): hw/pci-bridge: Add missing unref in case register-bus fails hw/pci-bridge: Convert pxb initialization functions to Error hw/pci-bridge/pci_expander_bridge.c | 60 ++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 27 deletions(-) -- 1.9.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] hw/pci-bridge: Add missing unref in case register-bus fails 2016-03-23 10:08 [Qemu-devel] [PATCH v3 0/2] Convert pxb initfn to Error Wei Jiangang @ 2016-03-23 10:08 ` Wei Jiangang 2016-03-23 10:08 ` [Qemu-devel] [PATCH v3 2/2] hw/pci-bridge: Convert pxb initialization functions to Error Wei Jiangang 1 sibling, 0 replies; 5+ messages in thread From: Wei Jiangang @ 2016-03-23 10:08 UTC (permalink / raw) To: qemu-devel; +Cc: marcel, caoj.fnst, Wei Jiangang, armbru, mst The error paths after a successful qdev_create/pci_bus_new should contain a object_unref/object_unparent. pxb_dev_init_common() did not yet, so add it. Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- hw/pci-bridge/pci_expander_bridge.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index 5e7e546..ba320bd 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -249,7 +249,7 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) PCI_HOST_BRIDGE(ds)->bus = bus; if (pxb_register_bus(dev, bus)) { - return -EINVAL; + goto err_register_bus; } qdev_init_nofail(ds); @@ -263,6 +263,12 @@ static int pxb_dev_init_common(PCIDevice *dev, bool pcie) pxb_dev_list = g_list_insert_sorted(pxb_dev_list, pxb, pxb_compare); return 0; + +err_register_bus: + object_unref(OBJECT(bds)); + object_unparent(OBJECT(bus)); + object_unref(OBJECT(ds)); + return -EINVAL; } static int pxb_dev_initfn(PCIDevice *dev) -- 1.9.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] hw/pci-bridge: Convert pxb initialization functions to Error 2016-03-23 10:08 [Qemu-devel] [PATCH v3 0/2] Convert pxb initfn to Error Wei Jiangang 2016-03-23 10:08 ` [Qemu-devel] [PATCH v3 1/2] hw/pci-bridge: Add missing unref in case register-bus fails Wei Jiangang @ 2016-03-23 10:08 ` Wei Jiangang 2016-03-23 12:05 ` Markus Armbruster 1 sibling, 1 reply; 5+ messages in thread From: Wei Jiangang @ 2016-03-23 10:08 UTC (permalink / raw) To: qemu-devel; +Cc: marcel, caoj.fnst, Wei Jiangang, armbru, mst 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. --- hw/pci-bridge/pci_expander_bridge.c | 54 ++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index ba320bd..6f53a6b 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,21 @@ 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 *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(&err, "Illegal numa node %d", pxb->numa_node); + if (err) { + error_propagate(errp, err); + } + return; } if (dev->qdev.id && *dev->qdev.id) { @@ -248,7 +247,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, &err); + if (err) { + error_propagate(errp, err); goto err_register_bus; } @@ -262,23 +263,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 +300,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 +318,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 +333,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; -- 1.9.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] hw/pci-bridge: Convert pxb initialization functions to Error 2016-03-23 10:08 ` [Qemu-devel] [PATCH v3 2/2] hw/pci-bridge: Convert pxb initialization functions to Error Wei Jiangang @ 2016-03-23 12:05 ` Markus Armbruster 2016-03-24 0:40 ` Wei, Jiangang 0 siblings, 1 reply; 5+ messages in thread From: Markus Armbruster @ 2016-03-23 12:05 UTC (permalink / raw) To: Wei Jiangang; +Cc: marcel, caoj.fnst, qemu-devel, mst Wei Jiangang <weijg.fnst@cn.fujitsu.com> writes: > 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. > --- > hw/pci-bridge/pci_expander_bridge.c | 54 ++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 27 deletions(-) > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > index ba320bd..6f53a6b 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,21 @@ 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 *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(&err, "Illegal numa node %d", pxb->numa_node); > + if (err) { > + error_propagate(errp, err); > + } No. v2 got it right. > + return; > } > > if (dev->qdev.id && *dev->qdev.id) { [...] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] hw/pci-bridge: Convert pxb initialization functions to Error 2016-03-23 12:05 ` Markus Armbruster @ 2016-03-24 0:40 ` Wei, Jiangang 0 siblings, 0 replies; 5+ messages in thread From: Wei, Jiangang @ 2016-03-24 0:40 UTC (permalink / raw) To: armbru@redhat.com Cc: marcel@redhat.com, Cao, Jin, qemu-devel@nongnu.org, mst@redhat.com On Wed, 2016-03-23 at 13:05 +0100, Markus Armbruster wrote: > Wei Jiangang <weijg.fnst@cn.fujitsu.com> writes: > > > 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. > > --- > > hw/pci-bridge/pci_expander_bridge.c | 54 ++++++++++++++++++------------------- > > 1 file changed, 27 insertions(+), 27 deletions(-) > > > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > > index ba320bd..6f53a6b 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,21 @@ 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 *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(&err, "Illegal numa node %d", pxb->numa_node); > > + if (err) { > > + error_propagate(errp, err); > > + } > > No. v2 got it right. Thanks for your response. OK, Let us discard v3. > > > + return; > > } > > > > if (dev->qdev.id && *dev->qdev.id) { > [...] > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-24 0:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-23 10:08 [Qemu-devel] [PATCH v3 0/2] Convert pxb initfn to Error Wei Jiangang 2016-03-23 10:08 ` [Qemu-devel] [PATCH v3 1/2] hw/pci-bridge: Add missing unref in case register-bus fails Wei Jiangang 2016-03-23 10:08 ` [Qemu-devel] [PATCH v3 2/2] hw/pci-bridge: Convert pxb initialization functions to Error Wei Jiangang 2016-03-23 12:05 ` Markus Armbruster 2016-03-24 0:40 ` Wei, Jiangang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).