| Message ID | 1470229005-152704-3-git-send-email-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Vladimir, Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > 109 iotest is broken for raw after 0965a41e998ab820b5 > [mirror: double performance of the bulk stage if the disc is full] > > The problem is with finishing block-job with error: before specified > patch mirror was not very async and it created one big request at disk > start, this request finished with error and qemu produced > BLOCK_JOB_COMPLETED with zero progress. > > After 0965a41, mirror starts several smaller requests in parallel, when > BLOCK_JOB_COMPLETED emited we have some successful non-zero progress. [...] > --- a/tests/qemu-iotests/109.out > +++ b/tests/qemu-iotests/109.out > @@ -135,7 +135,7 @@ Automatically detecting the format is dangerous for raw images, write operations > Specify the 'raw' format explicitly to remove the restrictions. > {"return": {}} > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}} > -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": OFFSET, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} What are the exact semantics of the "offset" field for BLOCK_JOB_COMPLETED? docs/qmp-events.txt is rather vague. As an API consumer I'd have assumed that everything up to offset has been completed successfully. If that interpretation is correct, offset must be 0 for this test because the very first sector wasn't mirrored successfully. Sascha
On 03.08.2016 17:22, Sascha Silbe wrote: > Dear Vladimir, > > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > >> 109 iotest is broken for raw after 0965a41e998ab820b5 >> [mirror: double performance of the bulk stage if the disc is full] >> >> The problem is with finishing block-job with error: before specified >> patch mirror was not very async and it created one big request at disk >> start, this request finished with error and qemu produced >> BLOCK_JOB_COMPLETED with zero progress. >> >> After 0965a41, mirror starts several smaller requests in parallel, when >> BLOCK_JOB_COMPLETED emited we have some successful non-zero progress. > [...] > >> --- a/tests/qemu-iotests/109.out >> +++ b/tests/qemu-iotests/109.out >> @@ -135,7 +135,7 @@ Automatically detecting the format is dangerous for raw images, write operations >> Specify the 'raw' format explicitly to remove the restrictions. >> {"return": {}} >> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}} >> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} >> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": OFFSET, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} > > What are the exact semantics of the "offset" field for > BLOCK_JOB_COMPLETED? > > docs/qmp-events.txt is rather vague. As an API consumer I'd have assumed > that everything up to offset has been completed successfully. If that > interpretation is correct, offset must be 0 for this test because the > very first sector wasn't mirrored successfully. As far as I'm aware, it doesn't have any real semantics besides the fact that $offset / $len is the progress of the block job; so it's the offset "in the job", but not the offset in the source disk. Max
Dear Max, Max Reitz <mreitz@redhat.com> writes: > On 03.08.2016 17:22, Sascha Silbe wrote: [...] >> What are the exact semantics of the "offset" field for >> BLOCK_JOB_COMPLETED? >> >> docs/qmp-events.txt is rather vague. As an API consumer I'd have assumed >> that everything up to offset has been completed successfully. If that >> interpretation is correct, offset must be 0 for this test because the >> very first sector wasn't mirrored successfully. > > As far as I'm aware, it doesn't have any real semantics besides the fact > that $offset / $len is the progress of the block job; so it's the offset > "in the job", but not the offset in the source disk. Ok. Would be nice to mention that in docs/qmp-events.txt, to avoid API consumers relying on more exact semantics (resuming at the block after offset to step over error locations for example). The name 'offset' is pretty suggestive... Sascha
diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109 index adf9889..280ed27 100755 --- a/tests/qemu-iotests/109 +++ b/tests/qemu-iotests/109 @@ -100,7 +100,7 @@ for sample_img in empty.bochs iotest-dirtylog-10G-4M.vhdx parallels-v1 \ _make_test_img 64M bzcat "$SAMPLE_IMG_DIR/$sample_img.bz2" > "$TEST_IMG.src" - run_qemu "$TEST_IMG" "$TEST_IMG.src" "" "BLOCK_JOB_ERROR" + run_qemu "$TEST_IMG" "$TEST_IMG.src" "" "BLOCK_JOB_ERROR" | _filter_block_job_offset $QEMU_IO -c 'read -P 0 0 64k' "$TEST_IMG" | _filter_qemu_io run_qemu "$TEST_IMG" "$TEST_IMG.src" "'format': 'raw'," "BLOCK_JOB_READY" diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out index 7c797ed..e5d70d7 100644 --- a/tests/qemu-iotests/109.out +++ b/tests/qemu-iotests/109.out @@ -135,7 +135,7 @@ Automatically detecting the format is dangerous for raw images, write operations Specify the 'raw' format explicitly to remove the restrictions. {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": OFFSET, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} {"return": []} read 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -155,7 +155,7 @@ Automatically detecting the format is dangerous for raw images, write operations Specify the 'raw' format explicitly to remove the restrictions. {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 31457280, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 31457280, "offset": OFFSET, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} {"return": []} read 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -175,7 +175,7 @@ Automatically detecting the format is dangerous for raw images, write operations Specify the 'raw' format explicitly to remove the restrictions. {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 327680, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 327680, "offset": OFFSET, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} {"return": []} read 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -195,7 +195,7 @@ Automatically detecting the format is dangerous for raw images, write operations Specify the 'raw' format explicitly to remove the restrictions. {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2048, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2048, "offset": OFFSET, "speed": 0, "type": "mirror", "error": "Operation not permitted"}} {"return": []} read 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 7853dbb..3ab6e4d 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -77,6 +77,12 @@ _filter_qmp() -e ' QMP_VERSION' } +# replace block job offset +_filter_block_job_offset() +{ + sed -e 's/, "offset": [0-9]\+,/, "offset": OFFSET,/' +} + # replace driver-specific options in the "Formatting..." line _filter_img_create() {
109 iotest is broken for raw after 0965a41e998ab820b5 [mirror: double performance of the bulk stage if the disc is full] The problem is with finishing block-job with error: before specified patch mirror was not very async and it created one big request at disk start, this request finished with error and qemu produced BLOCK_JOB_COMPLETED with zero progress. After 0965a41, mirror starts several smaller requests in parallel, when BLOCK_JOB_COMPLETED emited we have some successful non-zero progress. This patch solves the issue by filtering out progress from 109 test output. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/109 | 2 +- tests/qemu-iotests/109.out | 8 ++++---- tests/qemu-iotests/common.filter | 6 ++++++ 3 files changed, 11 insertions(+), 5 deletions(-)