[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 1/2] qcow2: Handle QCowL2Meta on error in preallocate_co()
From: |
Alberto Garcia |
Subject: |
[PATCH 1/2] qcow2: Handle QCowL2Meta on error in preallocate_co() |
Date: |
Tue, 8 Sep 2020 16:08:27 +0200 |
If qcow2_alloc_cluster_offset() or qcow2_alloc_cluster_link_l2() fail
then this function simply returns the error code, potentially leaking
the QCowL2Meta structure and leaving stale items in s->cluster_allocs.
A second problem is that this function calls qcow2_free_any_clusters()
on failure but passing a host cluster offset instead of an L2 entry.
Luckily for normal uncompressed clusters a raw offset also works like
a valid L2 entry so it works just the same, but we should be using
qcow2_free_clusters() instead.
This patch fixes both problems by using qcow2_handle_l2meta().
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block/qcow2.c | 40 +++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index da56b1a4df..eeb125c697 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2102,7 +2102,6 @@ static coroutine_fn int
qcow2_handle_l2meta(BlockDriverState *bs,
QCowL2Meta *next;
if (link_l2) {
- assert(!l2meta->prealloc);
ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
if (ret) {
goto out;
@@ -3126,7 +3125,7 @@ static int coroutine_fn preallocate_co(BlockDriverState
*bs, uint64_t offset,
int64_t file_length;
unsigned int cur_bytes;
int ret;
- QCowL2Meta *meta;
+ QCowL2Meta *meta = NULL, *m;
assert(offset <= new_length);
bytes = new_length - offset;
@@ -3137,27 +3136,17 @@ static int coroutine_fn preallocate_co(BlockDriverState
*bs, uint64_t offset,
&host_offset, &meta);
if (ret < 0) {
error_setg_errno(errp, -ret, "Allocating clusters failed");
- return ret;
+ goto out;
}
- while (meta) {
- QCowL2Meta *next = meta->next;
- meta->prealloc = true;
-
- ret = qcow2_alloc_cluster_link_l2(bs, meta);
- if (ret < 0) {
- error_setg_errno(errp, -ret, "Mapping clusters failed");
- qcow2_free_any_clusters(bs, meta->alloc_offset,
- meta->nb_clusters,
QCOW2_DISCARD_NEVER);
- return ret;
- }
-
- /* There are no dependent requests, but we need to remove our
- * request from the list of in-flight requests */
- QLIST_REMOVE(meta, next_in_flight);
+ for (m = meta; m != NULL; m = m->next) {
+ m->prealloc = true;
+ }
- g_free(meta);
- meta = next;
+ ret = qcow2_handle_l2meta(bs, &meta, true);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Mapping clusters failed");
+ goto out;
}
/* TODO Preallocate data if requested */
@@ -3174,7 +3163,8 @@ static int coroutine_fn preallocate_co(BlockDriverState
*bs, uint64_t offset,
file_length = bdrv_getlength(s->data_file->bs);
if (file_length < 0) {
error_setg_errno(errp, -file_length, "Could not get file size");
- return file_length;
+ ret = file_length;
+ goto out;
}
if (host_offset + cur_bytes > file_length) {
@@ -3184,11 +3174,15 @@ static int coroutine_fn preallocate_co(BlockDriverState
*bs, uint64_t offset,
ret = bdrv_co_truncate(s->data_file, host_offset + cur_bytes, false,
mode, 0, errp);
if (ret < 0) {
- return ret;
+ goto out;
}
}
- return 0;
+ ret = 0;
+
+out:
+ qcow2_handle_l2meta(bs, &meta, false);
+ return ret;
}
/* qcow2_refcount_metadata_size:
--
2.20.1