[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH for-4.2 08/13] qcow2: Fix broken snapshot table entr
From: |
Max Reitz |
Subject: |
[Qemu-devel] [PATCH for-4.2 08/13] qcow2: Fix broken snapshot table entries |
Date: |
Tue, 30 Jul 2019 19:25:03 +0200 |
The only case where we currently reject snapshot table entries is when
they have too much extra data. Fix them with qemu-img check -r all by
counting it as a corruption, reducing their extra_data_size, and then
letting qcow2_check_fix_snapshot_table() do the rest.
Signed-off-by: Max Reitz <address@hidden>
---
block/qcow2-snapshot.c | 69 ++++++++++++++++++++++++++++++++++--------
1 file changed, 56 insertions(+), 13 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 58609b6ea4..9956c32964 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -44,7 +44,23 @@ void qcow2_free_snapshots(BlockDriverState *bs)
s->nb_snapshots = 0;
}
-int qcow2_read_snapshots(BlockDriverState *bs, Error **errp)
+/*
+ * If @repair is true, try to repair a broken snapshot table instead
+ * of just returning an error:
+ *
+ * - If there were snapshots with too much extra metadata, increment
+ * *extra_data_dropped for each.
+ * This requires the caller to eventually rewrite the whole snapshot
+ * table, which requires cluster allocation. Therefore, this should
+ * be done only after qcow2_check_refcounts() made sure the refcount
+ * structures are valid.
+ * (In the meantime, the image is still valid because
+ * qcow2_check_refcounts() does not do anything with snapshots'
+ * extra data.)
+ */
+static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair,
+ int *extra_data_dropped,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
QCowSnapshotHeader h;
@@ -64,6 +80,8 @@ int qcow2_read_snapshots(BlockDriverState *bs, Error **errp)
s->snapshots = g_new0(QCowSnapshot, s->nb_snapshots);
for(i = 0; i < s->nb_snapshots; i++) {
+ bool discard_unknown_extra_data = false;
+
/* Read statically sized part of the snapshot header */
offset = ROUND_UP(offset, 8);
ret = bdrv_pread(bs->file, offset, &h, sizeof(h));
@@ -86,10 +104,21 @@ int qcow2_read_snapshots(BlockDriverState *bs, Error
**errp)
name_size = be16_to_cpu(h.name_size);
if (sn->extra_data_size > QCOW_MAX_SNAPSHOT_EXTRA_DATA) {
- ret = -EFBIG;
- error_setg(errp, "Too much extra metadata in snapshot table "
- "entry %i", i);
- goto fail;
+ if (!repair) {
+ ret = -EFBIG;
+ error_setg(errp, "Too much extra metadata in snapshot table "
+ "entry %i", i);
+ error_append_hint(errp, "You can force-remove this extra "
+ "metadata with qemu-img check -r all\n");
+ goto fail;
+ }
+
+ fprintf(stderr, "Discarding too much extra metadata in snapshot "
+ "table entry %i (%" PRIu32 " > %u)\n",
+ i, sn->extra_data_size, QCOW_MAX_SNAPSHOT_EXTRA_DATA);
+
+ (*extra_data_dropped)++;
+ discard_unknown_extra_data = true;
}
/* Read known extra data */
@@ -112,16 +141,22 @@ int qcow2_read_snapshots(BlockDriverState *bs, Error
**errp)
}
if (sn->extra_data_size > sizeof(extra)) {
- /* Store unknown extra data */
size_t unknown_extra_data_size =
sn->extra_data_size - sizeof(extra);
- sn->unknown_extra_data = g_malloc(unknown_extra_data_size);
- ret = bdrv_pread(bs->file, offset, sn->unknown_extra_data,
- unknown_extra_data_size);
- if (ret < 0) {
- error_setg_errno(errp, -ret, "Failed to read snapshot table");
- goto fail;
+ if (discard_unknown_extra_data) {
+ /* Discard unknown extra data */
+ sn->extra_data_size = sizeof(extra);
+ } else {
+ /* Store unknown extra data */
+ sn->unknown_extra_data = g_malloc(unknown_extra_data_size);
+ ret = bdrv_pread(bs->file, offset, sn->unknown_extra_data,
+ unknown_extra_data_size);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret,
+ "Failed to read snapshot table");
+ goto fail;
+ }
}
offset += unknown_extra_data_size;
}
@@ -162,6 +197,11 @@ fail:
return ret;
}
+int qcow2_read_snapshots(BlockDriverState *bs, Error **errp)
+{
+ return qcow2_do_read_snapshots(bs, false, NULL, errp);
+}
+
/* add at the end of the file a new list of snapshots */
int qcow2_write_snapshots(BlockDriverState *bs)
{
@@ -327,6 +367,7 @@ int coroutine_fn
qcow2_check_read_snapshot_table(BlockDriverState *bs,
{
BDRVQcow2State *s = bs->opaque;
Error *local_err = NULL;
+ int extra_data_dropped = 0;
int ret;
struct {
uint32_t nb_snapshots;
@@ -362,7 +403,8 @@ int coroutine_fn
qcow2_check_read_snapshot_table(BlockDriverState *bs,
}
qemu_co_mutex_unlock(&s->lock);
- ret = qcow2_read_snapshots(bs, &local_err);
+ ret = qcow2_do_read_snapshots(bs, fix & BDRV_FIX_ERRORS,
+ &extra_data_dropped, &local_err);
qemu_co_mutex_lock(&s->lock);
if (ret < 0) {
result->check_errors++;
@@ -375,6 +417,7 @@ int coroutine_fn
qcow2_check_read_snapshot_table(BlockDriverState *bs,
return ret;
}
+ result->corruptions += extra_data_dropped;
return 0;
}
--
2.21.0
- [Qemu-devel] [PATCH for-4.2 04/13] qcow2: Put qcow2_upgrade() into an own function, (continued)
- [Qemu-devel] [PATCH for-4.2 04/13] qcow2: Put qcow2_upgrade() into an own function, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 05/13] qcow2: Write v3-compliant snapshot list on upgrade, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 06/13] qcow2: Separate qcow2_check_read_snapshot_table(), Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 07/13] qcow2: Add qcow2_check_fix_snapshot_table(), Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 08/13] qcow2: Fix broken snapshot table entries,
Max Reitz <=
- [Qemu-devel] [PATCH for-4.2 10/13] qcow2: Repair snapshot table with too many entries, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 09/13] qcow2: Fix overly long snapshot tables, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 11/13] qcow2: Fix v3 snapshot table entry compliancy, Max Reitz, 2019/07/30