From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dEf5h-0000nJ-Nk for qemu-devel@nongnu.org; Sat, 27 May 2017 13:01:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dEf5e-0007BO-M0 for qemu-devel@nongnu.org; Sat, 27 May 2017 13:01:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44202) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dEf5e-0007As-Co for qemu-devel@nongnu.org; Sat, 27 May 2017 13:01:22 -0400 References: <20170526121516.6607-1-maozy.fnst@cn.fujitsu.com> <5ab8c115-a8de-1d48-155a-3daac7e0d841@cn.fujitsu.com> From: Marcel Apfelbaum Message-ID: <19dc61d7-9779-22a6-03cc-0eb718fe2875@redhat.com> Date: Sat, 27 May 2017 20:01:13 +0300 MIME-Version: 1.0 In-Reply-To: <5ab8c115-a8de-1d48-155a-3daac7e0d841@cn.fujitsu.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [Qemu-devel] [PATCH] pci-bridge/i82801b11: Convert to realize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mao Zhongyi , qemu-devel@nongnu.org Cc: mst@redhat.com, armbru@redhat.com 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 >>> --- >>> 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. 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. You may need to do several changes you didn't intend to in order to do the error propagation right... 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 > > > > > > > > > > > > > > > > > > > >> > >