* [Qemu-devel] [PATCH V2 0/2] qcow2: try cancel the modification on fail in snapshot creation @ 2013-05-17 7:56 Wenchao Xia 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 1/2] qcow2: free allocated cluster on fail in qcow2_write_snapshots() Wenchao Xia 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 2/2] qcow2: cancel the modification on fail in qcow2_snapshot_create() Wenchao Xia 0 siblings, 2 replies; 6+ messages in thread From: Wenchao Xia @ 2013-05-17 7:56 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, pbonzini, Wenchao Xia, stefanha V2: 1/2: all fail case will goto fail section. 2/2: add the goto code. Wenchao Xia (2): 1 qcow2: free allocated cluster on fail in qcow2_write_snapshots() 2 qcow2: cancel the modification on fail in qcow2_snapshot_create() block/qcow2-snapshot.c | 27 ++++++++++++++++++++------- 1 files changed, 20 insertions(+), 7 deletions(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH V2 1/2] qcow2: free allocated cluster on fail in qcow2_write_snapshots() 2013-05-17 7:56 [Qemu-devel] [PATCH V2 0/2] qcow2: try cancel the modification on fail in snapshot creation Wenchao Xia @ 2013-05-17 7:56 ` Wenchao Xia 2013-05-17 9:19 ` Stefan Hajnoczi 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 2/2] qcow2: cancel the modification on fail in qcow2_snapshot_create() Wenchao Xia 1 sibling, 1 reply; 6+ messages in thread From: Wenchao Xia @ 2013-05-17 7:56 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, pbonzini, Wenchao Xia, stefanha Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- block/qcow2-snapshot.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 992a5c8..45da32d 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -180,13 +180,13 @@ static int qcow2_write_snapshots(BlockDriverState *bs) /* Allocate space for the new snapshot list */ snapshots_offset = qcow2_alloc_clusters(bs, snapshots_size); - offset = snapshots_offset; - if (offset < 0) { - return offset; + ret = offset = snapshots_offset; + if (ret < 0) { + goto fail; } ret = bdrv_flush(bs); if (ret < 0) { - return ret; + goto fail; } /* Write all snapshots to the new list */ @@ -268,6 +268,8 @@ static int qcow2_write_snapshots(BlockDriverState *bs) return 0; fail: + /* free the new snapshot table */ + qcow2_free_clusters(bs, snapshots_offset, snapshots_size); return ret; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V2 1/2] qcow2: free allocated cluster on fail in qcow2_write_snapshots() 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 1/2] qcow2: free allocated cluster on fail in qcow2_write_snapshots() Wenchao Xia @ 2013-05-17 9:19 ` Stefan Hajnoczi 0 siblings, 0 replies; 6+ messages in thread From: Stefan Hajnoczi @ 2013-05-17 9:19 UTC (permalink / raw) To: Wenchao Xia; +Cc: kwolf, pbonzini, qemu-devel On Fri, May 17, 2013 at 03:56:44PM +0800, Wenchao Xia wrote: > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > block/qcow2-snapshot.c | 10 ++++++---- > 1 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c > index 992a5c8..45da32d 100644 > --- a/block/qcow2-snapshot.c > +++ b/block/qcow2-snapshot.c > @@ -180,13 +180,13 @@ static int qcow2_write_snapshots(BlockDriverState *bs) > > /* Allocate space for the new snapshot list */ > snapshots_offset = qcow2_alloc_clusters(bs, snapshots_size); > - offset = snapshots_offset; > - if (offset < 0) { > - return offset; > + ret = offset = snapshots_offset; > + if (ret < 0) { > + goto fail; > } If qcow2_alloc_clusters() failed then we must not call qcow2_free_clusters(). ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH V2 2/2] qcow2: cancel the modification on fail in qcow2_snapshot_create() 2013-05-17 7:56 [Qemu-devel] [PATCH V2 0/2] qcow2: try cancel the modification on fail in snapshot creation Wenchao Xia 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 1/2] qcow2: free allocated cluster on fail in qcow2_write_snapshots() Wenchao Xia @ 2013-05-17 7:56 ` Wenchao Xia 2013-05-17 9:27 ` Stefan Hajnoczi 1 sibling, 1 reply; 6+ messages in thread From: Wenchao Xia @ 2013-05-17 7:56 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, pbonzini, Wenchao Xia, stefanha Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- block/qcow2-snapshot.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 45da32d..033f705 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -367,7 +367,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table, s->l1_size * sizeof(uint64_t)); if (ret < 0) { - goto fail; + goto dealloc_cluster; } g_free(l1_table); @@ -380,7 +380,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) */ ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 1); if (ret < 0) { - goto fail; + goto dealloc_cluster; } /* Append the new snapshot to the snapshot list */ @@ -397,7 +397,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) if (ret < 0) { g_free(s->snapshots); s->snapshots = old_snapshot_list; - goto fail; + goto restore_refcount; } g_free(old_snapshot_list); @@ -410,6 +410,17 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) #endif return 0; +restore_refcount: + ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, + s->l1_size, -1); + if (ret < 0) { + /* Nothing can be done none now, need image check later */ + error_report("qcow2: Error in restoring refcount in snapshot"); + } + +dealloc_cluster: + qcow2_free_clusters(bs, sn->l1_table_offset, sn->l1_size); + fail: g_free(sn->id_str); g_free(sn->name); -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V2 2/2] qcow2: cancel the modification on fail in qcow2_snapshot_create() 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 2/2] qcow2: cancel the modification on fail in qcow2_snapshot_create() Wenchao Xia @ 2013-05-17 9:27 ` Stefan Hajnoczi 2013-05-20 2:17 ` Wenchao Xia 0 siblings, 1 reply; 6+ messages in thread From: Stefan Hajnoczi @ 2013-05-17 9:27 UTC (permalink / raw) To: Wenchao Xia; +Cc: kwolf, pbonzini, qemu-devel On Fri, May 17, 2013 at 03:56:45PM +0800, Wenchao Xia wrote: > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > block/qcow2-snapshot.c | 17 ++++++++++++++--- > 1 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c > index 45da32d..033f705 100644 > --- a/block/qcow2-snapshot.c > +++ b/block/qcow2-snapshot.c > @@ -367,7 +367,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) > ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table, > s->l1_size * sizeof(uint64_t)); > if (ret < 0) { > - goto fail; > + goto dealloc_cluster; > } > > g_free(l1_table); > @@ -380,7 +380,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) > */ > ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 1); > if (ret < 0) { > - goto fail; > + goto dealloc_cluster; > } > > /* Append the new snapshot to the snapshot list */ > @@ -397,7 +397,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) > if (ret < 0) { > g_free(s->snapshots); > s->snapshots = old_snapshot_list; > - goto fail; > + goto restore_refcount; > } > > g_free(old_snapshot_list); > @@ -410,6 +410,17 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) > #endif > return 0; > > +restore_refcount: > + ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, > + s->l1_size, -1); > + if (ret < 0) { > + /* Nothing can be done none now, need image check later */ > + error_report("qcow2: Error in restoring refcount in snapshot"); > + } > + > +dealloc_cluster: > + qcow2_free_clusters(bs, sn->l1_table_offset, sn->l1_size); sn->l1_size is in uint64_t units, not in bytes. Do you have test cases to exercise these code paths? Use blkdebug to inject errors at the right point so that the error code path is taken. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH V2 2/2] qcow2: cancel the modification on fail in qcow2_snapshot_create() 2013-05-17 9:27 ` Stefan Hajnoczi @ 2013-05-20 2:17 ` Wenchao Xia 0 siblings, 0 replies; 6+ messages in thread From: Wenchao Xia @ 2013-05-20 2:17 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: kwolf, pbonzini, qemu-devel 于 2013-5-17 17:27, Stefan Hajnoczi 写道: > On Fri, May 17, 2013 at 03:56:45PM +0800, Wenchao Xia wrote: >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> --- >> block/qcow2-snapshot.c | 17 ++++++++++++++--- >> 1 files changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c >> index 45da32d..033f705 100644 >> --- a/block/qcow2-snapshot.c >> +++ b/block/qcow2-snapshot.c >> @@ -367,7 +367,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) >> ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table, >> s->l1_size * sizeof(uint64_t)); >> if (ret < 0) { >> - goto fail; >> + goto dealloc_cluster; >> } >> >> g_free(l1_table); >> @@ -380,7 +380,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) >> */ >> ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 1); >> if (ret < 0) { >> - goto fail; >> + goto dealloc_cluster; >> } >> >> /* Append the new snapshot to the snapshot list */ >> @@ -397,7 +397,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) >> if (ret < 0) { >> g_free(s->snapshots); >> s->snapshots = old_snapshot_list; >> - goto fail; >> + goto restore_refcount; >> } >> >> g_free(old_snapshot_list); >> @@ -410,6 +410,17 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) >> #endif >> return 0; >> >> +restore_refcount: >> + ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, >> + s->l1_size, -1); >> + if (ret < 0) { >> + /* Nothing can be done none now, need image check later */ >> + error_report("qcow2: Error in restoring refcount in snapshot"); >> + } >> + >> +dealloc_cluster: >> + qcow2_free_clusters(bs, sn->l1_table_offset, sn->l1_size); > > sn->l1_size is in uint64_t units, not in bytes. > > Do you have test cases to exercise these code paths? Use blkdebug to > inject errors at the right point so that the error code path is taken. > OK, I'll try it. -- Best Regards Wenchao Xia ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-20 2:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-17 7:56 [Qemu-devel] [PATCH V2 0/2] qcow2: try cancel the modification on fail in snapshot creation Wenchao Xia 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 1/2] qcow2: free allocated cluster on fail in qcow2_write_snapshots() Wenchao Xia 2013-05-17 9:19 ` Stefan Hajnoczi 2013-05-17 7:56 ` [Qemu-devel] [PATCH V2 2/2] qcow2: cancel the modification on fail in qcow2_snapshot_create() Wenchao Xia 2013-05-17 9:27 ` Stefan Hajnoczi 2013-05-20 2:17 ` Wenchao Xia
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).