Am 16.10.2020 um 19:10 hat Vladimir Sementsov-Ogievskiy geschrieben: > Now bdrv_append returns status and we can drop all the local_err things > around it. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Reviewed-by: Greg Kurz <gr...@kaod.org> > Reviewed-by: Alberto Garcia <be...@igalia.com> > --- > block.c | 5 +---- > block/backup-top.c | 20 ++++++++------------ > block/commit.c | 5 +---- > block/mirror.c | 6 ++---- > blockdev.c | 4 +--- > tests/test-bdrv-graph-mod.c | 6 +++--- > 6 files changed, 16 insertions(+), 30 deletions(-) > > diff --git a/block.c b/block.c > index b05fbff42d..7b6818c681 100644 > --- a/block.c > +++ b/block.c > @@ -3161,7 +3161,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 > @@ -3208,9 +3207,7 @@ static BlockDriverState > *bdrv_append_temp_snapshot(BlockDriverState *bs, > * order to be able to return one, we have to increase > * bs_snapshot's refcount here */ > bdrv_ref(bs_snapshot); > - bdrv_append(bs_snapshot, bs, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + if (bdrv_append(bs_snapshot, bs, errp) < 0) {
We generally avoid calling functions with side effects inside a comparison. Let's use the usual pattern: ret = bdrv_append(bs_snapshot, bs, errp); if (ret < 0) { ... } (I mention it only once, but there are multiple instances in this patch.) > bs_snapshot = NULL; > goto out; > } > diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c > index 8cff13830e..2b71601c24 100644 > --- a/tests/test-bdrv-graph-mod.c > +++ b/tests/test-bdrv-graph-mod.c > @@ -101,7 +101,7 @@ static BlockDriverState *pass_through_node(const char > *name) > */ > static void test_update_perm_tree(void) > { > - Error *local_err = NULL; > + int ret; > > BlockBackend *root = blk_new(qemu_get_aio_context(), > BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ, > @@ -114,8 +114,8 @@ static void test_update_perm_tree(void) > bdrv_attach_child(filter, bs, "child", &child_of_bds, > BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, > &error_abort); > > - bdrv_append(filter, bs, &local_err); > - error_free_or_abort(&local_err); > + ret = bdrv_append(filter, bs, NULL); > + assert(ret < 0); Usually, test cases use g_assert_cmpint() which prints the expected and actual value if it fails. Kevin