Re: [PATCH v4 17/21] parallels: Check unused clusters in parallels_check_leak()
On 12/28/23 11:12, Alexander Ivanov wrote:
Since we have used bitmap, leak check is useless. Transform
parallels_truncate_unused_clusters() to parallels_check_unused_clusters()
helper and use it in leak check.
Signed-off-by: Alexander Ivanov
---
block/parallels.c | 121 +-
1 file changed, 67 insertions(+), 54 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 136865d53e..5ed58826bb 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -768,57 +768,87 @@ parallels_check_outside_image(BlockDriverState *bs,
BdrvCheckResult *res,
return 0;
}
+static int64_t GRAPH_RDLOCK
+parallels_check_unused_clusters(BlockDriverState *bs, bool truncate)
+{
+BDRVParallelsState *s = bs->opaque;
+int64_t leak, file_size, end_off = 0;
+int ret;
+
+file_size = bdrv_getlength(bs->file->bs);
+if (file_size < 0) {
+return file_size;
+}
+
+if (s->used_bmap_size > 0) {
+end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
+if (end_off == s->used_bmap_size) {
+end_off = 0;
+} else {
+end_off = (end_off + 1) * s->cluster_size;
+}
+}
+
+end_off += s->data_start * BDRV_SECTOR_SIZE;
+leak = file_size - end_off;
+if (leak < 0) {
+return -EINVAL;
+}
+if (!truncate || leak == 0) {
+return leak;
+}
+
+ret = bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
+if (ret) {
+return ret;
+}
+
+s->data_end = end_off / BDRV_SECTOR_SIZE;
+
+parallels_free_used_bitmap(bs);
+ret = parallels_fill_used_bitmap(bs);
+if (ret < 0) {
+return ret;
+}
+
+return leak;
+}
+
static int coroutine_fn GRAPH_RDLOCK
parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix, bool explicit)
{
BDRVParallelsState *s = bs->opaque;
-int64_t size, count;
-int ret;
+int64_t leak, count, size;
+
+leak = parallels_check_unused_clusters(bs, fix & BDRV_FIX_LEAKS);
+if (leak < 0) {
+res->check_errors++;
+return leak;
+}
+if (leak == 0) {
+return 0;
+}
size = bdrv_co_getlength(bs->file->bs);
if (size < 0) {
res->check_errors++;
return size;
}
-if (size <= res->image_end_offset) {
+res->image_end_offset = size;
+
+if (!explicit) {
return 0;
}
-count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
-if (explicit) {
-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;
-}
+count = DIV_ROUND_UP(leak, s->cluster_size);
+fprintf(stderr,
+"%s space leaked at the end of the image %" PRId64 "\n",
+fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", leak);
+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;
-}
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-
-parallels_free_used_bitmap(bs);
-ret = parallels_fill_used_bitmap(bs);
-if (ret == -ENOMEM) {
-res->check_errors++;
-return ret;
-}
-
-if (explicit) {
-res->leaks_fixed += count;
-}
+res->leaks_fixed += count;
}
return 0;
@@ -1454,23 +1484,6 @@ fail:
return ret;
}
-static int GRAPH_RDLOCK parallels_truncate_unused_clusters(BlockDriverState *bs)
-{
-BDRVParallelsState *s = bs->opaque;
-uint64_t end_off = 0;
-if (s->used_bmap_size > 0) {
-end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
-if (end_off == s->used_bmap_size) {
-end_off = 0;
-} else {
-end_off = (end_off + 1) * s->cluster_size;
-}
-}
-end_off += s->data_start * BDRV_SECTOR_SIZE;
-s->data_end = end_off / BDRV_SECTOR_SIZE;
-return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
-}
-
static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
@@ -1488,7 +1501,7 @@ static int GRAPH_RDLOCK
parallels_inactivate(BlockDriverState *bs)
parallels_update_header(bs);
/* errors are ignored, so we might as well pass exact=true */
-ret = parallels_truncate_unused_clusters(bs);
+ret =
[PATCH v4 17/21] parallels: Check unused clusters in parallels_check_leak()
Since we have used bitmap, leak check is useless. Transform
parallels_truncate_unused_clusters() to parallels_check_unused_clusters()
helper and use it in leak check.
Signed-off-by: Alexander Ivanov
---
block/parallels.c | 121 +-
1 file changed, 67 insertions(+), 54 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 136865d53e..5ed58826bb 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -768,57 +768,87 @@ parallels_check_outside_image(BlockDriverState *bs,
BdrvCheckResult *res,
return 0;
}
+static int64_t GRAPH_RDLOCK
+parallels_check_unused_clusters(BlockDriverState *bs, bool truncate)
+{
+BDRVParallelsState *s = bs->opaque;
+int64_t leak, file_size, end_off = 0;
+int ret;
+
+file_size = bdrv_getlength(bs->file->bs);
+if (file_size < 0) {
+return file_size;
+}
+
+if (s->used_bmap_size > 0) {
+end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
+if (end_off == s->used_bmap_size) {
+end_off = 0;
+} else {
+end_off = (end_off + 1) * s->cluster_size;
+}
+}
+
+end_off += s->data_start * BDRV_SECTOR_SIZE;
+leak = file_size - end_off;
+if (leak < 0) {
+return -EINVAL;
+}
+if (!truncate || leak == 0) {
+return leak;
+}
+
+ret = bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
+if (ret) {
+return ret;
+}
+
+s->data_end = end_off / BDRV_SECTOR_SIZE;
+
+parallels_free_used_bitmap(bs);
+ret = parallels_fill_used_bitmap(bs);
+if (ret < 0) {
+return ret;
+}
+
+return leak;
+}
+
static int coroutine_fn GRAPH_RDLOCK
parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix, bool explicit)
{
BDRVParallelsState *s = bs->opaque;
-int64_t size, count;
-int ret;
+int64_t leak, count, size;
+
+leak = parallels_check_unused_clusters(bs, fix & BDRV_FIX_LEAKS);
+if (leak < 0) {
+res->check_errors++;
+return leak;
+}
+if (leak == 0) {
+return 0;
+}
size = bdrv_co_getlength(bs->file->bs);
if (size < 0) {
res->check_errors++;
return size;
}
-if (size <= res->image_end_offset) {
+res->image_end_offset = size;
+
+if (!explicit) {
return 0;
}
-count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
-if (explicit) {
-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;
-}
+count = DIV_ROUND_UP(leak, s->cluster_size);
+fprintf(stderr,
+"%s space leaked at the end of the image %" PRId64 "\n",
+fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", leak);
+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;
-}
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-
-parallels_free_used_bitmap(bs);
-ret = parallels_fill_used_bitmap(bs);
-if (ret == -ENOMEM) {
-res->check_errors++;
-return ret;
-}
-
-if (explicit) {
-res->leaks_fixed += count;
-}
+res->leaks_fixed += count;
}
return 0;
@@ -1454,23 +1484,6 @@ fail:
return ret;
}
-static int GRAPH_RDLOCK parallels_truncate_unused_clusters(BlockDriverState
*bs)
-{
-BDRVParallelsState *s = bs->opaque;
-uint64_t end_off = 0;
-if (s->used_bmap_size > 0) {
-end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
-if (end_off == s->used_bmap_size) {
-end_off = 0;
-} else {
-end_off = (end_off + 1) * s->cluster_size;
-}
-}
-end_off += s->data_start * BDRV_SECTOR_SIZE;
-s->data_end = end_off / BDRV_SECTOR_SIZE;
-return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
-}
-
static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
@@ -1488,7 +1501,7 @@ static int GRAPH_RDLOCK
parallels_inactivate(BlockDriverState *bs)
parallels_update_header(bs);
/* errors are ignored, so we might as well pass exact=true */
-ret = parallels_truncate_unused_clusters(bs);
+ret = parallels_check_unused_clusters(bs, true);
if (ret < 0) {
error_report("Failed to truncate
