Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize

[Marcel Apfelbaum <marcel@redhat.com>]

On 27/05/2017 9:58, Mao Zhongyi wrote:
>
>
> On 05/26/2017 10:08 PM, Marcel Apfelbaum wrote:
>>
>>
>> On 26/05/2017 15:15, Mao Zhongyi wrote:
>>> The pci-birdge device i82801b11 still implements the old
>>> PCIDeviceClass .init() through i82801b11_bridge_init()
>>> instead of the new .realize(). All devices need to be
>>> converted to .realize(). So convert it and rename it to
>>> i82801b11_bridge_realize().
>>>
>>> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
>>> ---
>>>   hw/pci-bridge/i82801b11.c | 9 ++++-----
>>>   1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
>>> index 2404e7e..dca3162 100644
>>> --- a/hw/pci-bridge/i82801b11.c
>>> +++ b/hw/pci-bridge/i82801b11.c
>>> @@ -44,6 +44,7 @@
>>>   #include "qemu/osdep.h"
>>>   #include "hw/pci/pci.h"
>>>   #include "hw/i386/ich9.h"
>>> +#include "qapi/error.h"
>>> /*****************************************************************************/
>>> @@ -58,7 +59,7 @@ typedef struct I82801b11Bridge {
>>>       /*< public >*/
>>>   } I82801b11Bridge;
>>>   -static int i82801b11_bridge_initfn(PCIDevice *d)
>>> +static void i82801b11_bridge_realize(PCIDevice *d, Error **errp)
>>>   {
>>>       int rc;
>>>   @@ -70,12 +71,10 @@ static int i82801b11_bridge_initfn(PCIDevice *d)
>>>           goto err_bridge;
>>>       }
>>>       pci_config_set_prog_interface(d->config, 
>>> PCI_CLASS_BRIDGE_PCI_INF_SUB);
>>> -    return 0;
>>> +    return;
>>>     err_bridge:
>>>       pci_bridge_exitfn(d);
>>
>> Hi,
>>
>> Since you move to realize, you may want to leverage the errp to add
>> info on errors.
>>
>> Thanks,
>> Marcel
>
>
> Hi, Marcel
>

Hi!
> Thanks for your quick reply and advice. In fact, I have considered 
> adding an error
> message to errp when an error occurs. But I found that 
> pci_bridge_ssvid_init() has
> reported a specific info when it fails. If the error is reported here 
> again, it seems
> a bit more superfluous, so it's not adopted.

I agree we don't want to see the error twice, but that means we may want
to pass the error pointer to pci_bridge_ssvid_init and so on.
One of the main things we achieve when moving to 'realize' is better
error handling, so if we don't do that maybe is not worth it.

You may need to do several changes you didn't intend to
in order to do the error propagation right...

Thanks,
Marcel

> Of course, output a readable error info
> to make it easier for users is also a good choice. So, are you sure 
> you want to do
> that?
>
> Look forward to your feedback and suggestion soon.
>
> Thanks a lot.
> Mao
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>>
>
>