[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 2/9] parallels: Fix data_end field value in parallels_co_c
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v5 2/9] parallels: Fix data_end field value in parallels_co_check() |
Date: |
Tue, 23 Aug 2022 10:38:19 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 |
On 8/22/22 12:05, Alexander Ivanov wrote:
Make data_end pointing to the end of the last cluster if a leak was fixed.
Otherwise set the file size to data_end.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/block/parallels.c b/block/parallels.c
index c245ca35cd..2be56871bc 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -513,7 +513,15 @@ static int coroutine_fn
parallels_co_check(BlockDriverState *bs,
res->leaks_fixed += count;
}
}
-
+ /*
+ * If res->image_end_offset less than the file size,
+ * a leak was fixed and we should set the new offset to s->data_end.
+ * Otherwise set the file size to s->data_end.
I'm not sure about English :) For me "set A to B" means A := B, but you use it
visa versa..
+ */
+ if (res->image_end_offset < size && fix & BDRV_FIX_LEAKS) {
+ size = res->image_end_offset;
+ }
+ s->data_end = size >> BDRV_SECTOR_BITS;
Hmm, actually, when size < data_end, you can shrink data_end only if (fix &
BDRV_FIX_ERRORS), otherwise BAT is still not fixed.
out:
qemu_co_mutex_unlock(&s->lock);
return ret;
Actually I think we only need to modify s->data_end after successful BAT fixing
above. Then, bdrv_truncate is formally unrelated to s->data_end and shouldn't
touch it.
So, I think, more correct is something like
diff --git a/block/parallels.c b/block/parallels.c
index 2be56871bc..b268788974 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -479,20 +479,24 @@ static int coroutine_fn
parallels_co_check(BlockDriverState *bs,
prev_off = off;
}
ret = 0;
if (flush_bat) {
ret = bdrv_co_pwrite_sync(bs->file, 0, s->header_size, s->header, 0);
if (ret < 0) {
res->check_errors++;
goto out;
}
+
+ /* We have dropped some wrong clusters, update data_end */
+ assert(s->data_end * BDRV_SECTOR_SIZE >= high_off + s->cluster_size);
+ s->data_end = (high_off + s->cluster_size) / BDRV_SECTOR_SIZE;
}
res->image_end_offset = high_off + s->cluster_size;
if (size > res->image_end_offset) {
--
Best regards,
Vladimir
- Re: [PATCH v5 1/9] parallels: Out of image offset in BAT leads to image inflation, (continued)
[PATCH v5 4/9] parallels: Use generic infrastructure for BAT writing in parallels_co_check(), Alexander Ivanov, 2022/08/22
[PATCH v5 3/9] parallels: create parallels_set_bat_entry_helper() to assign BAT value, Alexander Ivanov, 2022/08/22
[PATCH v5 5/9] parallels: Move check of unclean image to a separate function, Alexander Ivanov, 2022/08/22
[PATCH v5 9/9] parallels: Replace qemu_co_mutex_lock by WITH_QEMU_LOCK_GUARD, Alexander Ivanov, 2022/08/22
[PATCH v5 6/9] parallels: Move check of cluster outside image to a separate function, Alexander Ivanov, 2022/08/22
[PATCH v5 7/9] parallels: Move check of leaks to a separate function, Alexander Ivanov, 2022/08/22
[PATCH v5 2/9] parallels: Fix data_end field value in parallels_co_check(), Alexander Ivanov, 2022/08/22
[PATCH v5 8/9] parallels: Move statistic collection to a separate function, Alexander Ivanov, 2022/08/22