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

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

[Anton Nefedov]

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 <anton.nefedov@virtuozzo.com>
---
 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

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 <anton.nefedov@virtuozzo.com>:
> 
> 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 <anton.nefedov@virtuozzo.com>
> ---
> 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 <pl@kamp.de <mailto:pl@kamp.de>>

Peter

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

[Kevin Wolf]

Am 26.04.2017 um 10:33 hat Anton Nefedov geschrieben:
> 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 <anton.nefedov@virtuozzo.com>

Thanks, applied to the block branch.

Kevin