qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] pci: Clean up error reporting in pci_nic_init()
Date: Tue, 28 Apr 2015 10:40:09 +0200	[thread overview]
Message-ID: <20150428103810-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <20150428092647.33360532@thh440s>

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?

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

-- 
MST

  reply	other threads:[~2015-04-28  8:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-27 17:51 [Qemu-devel] [PATCH] pci: Clean up error reporting in pci_nic_init() Thomas Huth
2015-04-27 17:55 ` Peter Crosthwaite
2015-04-28  7:26   ` Thomas Huth
2015-04-28  8:40     ` Michael S. Tsirkin [this message]
2015-04-28 10:56       ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150428103810-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=armbru@redhat.com \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).