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

[Peter Lieven]

Am 21.04.2017 um 14:19 schrieb Anton Nefedov:
> On 04/21/2017 01:44 PM, Peter Lieven wrote:
>> Am 21.04.2017 um 12:04 schrieb 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 <address@hidden>
>>> ---
>>> [..]
>>>
>>
>>
>> And what if we error out in the read path? Wouldn't be something like this 
>> easier?
>>
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 22f559a..4ff1085 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1903,6 +1903,16 @@ static int convert_do_copy(ImgConvertState *s)
>>          main_loop_wait(false);
>>      }
>>
>> +    /* on error path we need to enter all coroutines that are still
>> +     * running before cleaning up common structures */
>> +    if (s->ret) {
>> +        for (i = 0; i < s->num_coroutines; i++) {
>> +             if (s->co[i]) {
>> +                 qemu_coroutine_enter(s->co[i]);
>> +             }
>> +        }
>> +    }
>> +
>>      if (s->compressed && !s->ret) {
>>          /* signal EOF to align */
>>          ret = blk_pwrite_compressed(s->target, 0, NULL, 0);
>>
>>
>> Peter
>>
>
> seemed a bit too daring to me to re-enter every coroutine potentially 
> including the ones that yielded waiting for I/O completion.
> If that's ok - that is for sure easier :)

I think we should enter every coroutine that is still running and have it 
terminate correctly. It was my mistake that I have not
done this in the original patch. Can you check if the above fixes your test 
cases that triggered the bug?

Peter