| Message ID | 1368777405-8750-3-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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.
δΊ 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.
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);
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- block/qcow2-snapshot.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-)