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.
>
> /* Add 64 * 2 EEPROM. i82557 and i82558 support a 64 word EEPROM,
> * i82559 and later support 64 or 256 word EEPROM. */
--
Eduardo
