[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-4.2 08/13] qcow2: Fix broken snapshot table
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH for-4.2 08/13] qcow2: Fix broken snapshot table entries |
Date: |
Wed, 31 Jul 2019 11:06:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 30.07.19 21:02, Eric Blake wrote:
> On 7/30/19 12:25 PM, Max Reitz wrote:
>> The only case where we currently reject snapshot table entries is when
>> they have too much extra data. Fix them with qemu-img check -r all by
>> counting it as a corruption, reducing their extra_data_size, and then
>> letting qcow2_check_fix_snapshot_table() do the rest.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> block/qcow2-snapshot.c | 69 ++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 56 insertions(+), 13 deletions(-)
>>
>
>> @@ -112,16 +141,22 @@ int qcow2_read_snapshots(BlockDriverState *bs, Error
>> **errp)
>> }
>>
>> if (sn->extra_data_size > sizeof(extra)) {
>> - /* Store unknown extra data */
>> size_t unknown_extra_data_size =
>> sn->extra_data_size - sizeof(extra);
>>
>> - sn->unknown_extra_data = g_malloc(unknown_extra_data_size);
>> - ret = bdrv_pread(bs->file, offset, sn->unknown_extra_data,
>> - unknown_extra_data_size);
>> - if (ret < 0) {
>> - error_setg_errno(errp, -ret, "Failed to read snapshot
>> table");
>> - goto fail;
>> + if (discard_unknown_extra_data) {
>> + /* Discard unknown extra data */
>> + sn->extra_data_size = sizeof(extra);
>
> This truncates it down to just the data we know. Should it instead
> truncate down to the 1024 bytes of QCOW_MAX_SNAPSHOT_EXTRA_DATA defined
> in 2/13? (We can't keep all of the user's extra stuff, but we can at
> least try to preserve as much as possible)
On one hand, potentially cutting unknown data in half sounds like not
such a good idea to me.
On the other, a field can only be considered present if it is fully
present. So cutting any optional data in half shouldn’t have any
negative impact.
So, yes, truncating it down to 1024 bytes sounds good.
Max
> Otherwise, looks good.
> Reviewed-by: Eric Blake <address@hidden>
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH for-4.2 05/13] qcow2: Write v3-compliant snapshot list on upgrade, (continued)
- [Qemu-devel] [PATCH for-4.2 05/13] qcow2: Write v3-compliant snapshot list on upgrade, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 06/13] qcow2: Separate qcow2_check_read_snapshot_table(), Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 07/13] qcow2: Add qcow2_check_fix_snapshot_table(), Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 08/13] qcow2: Fix broken snapshot table entries, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 10/13] qcow2: Repair snapshot table with too many entries, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 09/13] qcow2: Fix overly long snapshot tables, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 11/13] qcow2: Fix v3 snapshot table entry compliancy, Max Reitz, 2019/07/30
- [Qemu-devel] [PATCH for-4.2 12/13] iotests: Add peek_file* functions, Max Reitz, 2019/07/30