Re: [PATCH v2 04/36] block: bdrv_append(): return status

2021-01-18 Thread Kevin Wolf
Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Return int status to avoid extra error propagation schemes.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Reviewed-by: Kevin Wolf 




Re: [PATCH v2 04/36] block: bdrv_append(): return status

2020-12-14 Thread Alberto Garcia
On Fri 27 Nov 2020 03:44:50 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> Return int status to avoid extra error propagation schemes.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Reviewed-by: Alberto Garcia 

Berto



[PATCH v2 04/36] block: bdrv_append(): return status

2020-11-27 Thread Vladimir Sementsov-Ogievskiy
Return int status to avoid extra error propagation schemes.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/block.h   |  4 ++--
 block.c | 15 ---
 block/commit.c  |  6 ++
 block/mirror.c  |  6 ++
 blockdev.c  |  6 +++---
 tests/test-bdrv-graph-mod.c |  6 +++---
 6 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index db37a35cee..ee3f5a6cca 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -344,8 +344,8 @@ int bdrv_create(BlockDriver *drv, const char* filename,
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
 
 BlockDriverState *bdrv_new(void);
-void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
- Error **errp);
+int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
+Error **errp);
 void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
Error **errp);
 
diff --git a/block.c b/block.c
index 55efef3c9d..916087ee1a 100644
--- a/block.c
+++ b/block.c
@@ -3103,7 +3103,6 @@ static BlockDriverState 
*bdrv_append_temp_snapshot(BlockDriverState *bs,
 int64_t total_size;
 QemuOpts *opts = NULL;
 BlockDriverState *bs_snapshot = NULL;
-Error *local_err = NULL;
 int ret;
 
 /* if snapshot, we create a temporary backing file and open it
@@ -3145,9 +3144,8 @@ static BlockDriverState 
*bdrv_append_temp_snapshot(BlockDriverState *bs,
 goto out;
 }
 
-bdrv_append(bs_snapshot, bs, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+ret = bdrv_append(bs_snapshot, bs, errp);
+if (ret < 0) {
 bs_snapshot = NULL;
 goto out;
 }
@@ -4606,22 +4604,25 @@ void bdrv_replace_node(BlockDriverState *from, 
BlockDriverState *to,
  * Recent update: bdrv_append does NOT eat bs_new reference for now. Drop this
  * comment several moths later.
  */
-void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
- Error **errp)
+int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
+Error **errp)
 {
 Error *local_err = NULL;
 
 bdrv_set_backing_hd(bs_new, bs_top, _err);
 if (local_err) {
 error_propagate(errp, local_err);
-return;
+return -EPERM;
 }
 
 bdrv_replace_node(bs_top, bs_new, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 bdrv_set_backing_hd(bs_new, NULL, _abort);
+return -EPERM;
 }
+
+return 0;
 }
 
 static void bdrv_delete(BlockDriverState *bs)
diff --git a/block/commit.c b/block/commit.c
index 61924bcf66..b89bb20b75 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -254,7 +254,6 @@ void commit_start(const char *job_id, BlockDriverState *bs,
 BlockDriverState *iter;
 BlockDriverState *commit_top_bs = NULL;
 BlockDriverState *filtered_base;
-Error *local_err = NULL;
 int64_t base_size, top_size;
 uint64_t base_perms, iter_shared_perms;
 int ret;
@@ -312,11 +311,10 @@ void commit_start(const char *job_id, BlockDriverState 
*bs,
 
 commit_top_bs->total_sectors = top->total_sectors;
 
-bdrv_append(commit_top_bs, top, _err);
+ret = bdrv_append(commit_top_bs, top, errp);
 bdrv_unref(commit_top_bs); /* referenced by new parents or failed */
-if (local_err) {
+if (ret < 0) {
 commit_top_bs = NULL;
-error_propagate(errp, local_err);
 goto fail;
 }
 
diff --git a/block/mirror.c b/block/mirror.c
index 13f7ecc998..c3fbe3e8bd 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1560,7 +1560,6 @@ static BlockJob *mirror_start_job(
 BlockDriverState *mirror_top_bs;
 bool target_is_backing;
 uint64_t target_perms, target_shared_perms;
-Error *local_err = NULL;
 int ret;
 
 if (granularity == 0) {
@@ -1606,12 +1605,11 @@ static BlockJob *mirror_start_job(
 mirror_top_bs->opaque = bs_opaque;
 
 bdrv_drained_begin(bs);
-bdrv_append(mirror_top_bs, bs, _err);
+ret = bdrv_append(mirror_top_bs, bs, errp);
 bdrv_drained_end(bs);
 
-if (local_err) {
+if (ret < 0) {
 bdrv_unref(mirror_top_bs);
-error_propagate(errp, local_err);
 return NULL;
 }
 
diff --git a/blockdev.c b/blockdev.c
index 96c96f8ba6..2af35d0958 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1432,6 +1432,7 @@ typedef struct ExternalSnapshotState {
 static void external_snapshot_prepare(BlkActionState *common,
   Error **errp)
 {
+int ret;
 int flags = 0;
 QDict *options = NULL;
 Error *local_err = NULL;
@@ -1587,9 +1588,8 @@ static void external_snapshot_prepare(BlkActionState 
*common,
 goto out;
 }
 
-bdrv_append(state->new_bs, state->old_bs, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+ret =