From patchwork Wed Apr 24 15:32:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Hrdina X-Patchwork-Id: 239237 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 8C6812C0111 for ; Thu, 25 Apr 2013 01:36:34 +1000 (EST) Received: from localhost ([::1]:49846 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV1kG-0003iW-P0 for incoming@patchwork.ozlabs.org; Wed, 24 Apr 2013 11:36:32 -0400 Received: from eggs.gnu.org ([208.118.235.92]:57815) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV1gZ-0006u5-L7 for qemu-devel@nongnu.org; Wed, 24 Apr 2013 11:32:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UV1gU-00044t-9O for qemu-devel@nongnu.org; Wed, 24 Apr 2013 11:32:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13019) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV1gT-00044P-Ky for qemu-devel@nongnu.org; Wed, 24 Apr 2013 11:32:37 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r3OFWPhM008034 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 24 Apr 2013 11:32:25 -0400 Received: from localhost.localdomain.com (dhcp-27-230.brq.redhat.com [10.34.27.230]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r3OFWDGM029227; Wed, 24 Apr 2013 11:32:23 -0400 From: Pavel Hrdina To: qemu-devel@nongnu.org Date: Wed, 24 Apr 2013 17:32:03 +0200 Message-Id: <057bb7e62d93ed0442c1fbf058c90c53e9931f49.1366817130.git.phrdina@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, phrdina@redhat.com, armbru@redhat.com, lcapitulino@redhat.com, xiawenc@linux.vnet.ibm.com Subject: [Qemu-devel] [PATCH v2 05/12] block: update error reporting for bdrv_snapshot_goto() and related functions X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Signed-off-by: Pavel Hrdina Reviewed-by: Eric Blake --- block.c | 34 ++++++++++++++++++---------------- block/qcow2-snapshot.c | 24 +++++++++++++++++------- block/qcow2.h | 4 +++- block/rbd.c | 10 +++++++--- block/sheepdog.c | 18 ++++++++---------- include/block/block.h | 5 +++-- include/block/block_int.h | 5 +++-- qemu-img.c | 7 ++----- savevm.c | 10 +++++----- 9 files changed, 66 insertions(+), 51 deletions(-) diff --git a/block.c b/block.c index 3f7ee38..c36d3d5 100644 --- a/block.c +++ b/block.c @@ -3412,30 +3412,32 @@ int bdrv_snapshot_create(BlockDriverState *bs, return -ENOTSUP; } -int bdrv_snapshot_goto(BlockDriverState *bs, - const char *snapshot_id) +void bdrv_snapshot_goto(BlockDriverState *bs, + const char *snapshot_id, + Error **errp) { BlockDriver *drv = bs->drv; - int ret, open_ret; - - if (!drv) - return -ENOMEDIUM; - if (drv->bdrv_snapshot_goto) - return drv->bdrv_snapshot_goto(bs, snapshot_id); + int ret; - if (bs->file) { + if (!drv) { + error_setg(errp, "Device has no medium"); + } else if (drv->bdrv_snapshot_goto) { + drv->bdrv_snapshot_goto(bs, snapshot_id, errp); + } else if (bs->file) { drv->bdrv_close(bs); - ret = bdrv_snapshot_goto(bs->file, snapshot_id); - open_ret = drv->bdrv_open(bs, NULL, bs->open_flags); - if (open_ret < 0) { + bdrv_snapshot_goto(bs->file, snapshot_id, errp); + ret = drv->bdrv_open(bs, NULL, bs->open_flags); + if (ret < 0) { bdrv_delete(bs->file); bs->drv = NULL; - return open_ret; + if (!error_is_set(errp)) { + error_setg_errno(errp, -ret, "Failed to open '%s'", + bdrv_get_device_name(bs)); + } } - return ret; + } else { + error_setg(errp, "Snapshots are not supported"); } - - return -ENOTSUP; } void bdrv_snapshot_delete(BlockDriverState *bs, diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index fd0e231..67a57fc 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -417,7 +417,9 @@ fail: } /* copy the snapshot 'snapshot_name' into the current disk image */ -int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) +void qcow2_snapshot_goto(BlockDriverState *bs, + const char *snapshot_id, + Error **errp) { BDRVQcowState *s = bs->opaque; QCowSnapshot *sn; @@ -429,14 +431,15 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) /* Search the snapshot */ snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id); if (snapshot_index < 0) { - return -ENOENT; + error_setg(errp, "Failed to find snapshot '%s' by id or name", + snapshot_id); + return; } sn = &s->snapshots[snapshot_index]; if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) { - error_report("qcow2: Loading snapshots with different disk " - "size is not implemented"); - ret = -ENOTSUP; + error_setg(errp, "Loading snapshots with different disk size is not " + "implemented"); goto fail; } @@ -447,6 +450,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) */ ret = qcow2_grow_l1_table(bs, sn->l1_size, true); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to grow L1 table"); goto fail; } @@ -465,18 +469,22 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) ret = bdrv_pread(bs->file, sn->l1_table_offset, sn_l1_table, sn_l1_bytes); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to load data into L1 table"); goto fail; } ret = qcow2_update_snapshot_refcount(bs, sn->l1_table_offset, sn->l1_size, 1); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to increase the cluster refcounts " + "of the snapshot to load"); goto fail; } ret = bdrv_pwrite_sync(bs->file, s->l1_table_offset, sn_l1_table, cur_l1_bytes); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to save L1 table"); goto fail; } @@ -502,6 +510,8 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) } if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to decrease the cluster refcounts " + "of the old snapshot"); goto fail; } @@ -514,6 +524,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) */ ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to update the cluster flags"); goto fail; } @@ -523,11 +534,10 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) qcow2_check_refcounts(bs, &result, 0); } #endif - return 0; + return; fail: g_free(sn_l1_table); - return ret; } void qcow2_snapshot_delete(BlockDriverState *bs, diff --git a/block/qcow2.h b/block/qcow2.h index dbd332d..b70387c 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -383,7 +383,9 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors); /* qcow2-snapshot.c functions */ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info); -int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id); +void qcow2_snapshot_goto(BlockDriverState *bs, + const char *snapshot_id, + Error **errp); void qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id, Error **errp); diff --git a/block/rbd.c b/block/rbd.c index 1e54c55..5047b59 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -913,14 +913,18 @@ static void qemu_rbd_snap_remove(BlockDriverState *bs, } } -static int qemu_rbd_snap_rollback(BlockDriverState *bs, - const char *snapshot_name) +static void qemu_rbd_snap_rollback(BlockDriverState *bs, + const char *snapshot_name, + Error **errp) { BDRVRBDState *s = bs->opaque; int r; r = rbd_snap_rollback(s->image, snapshot_name); - return r; + if (r < 0) { + error_setg_errno(errp, -r, "Failed to rollback snapshot '%s'", + snapshot_name); + } } static int qemu_rbd_snap_list(BlockDriverState *bs, diff --git a/block/sheepdog.c b/block/sheepdog.c index 7e0610f..868ab89 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1867,7 +1867,9 @@ cleanup: return ret; } -static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) +static void sd_snapshot_goto(BlockDriverState *bs, + const char *snapshot_id, + Error **errp) { BDRVSheepdogState *s = bs->opaque; BDRVSheepdogState *old_s; @@ -1892,13 +1894,13 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) ret = find_vdi_name(s, vdi, snapid, tag, &vid, 1); if (ret) { - error_report("Failed to find_vdi_name"); + error_setg_errno(errp, -ret, "Failed to find VDI snapshot '%s'", vdi); goto out; } fd = connect_to_sdog(s); if (fd < 0) { - ret = fd; + error_setg_errno(errp, -fd, "Failed to connect to sdog"); goto out; } @@ -1909,14 +1911,14 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) closesocket(fd); if (ret) { + error_setg_errno(errp, -ret, "Failed to open VDI snapshot '%s'", vdi); goto out; } memcpy(&s->inode, buf, sizeof(s->inode)); if (!s->inode.vm_state_size) { - error_report("Invalid snapshot"); - ret = -ENOENT; + error_setg(errp, "Invalid snapshot '%s'", snapshot_id); goto out; } @@ -1925,16 +1927,12 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) g_free(buf); g_free(old_s); - return 0; + return; out: /* recover bdrv_sd_state */ memcpy(s, old_s, sizeof(BDRVSheepdogState)); g_free(buf); g_free(old_s); - - error_report("failed to open. recover old bdrv_sd_state."); - - return ret; } static void sd_snapshot_delete(BlockDriverState *bs, diff --git a/include/block/block.h b/include/block/block.h index abb636d..3dccecc 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -335,8 +335,9 @@ int bdrv_is_snapshot(BlockDriverState *bs); BlockDriverState *bdrv_snapshots(void); int bdrv_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info); -int bdrv_snapshot_goto(BlockDriverState *bs, - const char *snapshot_id); +void bdrv_snapshot_goto(BlockDriverState *bs, + const char *snapshot_id, + Error **errp); void bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id, Error **errp); diff --git a/include/block/block_int.h b/include/block/block_int.h index c3f67c3..db67841 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -154,8 +154,9 @@ struct BlockDriver { int (*bdrv_snapshot_create)(BlockDriverState *bs, QEMUSnapshotInfo *sn_info); - int (*bdrv_snapshot_goto)(BlockDriverState *bs, - const char *snapshot_id); + void (*bdrv_snapshot_goto)(BlockDriverState *bs, + const char *snapshot_id, + Error **errp); void (*bdrv_snapshot_delete)(BlockDriverState *bs, const char *snapshot_id, Error **errp); diff --git a/qemu-img.c b/qemu-img.c index 9cf3467..4aca51f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2022,11 +2022,8 @@ static int img_snapshot(int argc, char **argv) break; case SNAPSHOT_APPLY: - ret = bdrv_snapshot_goto(bs, snapshot_name); - if (ret) { - error_report("Could not apply snapshot '%s': %d (%s)", - snapshot_name, ret, strerror(-ret)); - } + bdrv_snapshot_goto(bs, snapshot_name, &local_err); + ret = qemu_img_handle_error(local_err); break; case SNAPSHOT_DELETE: diff --git a/savevm.c b/savevm.c index 1b4aea8..f280aae 100644 --- a/savevm.c +++ b/savevm.c @@ -2529,11 +2529,11 @@ int load_vmstate(const char *name) bs = NULL; while ((bs = bdrv_next(bs))) { if (bdrv_can_snapshot(bs)) { - ret = bdrv_snapshot_goto(bs, name); - if (ret < 0) { - error_report("Error %d while activating snapshot '%s' on '%s'", - ret, name, bdrv_get_device_name(bs)); - return ret; + bdrv_snapshot_goto(bs, name, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + return -EIO; } } }