qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize


From: Mao Zhongyi
Subject: Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize
Date: Wed, 31 May 2017 09:46:59 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

Hi, Marcel & Markus

On 05/29/2017 07:37 PM, Markus Armbruster wrote:
Marcel Apfelbaum <address@hidden> writes:

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 <address@hidden>
---
  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.

Yes.

Callees that report errors differently are a common issue when
converting a function to Error.  The correct solution is to convert
these callees to Error, too.

Sometimes, this is just too much to be practical.  Then a "shallow"
conversion can make sense as a stop-gap, even though it'll result in
clearly inferior error reporting.

At a glance, this one doesn't look impractical.

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.

We need to convert all devices to realize() so we can get rid of the old
qdev methods.  But you're right in that we better convert them
correctly.

You may need to do several changes you didn't intend to
in order to do the error propagation right...

Yes.  An Error conversion can look simple at first, then balloon into a
multi-part series.


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

Any help with converting the remaining devices to realize() is much
appreciated.


Thanks for your detailed explanation, I think I know what to do.

Thanks,
Mao
































reply via email to

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