| Message ID | 20220815090216.1818622-7-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parallels: Refactor the code of images checks and fix a bug | expand |
On 8/15/22 12:02, 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> > --- > v2: No change. > v3: Fix commit message. > > block/parallels.c | 85 +++++++++++++++++++++++++++++------------------ > 1 file changed, 52 insertions(+), 33 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index 12104ba5ad..8737eadfb4 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -469,14 +469,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; 'i' should be kept to be uint32_t, as pre-patch. With it fixed: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
On 17.08.2022 23:00, Vladimir Sementsov-Ogievskiy wrote: > On 8/15/22 12:02, 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> >> --- >> v2: No change. >> v3: Fix commit message. >> >> block/parallels.c | 85 +++++++++++++++++++++++++++++------------------ >> 1 file changed, 52 insertions(+), 33 deletions(-) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index 12104ba5ad..8737eadfb4 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -469,14 +469,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; > > 'i' should be kept to be uint32_t, as pre-patch. > > With it fixed: > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > > Will fix it, thank you.
diff --git a/block/parallels.c b/block/parallels.c index 12104ba5ad..8737eadfb4 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -469,14 +469,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) { @@ -484,41 +483,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", @@ -536,11 +510,56 @@ 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; } } + 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);
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> --- v2: No change. v3: Fix commit message. block/parallels.c | 85 +++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 33 deletions(-)