On 18.01.2023 15:45, Hanna Czenczek wrote:
On 15.01.23 16:58, Alexander Ivanov wrote:
We will add more and more checks so we need a better code structure
in parallels_co_check. Let each check performs in a separate loop
in a separate helper.
Signed-off-by: Alexander Ivanov <alexander.iva...@virtuozzo.com>
Reviewed-by: Denis V. Lunev <d...@openvz.org>
---
block/parallels.c | 59 ++++++++++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 16 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index d48b447cca..3d06623355 100644
--- a/block/parallels.c
+++ b/block/parallels.c
[...]
@@ -469,19 +511,6 @@ static int coroutine_fn
parallels_co_check(BlockDriverState *bs,
continue;
}
- /* cluster outside the image */
- if (off > size) {
- fprintf(stderr, "%s cluster %u is outside image\n",
- fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
- res->corruptions++;
- if (fix & BDRV_FIX_ERRORS) {
- parallels_set_bat_entry(s, i, 0);
- res->corruptions_fixed++;
- }
- prev_off = 0;
- continue;
- }
-
res->bfi.allocated_clusters++;
if (off > high_off) {
high_off = off;
parallels_co_check() keeps the `high_off` variable, and now it is also
bumped for clusters that are outside the image. This seems to go
against patch 2’s intentions.
Consider an image whose file length is larger than all of its clusters
need (i.e. there’s leaked space), except for one cluster, which is
beyond the EOF. This one cluster is considered an error (because it’s
outside the image). Before this patch, we would have truncated the
image’s file length to match all the other non-error clusters (and
drop the leaked space). With this patch, the error cluster, if it
wasn’t fixed by parallels_check_outside_image(), the image’s file
length is no longer truncated. Basically, this seems to restore the
behavior from before patch 2.
Was this intentional?
Hanna
Good point!
I have missed the case with !BDRV_FIX_ERRORS.
Thank you!
