On 8/5/22 18:47, alexander.iva...@virtuozzo.com wrote:
From: Alexander Ivanov <alexander.iva...@virtuozzo.com>
A bit strange to see this. Is your name set up correctly in ~/.gitconfig ?
We will add more and more checks of images so we need to reorganize the code. Put each check to a separate helper function with a separate loop. Add two helpers: truncate_file() and sync_header(). They will be used in multiple functions.
It would be a lot simpler to review if you split out one check_ function per patch :)
Signed-off-by: Alexander Ivanov <alexander.iva...@virtuozzo.com> ---
[..]
- /* - * In order to really repair the image, we must shrink it. - * That means we have to pass exact=true. - */ - ret = bdrv_truncate(bs->file, res->image_end_offset, true, - PREALLOC_MODE_OFF, 0, &local_err); + ret = truncate_file(bs, res->image_end_offset); if (ret < 0) { - error_report_err(local_err); - res->check_errors++;
Why you remove check_errors++ ? It's an error of truncate_file counted. With it kept here: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
- goto out; + return ret; } res->leaks_fixed += count; }
[..]
+static int coroutine_fn parallels_co_check(BlockDriverState *bs, + BdrvCheckResult *res, + BdrvCheckMode fix) +{ + BDRVParallelsState *s = bs->opaque; + int ret; + + qemu_co_mutex_lock(&s->lock);
If touch this anyway, I'd move to QEMU_LOCK_GUARD().
+ + check_unclean(bs, res, fix); + + ret = check_cluster_outside_image(bs, res, fix); + if (ret != 0) { + goto out; + } + + check_fragmentation(bs, res, fix); + + ret = check_leak(bs, res, fix); + if (ret != 0) { + goto out; + } + + collect_statistics(bs, res);
probably, chack_fragmentation() should be part of collect_statistics() (called from it, or just merged into same loop, as it fills same res->bfi) Also while reviewing I noted that when FIX_ERRORS is not enabled we count broken clusters (with off > size) as allocated, and also they participate in fragmentation statistics. That's preexisting still.. -- Best regards, Vladimir