Re: [PATCH 06/21] parallels: refactor path when we need to re-check image in parallels_open

[Alexander Ivanov]

On 9/15/23 20:41, Denis V. Lunev wrote:
> More conditions follows thus the check should be more scalable.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>   block/parallels.c | 19 ++++++++-----------
>   1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 8f223bfd89..aa29df9f77 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -1065,7 +1065,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>       int ret, size, i;
>       int64_t file_nb_sectors, sector;
>       uint32_t data_start;
> -    bool data_off_is_correct;
> +    bool need_check = false;
>   
>       ret = parallels_opts_prealloc(bs, options, errp);
>       if (ret < 0) {
> @@ -1133,11 +1133,12 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>       s->bat_bitmap = (uint32_t *)(s->header + 1);
>   
>       if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
> -        s->header_unclean = true;
> +        need_check = s->header_unclean = true;
>       }
>   
> -    data_off_is_correct = parallels_test_data_off(s, file_nb_sectors,
> -                                                  &data_start);
> +    need_check = need_check ||
> +                 !parallels_test_data_off(s, file_nb_sectors, &data_start);
> +
>       s->data_start = data_start;
>       s->data_end = s->data_start;
>       if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
> @@ -1194,6 +1195,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>               s->data_end = sector + s->tracks;
>           }
>       }
> +    need_check = need_check || s->data_end > file_nb_sectors;
>   
>       /*
>        * We don't repair the image here if it's opened for checks. Also we don't
> @@ -1203,12 +1205,8 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>           return 0;
>       }
>   
> -    /*
> -     * Repair the image if it's dirty or
> -     * out-of-image corruption was detected.
> -     */
> -    if (s->data_end > file_nb_sectors || s->header_unclean
> -        || !data_off_is_correct) {
> +    /* Repair the image if corruption was detected. */
> +    if (need_check) {
>           BdrvCheckResult res;
>           ret = bdrv_check(bs, &res, BDRV_FIX_ERRORS | BDRV_FIX_LEAKS);
>           if (ret < 0) {
> @@ -1217,7 +1215,6 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>               goto fail;
>           }
>       }
> -
>       return 0;
>   
>   fail_format:

Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>