Re: [Qemu-devel] [PATCH V9 1/8] snapshot: add parameter *errp in snapshot create

2014-01-11 Thread Max Reitz

On 05.01.2014 20:43, Wenchao Xia wrote:

The return value is only used for error report before this patch,
so change the function protype to return void.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
  block/qcow2-snapshot.c|   30 +-
  block/qcow2.h |4 +++-
  block/rbd.c   |   19 ++-
  block/sheepdog.c  |   28 ++--
  block/snapshot.c  |   19 +--
  blockdev.c|   10 --
  include/block/block_int.h |5 +++--
  include/block/snapshot.h  |5 +++--
  qemu-img.c|   10 ++
  savevm.c  |   12 
  10 files changed, 93 insertions(+), 49 deletions(-)


Reviewed-by: Max Reitz mre...@redhat.com



[Qemu-devel] [PATCH V9 1/8] snapshot: add parameter *errp in snapshot create

2014-01-05 Thread Wenchao Xia
The return value is only used for error report before this patch,
so change the function protype to return void.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 block/qcow2-snapshot.c|   30 +-
 block/qcow2.h |4 +++-
 block/rbd.c   |   19 ++-
 block/sheepdog.c  |   28 ++--
 block/snapshot.c  |   19 +--
 blockdev.c|   10 --
 include/block/block_int.h |5 +++--
 include/block/snapshot.h  |5 +++--
 qemu-img.c|   10 ++
 savevm.c  |   12 
 10 files changed, 93 insertions(+), 49 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index ad8bf3d..8cac42d 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -347,7 +347,9 @@ static int find_snapshot_by_id_or_name(BlockDriverState *bs,
 }
 
 /* if no id is provided, a new one is constructed */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp)
 {
 BDRVQcowState *s = bs-opaque;
 QCowSnapshot *new_snapshot_list = NULL;
@@ -366,7 +368,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 /* Check that the ID is unique */
 if (find_snapshot_by_id_and_name(bs, sn_info-id_str, NULL) = 0) {
-return -EEXIST;
+error_setg(errp, Snapshot with id %s already exists, 
sn_info-id_str);
+return;
 }
 
 /* Populate sn with passed data */
@@ -382,7 +385,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 /* Allocate the L1 table of the snapshot and copy the current one there. */
 l1_table_offset = qcow2_alloc_clusters(bs, s-l1_size * sizeof(uint64_t));
 if (l1_table_offset  0) {
-ret = l1_table_offset;
+error_setg_errno(errp, -l1_table_offset,
+ Failed in allocation of snapshot L1 table);
 goto fail;
 }
 
@@ -397,12 +401,22 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 ret = qcow2_pre_write_overlap_check(bs, 0, sn-l1_table_offset,
 s-l1_size * sizeof(uint64_t));
 if (ret  0) {
+error_setg_errno(errp, -ret,
+ Failed in overlap check for snapshot L1 table at %
+ PRIu64  with size % PRIu64,
+ sn-l1_table_offset,
+ (uint64_t)(s-l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
 ret = bdrv_pwrite(bs-file, sn-l1_table_offset, l1_table,
   s-l1_size * sizeof(uint64_t));
 if (ret  0) {
+error_setg_errno(errp, -ret,
+ Failed in update of snapshot L1 table at %
+ PRIu64  with size % PRIu64,
+ sn-l1_table_offset,
+ (uint64_t)(s-l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
@@ -416,6 +430,10 @@ 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) {
+error_setg_errno(errp, -ret,
+ Failed in update of refcount for snapshot at %
+ PRIu64  with size %d,
+ s-l1_table_offset, s-l1_size);
 goto fail;
 }
 
@@ -431,6 +449,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 ret = qcow2_write_snapshots(bs);
 if (ret  0) {
+/* Following line will be replaced with more detailed error later */
+error_setg(errp, Failed in write of snapshot);
 g_free(s-snapshots);
 s-snapshots = old_snapshot_list;
 s-nb_snapshots--;
@@ -452,14 +472,14 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
   qcow2_check_refcounts(bs, result, 0);
 }
 #endif
-return 0;
+return;
 
 fail:
 g_free(sn-id_str);
 g_free(sn-name);
 g_free(l1_table);
 
-return ret;
+return;
 }
 
 /* copy the snapshot 'snapshot_name' into the current disk image */
diff --git a/block/qcow2.h b/block/qcow2.h
index 303eb26..c56a5b6 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -481,7 +481,9 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t 
offset, int nb_sectors);
 int qcow2_expand_zero_clusters(BlockDriverState *bs);
 
 /* qcow2-snapshot.c functions */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp);
 int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id);
 int