Re: [Qemu-devel] [PATCH RFC 01/10] pci: Convert core to realize

[Markus Armbruster <armbru@redhat.com>]

Gonglei <arei.gonglei@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 <armbru@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.

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!