[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-block
Subject:    Re: [Qemu-block] [PATCH for-4.2 05/13] qcow2: Write v3-compliant snapshot list on upgrade
From:       Max Reitz <mreitz () redhat ! com>
Date:       2019-07-31 8:56:50
Message-ID: efb03488-366e-8c92-e36a-32effdc1d66f () redhat ! com
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


On 30.07.19 20:10, Eric Blake wrote:
> On 7/30/19 12:25 PM, Max Reitz wrote:
>> qcow2 v3 requires every snapshot table entry to have two extra data
>> fields: The 64-bit VM state size, and the virtual disk size.  Both are
>> optional for v2 images, so they may not be present.
>>
>> qcow2_upgrade() therefore should update the snapshot table to ensure all
>> entries have these extra data fields.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1727347
>> Reported-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/qcow2.c | 29 +++++++++++++++++++++++++++--
>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>
> 
>> +
>> +    /*
>> +     * In v2, snapshots do not need to have extra data.  v3 requires
>> +     * the 64-bit VM state size and the virtual disk size to be
>> +     * present.
>> +     * qcow2_write_snapshots() will always write the list in the
>> +     * v3-compliant format.
>> +     */
>> +    need_snapshot_update = false;
>> +    for (i = 0; i < s->nb_snapshots; i++) {
>> +        if (s->snapshots[i].extra_data_size < 16) {
> 
> s/16/sizeof(extra)/ looks a bit nicer, but doesn't change semantics.

Hm, but it's not quite right.  I mean, right now it is, but if we were
to add a new field to snapshot metadata, it wouldn't be.  It should be 16.

I can make it something like sizeof(extra.vm_state_size_large) +
sizeof(extra.disk_size), though.

Max

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



["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]