On 11.10.19 18:23, Eric Blake wrote:
> On 10/11/19 10:28 AM, 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>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>   block/qcow2.c | 32 ++++++++++++++++++++++++++++++--
>>   1 file changed, 30 insertions(+), 2 deletions(-)
>>
> 
>> +    need_snapshot_update = false;
>> +    for (i = 0; i < s->nb_snapshots; i++) {
>> +        if (s->snapshots[i].extra_data_size <
>> +            sizeof_field(QCowSnapshotExtraData, vm_state_size_large) +
>> +            sizeof_field(QCowSnapshotExtraData, disk_size))
> 
> Shorter as:
> if (s->snapshots[i].extra_data_size < sizeof(QCowSnapshotExtraData))
> 
> but that's stylistic, so R-b still stands.

Yes, but if we ever add fields to QCowSnapshotExtraData, we shouldn’t
count them here.  Therefore, I think we need to count exactly the fields
that the standard says are mandatory in v3.

Max