[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 01/12] block: add error parameter to bdrv_sna
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 01/12] block: add error parameter to bdrv_snapshot_create() and related functions |
Date: |
Tue, 26 Mar 2013 08:22:50 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 |
On 03/22/2013 07:16 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <address@hidden>
> ---
> block.c | 22 ++++++++++++++++------
> block/qcow2-snapshot.c | 9 ++++++++-
> block/qcow2.h | 4 +++-
> block/rbd.c | 8 ++++++--
> block/sheepdog.c | 17 +++++++++--------
> include/block/block.h | 3 ++-
> include/block/block_int.h | 3 ++-
> qemu-img.c | 2 +-
> savevm.c | 2 +-
> 9 files changed, 48 insertions(+), 22 deletions(-)
>
> int bdrv_snapshot_create(BlockDriverState *bs,
> - QEMUSnapshotInfo *sn_info)
> + QEMUSnapshotInfo *sn_info,
> + Error **errp)
> {
> BlockDriver *drv = bs->drv;
> - if (!drv)
> +
> + if (!drv) {
> + error_setg(errp, "Device '%s' has no medium.",
In general, error_setg() should not print a trailing '.' (only two
offenders to git grep 'error_setg.*\."'). I think we also tend to start
messages with lower case, although that's not as obvious (49 cases of
'error_setg[^"*"[A-Z]' vs. 121 of error_setg[^"]*"[a-z]').
> +
> + error_setg(errp, "Snapshot is not supported for '%s'.",
And again, and probably throughout your series (although I'll quit
pointing it out).
> @@ -830,16 +832,18 @@ static int qemu_rbd_snap_create(BlockDriverState *bs,
> */
> if (sn_info->id_str[0] != '\0' &&
> strcmp(sn_info->id_str, sn_info->name) != 0) {
> + error_setg(errp, "ID and name have to be equal.");
> return -EINVAL;
> }
>
> if (strlen(sn_info->name) >= sizeof(sn_info->id_str)) {
> + error_setg(errp, "Parameter 'name' has to be shorter that 127
> chars.");
s/that/than/
> return -ERANGE;
> }
>
> r = rbd_snap_create(s->image, sn_info->name);
> if (r < 0) {
> - error_report("failed to create snap: %s", strerror(-r));
> + error_setg(errp, "Failed to create snapshot: '%s'.", strerror(-r));
Use error_setg_errno(errp, -r, "failed to create snapshot").
> @@ -1779,9 +1781,8 @@ static int sd_snapshot_create(BlockDriverState *bs,
> QEMUSnapshotInfo *sn_info)
> s->name, sn_info->vm_state_size, s->is_snapshot);
>
> if (s->is_snapshot) {
> - error_report("You can't create a snapshot of a snapshot VDI, "
> - "%s (%" PRIu32 ").", s->name, s->inode.vdi_id);
> -
> + error_setg(errp, "You can't create a snapshot '%s' of a VDI
> snapshot.",
> + sn_info->name);
Why are you losing information from the previous version of this error
message?
> if (ret < 0) {
> - error_report("failed to create inode for snapshot. %s",
> - strerror(errno));
> + error_setg(errp, "Failed to create inode for snapshot.");
Another case of losing information; error_setg_errno would be better
here, and anywhere else the old message included a strerror() call.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 00/12] convert savevm to use qapi and introduce qmp command, Pavel Hrdina, 2013/03/22
- [Qemu-devel] [PATCH v2 01/12] block: add error parameter to bdrv_snapshot_create() and related functions, Pavel Hrdina, 2013/03/22
- Re: [Qemu-devel] [PATCH v2 01/12] block: add error parameter to bdrv_snapshot_create() and related functions,
Eric Blake <=
- [Qemu-devel] [PATCH v2 02/12] block: add error parameter to del_existing_snapshots(), Pavel Hrdina, 2013/03/22
- [Qemu-devel] [PATCH v2 03/12] savevm: add error parameter to qemu_savevm_state_begin(), Pavel Hrdina, 2013/03/22
- [Qemu-devel] [PATCH v2 05/12] savevm: add error parameter to qemu_savevm_state_complete(), Pavel Hrdina, 2013/03/22
- [Qemu-devel] [PATCH v2 04/12] savevm: add error parameter to qemu_savevm_state_iterate(), Pavel Hrdina, 2013/03/22
- [Qemu-devel] [PATCH v2 07/12] qapi: Convert savevm, Pavel Hrdina, 2013/03/22