Hi, Eduardo
On 06/03/2017 01:45 AM, Eduardo Habkost wrote:
On Fri, Jun 02, 2017 at 03:54:41PM +0800, Mao Zhongyi wrote:
Add Error argument for pci_add_capability() to leverage the errp
to pass info on errors. This way is helpful for its callers to
make a better error handling when moving to 'realize'.
Cc: m...@redhat.com
Cc: pbonz...@redhat.com
Cc: r...@twiddle.net
Cc: ehabk...@redhat.com
Cc: dmi...@daynix.com
Cc: jasow...@redhat.com
Cc: mar...@redhat.com
Cc: alex.william...@redhat.com
Cc: arm...@redhat.com
Signed-off-by: Mao Zhongyi <maozy.f...@cn.fujitsu.com>
[...]
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 62e989c..f24046a 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -494,7 +494,7 @@ static void eepro100_fcp_interrupt(EEPRO100State *s)
}
#endif
-static void e100_pci_reset(EEPRO100State *s)
+static void e100_pci_reset(EEPRO100State *s, Error **errp)
{
E100PCIDeviceInfo *info = eepro100_get_class(s);
uint32_t device = s->device;
@@ -570,9 +570,13 @@ static void e100_pci_reset(EEPRO100State *s)
/* Power Management Capabilities */
int cfg_offset = 0xdc;
int r = pci_add_capability(&s->dev, PCI_CAP_ID_PM,
- cfg_offset, PCI_PM_SIZEOF);
- assert(r > 0);
- pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21);
+ cfg_offset, PCI_PM_SIZEOF,
+ errp);
+ if (r > 0) {
+ pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21);
+ } else {
+ return;
+ }
#if 0 /* TODO: replace dummy code for power management emulation. */
/* TODO: Power Management Control / Status. */
pci_set_word(pci_conf + cfg_offset + PCI_PM_CTRL, 0x0000);
@@ -1863,7 +1867,10 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error
**errp)
s->device = info->device;
- e100_pci_reset(s);
+ e100_pci_reset(s, errp);
+ if (errp && *errp) {
+ return;
+ }
This doesn't look right. The behavior of the function shouldn't
be different depending on the value of errp.
If you need to check for e100_pci_reset() errors inside
e100_nic_realize(), you'll need a local Error* variable and an
error_propagate() call. See the "Receive an error and pass it on
to the caller" example at include/qapi/error.h.
Thanks for your detailed explanation. I already know how to do it, will
fix it in the next version.
Mao
/* Add 64 * 2 EEPROM. i82557 and i82558 support a 64 word EEPROM,
* i82559 and later support 64 or 256 word EEPROM. */
