From 15ce94a68ca6730466c565c3d29971aab3087bf1 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 2 Feb 2021 15:49:55 +0300 Subject: [PATCH] block/qed: bdrv_qed_do_open: deal with errp Always set errp on failure. The generic bdrv_open_driver supports driver functions which can return a negative value but forget to set errp. That's a strange thing. Let's improve bdrv_qed_do_open to not behave this way. This allows the simplification of code in bdrv_qed_co_invalidate_cache(). Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Reviewed-by: Greg Kurz Message-Id: <20210202124956.63146-14-vsementsov@virtuozzo.com> [eblake: commit message grammar tweak] Signed-off-by: Eric Blake --- block/qed.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/block/qed.c b/block/qed.c index b27e7546ca..f45c640513 100644 --- a/block/qed.c +++ b/block/qed.c @@ -393,6 +393,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header)); if (ret < 0) { + error_setg(errp, "Failed to read QED header"); return ret; } qed_header_le_to_cpu(&le_header, &s->header); @@ -408,25 +409,30 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, return -ENOTSUP; } if (!qed_is_cluster_size_valid(s->header.cluster_size)) { + error_setg(errp, "QED cluster size is invalid"); return -EINVAL; } /* Round down file size to the last cluster */ file_size = bdrv_getlength(bs->file->bs); if (file_size < 0) { + error_setg(errp, "Failed to get file length"); return file_size; } s->file_size = qed_start_of_cluster(s, file_size); if (!qed_is_table_size_valid(s->header.table_size)) { + error_setg(errp, "QED table size is invalid"); return -EINVAL; } if (!qed_is_image_size_valid(s->header.image_size, s->header.cluster_size, s->header.table_size)) { + error_setg(errp, "QED image size is invalid"); return -EINVAL; } if (!qed_check_table_offset(s, s->header.l1_table_offset)) { + error_setg(errp, "QED table offset is invalid"); return -EINVAL; } @@ -438,6 +444,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, /* Header size calculation must not overflow uint32_t */ if (s->header.header_size > UINT32_MAX / s->header.cluster_size) { + error_setg(errp, "QED header size is too large"); return -EINVAL; } @@ -445,6 +452,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, if ((uint64_t)s->header.backing_filename_offset + s->header.backing_filename_size > s->header.cluster_size * s->header.header_size) { + error_setg(errp, "QED backing filename offset is invalid"); return -EINVAL; } @@ -453,6 +461,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, bs->auto_backing_file, sizeof(bs->auto_backing_file)); if (ret < 0) { + error_setg(errp, "Failed to read backing filename"); return ret; } pstrcpy(bs->backing_file, sizeof(bs->backing_file), @@ -475,6 +484,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, ret = qed_write_header_sync(s); if (ret) { + error_setg(errp, "Failed to update header"); return ret; } @@ -487,6 +497,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, ret = qed_read_l1_table_sync(s); if (ret) { + error_setg(errp, "Failed to read L1 table"); goto out; } @@ -503,6 +514,7 @@ static int coroutine_fn bdrv_qed_do_open(BlockDriverState *bs, QDict *options, ret = qed_check(s, &result, true); if (ret) { + error_setg(errp, "Image corrupted"); goto out; } } @@ -1537,22 +1549,16 @@ static void coroutine_fn bdrv_qed_co_invalidate_cache(BlockDriverState *bs, Error **errp) { BDRVQEDState *s = bs->opaque; - Error *local_err = NULL; int ret; bdrv_qed_close(bs); bdrv_qed_init_state(bs); qemu_co_mutex_lock(&s->table_lock); - ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err); + ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, errp); qemu_co_mutex_unlock(&s->table_lock); - if (local_err) { - error_propagate_prepend(errp, local_err, - "Could not reopen qed layer: "); - return; - } else if (ret < 0) { - error_setg_errno(errp, -ret, "Could not reopen qed layer"); - return; + if (ret < 0) { + error_prepend(errp, "Could not reopen qed layer: "); } }