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.ivanov@virtuozzo.com>
---
block/parallels.c | 87 +++++++++++++++++++++++++++++------------------
1 file changed, 54 insertions(+), 33 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index bf074f247c..12e8d382f3 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -468,14 +468,13 @@ static int parallels_check_outside_image(BlockDriverState *bs,
return 0;
}
-static int coroutine_fn parallels_co_check(BlockDriverState *bs,
- BdrvCheckResult *res,
- BdrvCheckMode fix)
+static int parallels_check_leak(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
{
BDRVParallelsState *s = bs->opaque;
- int64_t size, prev_off, high_off;
- int ret;
- uint32_t i;
+ int64_t size, off, high_off, count;
+ int i, ret;
size = bdrv_getlength(bs->file->bs);
if (size < 0) {
@@ -483,41 +482,16 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
return size;
}
- qemu_co_mutex_lock(&s->lock);
-
- parallels_check_unclean(bs, res, fix);
-
- ret = parallels_check_outside_image(bs, res, fix);
- if (ret < 0) {
- goto out;
- }
-
- res->bfi.total_clusters = s->bat_size;
- res->bfi.compressed_clusters = 0; /* compression is not supported */
-
high_off = 0;
- prev_off = 0;
for (i = 0; i < s->bat_size; i++) {
- int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
- if (off == 0) {
- prev_off = 0;
- continue;
- }
-
- res->bfi.allocated_clusters++;
+ off = bat2sect(s, i) << BDRV_SECTOR_BITS;
if (off > high_off) {
high_off = off;
}
-
- if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
- res->bfi.fragmented_clusters++;
- }
- prev_off = off;
}
res->image_end_offset = high_off + s->cluster_size;
if (size > res->image_end_offset) {
- int64_t count;
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",
@@ -535,11 +509,12 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
if (ret < 0) {
error_report_err(local_err);
res->check_errors++;
- goto out;
+ return ret;
}
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.
@@ -549,6 +524,52 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
size = res->image_end_offset;
}
s->data_end = size >> BDRV_SECTOR_BITS;
+
+ return 0;
+}
+
+static int coroutine_fn parallels_co_check(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int64_t prev_off;
+ int ret;
+ uint32_t i;
+
+ qemu_co_mutex_lock(&s->lock);
+
+ parallels_check_unclean(bs, res, fix);
+
+ ret = parallels_check_outside_image(bs, res, fix);
+ if (ret < 0) {
+ goto out;
+ }
+
+ ret = parallels_check_leak(bs, res, fix);
+ if (ret < 0) {
+ goto out;
+ }
+
+ res->bfi.total_clusters = s->bat_size;
+ res->bfi.compressed_clusters = 0; /* compression is not supported */
+
+ prev_off = 0;
+ for (i = 0; i < s->bat_size; i++) {
+ int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+ if (off == 0) {
+ prev_off = 0;
+ continue;
+ }
+
+ res->bfi.allocated_clusters++;
+
+ if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
+ res->bfi.fragmented_clusters++;
+ }
+ prev_off = off;
+ }
+
out:
qemu_co_mutex_unlock(&s->lock);
--
2.34.1On 22.08.2022 11:05, 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.ivanov@virtuozzo.com>
> ---
> block/parallels.c | 87 +++++++++++++++++++++++++++++------------------
> 1 file changed, 54 insertions(+), 33 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index bf074f247c..12e8d382f3 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -468,14 +468,13 @@ static int parallels_check_outside_image(BlockDriverState *bs,
> return 0;
> }
>
> -static int coroutine_fn parallels_co_check(BlockDriverState *bs,
> - BdrvCheckResult *res,
> - BdrvCheckMode fix)
> +static int parallels_check_leak(BlockDriverState *bs,
> + BdrvCheckResult *res,
> + BdrvCheckMode fix)
> {
> BDRVParallelsState *s = bs->opaque;
> - int64_t size, prev_off, high_off;
> - int ret;
> - uint32_t i;
> + int64_t size, off, high_off, count;
> + int i, ret;
This is not we have agreed on. 'i' should be uint32_t
You have fixed parallels_co_check, but this needs
the fix too.
>
> size = bdrv_getlength(bs->file->bs);
> if (size < 0) {
> @@ -483,41 +482,16 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
> return size;
> }
>
> - qemu_co_mutex_lock(&s->lock);
> -
> - parallels_check_unclean(bs, res, fix);
> -
> - ret = parallels_check_outside_image(bs, res, fix);
> - if (ret < 0) {
> - goto out;
> - }
> -
> - res->bfi.total_clusters = s->bat_size;
> - res->bfi.compressed_clusters = 0; /* compression is not supported */
> -
> high_off = 0;
> - prev_off = 0;
> for (i = 0; i < s->bat_size; i++) {
> - int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
> - if (off == 0) {
> - prev_off = 0;
> - continue;
> - }
> -
> - res->bfi.allocated_clusters++;
> + off = bat2sect(s, i) << BDRV_SECTOR_BITS;
> if (off > high_off) {
> high_off = off;
> }
> -
> - if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
> - res->bfi.fragmented_clusters++;
> - }
> - prev_off = off;
> }
>
> res->image_end_offset = high_off + s->cluster_size;
> if (size > res->image_end_offset) {
> - int64_t count;
> 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",
> @@ -535,11 +509,12 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
> if (ret < 0) {
> error_report_err(local_err);
> res->check_errors++;
> - goto out;
> + return ret;
> }
> 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.
> @@ -549,6 +524,52 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
> size = res->image_end_offset;
> }
> s->data_end = size >> BDRV_SECTOR_BITS;
> +
> + return 0;
> +}
> +
> +static int coroutine_fn parallels_co_check(BlockDriverState *bs,
> + BdrvCheckResult *res,
> + BdrvCheckMode fix)
> +{
> + BDRVParallelsState *s = bs->opaque;
> + int64_t prev_off;
> + int ret;
> + uint32_t i;
> +
> + qemu_co_mutex_lock(&s->lock);
> +
> + parallels_check_unclean(bs, res, fix);
> +
> + ret = parallels_check_outside_image(bs, res, fix);
> + if (ret < 0) {
> + goto out;
> + }
> +
> + ret = parallels_check_leak(bs, res, fix);
> + if (ret < 0) {
> + goto out;
> + }
> +
> + res->bfi.total_clusters = s->bat_size;
> + res->bfi.compressed_clusters = 0; /* compression is not supported */
> +
> + prev_off = 0;
> + for (i = 0; i < s->bat_size; i++) {
> + int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
> + if (off == 0) {
> + prev_off = 0;
> + continue;
> + }
> +
> + res->bfi.allocated_clusters++;
> +
> + if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
> + res->bfi.fragmented_clusters++;
> + }
> + prev_off = off;
> + }
> +
> out:
> qemu_co_mutex_unlock(&s->lock);
>
On 23.08.2022 11:40, Denis V. Lunev wrote:
> On 22.08.2022 11:05, 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.ivanov@virtuozzo.com>
>> ---
>> block/parallels.c | 87 +++++++++++++++++++++++++++++------------------
>> 1 file changed, 54 insertions(+), 33 deletions(-)
>>
>> diff --git a/block/parallels.c b/block/parallels.c
>> index bf074f247c..12e8d382f3 100644
>> --- a/block/parallels.c
>> +++ b/block/parallels.c
>> @@ -468,14 +468,13 @@ static int
>> parallels_check_outside_image(BlockDriverState *bs,
>> return 0;
>> }
>> -static int coroutine_fn parallels_co_check(BlockDriverState *bs,
>> - BdrvCheckResult *res,
>> - BdrvCheckMode fix)
>> +static int parallels_check_leak(BlockDriverState *bs,
>> + BdrvCheckResult *res,
>> + BdrvCheckMode fix)
>> {
>> BDRVParallelsState *s = bs->opaque;
>> - int64_t size, prev_off, high_off;
>> - int ret;
>> - uint32_t i;
>> + int64_t size, off, high_off, count;
>> + int i, ret;
> This is not we have agreed on. 'i' should be uint32_t
> You have fixed parallels_co_check, but this needs
> the fix too.
Damn! Missed it when rebased on changed patches.
>
>> size = bdrv_getlength(bs->file->bs);
>> if (size < 0) {
>> @@ -483,41 +482,16 @@ static int coroutine_fn
>> parallels_co_check(BlockDriverState *bs,
>> return size;
>> }
>> - qemu_co_mutex_lock(&s->lock);
>> -
>> - parallels_check_unclean(bs, res, fix);
>> -
>> - ret = parallels_check_outside_image(bs, res, fix);
>> - if (ret < 0) {
>> - goto out;
>> - }
>> -
>> - res->bfi.total_clusters = s->bat_size;
>> - res->bfi.compressed_clusters = 0; /* compression is not
>> supported */
>> -
>> high_off = 0;
>> - prev_off = 0;
>> for (i = 0; i < s->bat_size; i++) {
>> - int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
>> - if (off == 0) {
>> - prev_off = 0;
>> - continue;
>> - }
>> -
>> - res->bfi.allocated_clusters++;
>> + off = bat2sect(s, i) << BDRV_SECTOR_BITS;
>> if (off > high_off) {
>> high_off = off;
>> }
>> -
>> - if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
>> - res->bfi.fragmented_clusters++;
>> - }
>> - prev_off = off;
>> }
>> res->image_end_offset = high_off + s->cluster_size;
>> if (size > res->image_end_offset) {
>> - int64_t count;
>> 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",
>> @@ -535,11 +509,12 @@ static int coroutine_fn
>> parallels_co_check(BlockDriverState *bs,
>> if (ret < 0) {
>> error_report_err(local_err);
>> res->check_errors++;
>> - goto out;
>> + return ret;
>> }
>> 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.
>> @@ -549,6 +524,52 @@ static int coroutine_fn
>> parallels_co_check(BlockDriverState *bs,
>> size = res->image_end_offset;
>> }
>> s->data_end = size >> BDRV_SECTOR_BITS;
>> +
>> + return 0;
>> +}
>> +
>> +static int coroutine_fn parallels_co_check(BlockDriverState *bs,
>> + BdrvCheckResult *res,
>> + BdrvCheckMode fix)
>> +{
>> + BDRVParallelsState *s = bs->opaque;
>> + int64_t prev_off;
>> + int ret;
>> + uint32_t i;
>> +
>> + qemu_co_mutex_lock(&s->lock);
>> +
>> + parallels_check_unclean(bs, res, fix);
>> +
>> + ret = parallels_check_outside_image(bs, res, fix);
>> + if (ret < 0) {
>> + goto out;
>> + }
>> +
>> + ret = parallels_check_leak(bs, res, fix);
>> + if (ret < 0) {
>> + goto out;
>> + }
>> +
>> + res->bfi.total_clusters = s->bat_size;
>> + res->bfi.compressed_clusters = 0; /* compression is not
>> supported */
>> +
>> + prev_off = 0;
>> + for (i = 0; i < s->bat_size; i++) {
>> + int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
>> + if (off == 0) {
>> + prev_off = 0;
>> + continue;
>> + }
>> +
>> + res->bfi.allocated_clusters++;
>> +
>> + if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
>> + res->bfi.fragmented_clusters++;
>> + }
>> + prev_off = off;
>> + }
>> +
>> out:
>> qemu_co_mutex_unlock(&s->lock);
>
© 2016 - 2026 Red Hat, Inc.