Re: [Qemu-devel] [PATCH for-4.2 08/13] qcow2: Fix broken snapshot table entries

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1805 bytes --]

On 7/30/19 12:25 PM, Max Reitz wrote:
> 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 <mreitz@redhat.com>
> ---
>  block/qcow2-snapshot.c | 69 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 56 insertions(+), 13 deletions(-)
> 

> @@ -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);

This truncates it down to just the data we know. Should it instead
truncate down to the 1024 bytes of QCOW_MAX_SNAPSHOT_EXTRA_DATA defined
in 2/13?  (We can't keep all of the user's extra stuff, but we can at
least try to preserve as much as possible)

Otherwise, looks good.
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]