On 8/19/19 1:55 PM, Max Reitz wrote:
> The qcow2 specification says to ignore unknown extra data fields in
> snapshot table entries.  Currently, we discard it whenever we update the
> image, which is a bit different from "ignore".
> 
> This patch makes the qcow2 driver keep all unknown extra data fields
> when updating an image's snapshot table.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.h          |  5 ++++
>  block/qcow2-snapshot.c | 61 +++++++++++++++++++++++++++++++++++-------
>  2 files changed, 56 insertions(+), 10 deletions(-)
> 

> @@ -162,7 +184,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
>          sn = s->snapshots + i;
>          offset = ROUND_UP(offset, 8);
>          offset += sizeof(h);
> -        offset += sizeof(extra);
> +        offset += MAX(sizeof(extra), sn->extra_data_size);

Why would we ever have less than sizeof(extra) bytes to write on output,
since we always produce the fields on creation and synthesize the
missing fields of extra on read?  Can't you rewrite this as:

assert(sn->extra_data_size >= sizeof(extra));
offset += sn->extra_data_size;

Otherwise,

Reviewed-by: Eric Blake <eblake@redhat.com>

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