On 20.08.19 13:42, Max Reitz wrote:
> On 19.08.19 21:23, Eric Blake wrote:
>> 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;
> 
> Hm, but I don’t prop up extra_data_size to be at least sizeof(extra).  I
> can do that, but it would add a few extra lines here and there.

On second thought, it doesn’t work at all because then there is no way
to later detect whether the image is compliant or not.

Max