On Tue, 2014-10-28 at 10:38 +0100, Markus Armbruster wrote:
> Gonglei <arei.gong...@huawei.com> writes:
>
> > On 2014/10/28 15:35, Markus Armbruster wrote:
> >
> >> Implement DeviceClass methods realize() and unrealize() instead of
> >> init() and exit(). The core's initialization errors now get
> >> propagated properly, and QMP sends them instead of an unspecific
> >> "Device initialization failed" error. Unrealize can't fail, so no
> >> change there.
> >>
> >> PCIDeviceClass is unchanged: it still provides init() and exit().
> >> Therefore, device models' errors are still not propagated.
> >>
> >> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> >> ---
> >> hw/pci/pci.c | 91
> >> +++++++++++++++++++++++++++++++-----------------------------
> >> 1 file changed, 47 insertions(+), 44 deletions(-)
> >>
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index cd7a403..aef95c3 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> [...]
> >> void pci_register_bar(PCIDevice *pci_dev, int region_num,
> >> @@ -1742,10 +1747,11 @@ PCIDevice *pci_find_device(PCIBus *bus, int
> >> bus_num, uint8_t devfn)
> >> return bus->devices[devfn];
> >> }
> >>
> >> -static int pci_qdev_init(DeviceState *qdev)
> >> +static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> >> {
> >> PCIDevice *pci_dev = (PCIDevice *)qdev;
> >> PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> >> + Error *local_err = NULL;
> >> PCIBus *bus;
> >> int rc;
> >> bool is_default_rom;
> >> @@ -1758,15 +1764,16 @@ static int pci_qdev_init(DeviceState *qdev)
> >> bus = PCI_BUS(qdev_get_parent_bus(qdev));
> >> pci_dev = do_pci_register_device(pci_dev, bus,
> >> object_get_typename(OBJECT(qdev)),
> >> - pci_dev->devfn);
> >> + pci_dev->devfn, errp);
> >> if (pci_dev == NULL)
> >
> >> - return -1;
> >> + return;
> >>
> >
> > Maybe we can use '{}' for if statement follow Qemu's coding style.
I suggest adding a trivial patch on top of Markus's series
Thanks,
Marcel
>
> scripts/checkpatch.pl is happy with the patch as is.
>
> I prefer to add braces only when I touch the conditional.
>
> Naturally, I also add them when a maintainer asks me to :)
>
> >> if (pc->init) {
> >> rc = pc->init(pci_dev);
> >> if (rc != 0) {
> >> do_pci_unregister_device(pci_dev);
> >> - return rc;
> >> + error_setg(errp, "Device initialization failed");
> >> + return;
> >> }
> >> }
> >>
> [...]
>
> Thanks!
>
