On 31.01.19 03:21, John Snow wrote: > > > On 1/30/19 6:52 PM, Max Reitz wrote: >> Without this filter, this test sometimes fails. >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> I intended to send this as part of my iotest fixes series, but it ended >> up on the wrong branch... Doesn't really matter, though, as there is no >> functional dependency. >> --- >> tests/qemu-iotests/229 | 6 +++++- >> tests/qemu-iotests/229.out | 1 - >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229 >> index 893d098ad2..b0d4885fa6 100755 >> --- a/tests/qemu-iotests/229 >> +++ b/tests/qemu-iotests/229 >> @@ -81,11 +81,15 @@ echo >> echo '=== Force cancel job paused in error state ===' >> echo >> >> +# Filter out BLOCK_JOB_ERROR events because they may or may not occur. >> +# Cancelling the job means resuming it for a bit before it is actually >> +# aborted, and in that time it may or may not re-encounter the error. > > Oh, because the job is "paused" and cancelling it involves job_enter, > which we then allow the job to gracefully fail through it's own pathways > -- but depending on where it failed originally, it may-or-may-not wind > up trying something else that fails before it finds the "exit > gracefully" signal, is that right?
That's at least how I explained it to me, yes.
> I guess there's no real way to adjust that behavior.
>
>> success_or_failure="y" _send_qemu_cmd $QEMU_HANDLE \
>> "{'execute': 'block-job-cancel',
>> 'arguments': { 'device': 'testdisk',
>> 'force': true}}" \
>> - "BLOCK_JOB_CANCELLED" "Assertion"
>> + "BLOCK_JOB_CANCELLED" "Assertion" \
>> + | grep -v '"BLOCK_JOB_ERROR"'
>>
>> # success, all done
>> echo "*** done"
>> diff --git a/tests/qemu-iotests/229.out b/tests/qemu-iotests/229.out
>> index 4c4112805f..a3eb33788a 100644
>> --- a/tests/qemu-iotests/229.out
>> +++ b/tests/qemu-iotests/229.out
>> @@ -17,7 +17,6 @@ wrote 2097152/2097152 bytes at offset 0
>>
>> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
>> {"return": {}}
>> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "BLOCK_JOB_ERROR", "data": {"device": "testdisk", "operation": "write",
>> "action": "stop"}}
>> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "testdisk"}}
>> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
>> "BLOCK_JOB_CANCELLED", "data": {"device": "testdisk", "len": 2097152,
>> "offset": 1048576, "speed": 0, "type": "mirror"}}
>> *** done
>>
>
> I think this is fine, if we cannot help to make this any more
> deterministic, so I'm fine with:
>
> Reviewed-by: John Snow <js...@redhat.com>
>
> but I am curious to know if this poses any theoretical problems for
> libvirt having to deal with possibly an extra hiccup before the cancel
> registers.
Hm... I would say the bigger issue is the error event appearing than
that not being the case. Because it appears most of the time, I think
libvirt can deal with it.
Max
signature.asc
Description: OpenPGP digital signature
