Re: [Qemu-devel] [PATCH for-4.2 06/13] qcow2: Separate qcow2_check_read_snapshot_table()

[Eric Blake <eblake@redhat.com>]

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

On 7/30/19 12:25 PM, Max Reitz wrote:
> Reading the snapshot table can fail.  That is a problem when we want to
> repair the image.
> 
> Therefore, stop reading the snapshot table in qcow2_do_open() in check
> mode.  Instead, add a new function qcow2_check_read_snapshot_table()
> that reads the snapshot table at a later point.  In the future, we want
> to handle errors here and fix them.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.h          |  4 +++
>  block/qcow2-snapshot.c | 58 ++++++++++++++++++++++++++++++++
>  block/qcow2.c          | 76 ++++++++++++++++++++++++++++++++----------
>  3 files changed, 120 insertions(+), 18 deletions(-)
> 

> +++ b/block/qcow2-snapshot.c
> @@ -321,6 +321,64 @@ fail:
>      return ret;
>  }
>  
> +int coroutine_fn qcow2_check_read_snapshot_table(BlockDriverState *bs,
> +                                                 BdrvCheckResult *result,
> +                                                 BdrvCheckMode fix)
> +{
> +    BDRVQcow2State *s = bs->opaque;
> +    Error *local_err = NULL;
> +    int ret;
> +    struct {
> +        uint32_t nb_snapshots;
> +        uint64_t snapshots_offset;
> +    } QEMU_PACKED snapshot_table_pointer;
> +
> +    /* qcow2_do_open() discards this information in check mode */
> +    ret = bdrv_pread(bs->file, 60, &snapshot_table_pointer,
> +                     sizeof(snapshot_table_pointer));

Should that '60' be a named constant or offsetof() expression?  (I know,
you just copied this instance from elsewhere)

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 --]