qemu-devel
[Top][All Lists]
Advanced

[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: Mao Zhongyi
Subject: Re: [Qemu-devel] [PATCH v2 5/6] pci: Make errp the last parameter of pci_add_capability()
Date: Mon, 5 Jun 2017 11:06:15 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

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

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






reply via email to

[Prev in Thread] Current Thread [Next in Thread]