Re: [PATCH 6/6] parallels: Image repairing in parallels_open()

[Denis V. Lunev]

On 9/2/22 10:53, Alexander Ivanov wrote:
> Repair an image at opening if the image is unclean or
> out-of-image corruption was detected.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   block/parallels.c | 95 ++++++++++++++++++++++++++++++++---------------
>   1 file changed, 65 insertions(+), 30 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 08526196da..a7c3af4ef2 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -735,6 +735,18 @@ static int coroutine_fn 
> parallels_co_check(BlockDriverState *bs,
>       return ret;
>   }
>   
> +typedef struct ParallelsOpenCheckCo {
> +    BlockDriverState *bs;
> +    BdrvCheckResult *res;
> +    BdrvCheckMode fix;
> +    int ret;
> +} ParallelsOpenCheckCo;
> +
> +static void coroutine_fn parallels_co_open_check(void *opaque)
> +{
> +    ParallelsOpenCheckCo *poc = opaque;
> +    poc->ret = parallels_co_check(poc->bs, poc->res, poc->fix);
> +}
>   
>   static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
>                                               Error **errp)
> @@ -947,8 +959,8 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>   {
>       BDRVParallelsState *s = bs->opaque;
>       ParallelsHeader ph;
> -    int ret, size, i;
> -    int64_t file_size;
> +    int ret, size;
> +    int64_t file_size, high_off;
>       QemuOpts *opts = NULL;
>       Error *local_err = NULL;
>       char *buf;
> @@ -1027,34 +1039,6 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>       }
>       s->bat_bitmap = (uint32_t *)(s->header + 1);
>   
> -    for (i = 0; i < s->bat_size; i++) {
> -        int64_t off = bat2sect(s, i);
> -        if (off >= file_size) {
> -            if (flags & BDRV_O_CHECK) {
> -                continue;
> -            }
> -            error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry "
> -                       "is larger than file size (%" PRIi64 ")",
> -                       off, i, file_size);
> -            ret = -EINVAL;
> -            goto fail;
> -        }
> -        if (off >= s->data_end) {
> -            s->data_end = off + s->tracks;
> -        }
> -    }
> -
> -    if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
> -        /* Image was not closed correctly. The check is mandatory */
> -        s->header_unclean = true;
> -        if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_CHECK)) {
> -            error_setg(errp, "parallels: Image was not closed correctly; "
> -                       "cannot be opened read/write");
> -            ret = -EACCES;
> -            goto fail;
> -        }
> -    }
> -
>       opts = qemu_opts_create(&parallels_runtime_opts, NULL, 0, errp);
>       if (!opts) {
>           goto fail_options;
> @@ -1116,7 +1100,58 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>           error_free(s->migration_blocker);
>           goto fail;
>       }
> +
>       qemu_co_mutex_init(&s->lock);
> +
> +    if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
> +        s->header_unclean = true;
> +    }
> +
> +    high_off = highest_offset(s) >> BDRV_SECTOR_BITS;
> +    if (high_off >= s->data_end) {
> +        s->data_end = high_off + s->tracks;
> +    }
> +
> +    /*
> +     * We don't repair the image here if it is opened for checks.
> +     * Also let to work with images in RO mode.
> +     */
> +    if ((flags & BDRV_O_CHECK) || !(flags & BDRV_O_RDWR)) {
> +        return 0;
> +    }
> +
> +    /*
> +     * Repair the image if it's dirty or
> +     * out-of-image corruption was detected.
> +     */
> +    if (s->data_end > file_size ||
> +        le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
> +        BdrvCheckResult res = {0};
> +        Coroutine *co;
> +        ParallelsOpenCheckCo poc = {
> +            .bs = bs,
> +            .res = &res,
> +            .fix = BDRV_FIX_ERRORS | BDRV_FIX_LEAKS,
> +            .ret = -EINPROGRESS
> +        };
> +
> +        if (qemu_in_coroutine()) {
> +            /* From bdrv_co_create.  */
> +            parallels_co_open_check(&poc);
> +        } else {
> +            assert(qemu_get_current_aio_context() == qemu_get_aio_context());
> +            co = qemu_coroutine_create(parallels_co_open_check, &poc);
> +            qemu_coroutine_enter(co);
> +            BDRV_POLL_WHILE(bs, poc.ret == -EINPROGRESS);
> +        }
> +
> +        if (poc.ret < 0) {
> +            error_setg_errno(errp, -poc.ret,
> +                             "Could not repair corrupted image");
> +            goto fail;
> +        }
> +    }
> +
bdrv_check() is your friend. No need to duplicate the code


>       return 0;
>   
>   fail_format: