[PATCH 0/3] Check and repair duplicated clusters in parallels images

alexander.ivanov@virtuozzo.com posted 3 patches 1 year, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220804145200.564072-1-alexander.ivanov@virtuozzo.com
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, "Denis V. Lunev" <den@openvz.org>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
block/parallels.c                             | 112 ++++++++++++++++--
tests/qemu-iotests/314                        |  88 ++++++++++++++
tests/qemu-iotests/314.out                    |  36 ++++++
.../parallels-2-duplicated-cluster.bz2        | Bin 0 -> 148 bytes
4 files changed, 227 insertions(+), 9 deletions(-)
create mode 100755 tests/qemu-iotests/314
create mode 100644 tests/qemu-iotests/314.out
create mode 100644 tests/qemu-iotests/sample_images/parallels-2-duplicated-cluster.bz2
[PATCH 0/3] Check and repair duplicated clusters in parallels images
Posted by alexander.ivanov@virtuozzo.com 1 year, 7 months ago
From: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Parallels image file can be corrupted this way: two guest memory areas
refer to the same host memory area (duplicated offsets in BAT).
qemu-img check copies data from duplicated cluster to the new cluster and
writes new corresponding offset to BAT instead of duplicated one.

Test 314 uses sample corrupted image parallels-2-duplicated-cluster.bz2.
Reading from duplicated offset and from original offset returns the same
data. After repairing changing either of these blocks of data
does not affect another one.

Alexander Ivanov (3):
  parallels: Add checking and repairing duplicate offsets in BAT
  parallels: Let duplicates repairing pass without unwanted messages
  iotests, parallels: Add a test for duplicated clusters

 block/parallels.c                             | 112 ++++++++++++++++--
 tests/qemu-iotests/314                        |  88 ++++++++++++++
 tests/qemu-iotests/314.out                    |  36 ++++++
 .../parallels-2-duplicated-cluster.bz2        | Bin 0 -> 148 bytes
 4 files changed, 227 insertions(+), 9 deletions(-)
 create mode 100755 tests/qemu-iotests/314
 create mode 100644 tests/qemu-iotests/314.out
 create mode 100644 tests/qemu-iotests/sample_images/parallels-2-duplicated-cluster.bz2

-- 
2.34.1
Re: [PATCH 0/3] Check and repair duplicated clusters in parallels images
Posted by Denis V. Lunev 1 year, 7 months ago
On 04.08.2022 16:51, alexander.ivanov@virtuozzo.com wrote:
> From: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>
> Parallels image file can be corrupted this way: two guest memory areas
> refer to the same host memory area (duplicated offsets in BAT).
> qemu-img check copies data from duplicated cluster to the new cluster and
> writes new corresponding offset to BAT instead of duplicated one.
>
> Test 314 uses sample corrupted image parallels-2-duplicated-cluster.bz2.
> Reading from duplicated offset and from original offset returns the same
> data. After repairing changing either of these blocks of data
> does not affect another one.
>
> Alexander Ivanov (3):
>    parallels: Add checking and repairing duplicate offsets in BAT
>    parallels: Let duplicates repairing pass without unwanted messages
>    iotests, parallels: Add a test for duplicated clusters
>
>   block/parallels.c                             | 112 ++++++++++++++++--
>   tests/qemu-iotests/314                        |  88 ++++++++++++++
>   tests/qemu-iotests/314.out                    |  36 ++++++
>   .../parallels-2-duplicated-cluster.bz2        | Bin 0 -> 148 bytes
>   4 files changed, 227 insertions(+), 9 deletions(-)
>   create mode 100755 tests/qemu-iotests/314
>   create mode 100644 tests/qemu-iotests/314.out
>   create mode 100644 tests/qemu-iotests/sample_images/parallels-2-duplicated-cluster.bz2
>
the series itself should be marked as v2 indicating that this is based on
the original work of Natalia. Natalia should be in CC list.

Also list of CC persons is too small, pls run

iris ~/src/qemu $ ./scripts/get_maintainer.pl 
0001-parallels-Add-checking-and-repairing-duplicate-offse.patch
Stefan Hajnoczi <stefanha@redhat.com> (supporter:parallels)
"Denis V. Lunev" <den@openvz.org> (supporter:parallels)
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> 
(supporter:parallels)
Kevin Wolf <kwolf@redhat.com> (supporter:Block layer core)
Hanna Reitz <hreitz@redhat.com> (supporter:Block layer core)
qemu-block@nongnu.org (open list:parallels)
qemu-devel@nongnu.org (open list:All patches CC here)
iris ~/src/qemu $

Additionally, you should list of changes from previous version
at least in the main message.
Re: [PATCH 0/3] Check and repair duplicated clusters in parallels images
Posted by Denis V. Lunev 1 year, 7 months ago
On 04.08.2022 17:01, Denis V. Lunev wrote:
> On 04.08.2022 16:51, alexander.ivanov@virtuozzo.com wrote:
>> From: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>>
>> Parallels image file can be corrupted this way: two guest memory areas
>> refer to the same host memory area (duplicated offsets in BAT).
>> qemu-img check copies data from duplicated cluster to the new cluster 
>> and
>> writes new corresponding offset to BAT instead of duplicated one.
>>
>> Test 314 uses sample corrupted image parallels-2-duplicated-cluster.bz2.
>> Reading from duplicated offset and from original offset returns the same
>> data. After repairing changing either of these blocks of data
>> does not affect another one.
>>
>> Alexander Ivanov (3):
>>    parallels: Add checking and repairing duplicate offsets in BAT
>>    parallels: Let duplicates repairing pass without unwanted messages
>>    iotests, parallels: Add a test for duplicated clusters
>>
>>   block/parallels.c                             | 112 ++++++++++++++++--
>>   tests/qemu-iotests/314                        |  88 ++++++++++++++
>>   tests/qemu-iotests/314.out                    |  36 ++++++
>>   .../parallels-2-duplicated-cluster.bz2        | Bin 0 -> 148 bytes
>>   4 files changed, 227 insertions(+), 9 deletions(-)
>>   create mode 100755 tests/qemu-iotests/314
>>   create mode 100644 tests/qemu-iotests/314.out
>>   create mode 100644 
>> tests/qemu-iotests/sample_images/parallels-2-duplicated-cluster.bz2
>>
> the series itself should be marked as v2 indicating that this is based on
> the original work of Natalia. Natalia should be in CC list.
>
> Also list of CC persons is too small, pls run
>
> iris ~/src/qemu $ ./scripts/get_maintainer.pl 
> 0001-parallels-Add-checking-and-repairing-duplicate-offse.patch
> Stefan Hajnoczi <stefanha@redhat.com> (supporter:parallels)
> "Denis V. Lunev" <den@openvz.org> (supporter:parallels)
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> 
> (supporter:parallels)
> Kevin Wolf <kwolf@redhat.com> (supporter:Block layer core)
> Hanna Reitz <hreitz@redhat.com> (supporter:Block layer core)
> qemu-block@nongnu.org (open list:parallels)
> qemu-devel@nongnu.org (open list:All patches CC here)
> iris ~/src/qemu $
>
> Additionally, you should list of changes from previous version
> at least in the main message.
and generic note, to think.

We will add more and more checks. Should we start thinking on a better
code structure in parallels_co_check, f.e. we could performs each
check in a separate loop in separate helper and calculate statisctics
once all checks were done.

This is just a quick note, but I think it worth to keep this in mind.

Den