[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/6] pci: Make errp the last parameter of pci
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/6] pci: Make errp the last parameter of pci_add_capability() |
Date: |
Fri, 2 Jun 2017 14:45:16 -0300 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
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: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Signed-off-by: Mao Zhongyi <address@hidden>
[...]
> 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
[Qemu-devel] [PATCH v2 3/6] pci: Fix the wrong return value judgment condition, Mao Zhongyi, 2017/06/02
[Qemu-devel] [PATCH v2 5/6] pci: Make errp the last parameter of pci_add_capability(), Mao Zhongyi, 2017/06/02
- Re: [Qemu-devel] [PATCH v2 5/6] pci: Make errp the last parameter of pci_add_capability(),
Eduardo Habkost <=
[Qemu-devel] [PATCH v2 2/6] pci: Add comment for pci_add_capability2(), Mao Zhongyi, 2017/06/02
[Qemu-devel] [PATCH v2 6/6] pci: Convert to realize, Mao Zhongyi, 2017/06/02
[Qemu-devel] [PATCH v2 4/6] net/eepro100: Fixed code style, Mao Zhongyi, 2017/06/02
Re: [Qemu-devel] [PATCH v2 0/6] Convert to realize and cleanup, Michael S. Tsirkin, 2017/06/06