| Message ID | 20190730172508.19911-11-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: Let check -r all repair some snapshot bits | expand |
On 7/30/19 12:25 PM, Max Reitz wrote: > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/qcow2-snapshot.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > Same problem as for 9/13 - should we really be throwing away the user's data like this? (9/13 hits if the user has a small number of snapshots, but each has enough extra data, that the overall table is bigger than we like; 10/13 hits if the user has more snapshots than we like, but otherwise they do the same thing).
On 30.07.19 21:10, Eric Blake wrote: > On 7/30/19 12:25 PM, Max Reitz wrote: >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> block/qcow2-snapshot.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> > > Same problem as for 9/13 - should we really be throwing away the user's > data like this? (9/13 hits if the user has a small number of snapshots, > but each has enough extra data, that the overall table is bigger than we > like; 10/13 hits if the user has more snapshots than we like, but > otherwise they do the same thing). The same arguments apply (though the “it must be a corruption” argument applies even more, because having more than 65536 snapshots just isn’t right.) Max
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index bd8e56a99e..9e8c7c1f7f 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -430,6 +430,14 @@ int coroutine_fn qcow2_check_read_snapshot_table(BlockDriverState *bs, s->snapshots_offset = be64_to_cpu(snapshot_table_pointer.snapshots_offset); s->nb_snapshots = be32_to_cpu(snapshot_table_pointer.nb_snapshots); + if (s->nb_snapshots > QCOW_MAX_SNAPSHOTS && (fix & BDRV_FIX_ERRORS)) { + fprintf(stderr, "Discarding %u overhanging snapshots\n", + s->nb_snapshots - QCOW_MAX_SNAPSHOTS); + + nb_clusters_reduced += s->nb_snapshots - QCOW_MAX_SNAPSHOTS; + s->nb_snapshots = QCOW_MAX_SNAPSHOTS; + } + ret = qcow2_validate_table(bs, s->snapshots_offset, s->nb_snapshots, sizeof(QCowSnapshotHeader), sizeof(QCowSnapshotHeader) * QCOW_MAX_SNAPSHOTS, @@ -438,6 +446,12 @@ int coroutine_fn qcow2_check_read_snapshot_table(BlockDriverState *bs, result->check_errors++; error_reportf_err(local_err, "ERROR "); + if (s->nb_snapshots > QCOW_MAX_SNAPSHOTS) { + fprintf(stderr, "You can force-remove all %u overhanging snapshots " + "with qemu-img check -r all\n", + s->nb_snapshots - QCOW_MAX_SNAPSHOTS); + } + /* We did not read the snapshot table, so invalidate this information */ s->snapshots_offset = 0; s->nb_snapshots = 0;
Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2-snapshot.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)