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);
-
-    return rc;
 }
 
 static const VMStateDescription i82801b11_bridge_dev_vmstate = {
@@ -95,7 +94,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11;
     k->revision = ICH9_D2P_A2_REVISION;
-    k->init = i82801b11_bridge_initfn;
+    k->realize = i82801b11_bridge_realize;
     k->config_write = pci_bridge_write_config;
     dc->vmsd = &i82801b11_bridge_dev_vmstate;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-- 
2.9.3

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

> -
> -    return rc;
>   }
>   
>   static const VMStateDescription i82801b11_bridge_dev_vmstate = {
> @@ -95,7 +94,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, void *data)
>       k->vendor_id = PCI_VENDOR_ID_INTEL;
>       k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11;
>       k->revision = ICH9_D2P_A2_REVISION;
> -    k->init = i82801b11_bridge_initfn;
> +    k->realize = i82801b11_bridge_realize;
>       k->config_write = pci_bridge_write_config;
>       dc->vmsd = &i82801b11_bridge_dev_vmstate;
>       set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);

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

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. 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



















>

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
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>>
>
>