qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Juan Quintela <quintela@redhat.com>
Cc: lvivier@redhat.com, qemu-devel@nongnu.org, peterx@redhat.com
Subject: Re: [Qemu-devel] [PATCH 6/6] migration: Pass Error ** argument to {save, load}_vmstate
Date: Fri, 28 Apr 2017 10:20:40 -0500	[thread overview]
Message-ID: <04eb0f88-4e4b-ffcf-8f5a-e1d5f3414ce9@redhat.com> (raw)
In-Reply-To: <20170428144729.GB3276@work-vm>

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

On 04/28/2017 09:47 AM, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> This way we use the "normal" way of printing errors for hmp commands.
>>
>> --

>>      if (!bdrv_all_can_snapshot(&bs)) {
>> -        error_report("Device '%s' is writable but does not support snapshots",
>> -                     bdrv_get_device_name(bs));
>> +        error_setg(errp, "Device '%s' is writable but does not support "
>> +                   "snapshots", bdrv_get_device_name(bs));
>>          return ret;
>>      }
>>  
>>      /* Delete old snapshots of the same name */
>>      if (name) {
>> -        ret = bdrv_all_delete_snapshot(name, &bs1, &local_err);
>> +        ret = bdrv_all_delete_snapshot(name, &bs1, errp);
>>          if (ret < 0) {
>> -            error_reportf_err(local_err,
>> -                              "Error while deleting snapshot on device '%s': ",
>> -                              bdrv_get_device_name(bs1));
>> +            error_prepend(errp, "Error while deleting snapshot on device "
>> +                          "'%s': ", bdrv_get_device_name(bs1));
> 
> I thought the rule was that you had to use a local_err and use error_propagate?
> (I hate that rule)

The rule is that if you want to make code conditional on whether an
error occurred, you can't do it by checking errp (because the caller may
have passed NULL), so you instead have to use local_err and
error_propagate(). But if there are other ways to tell if an error
occurred (such as a return value), then passing errp directly through is
fine (error_prepend does the right thing even if errp is NULL because
the caller doesn't care about an error).

include/qapi/error.h has some good examples, and if it is missing
something, we'd be glad to patch it further to serve as a good reference.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

  parent reply	other threads:[~2017-04-28 15:20 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-25 10:24 [Qemu-devel] [PATCH 0/6] Move snapshots commands to hmp Juan Quintela
2017-04-25 10:24 ` [Qemu-devel] [PATCH 1/6] monitor: Remove monitor parameter from save_vmstate Juan Quintela
2017-04-25 13:27   ` Laurent Vivier
2017-04-25 10:24 ` [Qemu-devel] [PATCH 2/6] monitor: Move hmp_loadvm from monitor.c to hmp.c Juan Quintela
2017-04-25 13:37   ` Laurent Vivier
2017-04-25 10:24 ` [Qemu-devel] [PATCH 3/6] monitor: Move hmp_savevm from savevm.c " Juan Quintela
2017-04-25 13:33   ` Laurent Vivier
2017-04-25 10:24 ` [Qemu-devel] [PATCH 4/6] monitor: Move hmp_delvm " Juan Quintela
2017-04-25 13:34   ` Laurent Vivier
2017-04-25 10:24 ` [Qemu-devel] [PATCH 5/6] monitor: Move hmp_info_snapshots " Juan Quintela
2017-04-25 14:15   ` Laurent Vivier
2017-04-25 16:48     ` Juan Quintela
2017-04-25 10:24 ` [Qemu-devel] [PATCH 6/6] migration: Pass Error ** argument to {save, load}_vmstate Juan Quintela
2017-04-25 15:21   ` Laurent Vivier
2017-04-25 17:00     ` Juan Quintela
2017-04-25 17:10       ` Laurent Vivier
2017-04-25 17:31         ` Juan Quintela
2017-04-28 14:47   ` Dr. David Alan Gilbert
2017-04-28 15:19     ` Markus Armbruster
2017-04-28 16:13       ` Dr. David Alan Gilbert
2017-04-28 15:20     ` Eric Blake [this message]
2017-04-28 15:07 ` [Qemu-devel] [PATCH 0/6] Move snapshots commands to hmp Dr. David Alan Gilbert

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=04eb0f88-4e4b-ffcf-8f5a-e1d5f3414ce9@redhat.com \
    --to=eblake@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@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).