[1/1] qemu-img: wait for convert coroutines to complete
diff mbox

Message ID 1492511251-16720-1-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev April 18, 2017, 10:27 a.m. UTC
From: Anton Nefedov <anton.nefedov@virtuozzo.com>

We should wait for other coroutines on error path, i.e. one of coroutines
terminates with i/o error, before cleaning the common structures. In the
other case we would crash in a lot of different places. This behaviour
was introduced by commit 2d9187bc65.

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Peter Lieven <pl@kamp.de>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Lieven April 18, 2017, 11:22 a.m. UTC | #1
Am 18.04.2017 um 12:27 schrieb Denis V. Lunev:
> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>
> We should wait for other coroutines on error path, i.e. one of coroutines
> terminates with i/o error, before cleaning the common structures. In the
> other case we would crash in a lot of different places. This behaviour
> was introduced by commit 2d9187bc65.
>
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Peter Lieven <pl@kamp.de>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>
> ---
>   qemu-img.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index b220cf7..3b04c5f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1899,7 +1899,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);
>       }
>   

Reviewed-by: Peter Lieven <pl@kamp.de>


Thanks for catching this.


Peter
Peter Lieven April 21, 2017, 9:18 a.m. UTC | #2
Am 18.04.2017 um 12:27 schrieb Denis V. Lunev:
> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>
> We should wait for other coroutines on error path, i.e. one of coroutines
> terminates with i/o error, before cleaning the common structures. In the
> other case we would crash in a lot of different places. This behaviour
> was introduced by commit 2d9187bc65.
>
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Peter Lieven <pl@kamp.de>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Max Reitz <mreitz@redhat.com>

Cc: qemu-stable@nongnu.org

This should go into 2.9.1

Peter
Anton Nefedov April 21, 2017, 9:43 a.m. UTC | #3
On 04/21/2017 12:18 PM, Peter Lieven wrote:
> Am 18.04.2017 um 12:27 schrieb Denis V. Lunev:
>> From: Anton Nefedov <anton.nefedov@virtuozzo.com>
>>
>> We should wait for other coroutines on error path, i.e. one of coroutines
>> terminates with i/o error, before cleaning the common structures. In the
>> other case we would crash in a lot of different places. This behaviour
>> was introduced by commit 2d9187bc65.
>>
>> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Peter Lieven <pl@kamp.de>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Max Reitz <mreitz@redhat.com>
>
> Cc: qemu-stable@nongnu.org
>
> This should go into 2.9.1
>
> Peter
>

Actually I'm afraid the patch is incorrect.. The erroneous coroutine 
bails out without reentering its possible dependent coroutine so it (and 
the following dependents) will never finish.

I'll send out the 2nd version soon

/Anton

Patch
diff mbox

diff --git a/qemu-img.c b/qemu-img.c
index b220cf7..3b04c5f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1899,7 +1899,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);
     }