Re: [Qemu-devel] [Qemu-stable] [PATCH v3 1/1] qemu-img: wait for convert coroutines to complete

[Peter Lieven]

> Am 26.04.2017 um 10:33 schrieb Anton Nefedov <address@hidden>:
> 
> On error path (like i/o error in one of the coroutines), it's required to
>  - wait for coroutines completion before cleaning the common structures
>  - reenter dependent coroutines so they ever finish
> 
> Introduced in 2d9187bc65.
> 
> Signed-off-by: Anton Nefedov <address@hidden>
> ---
> qemu-img.c | 26 +++++++++++---------------
> 1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index bbe1574..8c50379 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1761,13 +1761,13 @@ static void coroutine_fn convert_co_do_copy(void 
> *opaque)
>         qemu_co_mutex_lock(&s->lock);
>         if (s->ret != -EINPROGRESS || s->sector_num >= s->total_sectors) {
>             qemu_co_mutex_unlock(&s->lock);
> -            goto out;
> +            break;
>         }
>         n = convert_iteration_sectors(s, s->sector_num);
>         if (n < 0) {
>             qemu_co_mutex_unlock(&s->lock);
>             s->ret = n;
> -            goto out;
> +            break;
>         }
>         /* save current sector and allocation status to local variables */
>         sector_num = s->sector_num;
> @@ -1792,7 +1792,6 @@ static void coroutine_fn convert_co_do_copy(void 
> *opaque)
>                 error_report("error while reading sector %" PRId64
>                              ": %s", sector_num, strerror(-ret));
>                 s->ret = ret;
> -                goto out;
>             }
>         } else if (!s->min_sparse && status == BLK_ZERO) {
>             status = BLK_DATA;
> @@ -1801,22 +1800,20 @@ static void coroutine_fn convert_co_do_copy(void 
> *opaque)
> 
>         if (s->wr_in_order) {
>             /* keep writes in order */
> -            while (s->wr_offs != sector_num) {
> -                if (s->ret != -EINPROGRESS) {
> -                    goto out;
> -                }
> +            while (s->wr_offs != sector_num && s->ret == -EINPROGRESS) {
>                 s->wait_sector_num[index] = sector_num;
>                 qemu_coroutine_yield();
>             }
>             s->wait_sector_num[index] = -1;
>         }
> 
> -        ret = convert_co_write(s, sector_num, n, buf, status);
> -        if (ret < 0) {
> -            error_report("error while writing sector %" PRId64
> -                         ": %s", sector_num, strerror(-ret));
> -            s->ret = ret;
> -            goto out;
> +        if (s->ret == -EINPROGRESS) {
> +            ret = convert_co_write(s, sector_num, n, buf, status);
> +            if (ret < 0) {
> +                error_report("error while writing sector %" PRId64
> +                             ": %s", sector_num, strerror(-ret));
> +                s->ret = ret;
> +            }
>         }
> 
>         if (s->wr_in_order) {
> @@ -1837,7 +1834,6 @@ static void coroutine_fn convert_co_do_copy(void 
> *opaque)
>         }
>     }
> 
> -out:
>     qemu_vfree(buf);
>     s->co[index] = NULL;
>     s->running_coroutines--;
> @@ -1899,7 +1895,7 @@ static int convert_do_copy(ImgConvertState *s)
>         qemu_coroutine_enter(s->co[i]);
>     }
> 
> -    while (s->ret == -EINPROGRESS) {
> +    while (s->running_coroutines) {
>         main_loop_wait(false);
>     }
> 
> -- 
> 2.7.4
> 
> 

Reviewed-by: Peter Lieven <address@hidden <mailto:address@hidden>>

Peter