Re: [PATCH v2 4/8] parallels: Move check of unclean image to a separate function

[Denis V. Lunev]

On 11.08.2022 17:00, Alexander Ivanov wrote:
> v2: Revert the condition with s->header_unclean.
same comment about change log as previously

And commit message misses motivation part, why we are
doing this rework. What is the goal of this change?

The code part is clean.

> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   block/parallels.c | 31 +++++++++++++++++++++----------
>   1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 6879ea4597..c53b2810cf 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -419,6 +419,25 @@ static coroutine_fn int 
> parallels_co_readv(BlockDriverState *bs,
>       return ret;
>   }
>   
> +static void parallels_check_unclean(BlockDriverState *bs,
> +                                    BdrvCheckResult *res,
> +                                    BdrvCheckMode fix)
> +{
> +    BDRVParallelsState *s = bs->opaque;
> +
> +    if (!s->header_unclean) {
> +        return;
> +    }
> +
> +    fprintf(stderr, "%s image was not closed correctly\n",
> +            fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
> +    res->corruptions++;
> +    if (fix & BDRV_FIX_ERRORS) {
> +        /* parallels_close will do the job right */
> +        res->corruptions_fixed++;
> +        s->header_unclean = false;
> +    }
> +}
>   
>   static int coroutine_fn parallels_co_check(BlockDriverState *bs,
>                                              BdrvCheckResult *res,
> @@ -436,16 +455,8 @@ static int coroutine_fn 
> parallels_co_check(BlockDriverState *bs,
>       }
>   
>       qemu_co_mutex_lock(&s->lock);
> -    if (s->header_unclean) {
> -        fprintf(stderr, "%s image was not closed correctly\n",
> -                fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
> -        res->corruptions++;
> -        if (fix & BDRV_FIX_ERRORS) {
> -            /* parallels_close will do the job right */
> -            res->corruptions_fixed++;
> -            s->header_unclean = false;
> -        }
> -    }
> +
> +    parallels_check_unclean(bs, res, fix);
>   
>       res->bfi.total_clusters = s->bat_size;
>       res->bfi.compressed_clusters = 0; /* compression is not supported */