Re: [Qemu-devel] [PATCH] pci: Clean up error reporting in pci_nic_init()

[Thomas Huth <thuth@redhat.com>]

On Tue, 28 Apr 2015 10:40:09 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Apr 28, 2015 at 09:26:47AM +0200, Thomas Huth wrote:
> > On Mon, 27 Apr 2015 10:55:41 -0700
> > Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> > 
> > > On Mon, Apr 27, 2015 at 10:51 AM, Thomas Huth <thuth@redhat.com> wrote:
> > > > The error reporting in pci_nic_init() is quite erratic: Some errors
> > > > are printed directly with error_report(), and some are passed back
> > > > to the (only) caller pci_nic_init_nofail() via an Error pointer.
> > > > Let's fix up this inconsistency by always printing the error in
> > > > pci_nic_init() and by getting rid of the Error pointer this way.
> > > >
> > > 
> > > Can it be made consistent the other way? - always propagate? Usually
> > > we move towards consistent error propagation rather that
> > > deep-call-chain error reports.
> > 
> > In that case, I'd need to rework qemu_find_nic_model() in net/net.c,
> > too, since this is also printing errors directly. ... I could certainly
> > do that, too, but I slowly start wondering whether this all is worth
> > the effort, just to make the code for the _legacy_ "-net" option a
> > little bit nicer. Maybe we should rather focus on thinking about ways
> > to finally get rid of "-net" one day?
> 
> Yes please.
> I think what it would take is supporting dump option in netdevs.
> Need some careful coding to bail out if people try to use
> this with vhost.
> Want to work on it?

Sure, I can have a look.

> > Another idea: What about merging pci_nic_init() into
> > pci_nic_init_no_fail()? pci_nic_init() is only used by the _no_fail()
> > function, so there seems very few benefit by having this code in a
> > separate function. If they got merged, the error printing inconsistency
> > would be solved, too.
> > 
> >  Thomas
> 
> This later one sounds good to me.

Patch sent.

 Thomas