From patchwork Fri Jan 3 03:08:49 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wayne Xia X-Patchwork-Id: 306560 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id D21B42C00A8 for ; Fri, 3 Jan 2014 22:13:44 +1100 (EST) Received: from localhost ([::1]:49226 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vz2hC-0002B8-D7 for incoming@patchwork.ozlabs.org; Fri, 03 Jan 2014 06:13:42 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35248) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vz2ep-0007gt-Kv for qemu-devel@nongnu.org; Fri, 03 Jan 2014 06:11:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vz2ec-0003fK-OH for qemu-devel@nongnu.org; Fri, 03 Jan 2014 06:11:15 -0500 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:44077) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vz2ea-0003eG-Em for qemu-devel@nongnu.org; Fri, 03 Jan 2014 06:11:02 -0500 Received: from /spool/local by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 3 Jan 2014 16:40:51 +0530 Received: from d28dlp02.in.ibm.com (9.184.220.127) by e28smtp06.in.ibm.com (192.168.1.136) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 3 Jan 2014 16:40:49 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 5D04B394004E for ; Fri, 3 Jan 2014 16:40:49 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s03BAki237224698 for ; Fri, 3 Jan 2014 16:40:46 +0530 Received: from d28av02.in.ibm.com (localhost [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s03BAmUR014219 for ; Fri, 3 Jan 2014 16:40:48 +0530 Received: from RH64wenchao ([9.181.129.59]) by d28av02.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s03BAedj013788; Fri, 3 Jan 2014 16:40:47 +0530 From: Wenchao Xia To: qemu-devel@nongnu.org Date: Fri, 3 Jan 2014 11:08:49 +0800 Message-Id: <1388718532-18264-6-git-send-email-xiawenc@linux.vnet.ibm.com> X-Mailer: git-send-email 1.7.1 In-Reply-To: <1388718532-18264-1-git-send-email-xiawenc@linux.vnet.ibm.com> References: <1388718532-18264-1-git-send-email-xiawenc@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14010311-9574-0000-0000-00000B515BEC X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 122.248.162.6 Cc: kwolf@redhat.com, jcody@redhat.com, Wenchao Xia , stefanha@redhat.com, mreitz@redhat.com Subject: [Qemu-devel] [PATCH V8 5/8] qcow2: full rollback on fail in qcow2_write_snapshots() 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 Header restore step is added, and old code section "fail" is renamed to "dealloc_sn_table" to tip better, now "fail" section does not rollback anything on disk. When one step in rollback fails, following steps will be skipped, to make sure no dangling pointer is left. A new parameter "*errp_rollback" is added to tell caller the result of rollback procedure. Signed-off-by: Wenchao Xia --- block/qcow2-snapshot.c | 63 +++++++++++++++++++++++++++++++++++------------ 1 files changed, 47 insertions(+), 16 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 5619fc3..5cac714 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -152,7 +152,9 @@ fail: } /* add at the end of the file a new list of snapshots */ -static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) +static int qcow2_write_snapshots(BlockDriverState *bs, + Error **errp, + Error **errp_rollback) { BDRVQcowState *s = bs->opaque; QCowSnapshot *sn; @@ -162,9 +164,9 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) struct { uint32_t nb_snapshots; uint64_t snapshots_offset; - } QEMU_PACKED header_data; + } QEMU_PACKED header_data, header_data_old; int64_t offset, snapshots_offset; - int ret; + int ret, ret0; /* compute the size of the snapshots */ offset = 0; @@ -192,7 +194,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) error_setg_errno(errp, -ret, "Failed in flush after snapshot list cluster " "allocation"); - goto fail; + goto dealloc_sn_table; } /* The snapshot list position has not yet been updated, so these clusters @@ -203,7 +205,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) "Failed in overlap check for snapshot list cluster " "at %" PRIi64 " with size %d", offset, snapshots_size); - goto fail; + goto dealloc_sn_table; } @@ -240,7 +242,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) "Failed in write of snapshot header at %" PRIi64 " with size %zu", offset, sizeof(h)); - goto fail; + goto dealloc_sn_table; } offset += sizeof(h); @@ -250,7 +252,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) "Failed in write of extra snapshot data at %" PRIi64 " with size %zu", offset, sizeof(extra)); - goto fail; + goto dealloc_sn_table; } offset += sizeof(extra); @@ -260,7 +262,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) "Failed in write of snapshot id string at %" PRIi64 " with size %d", offset, id_str_size); - goto fail; + goto dealloc_sn_table; } offset += id_str_size; @@ -270,7 +272,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) "Failed in write of snapshot name string at %" PRIi64 " with size %d", offset, name_size); - goto fail; + goto dealloc_sn_table; } offset += name_size; } @@ -283,7 +285,18 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) if (ret < 0) { error_setg_errno(errp, -ret, "Failed in flush after snapshot list update"); - goto fail; + goto dealloc_sn_table; + } + + /* Start the qcow2 header update */ + ret = bdrv_pread(bs->file, offsetof(QCowHeader, nb_snapshots), + &header_data_old, sizeof(header_data_old)); + if (ret < 0) { + error_setg_errno(errp, -ret, + "Failed in read of image header at %zu with size %zu", + offsetof(QCowHeader, nb_snapshots), + sizeof(header_data_old)); + goto dealloc_sn_table; } QEMU_BUILD_BUG_ON(offsetof(QCowHeader, snapshots_offset) != @@ -299,7 +312,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) "Failed in update of image header at %zu with size %zu", offsetof(QCowHeader, nb_snapshots), sizeof(header_data)); - goto fail; + goto restore_header; } /* free the old snapshot table */ @@ -309,11 +322,29 @@ static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp) s->snapshots_size = snapshots_size; return 0; -fail: +restore_header: + ret0 = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots), + &header_data_old, sizeof(header_data_old)); + if (ret0 < 0) { + error_setg_errno(errp_rollback, -ret0, + "In rollback failed to restore image header at %zu " + "with size %zu", + offsetof(QCowHeader, nb_snapshots), + sizeof(header_data)); + goto fail; + } + +dealloc_sn_table: if (snapshots_offset > 0) { - qcow2_free_clusters(bs, snapshots_offset, snapshots_size, - QCOW2_DISCARD_ALWAYS); + ret0 = qcow2_free_clusters(bs, snapshots_offset, snapshots_size, + QCOW2_DISCARD_ALWAYS); + if (ret0 < 0) { + error_setg_errno(errp_rollback, -ret0, + "In rollback failed to free snapshot table"); + } } + +fail: if (errp) { g_assert(error_is_set(errp)); } @@ -481,7 +512,7 @@ void qcow2_snapshot_create(BlockDriverState *bs, s->snapshots = new_snapshot_list; s->snapshots[s->nb_snapshots++] = *sn; - ret = qcow2_write_snapshots(bs, errp); + ret = qcow2_write_snapshots(bs, errp, NULL); if (ret < 0) { g_free(s->snapshots); s->snapshots = old_snapshot_list; @@ -656,7 +687,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, s->snapshots + snapshot_index + 1, (s->nb_snapshots - snapshot_index - 1) * sizeof(sn)); s->nb_snapshots--; - ret = qcow2_write_snapshots(bs, errp); + ret = qcow2_write_snapshots(bs, errp, NULL); if (ret < 0) { return ret; }