Without this filter, this test sometimes fails.
Signed-off-by: Max Reitz <mreitz@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.
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
--
2.20.1
On 1/30/19 6:52 PM, Max Reitz wrote: > Without this filter, this test sometimes fails. > > Signed-off-by: Max Reitz <mreitz@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? 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 <jsnow@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.
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 <mreitz@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 <jsnow@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
Am 31.01.2019 um 00:52 hat Max Reitz geschrieben: > Without this filter, this test sometimes fails. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Thanks, applied to the block branch. Kevin
© 2016 - 2024 Red Hat, Inc.