On 1/12/23 16:01, Alexander Ivanov wrote:
We need to fix leak after deduplication in the next patch. Move leak
fixing to a separate helper parallels_fix_leak() and add
parallels_get_leak_size() helper wich used in parallels_fix_leak() and
parallels_check_leak().
Signed-off-by: Alexander Ivanov
---
block/parallels.c | 88 ---
1 file changed, 61 insertions(+), 27 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index ed2cf27abc..da1e75096c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -475,21 +475,53 @@ static int parallels_check_outside_image(BlockDriverState
*bs,
return 0;
}
+static int64_t parallels_get_leak_size(BlockDriverState *bs,
+ BdrvCheckResult *res)
+{
+int64_t size;
+size = bdrv_getlength(bs->file->bs);
+/*
+ * Before any usage of this function out-of-image corruption has been
+ * fixed. If the function returns a negative value, it means an error.
+ */
+return (size < 0) ? size : (size - res->image_end_offset);
I'd better use normal 'if' here and will add an assert that
'size' >= 'res->image_end_offset' on success path.
+}
+
+static int parallels_fix_leak(BlockDriverState *bs,
+ BdrvCheckResult *res)
+{
+Error *local_err = NULL;
+int64_t size;
+int ret;
+
+size = parallels_get_leak_size(bs, res);
+if (size <= 0) {
+return size;
+}
+
+/*
+ * In order to really repair the image, we must shrink it.
+ * That means we have to pass exact=true.
+ */
+ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
+ PREALLOC_MODE_OFF, 0, _err);
+if (ret < 0) {
+error_report_err(local_err);
+return ret;
+}
+
+return 0;
+}
+
static int parallels_check_leak(BlockDriverState *bs,
BdrvCheckResult *res,
BdrvCheckMode fix)
{
BDRVParallelsState *s = bs->opaque;
-int64_t size, off, high_off, count;
+int64_t off, high_off, count, leak_size;
uint32_t i;
int ret;
-size = bdrv_getlength(bs->file->bs);
-if (size < 0) {
-res->check_errors++;
-return size;
-}
-
high_off = 0;
for (i = 0; i < s->bat_size; i++) {
off = bat2sect(s, i) << BDRV_SECTOR_BITS;
@@ -499,30 +531,32 @@ static int parallels_check_leak(BlockDriverState *bs,
}
res->image_end_offset = high_off + s->cluster_size;
-if (size > res->image_end_offset) {
-count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
-fprintf(stderr, "%s space leaked at the end of the image %" PRId64
"\n",
-fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
-size - res->image_end_offset);
-res->leaks += count;
-if (fix & BDRV_FIX_LEAKS) {
-Error *local_err = NULL;
-/*
- * In order to really repair the image, we must shrink it.
- * That means we have to pass exact=true.
- */
-ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
- PREALLOC_MODE_OFF, 0, _err);
-if (ret < 0) {
-error_report_err(local_err);
-res->check_errors++;
-return ret;
-}
-res->leaks_fixed += count;
+leak_size = parallels_get_leak_size(bs, res);
+if (leak_size < 0) {
+res->check_errors++;
+return leak_size;
+}
+if (leak_size == 0) {
+return 0;
+}
+
+if (fix & BDRV_FIX_LEAKS) {
+ret = parallels_fix_leak(bs, res);
you have changed semantics a bit - with a error here the message
that we do get leak will be lost. I think the code is to be restructured
a bit here:
* move printing above this if
* fill leaks_fixed inside this if on the success path
This will be much straight forward
Den
+if (ret < 0) {
+return ret;
}
}
+count = DIV_ROUND_UP(leak_size, s->cluster_size);
+fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n",
+fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", leak_size);
+
+res->leaks += count;
+if (fix & BDRV_FIX_LEAKS) {
+res->leaks_fixed += count;
+}
+
return 0;
}
