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 <vsementsov@virtuozzo.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20210202124956.63146-14-vsementsov@virtuozzo.com> [eblake: commit message grammar tweak] Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
parent
e6247c9c9f
commit
15ce94a68c
24
block/qed.c
24
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: ");
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user