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

Commit Message

Denis V. Lunev April 18, 2017, 10:27 a.m. UTC

From: Anton Nefedov <[e-mail]>

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 <[e-mail]>
Signed-off-by: Denis V. Lunev <[e-mail]>
CC: Peter Lieven <[e-mail]>
CC: Kevin Wolf <[e-mail]>
CC: Max Reitz <[e-mail]>
---
 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 <[e-mail]>
>
> 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 <[e-mail]>
> Signed-off-by: Denis V. Lunev <[e-mail]>
> CC: Peter Lieven <[e-mail]>
> CC: Kevin Wolf <[e-mail]>
> CC: Max Reitz <[e-mail]>
> ---
>   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 <[e-mail]>


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 <[e-mail]>
>
> 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 <[e-mail]>
> Signed-off-by: Denis V. Lunev <[e-mail]>
> CC: Peter Lieven <[e-mail]>
> CC: Kevin Wolf <[e-mail]>
> CC: Max Reitz <[e-mail]>

Cc: [e-mail]

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 <[e-mail]>
>>
>> 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 <[e-mail]>
>> Signed-off-by: Denis V. Lunev <[e-mail]>
>> CC: Peter Lieven <[e-mail]>
>> CC: Kevin Wolf <[e-mail]>
>> CC: Max Reitz <[e-mail]>
>
> Cc: [e-mail]
>
> 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 --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);
     }