2011/8/4 Kevin Wolf <kw...@redhat.com>:
> When loading an internal snapshot whose L1 table is smaller than the current
> L1
> table, the size of the current L1 would be shrunk to the snapshot's L1 size in
> memory, but not on disk. This lead to incorrect refcount updates and
> eventuelly
> to image corruption.
>
> Instead of writing the new L1 size to disk, this simply retains the bigger L1
> size that is currently in use and makes sure that the unused part is zeroed.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
>
> And the moment you send it out, you notice that it's wrong... *sigh*
>
> v2:
> - Check for s->l1_size > sn->l1_size in order to avoid disasters...
>
> Philipp, I think this should fix your corruption. Please give it a try.
>
> Anthony, this must go into 0.15. Given the short time until -rc2, do you
> prefer
> to pick it up directly or should I send a pull request tomorrow? The patch
> looks obvious, is tested with the given testcase and survives a basic
> qemu-iotests run (though qemu-iotests doesn't exercise snapshots a lot)
>
> Stefan, please review :-)
>
> block/qcow2-snapshot.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 74823a5..6972e66 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -330,8 +330,11 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char
> *snapshot_id)
> if (qcow2_grow_l1_table(bs, sn->l1_size, true) < 0)
> goto fail;
>
> - s->l1_size = sn->l1_size;
> + if (s->l1_size > sn->l1_size) {
> + memset(s->l1_table + sn->l1_size, 0, s->l1_size - sn->l1_size);
> + }
> l1_size2 = s->l1_size * sizeof(uint64_t);
> +
> /* copy the snapshot l1 table to the current l1 table */
> if (bdrv_pread(bs->file, sn->l1_table_offset,
> s->l1_table, l1_size2) != l1_size2)
> --
> 1.7.6
>
This patch looked odd at first sight. First a qcow2_grow_l1_table is
called to shrink L1 so perhaps should be qcow2_resize_l1_table.
Perhaps also it would be better to clean entries in
qcow2_grow_l1_table instead of qcow2_snapshot_goto to avoid same
problem in different calls to qcow2_grow_l1_table. The other oddity
(still to understand) is: why does some code use l1_table above
l1_size ??
Frediano
