qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>, Kevin Wolf <kwolf@redhat.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 0/9] Miscellaneous error reporting improvements
Date: Fri, 29 May 2015 08:00:28 -0600	[thread overview]
Message-ID: <556870FC.3020806@redhat.com> (raw)
In-Reply-To: <877frr4oig.fsf@blackfin.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On 05/29/2015 05:22 AM, Markus Armbruster wrote:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
>> Am 28.05.2015 um 14:21 hat Markus Armbruster geschrieben:
>>> Touches vl.c, which gives me pretext to ask Paolo: would you be
>>> willing to take this through your tree?  Or should I take it through
>>> mine?
>>
>> After this series we have an ugly half-converted state where
>> qemu_opts_foreach() has both a return code and an Error object,
>> and it's not generally true that an error is set for a failing
>> return code.
>>
>> The most confusing part about this is that you have &error_abort almost
>> everywhere, but the function doesn't actually abort on error, but rather
>> returns a negative error code and leaves errp alone.
> 
> True.  The function contract spells it out, which hopefully reduces the
> confusion somewhat.

Except that you don't enforce the contract; I suggested adding
assert(!*errp) at the right place in the two conversions.

> 
> Would you find NULL less confusing than &error_abort?

NULL says to ignore errors, &error_abort says to diagnose errors as
programming bugs.  If we know we aren't going to have an error, I prefer
diagnosing coding bugs.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  parent reply	other threads:[~2015-05-29 14:00 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-28 12:21 [Qemu-devel] [PATCH 0/9] Miscellaneous error reporting improvements Markus Armbruster
2015-05-28 12:21 ` [Qemu-devel] [PATCH 1/9] vl: Report failure to sandbox at most once Markus Armbruster
2015-05-28 14:24   ` Eric Blake
2015-05-28 12:21 ` [Qemu-devel] [PATCH 2/9] vl: Print -device help " Markus Armbruster
2015-05-28 14:47   ` Eric Blake
2015-05-28 12:21 ` [Qemu-devel] [PATCH 3/9] vl: Fail right after first bad -object Markus Armbruster
2015-05-28 14:52   ` Eric Blake
2015-06-02  8:41     ` Markus Armbruster
2015-05-28 12:21 ` [Qemu-devel] [PATCH 4/9] QemuOpts: Drop qemu_opts_foreach() parameter abort_on_failure Markus Armbruster
2015-05-28 15:10   ` Eric Blake
2015-06-02  8:42     ` Markus Armbruster
2015-05-28 12:21 ` [Qemu-devel] [PATCH 5/9] QemuOpts: Convert qemu_opts_foreach() to Error Markus Armbruster
2015-05-28 16:18   ` Eric Blake
2015-06-02 11:33     ` Markus Armbruster
2015-06-02 12:34       ` Eric Blake
2015-06-02 14:13         ` Paolo Bonzini
2015-05-28 12:21 ` [Qemu-devel] [PATCH 6/9] blkdebug: Simplify passing of Error through qemu_opts_foreach() Markus Armbruster
2015-05-28 17:15   ` Eric Blake
2015-05-28 12:21 ` [Qemu-devel] [PATCH 7/9] QemuOpts: Drop qemu_opt_foreach() parameter abort_on_failure Markus Armbruster
2015-05-28 18:57   ` Eric Blake
2015-05-28 12:21 ` [Qemu-devel] [PATCH 8/9] QemuOpts: Convert qemu_opt_foreach() to Error Markus Armbruster
2015-05-28 19:07   ` Eric Blake
2015-05-28 12:21 ` [Qemu-devel] [PATCH 9/9] vhost-user: Improve -netdev/netdev_add/-net/... error reporting Markus Armbruster
2015-05-28 19:20   ` Eric Blake
2015-06-02 16:32   ` Stefan Hajnoczi
2015-05-29  8:51 ` [Qemu-devel] [PATCH 0/9] Miscellaneous error reporting improvements Kevin Wolf
2015-05-29 11:22   ` Markus Armbruster
2015-05-29 12:42     ` Kevin Wolf
2015-05-29 14:00     ` Eric Blake [this message]
2015-06-02 11:51       ` Markus Armbruster
2015-06-02 12:51         ` Eric Blake

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=556870FC.3020806@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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).