| Message ID | 20191015193503.25591-2-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests: More iotest 223 improvements | expand |
On 15.10.19 21:35, Eric Blake wrote: > This test has been broken since 3.0. It used TEST_IMG to influence > the name of a file created during _make_test_img, but commit 655ae6bb > changed things so that the wrong file name is being created, which > then caused _launch_qemu to fail. In the meantime, the set of events > issued for the actions of the test has increased. > > Why haven't we noticed the failure? Because the test rarely gets run: > './check -qcow2 173' is insufficient (that defaults to using file protocol) > './check -nfs 173' is insufficient (that defaults to using raw format) > so the test is only run with: > ./check -qcow2 -nfs 173 > > Note that we already have a number of other problems with -nfs: > ./check -nfs (fails 18/30) > ./check -qcow2 -nfs (fails 45/76 after this patch) > and it's not on my priority list to fix those. Rather, I found this > because of my next patch's work on tests using _send_qemu_cmd. > > Fixes: 655ae6b > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > tests/qemu-iotests/173 | 4 ++-- > tests/qemu-iotests/173.out | 6 +++++- > 2 files changed, 7 insertions(+), 3 deletions(-) Reviewed-by: Max Reitz <mreitz@redhat.com>
On 15.10.19 21:35, Eric Blake wrote: > This test has been broken since 3.0. It used TEST_IMG to influence > the name of a file created during _make_test_img, but commit 655ae6bb > changed things so that the wrong file name is being created, which > then caused _launch_qemu to fail. In the meantime, the set of events > issued for the actions of the test has increased. > > Why haven't we noticed the failure? Because the test rarely gets run: > './check -qcow2 173' is insufficient (that defaults to using file protocol) > './check -nfs 173' is insufficient (that defaults to using raw format) > so the test is only run with: > ./check -qcow2 -nfs 173 > > Note that we already have a number of other problems with -nfs: > ./check -nfs (fails 18/30) > ./check -qcow2 -nfs (fails 45/76 after this patch) > and it's not on my priority list to fix those. Rather, I found this > because of my next patch's work on tests using _send_qemu_cmd. > > Fixes: 655ae6b > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > tests/qemu-iotests/173 | 4 ++-- > tests/qemu-iotests/173.out | 6 +++++- > 2 files changed, 7 insertions(+), 3 deletions(-) On second thought, I wonder whether this test actually does anything with NFS. It doesn’t look like it to me. I wonder because for some reason I can’t get NFS to work with qemu at all. I don’t think the iotests are at fault why so many tests fail, actually. Max
On 17.10.19 13:46, Max Reitz wrote: > On 15.10.19 21:35, Eric Blake wrote: >> This test has been broken since 3.0. It used TEST_IMG to influence >> the name of a file created during _make_test_img, but commit 655ae6bb >> changed things so that the wrong file name is being created, which >> then caused _launch_qemu to fail. In the meantime, the set of events >> issued for the actions of the test has increased. >> >> Why haven't we noticed the failure? Because the test rarely gets run: >> './check -qcow2 173' is insufficient (that defaults to using file protocol) >> './check -nfs 173' is insufficient (that defaults to using raw format) >> so the test is only run with: >> ./check -qcow2 -nfs 173 >> >> Note that we already have a number of other problems with -nfs: >> ./check -nfs (fails 18/30) >> ./check -qcow2 -nfs (fails 45/76 after this patch) >> and it's not on my priority list to fix those. Rather, I found this >> because of my next patch's work on tests using _send_qemu_cmd. >> >> Fixes: 655ae6b >> Signed-off-by: Eric Blake <eblake@redhat.com> >> --- >> tests/qemu-iotests/173 | 4 ++-- >> tests/qemu-iotests/173.out | 6 +++++- >> 2 files changed, 7 insertions(+), 3 deletions(-) > > On second thought, I wonder whether this test actually does anything > with NFS. It doesn’t look like it to me. > > I wonder because for some reason I can’t get NFS to work with qemu at > all. I don’t think the iotests are at fault why so many tests fail, > actually. OK, I was just missing an “insecure” in my exports. I hate debugging NFS. Now I’m down to 16/76 for qcow2, and most of those look benign. (As in, they simply don’t support nfs.) Max
On 10/17/19 7:17 AM, Max Reitz wrote: >>> Why haven't we noticed the failure? Because the test rarely gets run: >>> './check -qcow2 173' is insufficient (that defaults to using file protocol) >>> './check -nfs 173' is insufficient (that defaults to using raw format) >>> so the test is only run with: >>> ./check -qcow2 -nfs 173 >>> >>> Note that we already have a number of other problems with -nfs: >>> ./check -nfs (fails 18/30) >>> ./check -qcow2 -nfs (fails 45/76 after this patch) >>> and it's not on my priority list to fix those. Rather, I found this >>> because of my next patch's work on tests using _send_qemu_cmd. >>> >>> Fixes: 655ae6b >>> Signed-off-by: Eric Blake <eblake@redhat.com> >>> --- >>> tests/qemu-iotests/173 | 4 ++-- >>> tests/qemu-iotests/173.out | 6 +++++- >>> 2 files changed, 7 insertions(+), 3 deletions(-) >> >> On second thought, I wonder whether this test actually does anything >> with NFS. It doesn’t look like it to me. >> >> I wonder because for some reason I can’t get NFS to work with qemu at >> all. I don’t think the iotests are at fault why so many tests fail, >> actually. > > OK, I was just missing an “insecure” in my exports. I hate debugging NFS. Hmm, that probably explains some of my failures as well. It is not obvious what has to be done to a system prior to being able to even run './check -nfs'; and it would be nice if ./check could give better heads-up warnings about an insufficient setup. You may indeed have a point that the test may work with other non-local setups. But with this a quick hack: diff --git i/tests/qemu-iotests/173 w/tests/qemu-iotests/173 index 29dcaa1960df..c01d0186c6ba 100755 --- i/tests/qemu-iotests/173 +++ w/tests/qemu-iotests/173 @@ -40,7 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.qemu _supported_fmt qcow2 -_supported_proto nfs +_supported_proto nbd nfs size=100M ./check -qcow2 -nfs 173 # still passes ./check -qcow2 -nbd 173 # fails: +{"error": {"class": "GenericError", "desc": "Failed to get "write" lock"}} +Timeout waiting for return on handle 0 there may be more I can do to let this test work with nbd as well, but for the sake of getting this email sent, that's as far as I've gotten for now. > > Now I’m down to 16/76 for qcow2, and most of those look benign. (As in, > they simply don’t support nfs.) > > Max >
diff --git a/tests/qemu-iotests/173 b/tests/qemu-iotests/173 index 9e2fa2e73cb9..29dcaa1960df 100755 --- a/tests/qemu-iotests/173 +++ b/tests/qemu-iotests/173 @@ -47,9 +47,9 @@ size=100M BASE_IMG="${TEST_DIR}/image.base" TOP_IMG="${TEST_DIR}/image.snp1" -TEST_IMG="${BASE_IMG}" _make_test_img $size +TEST_IMG_FILE="${BASE_IMG}" _make_test_img $size -TEST_IMG="${TOP_IMG}" _make_test_img $size +TEST_IMG_FILE="${TOP_IMG}" _make_test_img $size echo echo === Running QEMU, using block-stream to find backing image === diff --git a/tests/qemu-iotests/173.out b/tests/qemu-iotests/173.out index f477a0099a32..e83d17ec2f64 100644 --- a/tests/qemu-iotests/173.out +++ b/tests/qemu-iotests/173.out @@ -7,6 +7,10 @@ Formatting 'TEST_DIR/image.snp1', fmt=IMGFMT size=104857600 {"return": {}} {"return": {}} {"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "disk2"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "disk2"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk2", "len": 104857600, "offset": 104857600, "speed": 0, "type": "stream"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "disk2"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "disk2"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk2", "len": 0, "offset": 0, "speed": 0, "type": "stream"}} *** done
This test has been broken since 3.0. It used TEST_IMG to influence the name of a file created during _make_test_img, but commit 655ae6bb changed things so that the wrong file name is being created, which then caused _launch_qemu to fail. In the meantime, the set of events issued for the actions of the test has increased. Why haven't we noticed the failure? Because the test rarely gets run: './check -qcow2 173' is insufficient (that defaults to using file protocol) './check -nfs 173' is insufficient (that defaults to using raw format) so the test is only run with: ./check -qcow2 -nfs 173 Note that we already have a number of other problems with -nfs: ./check -nfs (fails 18/30) ./check -qcow2 -nfs (fails 45/76 after this patch) and it's not on my priority list to fix those. Rather, I found this because of my next patch's work on tests using _send_qemu_cmd. Fixes: 655ae6b Signed-off-by: Eric Blake <eblake@redhat.com> --- tests/qemu-iotests/173 | 4 ++-- tests/qemu-iotests/173.out | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-)