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

[Max Reitz]

On 30.07.19 20:53, Eric Blake wrote:
> 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 <address@hidden>
>> ---
>>  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)

Well, I copied it from the specification. O:-)

You’re completely right.  It should be offsetof(QCowHeader, nb_snapshots).

(I blame the fact that I had started writing the test by this point, so
I was already immersed in so many magic numbers.)

> Reviewed-by: Eric Blake <address@hidden>

signature.asc
Description: OpenPGP digital signature