* [PATCH v2 01/17] iotests: replace calls to log(qemu_io(...)) with qemu_io_log()
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-25 13:25 ` Hanna Reitz
2022-03-24 18:30 ` [PATCH v2 02/17] iotests/163: Fix broken qemu-io invocation John Snow
` (15 subsequent siblings)
16 siblings, 1 reply; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
This makes these callsites a little simpler, but the real motivation is
a forthcoming commit will change the return type of qemu_io(), so removing
users of the return value now is helpful.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/242 | 6 +++---
tests/qemu-iotests/255 | 4 +---
tests/qemu-iotests/303 | 4 ++--
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
index b3afd36d72..c89f0c6cb3 100755
--- a/tests/qemu-iotests/242
+++ b/tests/qemu-iotests/242
@@ -22,8 +22,8 @@
import iotests
import json
import struct
-from iotests import qemu_img_create, qemu_io, qemu_img_info, \
- file_path, img_info_log, log, filter_qemu_io
+from iotests import qemu_img_create, qemu_io_log, qemu_img_info, \
+ file_path, img_info_log, log
iotests.script_initialize(supported_fmts=['qcow2'],
supported_protocols=['file'],
@@ -61,7 +61,7 @@ def add_bitmap(bitmap_number, persistent, disabled):
def write_to_disk(offset, size):
write = 'write {} {}'.format(offset, size)
- log(qemu_io('-c', write, disk), filters=[filter_qemu_io])
+ qemu_io_log('-c', write, disk)
def toggle_flag(offset):
diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255
index f86fa851b6..88b29d64b4 100755
--- a/tests/qemu-iotests/255
+++ b/tests/qemu-iotests/255
@@ -95,9 +95,7 @@ with iotests.FilePath('src.qcow2') as src_path, \
iotests.qemu_img_create('-f', iotests.imgfmt, src_path, size_str)
iotests.qemu_img_create('-f', iotests.imgfmt, dst_path, size_str)
- iotests.log(iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write 0 1M',
- src_path),
- filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
+ iotests.qemu_io_log('-f', iotests.imgfmt, '-c', 'write 0 1M', src_path),
vm.add_object('throttle-group,x-bps-read=4096,id=throttle0')
diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
index 93aa5ce9b7..32128b1d32 100755
--- a/tests/qemu-iotests/303
+++ b/tests/qemu-iotests/303
@@ -21,7 +21,7 @@
import iotests
import subprocess
-from iotests import qemu_img_create, qemu_io, file_path, log, filter_qemu_io
+from iotests import qemu_img_create, qemu_io_log, file_path, log
iotests.script_initialize(supported_fmts=['qcow2'],
unsupported_imgopts=['refcount_bits', 'compat'])
@@ -43,7 +43,7 @@ def create_bitmap(bitmap_number, disabled):
def write_to_disk(offset, size):
write = f'write {offset} {size}'
- log(qemu_io('-c', write, disk), filters=[filter_qemu_io])
+ qemu_io_log('-c', write, disk)
def add_bitmap(num, begin, end, disabled):
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 01/17] iotests: replace calls to log(qemu_io(...)) with qemu_io_log()
2022-03-24 18:30 ` [PATCH v2 01/17] iotests: replace calls to log(qemu_io(...)) with qemu_io_log() John Snow
@ 2022-03-25 13:25 ` Hanna Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Hanna Reitz @ 2022-03-25 13:25 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, Eric Blake, qemu-block
On 24.03.22 19:30, John Snow wrote:
> This makes these callsites a little simpler, but the real motivation is
> a forthcoming commit will change the return type of qemu_io(), so removing
> users of the return value now is helpful.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> tests/qemu-iotests/242 | 6 +++---
> tests/qemu-iotests/255 | 4 +---
> tests/qemu-iotests/303 | 4 ++--
> 3 files changed, 6 insertions(+), 8 deletions(-)
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 02/17] iotests/163: Fix broken qemu-io invocation
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
2022-03-24 18:30 ` [PATCH v2 01/17] iotests: replace calls to log(qemu_io(...)) with qemu_io_log() John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 03/17] iotests: Don't check qemu_io() output for specific error strings John Snow
` (14 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
The 'read' commands to qemu-io were malformed, and this invocation only
worked by coincidence because the error messages were identical. Oops.
There's no point in checking the patterning of the reference image, so
just check the empty image by itself instead.
(Note: as of this commit, nothing actually enforces that this command
completes successfully, but a forthcoming commit in this series will
enforce that qemu_io() must have a zero status code.)
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
tests/qemu-iotests/163 | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
index e4cd4b230f..c94ad16f4a 100755
--- a/tests/qemu-iotests/163
+++ b/tests/qemu-iotests/163
@@ -113,10 +113,7 @@ class ShrinkBaseClass(iotests.QMPTestCase):
qemu_img('resize', '-f', iotests.imgfmt, '--shrink', test_img,
self.shrink_size)
- self.assertEqual(
- qemu_io('-c', 'read -P 0x00 %s'%self.shrink_size, test_img),
- qemu_io('-c', 'read -P 0x00 %s'%self.shrink_size, check_img),
- "Verifying image content")
+ qemu_io('-c', f"read -P 0x00 0 {self.shrink_size}", test_img)
self.image_verify()
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 03/17] iotests: Don't check qemu_io() output for specific error strings
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
2022-03-24 18:30 ` [PATCH v2 01/17] iotests: replace calls to log(qemu_io(...)) with qemu_io_log() John Snow
2022-03-24 18:30 ` [PATCH v2 02/17] iotests/163: Fix broken qemu-io invocation John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 04/17] iotests/040: Don't check image pattern on zero-length image John Snow
` (13 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
A forthcoming commit updates qemu_io() to raise an exception on non-zero
return by default, and changes its return type.
In preparation, simplify some calls to qemu_io() that assert that
specific error message strings do not appear in qemu-io's
output. Asserting that all of these calls return a status code of zero
will be a more robust way to guard against failure.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
tests/qemu-iotests/040 | 33 ++++++++++++++++-----------------
tests/qemu-iotests/056 | 2 +-
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 0e1cfd7e49..adf5815781 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -101,13 +101,13 @@ class TestSingleDrive(ImageCommitTestCase):
def test_commit(self):
self.run_commit_test(mid_img, backing_img)
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
+ qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
+ qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
def test_commit_node(self):
self.run_commit_test("mid", "base", node_names=True)
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
+ qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
+ qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
@iotests.skip_if_unsupported(['throttle'])
def test_commit_with_filter_and_quit(self):
@@ -192,13 +192,13 @@ class TestSingleDrive(ImageCommitTestCase):
def test_top_is_active(self):
self.run_commit_test(test_img, backing_img, need_ready=True)
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
+ qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
+ qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
def test_top_is_default_active(self):
self.run_default_commit_test()
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
+ qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
+ qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
def test_top_and_base_reversed(self):
self.assert_no_active_block_jobs()
@@ -334,8 +334,8 @@ class TestRelativePaths(ImageCommitTestCase):
def test_commit(self):
self.run_commit_test(self.mid_img, self.backing_img)
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
+ qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs)
+ qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs)
def test_device_not_found(self):
result = self.vm.qmp('block-commit', device='nonexistent', top='%s' % self.mid_img)
@@ -361,8 +361,8 @@ class TestRelativePaths(ImageCommitTestCase):
def test_top_is_active(self):
self.run_commit_test(self.test_img, self.backing_img)
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
- self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
+ qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs)
+ qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs)
def test_top_and_base_reversed(self):
self.assert_no_active_block_jobs()
@@ -738,11 +738,10 @@ class TestCommitWithFilters(iotests.QMPTestCase):
def do_test_io(self, read_or_write):
for index, pattern_file in enumerate(self.pattern_files):
- result = qemu_io('-f', iotests.imgfmt,
- '-c',
- f'{read_or_write} -P {index + 1} {index}M 1M',
- pattern_file)
- self.assertFalse('Pattern verification failed' in result)
+ qemu_io('-f', iotests.imgfmt,
+ '-c',
+ f'{read_or_write} -P {index + 1} {index}M 1M',
+ pattern_file)
@iotests.skip_if_unsupported(['throttle'])
def setUp(self):
diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
index b459a3f1e8..bef865eec4 100755
--- a/tests/qemu-iotests/056
+++ b/tests/qemu-iotests/056
@@ -102,7 +102,7 @@ class TestSyncModesNoneAndTop(iotests.QMPTestCase):
self.vm.shutdown()
time.sleep(1)
- self.assertEqual(-1, qemu_io('-c', 'read -P0x41 0 512', target_img).find("verification failed"))
+ qemu_io('-c', 'read -P0x41 0 512', target_img)
class TestBeforeWriteNotifier(iotests.QMPTestCase):
def setUp(self):
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 04/17] iotests/040: Don't check image pattern on zero-length image
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (2 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 03/17] iotests: Don't check qemu_io() output for specific error strings John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test John Snow
` (12 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
qemu-io fails on read/write beyond end-of-file on raw images, so skip
these invocations when running the zero-length image tests.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
tests/qemu-iotests/040 | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index adf5815781..c4a90937dc 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -86,8 +86,10 @@ class TestSingleDrive(ImageCommitTestCase):
qemu_img('create', '-f', iotests.imgfmt,
'-o', 'backing_file=%s' % mid_img,
'-F', iotests.imgfmt, test_img)
- qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
- qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
+ if self.image_len:
+ qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288',
+ mid_img)
self.vm = iotests.VM().add_drive(test_img, "node-name=top,backing.node-name=mid,backing.backing.node-name=base", interface="none")
self.vm.add_device('virtio-scsi')
self.vm.add_device("scsi-hd,id=scsi0,drive=drive0")
@@ -101,11 +103,15 @@ class TestSingleDrive(ImageCommitTestCase):
def test_commit(self):
self.run_commit_test(mid_img, backing_img)
+ if not self.image_len:
+ return
qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
def test_commit_node(self):
self.run_commit_test("mid", "base", node_names=True)
+ if not self.image_len:
+ return
qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
@@ -192,11 +198,15 @@ class TestSingleDrive(ImageCommitTestCase):
def test_top_is_active(self):
self.run_commit_test(test_img, backing_img, need_ready=True)
+ if not self.image_len:
+ return
qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
def test_top_is_default_active(self):
self.run_default_commit_test()
+ if not self.image_len:
+ return
qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (3 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 04/17] iotests/040: Don't check image pattern on zero-length image John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-25 1:33 ` Eric Blake
2022-03-25 13:40 ` Hanna Reitz
2022-03-24 18:30 ` [PATCH v2 06/17] iotests: create generic qemu_tool() function John Snow
` (11 subsequent siblings)
16 siblings, 2 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
Without this change, asserting that qemu_io always returns 0 causes this
test to fail in a way we happened not to be catching previously:
qemu.utils.VerboseProcessError: Command
'('/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/../../qemu-io',
'--cache', 'writeback', '--aio', 'threads', '-f', 'qcow2', '-c',
'read -P 4 3M 1M',
'/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img')'
returned non-zero exit status 1.
┏━ output ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
┃ qemu-io: can't open device
┃ /home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img:
┃ Could not open backing file: Could not open backing file: Throttle
┃ group 'tg' does not exist
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Explicitly provide the backing file so that opening the file outside of
QEMU (Where we will not have throttle groups) will succeed.
[Patch entirely written by Hanna but I don't have her S-o-B]
[My commit message is probably also garbage, sorry]
[Feel free to suggest a better one]
[I hope your day is going well]
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/040 | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index c4a90937dc..30eb97829e 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -836,7 +836,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
job_id='commit',
device='top-filter',
top_node='cow-2',
- base_node='cow-1')
+ base_node='cow-1',
+ backing_file=self.img1)
self.assert_qmp(result, 'return', {})
self.wait_until_completed(drive='commit')
@@ -852,7 +853,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
job_id='commit',
device='top-filter',
top_node='cow-1',
- base_node='cow-0')
+ base_node='cow-0',
+ backing_file=self.img0)
self.assert_qmp(result, 'return', {})
self.wait_until_completed(drive='commit')
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
2022-03-24 18:30 ` [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test John Snow
@ 2022-03-25 1:33 ` Eric Blake
2022-03-31 16:36 ` John Snow
2022-03-25 13:40 ` Hanna Reitz
1 sibling, 1 reply; 28+ messages in thread
From: Eric Blake @ 2022-03-25 1:33 UTC (permalink / raw)
To: John Snow; +Cc: Kevin Wolf, Hanna Reitz, qemu-devel, qemu-block
On Thu, Mar 24, 2022 at 02:30:06PM -0400, John Snow wrote:
> Without this change, asserting that qemu_io always returns 0 causes this
> test to fail in a way we happened not to be catching previously:
>
> qemu.utils.VerboseProcessError: Command
> '('/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/../../qemu-io',
> '--cache', 'writeback', '--aio', 'threads', '-f', 'qcow2', '-c',
> 'read -P 4 3M 1M',
> '/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img')'
> returned non-zero exit status 1.
> ┏━ output ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> ┃ qemu-io: can't open device
> ┃ /home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img:
> ┃ Could not open backing file: Could not open backing file: Throttle
> ┃ group 'tg' does not exist
> ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
>
> Explicitly provide the backing file so that opening the file outside of
> QEMU (Where we will not have throttle groups) will succeed.
>
> [Patch entirely written by Hanna but I don't have her S-o-B]
Yeah, you'll want that.
> [My commit message is probably also garbage, sorry]
No, it was actually decent.
> [Feel free to suggest a better one]
> [I hope your day is going well]
> Signed-off-by: John Snow <jsnow@redhat.com>
>
> Signed-off-by: John Snow <jsnow@redhat.com>
So giving your S-o-b twice makes up for it, right ;)
Well, you did say v3 would fix this. But while you're having fun
fixing it, you can add:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
2022-03-25 1:33 ` Eric Blake
@ 2022-03-31 16:36 ` John Snow
0 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-31 16:36 UTC (permalink / raw)
To: Eric Blake; +Cc: Kevin Wolf, Hanna Reitz, qemu-devel, Qemu-block
On Thu, Mar 24, 2022 at 9:33 PM Eric Blake <eblake@redhat.com> wrote:
>
> On Thu, Mar 24, 2022 at 02:30:06PM -0400, John Snow wrote:
> > Without this change, asserting that qemu_io always returns 0 causes this
> > test to fail in a way we happened not to be catching previously:
> >
> > qemu.utils.VerboseProcessError: Command
> > '('/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/../../qemu-io',
> > '--cache', 'writeback', '--aio', 'threads', '-f', 'qcow2', '-c',
> > 'read -P 4 3M 1M',
> > '/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img')'
> > returned non-zero exit status 1.
> > ┏━ output ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> > ┃ qemu-io: can't open device
> > ┃ /home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img:
> > ┃ Could not open backing file: Could not open backing file: Throttle
> > ┃ group 'tg' does not exist
> > ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> >
> > Explicitly provide the backing file so that opening the file outside of
> > QEMU (Where we will not have throttle groups) will succeed.
> >
> > [Patch entirely written by Hanna but I don't have her S-o-B]
>
> Yeah, you'll want that.
>
> > [My commit message is probably also garbage, sorry]
>
> No, it was actually decent.
>
> > [Feel free to suggest a better one]
> > [I hope your day is going well]
> > Signed-off-by: John Snow <jsnow@redhat.com>
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
> So giving your S-o-b twice makes up for it, right ;)
This happens when I add a '---' myself into the commit message, and
git-publish sees that the end of the commit message doesn't have a
S-o-B and adds one into the ignored region.
Haven't bothered to fix it yet.
>
> Well, you did say v3 would fix this. But while you're having fun
> fixing it, you can add:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
2022-03-24 18:30 ` [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test John Snow
2022-03-25 1:33 ` Eric Blake
@ 2022-03-25 13:40 ` Hanna Reitz
2022-03-25 15:06 ` John Snow
1 sibling, 1 reply; 28+ messages in thread
From: Hanna Reitz @ 2022-03-25 13:40 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block
On 24.03.22 19:30, John Snow wrote:
> Without this change, asserting that qemu_io always returns 0 causes this
> test to fail in a way we happened not to be catching previously:
>
> qemu.utils.VerboseProcessError: Command
> '('/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/../../qemu-io',
> '--cache', 'writeback', '--aio', 'threads', '-f', 'qcow2', '-c',
> 'read -P 4 3M 1M',
> '/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img')'
> returned non-zero exit status 1.
> ┏━ output ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> ┃ qemu-io: can't open device
> ┃ /home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img:
> ┃ Could not open backing file: Could not open backing file: Throttle
> ┃ group 'tg' does not exist
> ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
>
> Explicitly provide the backing file so that opening the file outside of
> QEMU (Where we will not have throttle groups) will succeed.
>
> [Patch entirely written by Hanna but I don't have her S-o-B]
Er, well, not sure if this even meets the necessary threshold of
originality, so a Proposed-by would’ve been fine.
Anyway, here you go:
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> [My commit message is probably also garbage, sorry]
I don’t find it too bad, but a paragraph describing the actual problem
might improve it. E.g. (below the exception output):
The commit jobs changes the backing file string stored in the image file
header belonging to the node above the commit’s top node to point to the
commit target (the base node). QEMU tries to be as accurate as
possible, and so in these test cases will include the filter that is
part of the block graph in that backing file string (by virtue of making
it a json:{} description of the post-commit subgraph). This makes
little sense outside of QEMU, though: Specifically, the throttle node in
that subgraph will dearly miss its supposedly associated throttle group
object.
When starting the commit job, we can specify a custom backing file
string to write into said image file, so let’s use that feature to write
the plain filename of the backing chain’s next actual image file there.
> [Feel free to suggest a better one]
> [I hope your day is going well]
Fridays tend to be on the better side of days.
Hanna
> Signed-off-by: John Snow <jsnow@redhat.com>
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/040 | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> index c4a90937dc..30eb97829e 100755
> --- a/tests/qemu-iotests/040
> +++ b/tests/qemu-iotests/040
> @@ -836,7 +836,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
> job_id='commit',
> device='top-filter',
> top_node='cow-2',
> - base_node='cow-1')
> + base_node='cow-1',
> + backing_file=self.img1)
> self.assert_qmp(result, 'return', {})
> self.wait_until_completed(drive='commit')
>
> @@ -852,7 +853,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
> job_id='commit',
> device='top-filter',
> top_node='cow-1',
> - base_node='cow-0')
> + base_node='cow-0',
> + backing_file=self.img0)
> self.assert_qmp(result, 'return', {})
> self.wait_until_completed(drive='commit')
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
2022-03-25 13:40 ` Hanna Reitz
@ 2022-03-25 15:06 ` John Snow
0 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-25 15:06 UTC (permalink / raw)
To: Hanna Reitz; +Cc: Kevin Wolf, qemu-devel, Qemu-block
[-- Attachment #1: Type: text/plain, Size: 4588 bytes --]
On Fri, Mar 25, 2022, 9:40 AM Hanna Reitz <hreitz@redhat.com> wrote:
> On 24.03.22 19:30, John Snow wrote:
> > Without this change, asserting that qemu_io always returns 0 causes this
> > test to fail in a way we happened not to be catching previously:
> >
> > qemu.utils.VerboseProcessError: Command
> > '('/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/../../qemu-io',
> > '--cache', 'writeback', '--aio', 'threads', '-f', 'qcow2', '-c',
> > 'read -P 4 3M 1M',
> > '/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img')'
> > returned non-zero exit status 1.
> > ┏━ output ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> > ┃ qemu-io: can't open device
> > ┃ /home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img:
> > ┃ Could not open backing file: Could not open backing file: Throttle
> > ┃ group 'tg' does not exist
> > ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> >
> > Explicitly provide the backing file so that opening the file outside of
> > QEMU (Where we will not have throttle groups) will succeed.
> >
> > [Patch entirely written by Hanna but I don't have her S-o-B]
>
> Er, well, not sure if this even meets the necessary threshold of
> originality, so a Proposed-by would’ve been fine.
>
I have a dogmatic devotion to crediting others. You diagnosed and fixed the
problem, not me!
(I realize it's a small thing, but still. I can't bring myself to put my
name on something that isn't mine, even if it's tiny.)
> Anyway, here you go:
>
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>
> > [My commit message is probably also garbage, sorry]
>
> I don’t find it too bad, but a paragraph describing the actual problem
> might improve it. E.g. (below the exception output):
>
> The commit jobs changes the backing file string stored in the image file
> header belonging to the node above the commit’s top node to point to the
> commit target (the base node). QEMU tries to be as accurate as
> possible, and so in these test cases will include the filter that is
> part of the block graph in that backing file string (by virtue of making
> it a json:{} description of the post-commit subgraph). This makes
> little sense outside of QEMU, though: Specifically, the throttle node in
> that subgraph will dearly miss its supposedly associated throttle group
> object.
>
> When starting the commit job, we can specify a custom backing file
> string to write into said image file, so let’s use that feature to write
> the plain filename of the backing chain’s next actual image file there.
>
Thanks :3
> > [Feel free to suggest a better one]
> > [I hope your day is going well]
>
> Fridays tend to be on the better side of days.
>
> Hanna
>
Up there in my top three kinds of days.
> > Signed-off-by: John Snow <jsnow@redhat.com>
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> > tests/qemu-iotests/040 | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> > index c4a90937dc..30eb97829e 100755
> > --- a/tests/qemu-iotests/040
> > +++ b/tests/qemu-iotests/040
> > @@ -836,7 +836,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
> > job_id='commit',
> > device='top-filter',
> > top_node='cow-2',
> > - base_node='cow-1')
> > + base_node='cow-1',
> > + backing_file=self.img1)
> > self.assert_qmp(result, 'return', {})
> > self.wait_until_completed(drive='commit')
> >
> > @@ -852,7 +853,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
> > job_id='commit',
> > device='top-filter',
> > top_node='cow-1',
> > - base_node='cow-0')
> > + base_node='cow-0',
> > + backing_file=self.img0)
> > self.assert_qmp(result, 'return', {})
> > self.wait_until_completed(drive='commit')
> >
>
>
[-- Attachment #2: Type: text/html, Size: 6676 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 06/17] iotests: create generic qemu_tool() function
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (4 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 07/17] iotests: rebase qemu_io() on top of qemu_tool() John Snow
` (10 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
reimplement qemu_img() in terms of qemu_tool() in preparation for doing
the same with qemu_io().
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
tests/qemu-iotests/iotests.py | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index fcec3e51e5..e4e18a5889 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -207,15 +207,13 @@ def qemu_img_create_prepare_args(args: List[str]) -> List[str]:
return result
-def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
- ) -> 'subprocess.CompletedProcess[str]':
+
+def qemu_tool(*args: str, check: bool = True, combine_stdio: bool = True
+ ) -> 'subprocess.CompletedProcess[str]':
"""
- Run qemu_img and return the status code and console output.
+ Run a qemu tool and return its status code and console output.
- This function always prepends QEMU_IMG_OPTIONS and may further alter
- the args for 'create' commands.
-
- :param args: command-line arguments to qemu-img.
+ :param args: full command line to run.
:param check: Enforce a return code of zero.
:param combine_stdio: set to False to keep stdout/stderr separated.
@@ -232,10 +230,8 @@ def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
properties. If streams are not combined, it will also have a
stderr property.
"""
- full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
-
subp = subprocess.run(
- full_args,
+ args,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT if combine_stdio else subprocess.PIPE,
universal_newlines=True,
@@ -244,7 +240,7 @@ def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
if check and subp.returncode or (subp.returncode < 0):
raise VerboseProcessError(
- subp.returncode, full_args,
+ subp.returncode, args,
output=subp.stdout,
stderr=subp.stderr,
)
@@ -252,6 +248,20 @@ def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
return subp
+def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
+ ) -> 'subprocess.CompletedProcess[str]':
+ """
+ Run QEMU_IMG_PROG and return its status code and console output.
+
+ This function always prepends QEMU_IMG_OPTIONS and may further alter
+ the args for 'create' commands.
+
+ See `qemu_tool()` for greater detail.
+ """
+ full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
+ return qemu_tool(*full_args, check=check, combine_stdio=combine_stdio)
+
+
def ordered_qmp(qmsg, conv_keys=True):
# Dictionaries are not ordered prior to 3.6, therefore:
if isinstance(qmsg, list):
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 07/17] iotests: rebase qemu_io() on top of qemu_tool()
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (5 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 06/17] iotests: create generic qemu_tool() function John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 08/17] iotests/030: fixup John Snow
` (9 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
Rework qemu_io() to be analogous to qemu_img(); a function that requires
a return code of zero by default unless disabled explicitly.
Tests that use qemu_io():
030 040 041 044 055 056 093 124 129 132 136 148 149 151 152 163 165 205
209 219 236 245 248 254 255 257 260 264 280 298 300 302 304
image-fleecing migrate-bitmaps-postcopy-test migrate-bitmaps-test
migrate-during-backup migration-permissions
Test that use qemu_io_log():
242 245 255 274 303 307 nbd-reconnect-on-open
Signed-off-by: John Snow <jsnow@redhat.com>
---
Note: This breaks several tests at this point. I'll be fixing each
broken test one by one in the subsequent commits. We can squash them all
on merge to avoid test regressions.
(Seems like a way to have your cake and eat it too with regards to
maintaining bisectability while also having nice mailing list patches.)
Copy-pastables:
./check -qcow2 030 040 041 044 055 056 124 129 132 151 152 163 165 209 \
219 236 242 245 248 254 255 257 260 264 274 \
280 298 300 302 303 304 307 image-fleecing \
migrate-bitmaps-postcopy-test migrate-bitmaps-test \
migrate-during-backup nbd-reconnect-on-open
./check -raw 093 136 148 migration-permissions
./check -nbd 205
# ./configure configure --disable-gnutls --enable-gcrypt
# this ALSO requires passwordless sudo.
./check -luks 149
# Just the ones that fail:
./check -qcow2 030 040 242 245
./check -raw migration-permissions
./check -nbd 205
./check -luks 149
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/iotests.py | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e4e18a5889..baf4995089 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -354,16 +354,23 @@ def qemu_io_wrap_args(args: Sequence[str]) -> List[str]:
def qemu_io_popen(*args):
return qemu_tool_popen(qemu_io_wrap_args(args))
-def qemu_io(*args):
- '''Run qemu-io and return the stdout data'''
- return qemu_tool_pipe_and_status('qemu-io', qemu_io_wrap_args(args))[0]
+def qemu_io(*args: str, check: bool = True, combine_stdio: bool = True
+ ) -> 'subprocess.CompletedProcess[str]':
+ """
+ Run QEMU_IO_PROG and return the status code and console output.
+
+ This function always prepends either QEMU_IO_OPTIONS or
+ QEMU_IO_OPTIONS_NO_FMT.
+ """
+ return qemu_tool(*qemu_io_wrap_args(args),
+ check=check, combine_stdio=combine_stdio)
def qemu_io_pipe_and_status(*args):
return qemu_tool_pipe_and_status('qemu-io', qemu_io_wrap_args(args))
-def qemu_io_log(*args):
- result = qemu_io(*args)
- log(result, filters=[filter_testfiles, filter_qemu_io])
+def qemu_io_log(*args: str) -> 'subprocess.CompletedProcess[str]':
+ result = qemu_io(*args, check=False)
+ log(result.stdout, filters=[filter_testfiles, filter_qemu_io])
return result
def qemu_io_silent(*args):
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 08/17] iotests/030: fixup
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (6 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 07/17] iotests: rebase qemu_io() on top of qemu_tool() John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 09/17] iotests/149: fixup John Snow
` (8 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
(Merge into prior patch.)
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/030 | 85 ++++++++++++++++++++++++------------------
1 file changed, 49 insertions(+), 36 deletions(-)
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 567bf1da67..3a2de920a3 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -64,16 +64,18 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
- 'image file map does not match backing file after streaming')
+ self.assertEqual(
+ qemu_io('-f', 'raw', '-c', 'map', backing_img).stdout,
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img).stdout,
+ 'image file map does not match backing file after streaming')
def test_stream_intermediate(self):
self.assert_no_active_block_jobs()
- self.assertNotEqual(qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img),
- qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img),
- 'image file map matches backing file before streaming')
+ self.assertNotEqual(
+ qemu_io('-f', 'raw', '-rU', '-c', 'map', backing_img).stdout,
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', mid_img).stdout,
+ 'image file map matches backing file before streaming')
result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
self.assert_qmp(result, 'return', {})
@@ -83,9 +85,10 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
- 'image file map does not match backing file after streaming')
+ self.assertEqual(
+ qemu_io('-f', 'raw', '-c', 'map', backing_img).stdout,
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img).stdout,
+ 'image file map does not match backing file after streaming')
def test_stream_pause(self):
self.assert_no_active_block_jobs()
@@ -113,15 +116,17 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
- 'image file map does not match backing file after streaming')
+ self.assertEqual(
+ qemu_io('-f', 'raw', '-c', 'map', backing_img).stdout,
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img).stdout,
+ 'image file map does not match backing file after streaming')
def test_stream_no_op(self):
self.assert_no_active_block_jobs()
# The image map is empty before the operation
- empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
+ empty_map = qemu_io(
+ '-f', iotests.imgfmt, '-rU', '-c', 'map', test_img).stdout
# This is a no-op: no data should ever be copied from the base image
result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
@@ -132,8 +137,9 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
- empty_map, 'image file map changed after a no-op')
+ self.assertEqual(
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img).stdout,
+ empty_map, 'image file map changed after a no-op')
def test_stream_partial(self):
self.assert_no_active_block_jobs()
@@ -146,9 +152,10 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
- 'image file map does not match backing file after streaming')
+ self.assertEqual(
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img).stdout,
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img).stdout,
+ 'image file map does not match backing file after streaming')
def test_device_not_found(self):
result = self.vm.qmp('block-stream', device='nonexistent')
@@ -236,9 +243,10 @@ class TestParallelOps(iotests.QMPTestCase):
# Check that the maps don't match before the streaming operations
for i in range(2, self.num_imgs, 2):
- self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[i]),
- qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[i-1]),
- 'image file map matches backing file before streaming')
+ self.assertNotEqual(
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[i]).stdout,
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[i-1]).stdout,
+ 'image file map matches backing file before streaming')
# Create all streaming jobs
pending_jobs = []
@@ -278,9 +286,10 @@ class TestParallelOps(iotests.QMPTestCase):
# Check that all maps match now
for i in range(2, self.num_imgs, 2):
- self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i]),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i-1]),
- 'image file map does not match backing file after streaming')
+ self.assertEqual(
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i]).stdout,
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[i-1]).stdout,
+ 'image file map does not match backing file after streaming')
# Test that it's not possible to perform two block-stream
# operations if there are nodes involved in both.
@@ -509,9 +518,10 @@ class TestParallelOps(iotests.QMPTestCase):
def test_stream_base_node_name(self):
self.assert_no_active_block_jobs()
- self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[4]),
- qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[3]),
- 'image file map matches backing file before streaming')
+ self.assertNotEqual(
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[4]).stdout,
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.imgs[3]).stdout,
+ 'image file map matches backing file before streaming')
# Error: the base node does not exist
result = self.vm.qmp('block-stream', device='node4', base_node='none', job_id='stream')
@@ -542,9 +552,10 @@ class TestParallelOps(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[4]),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[3]),
- 'image file map matches backing file after streaming')
+ self.assertEqual(
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[4]).stdout,
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', self.imgs[3]).stdout,
+ 'image file map matches backing file after streaming')
class TestQuorum(iotests.QMPTestCase):
num_children = 3
@@ -583,9 +594,10 @@ class TestQuorum(iotests.QMPTestCase):
os.remove(img)
def test_stream_quorum(self):
- self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.children[0]),
- qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.backing[0]),
- 'image file map matches backing file before streaming')
+ self.assertNotEqual(
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.children[0]).stdout,
+ qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.backing[0]).stdout,
+ 'image file map matches backing file before streaming')
self.assert_no_active_block_jobs()
@@ -597,9 +609,10 @@ class TestQuorum(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]),
- qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]),
- 'image file map does not match backing file after streaming')
+ self.assertEqual(
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', self.children[0]).stdout,
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', self.backing[0]).stdout,
+ 'image file map does not match backing file after streaming')
class TestSmallerBackingFile(iotests.QMPTestCase):
backing_len = 1 * 1024 * 1024 # MB
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 09/17] iotests/149: fixup
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (7 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 08/17] iotests/030: fixup John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 10/17] iotests/205: fixup John Snow
` (7 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
(Merge into prior patch.)
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/149 | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 9bb96d6a1d..2ae318f16f 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -295,7 +295,8 @@ def qemu_io_write_pattern(config, pattern, offset_mb, size_mb, dev=False):
args = ["-c", "write -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
args.extend(qemu_io_image_args(config, dev))
iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
- iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
+ output = iotests.qemu_io(*args, check=False).stdout
+ iotests.log(check_cipher_support(config, output),
filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
@@ -307,7 +308,8 @@ def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
args = ["-c", "read -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
args.extend(qemu_io_image_args(config, dev))
iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
- iotests.log(check_cipher_support(config, iotests.qemu_io(*args)),
+ output = iotests.qemu_io(*args, check=False).stdout
+ iotests.log(check_cipher_support(config, output),
filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 10/17] iotests/205: fixup
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (8 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 09/17] iotests/149: fixup John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 11/17] iotests/245: fixup John Snow
` (6 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
(Merge into prior patch.)
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/205 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/205 b/tests/qemu-iotests/205
index c0e107328f..15f798288a 100755
--- a/tests/qemu-iotests/205
+++ b/tests/qemu-iotests/205
@@ -85,13 +85,13 @@ class TestNbdServerRemove(iotests.QMPTestCase):
def do_test_connect_after_remove(self, mode=None):
args = ('-r', '-f', 'raw', '-c', 'read 0 512', nbd_uri)
- self.assertReadOk(qemu_io(*args))
+ self.assertReadOk(qemu_io(*args).stdout)
result = self.remove_export('exp', mode)
self.assert_qmp(result, 'return', {})
self.assertExportNotFound('exp')
- self.assertConnectFailed(qemu_io(*args))
+ self.assertConnectFailed(qemu_io(*args, check=False).stdout)
def test_connect_after_remove_default(self):
self.do_test_connect_after_remove()
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 11/17] iotests/245: fixup
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (9 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 10/17] iotests/205: fixup John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 12/17] iotests/migration-permissions: fixup John Snow
` (5 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
Merge with prior.
This is a little more involved, now, but maybe it's a small price to pay
for the better debug information being deployed universally.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/245 | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 8cbed7821b..edaf29094b 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -20,11 +20,13 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
+import copy
+import json
import os
import re
+from subprocess import CalledProcessError
+
import iotests
-import copy
-import json
from iotests import qemu_img, qemu_io
hd_path = [
@@ -216,11 +218,14 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# Reopen an image several times changing some of its options
def test_reopen(self):
- # Check whether the filesystem supports O_DIRECT
- if 'O_DIRECT' in qemu_io('-f', 'raw', '-t', 'none', '-c', 'quit', hd_path[0]):
- supports_direct = False
- else:
+ try:
+ qemu_io('-f', 'raw', '-t', 'none', '-c', 'quit', hd_path[0])
supports_direct = True
+ except CalledProcessError as exc:
+ if 'O_DIRECT' in exc.stdout:
+ supports_direct = False
+ else:
+ raise
# Open the hd1 image passing all backing options
opts = hd_opts(1)
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 12/17] iotests/migration-permissions: fixup
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (10 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 11/17] iotests/245: fixup John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 13/17] iotests/migration-permissions: use assertRaises() for qemu_io() negative test John Snow
` (4 subsequent siblings)
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
(Merge into prior commit)
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/tests/migration-permissions | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/tests/migration-permissions b/tests/qemu-iotests/tests/migration-permissions
index 6be02581c7..c7afb1bd2c 100755
--- a/tests/qemu-iotests/tests/migration-permissions
+++ b/tests/qemu-iotests/tests/migration-permissions
@@ -69,7 +69,7 @@ class TestMigrationPermissions(iotests.QMPTestCase):
def test_post_migration_permissions(self):
# Try to access the image R/W, which should fail because virtio-blk
# has not been configured with share-rw=on
- log = qemu_io('-f', imgfmt, '-c', 'quit', test_img)
+ log = qemu_io('-f', imgfmt, '-c', 'quit', test_img, check=False).stdout
if not log.strip():
print('ERROR (pre-migration): qemu-io should not be able to '
'access this image, but it reported no error')
@@ -84,7 +84,7 @@ class TestMigrationPermissions(iotests.QMPTestCase):
# Try the same qemu-io access again, verifying that the WRITE
# permission remains unshared
- log = qemu_io('-f', imgfmt, '-c', 'quit', test_img)
+ log = qemu_io('-f', imgfmt, '-c', 'quit', test_img, check=False).stdout
if not log.strip():
print('ERROR (post-migration): qemu-io should not be able to '
'access this image, but it reported no error')
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 13/17] iotests/migration-permissions: use assertRaises() for qemu_io() negative test
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (11 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 12/17] iotests/migration-permissions: fixup John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-25 1:36 ` Eric Blake
2022-03-24 18:30 ` [PATCH v2 14/17] iotests/image-fleecing: switch to qemu_io() John Snow
` (3 subsequent siblings)
16 siblings, 1 reply; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
Modify this test to use assertRaises for its negative testing of
qemu_io. If the exception raised does not match the one we tell it to
expect, we get *that* exception unhandled. If we get no exception, we
get a unittest assertion failure and the provided emsg printed to
screen.
If we get the CalledProcessError exception but the output is not what we
expect, we re-raise the original CalledProcessError.
Tidy.
(Note: Yes, you can reference "with" objects after that block ends; it
just means that ctx.__exit__(...) will have been called on it. It does
not *actually* go out of scope. unittests expects you to want to inspect
the Exception object, so they leave it defined post-exit.)
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
.../qemu-iotests/tests/migration-permissions | 28 +++++++++----------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/tests/qemu-iotests/tests/migration-permissions b/tests/qemu-iotests/tests/migration-permissions
index c7afb1bd2c..4e1da369c9 100755
--- a/tests/qemu-iotests/tests/migration-permissions
+++ b/tests/qemu-iotests/tests/migration-permissions
@@ -18,6 +18,8 @@
#
import os
+from subprocess import CalledProcessError
+
import iotests
from iotests import imgfmt, qemu_img_create, qemu_io
@@ -69,13 +71,12 @@ class TestMigrationPermissions(iotests.QMPTestCase):
def test_post_migration_permissions(self):
# Try to access the image R/W, which should fail because virtio-blk
# has not been configured with share-rw=on
- log = qemu_io('-f', imgfmt, '-c', 'quit', test_img, check=False).stdout
- if not log.strip():
- print('ERROR (pre-migration): qemu-io should not be able to '
- 'access this image, but it reported no error')
- else:
- # This is the expected output
- assert 'Is another process using the image' in log
+ emsg = ('ERROR (pre-migration): qemu-io should not be able to '
+ 'access this image, but it reported no error')
+ with self.assertRaises(CalledProcessError, msg=emsg) as ctx:
+ qemu_io('-f', imgfmt, '-c', 'quit', test_img)
+ if 'Is another process using the image' not in ctx.exception.stdout:
+ raise ctx.exception
# Now migrate the VM
self.vm_s.qmp('migrate', uri=f'unix:{mig_sock}')
@@ -84,13 +85,12 @@ class TestMigrationPermissions(iotests.QMPTestCase):
# Try the same qemu-io access again, verifying that the WRITE
# permission remains unshared
- log = qemu_io('-f', imgfmt, '-c', 'quit', test_img, check=False).stdout
- if not log.strip():
- print('ERROR (post-migration): qemu-io should not be able to '
- 'access this image, but it reported no error')
- else:
- # This is the expected output
- assert 'Is another process using the image' in log
+ emsg = ('ERROR (post-migration): qemu-io should not be able to '
+ 'access this image, but it reported no error')
+ with self.assertRaises(CalledProcessError, msg=emsg) as ctx:
+ qemu_io('-f', imgfmt, '-c', 'quit', test_img)
+ if 'Is another process using the image' not in ctx.exception.stdout:
+ raise ctx.exception
if __name__ == '__main__':
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 13/17] iotests/migration-permissions: use assertRaises() for qemu_io() negative test
2022-03-24 18:30 ` [PATCH v2 13/17] iotests/migration-permissions: use assertRaises() for qemu_io() negative test John Snow
@ 2022-03-25 1:36 ` Eric Blake
0 siblings, 0 replies; 28+ messages in thread
From: Eric Blake @ 2022-03-25 1:36 UTC (permalink / raw)
To: John Snow; +Cc: Kevin Wolf, Hanna Reitz, qemu-devel, qemu-block
On Thu, Mar 24, 2022 at 02:30:14PM -0400, John Snow wrote:
> Modify this test to use assertRaises for its negative testing of
> qemu_io. If the exception raised does not match the one we tell it to
> expect, we get *that* exception unhandled. If we get no exception, we
> get a unittest assertion failure and the provided emsg printed to
> screen.
>
> If we get the CalledProcessError exception but the output is not what we
> expect, we re-raise the original CalledProcessError.
>
> Tidy.
>
> (Note: Yes, you can reference "with" objects after that block ends; it
> just means that ctx.__exit__(...) will have been called on it. It does
> not *actually* go out of scope. unittests expects you to want to inspect
> the Exception object, so they leave it defined post-exit.)
Thanks for adding that paragraph.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Tested-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Hanna Reitz <hreitz@redhat.com>
> ---
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 14/17] iotests/image-fleecing: switch to qemu_io()
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (12 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 13/17] iotests/migration-permissions: use assertRaises() for qemu_io() negative test John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-25 1:38 ` Eric Blake
2022-03-25 14:24 ` Hanna Reitz
2022-03-24 18:30 ` [PATCH v2 15/17] iotests: remove qemu_io_pipe_and_status() John Snow
` (2 subsequent siblings)
16 siblings, 2 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
This test expects failure ... but only sometimes. When? Why?
It's for reads of a region not defined by a bitmap. Adjust the test to
be more explicit about what it expects to fail and why.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/tests/image-fleecing | 28 +++++++++++++++++--------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index b7e5076104..ac749702f8 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -22,9 +22,10 @@
#
# Creator/Owner: John Snow <jsnow@redhat.com>
+from subprocess import CalledProcessError
+
import iotests
-from iotests import log, qemu_img, qemu_io, qemu_io_silent, \
- qemu_io_pipe_and_status
+from iotests import log, qemu_img, qemu_io, qemu_io_silent
iotests.script_initialize(
supported_fmts=['qcow2'],
@@ -185,10 +186,14 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
for p in patterns + zeroes:
cmd = 'read -P%s %s %s' % p
log(cmd)
- out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd,
- nbd_uri)
- if ret != 0:
- print(out)
+
+ try:
+ qemu_io('-r', '-f', 'raw', '-c', cmd, nbd_uri)
+ except CalledProcessError as exc:
+ if bitmap and p in zeroes:
+ log(exc.stdout)
+ else:
+ raise
log('')
log('--- Testing COW ---')
@@ -228,9 +233,14 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
args += [target_img_path]
else:
args += ['-f', 'raw', nbd_uri]
- out, ret = qemu_io_pipe_and_status(*args)
- if ret != 0:
- print(out)
+
+ try:
+ qemu_io(*args)
+ except CalledProcessError as exc:
+ if bitmap and p in zeroes:
+ log(exc.stdout)
+ else:
+ raise
log('')
log('--- Cleanup ---')
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 14/17] iotests/image-fleecing: switch to qemu_io()
2022-03-24 18:30 ` [PATCH v2 14/17] iotests/image-fleecing: switch to qemu_io() John Snow
@ 2022-03-25 1:38 ` Eric Blake
2022-03-25 14:24 ` Hanna Reitz
1 sibling, 0 replies; 28+ messages in thread
From: Eric Blake @ 2022-03-25 1:38 UTC (permalink / raw)
To: John Snow; +Cc: Kevin Wolf, Hanna Reitz, qemu-devel, qemu-block
On Thu, Mar 24, 2022 at 02:30:15PM -0400, John Snow wrote:
> This test expects failure ... but only sometimes. When? Why?
>
> It's for reads of a region not defined by a bitmap. Adjust the test to
> be more explicit about what it expects to fail and why.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/tests/image-fleecing | 28 +++++++++++++++++--------
> 1 file changed, 19 insertions(+), 9 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 14/17] iotests/image-fleecing: switch to qemu_io()
2022-03-24 18:30 ` [PATCH v2 14/17] iotests/image-fleecing: switch to qemu_io() John Snow
2022-03-25 1:38 ` Eric Blake
@ 2022-03-25 14:24 ` Hanna Reitz
1 sibling, 0 replies; 28+ messages in thread
From: Hanna Reitz @ 2022-03-25 14:24 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block
On 24.03.22 19:30, John Snow wrote:
> This test expects failure ... but only sometimes. When? Why?
>
> It's for reads of a region not defined by a bitmap.
Ah, yes, right. The bitmaps defines what regions are to be backed up,
so other regions are not covered by the fleecing scheme, and might be
invalid if read. Hence the errors.
> Adjust the test to
> be more explicit about what it expects to fail and why.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/tests/image-fleecing | 28 +++++++++++++++++--------
> 1 file changed, 19 insertions(+), 9 deletions(-)
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 15/17] iotests: remove qemu_io_pipe_and_status()
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (13 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 14/17] iotests/image-fleecing: switch to qemu_io() John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-25 1:39 ` Eric Blake
2022-03-25 14:25 ` Hanna Reitz
2022-03-24 18:30 ` [PATCH v2 16/17] iotests: remove qemu_io_silent() and qemu_io_silent_check() John Snow
2022-03-24 18:30 ` [PATCH v2 17/17] iotests: make qemu_io_log() check return codes by default John Snow
16 siblings, 2 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, John Snow, qemu-block
I know we just added it, sorry. This is done in favor of qemu_io() which
*also* returns the console output and status, but with more robust error
handling on failure.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/iotests.py | 3 ---
1 file changed, 3 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index baf4995089..e903c8ede0 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -365,9 +365,6 @@ def qemu_io(*args: str, check: bool = True, combine_stdio: bool = True
return qemu_tool(*qemu_io_wrap_args(args),
check=check, combine_stdio=combine_stdio)
-def qemu_io_pipe_and_status(*args):
- return qemu_tool_pipe_and_status('qemu-io', qemu_io_wrap_args(args))
-
def qemu_io_log(*args: str) -> 'subprocess.CompletedProcess[str]':
result = qemu_io(*args, check=False)
log(result.stdout, filters=[filter_testfiles, filter_qemu_io])
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 15/17] iotests: remove qemu_io_pipe_and_status()
2022-03-24 18:30 ` [PATCH v2 15/17] iotests: remove qemu_io_pipe_and_status() John Snow
@ 2022-03-25 1:39 ` Eric Blake
2022-03-25 14:25 ` Hanna Reitz
1 sibling, 0 replies; 28+ messages in thread
From: Eric Blake @ 2022-03-25 1:39 UTC (permalink / raw)
To: John Snow; +Cc: Kevin Wolf, Hanna Reitz, qemu-devel, qemu-block
On Thu, Mar 24, 2022 at 02:30:16PM -0400, John Snow wrote:
> I know we just added it, sorry. This is done in favor of qemu_io() which
> *also* returns the console output and status, but with more robust error
> handling on failure.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 3 ---
> 1 file changed, 3 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 15/17] iotests: remove qemu_io_pipe_and_status()
2022-03-24 18:30 ` [PATCH v2 15/17] iotests: remove qemu_io_pipe_and_status() John Snow
2022-03-25 1:39 ` Eric Blake
@ 2022-03-25 14:25 ` Hanna Reitz
1 sibling, 0 replies; 28+ messages in thread
From: Hanna Reitz @ 2022-03-25 14:25 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block
On 24.03.22 19:30, John Snow wrote:
> I know we just added it, sorry. This is done in favor of qemu_io() which
> *also* returns the console output and status, but with more robust error
> handling on failure.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 3 ---
> 1 file changed, 3 deletions(-)
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 16/17] iotests: remove qemu_io_silent() and qemu_io_silent_check().
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (14 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 15/17] iotests: remove qemu_io_pipe_and_status() John Snow
@ 2022-03-24 18:30 ` John Snow
2022-03-24 18:30 ` [PATCH v2 17/17] iotests: make qemu_io_log() check return codes by default John Snow
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
Like qemu-img, qemu-io returning 0 should be the norm and not the
exception. Remove all calls to qemu_io_silent that just assert the
return code is zero (That's every last call, as it turns out), and
replace them with a normal qemu_io() call.
qemu_io_silent_check() appeared to have been unused already.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
tests/qemu-iotests/216 | 12 +++++-----
tests/qemu-iotests/218 | 5 ++---
tests/qemu-iotests/224 | 4 ++--
tests/qemu-iotests/258 | 11 +++++-----
tests/qemu-iotests/298 | 15 +++++--------
tests/qemu-iotests/310 | 22 +++++++++----------
tests/qemu-iotests/iotests.py | 16 --------------
tests/qemu-iotests/tests/image-fleecing | 4 ++--
.../tests/mirror-ready-cancel-error | 2 +-
.../qemu-iotests/tests/stream-error-on-reset | 4 ++--
10 files changed, 37 insertions(+), 58 deletions(-)
diff --git a/tests/qemu-iotests/216 b/tests/qemu-iotests/216
index 88b385afa3..97de1cda61 100755
--- a/tests/qemu-iotests/216
+++ b/tests/qemu-iotests/216
@@ -21,7 +21,7 @@
# Creator/Owner: Max Reitz <mreitz@redhat.com>
import iotests
-from iotests import log, qemu_img, qemu_io_silent
+from iotests import log, qemu_img, qemu_io
# Need backing file support
iotests.script_initialize(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk'],
@@ -52,10 +52,10 @@ with iotests.FilePath('base.img') as base_img_path, \
log('')
qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M')
- assert qemu_io_silent(base_img_path, '-c', 'write -P 1 0M 1M') == 0
+ qemu_io(base_img_path, '-c', 'write -P 1 0M 1M')
qemu_img('create', '-f', iotests.imgfmt, '-b', base_img_path,
'-F', iotests.imgfmt, top_img_path)
- assert qemu_io_silent(top_img_path, '-c', 'write -P 2 1M 1M') == 0
+ qemu_io(top_img_path, '-c', 'write -P 2 1M 1M')
log('Done')
@@ -110,8 +110,8 @@ with iotests.FilePath('base.img') as base_img_path, \
log('--- Checking COR result ---')
log('')
- assert qemu_io_silent(base_img_path, '-c', 'discard 0 64M') == 0
- assert qemu_io_silent(top_img_path, '-c', 'read -P 1 0M 1M') == 0
- assert qemu_io_silent(top_img_path, '-c', 'read -P 2 1M 1M') == 0
+ qemu_io(base_img_path, '-c', 'discard 0 64M')
+ qemu_io(top_img_path, '-c', 'read -P 1 0M 1M')
+ qemu_io(top_img_path, '-c', 'read -P 2 1M 1M')
log('Done')
diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
index 853ed52b34..0c717c9b7f 100755
--- a/tests/qemu-iotests/218
+++ b/tests/qemu-iotests/218
@@ -28,7 +28,7 @@
# Creator/Owner: Max Reitz <mreitz@redhat.com>
import iotests
-from iotests import log, qemu_img, qemu_io_silent
+from iotests import log, qemu_img, qemu_io
iotests.script_initialize(supported_fmts=['qcow2', 'raw'])
@@ -146,8 +146,7 @@ with iotests.VM() as vm, \
iotests.FilePath('src.img') as src_img_path:
qemu_img('create', '-f', iotests.imgfmt, src_img_path, '64M')
- assert qemu_io_silent('-f', iotests.imgfmt, src_img_path,
- '-c', 'write -P 42 0M 64M') == 0
+ qemu_io('-f', iotests.imgfmt, src_img_path, '-c', 'write -P 42 0M 64M')
vm.launch()
diff --git a/tests/qemu-iotests/224 b/tests/qemu-iotests/224
index c31c55b49d..8eb3ceb8d1 100755
--- a/tests/qemu-iotests/224
+++ b/tests/qemu-iotests/224
@@ -22,7 +22,7 @@
# Creator/Owner: Max Reitz <mreitz@redhat.com>
import iotests
-from iotests import log, qemu_img, qemu_io_silent, filter_qmp_testfiles, \
+from iotests import log, qemu_img, qemu_io, filter_qmp_testfiles, \
filter_qmp_imgfmt
import json
@@ -54,7 +54,7 @@ for filter_node_name in False, True:
'-F', iotests.imgfmt, top_img_path)
# Something to commit
- assert qemu_io_silent(mid_img_path, '-c', 'write -P 1 0 1M') == 0
+ qemu_io(mid_img_path, '-c', 'write -P 1 0 1M')
vm.launch()
diff --git a/tests/qemu-iotests/258 b/tests/qemu-iotests/258
index 7798a04d7d..35286216d3 100755
--- a/tests/qemu-iotests/258
+++ b/tests/qemu-iotests/258
@@ -21,7 +21,7 @@
# Creator/Owner: Max Reitz <mreitz@redhat.com>
import iotests
-from iotests import log, qemu_img, qemu_io_silent, \
+from iotests import log, qemu_img, qemu_io, \
filter_qmp_testfiles, filter_qmp_imgfmt
# Returns a node for blockdev-add
@@ -86,15 +86,14 @@ def test_concurrent_finish(write_to_stream_node):
if write_to_stream_node:
# This is what (most of the time) makes commit finish
# earlier and then pull in stream
- assert qemu_io_silent(node2_path,
- '-c', 'write %iK 64K' % (65536 - 192),
- '-c', 'write %iK 64K' % (65536 - 64)) == 0
+ qemu_io(node2_path,
+ '-c', 'write %iK 64K' % (65536 - 192),
+ '-c', 'write %iK 64K' % (65536 - 64))
stream_throttle='tg'
else:
# And this makes stream finish earlier
- assert qemu_io_silent(node1_path,
- '-c', 'write %iK 64K' % (65536 - 64)) == 0
+ qemu_io(node1_path, '-c', 'write %iK 64K' % (65536 - 64))
commit_throttle='tg'
diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
index fae72211b1..ad560e2941 100755
--- a/tests/qemu-iotests/298
+++ b/tests/qemu-iotests/298
@@ -129,16 +129,13 @@ class TestTruncate(iotests.QMPTestCase):
os.remove(refdisk)
def do_test(self, prealloc_mode, new_size):
- ret = iotests.qemu_io_silent('--image-opts', '-c', 'write 0 10M', '-c',
- f'truncate -m {prealloc_mode} {new_size}',
- drive_opts)
- self.assertEqual(ret, 0)
+ iotests.qemu_io('--image-opts', '-c', 'write 0 10M', '-c',
+ f'truncate -m {prealloc_mode} {new_size}',
+ drive_opts)
- ret = iotests.qemu_io_silent('-f', iotests.imgfmt, '-c', 'write 0 10M',
- '-c',
- f'truncate -m {prealloc_mode} {new_size}',
- refdisk)
- self.assertEqual(ret, 0)
+ iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write 0 10M',
+ '-c', f'truncate -m {prealloc_mode} {new_size}',
+ refdisk)
stat = os.stat(disk)
refstat = os.stat(refdisk)
diff --git a/tests/qemu-iotests/310 b/tests/qemu-iotests/310
index e3bfedc7fd..2496495f50 100755
--- a/tests/qemu-iotests/310
+++ b/tests/qemu-iotests/310
@@ -21,7 +21,7 @@
#
import iotests
-from iotests import log, qemu_img, qemu_io_silent
+from iotests import log, qemu_img, qemu_io
# Need backing file support
iotests.script_initialize(supported_fmts=['qcow2'],
@@ -44,15 +44,15 @@ with iotests.FilePath('base.img') as base_img_path, \
log('')
qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M')
- assert qemu_io_silent(base_img_path, '-c', 'write -P 1 0M 1M') == 0
- assert qemu_io_silent(base_img_path, '-c', 'write -P 1 3M 1M') == 0
+ qemu_io(base_img_path, '-c', 'write -P 1 0M 1M')
+ qemu_io(base_img_path, '-c', 'write -P 1 3M 1M')
qemu_img('create', '-f', iotests.imgfmt, '-b', base_img_path,
'-F', iotests.imgfmt, mid_img_path)
- assert qemu_io_silent(mid_img_path, '-c', 'write -P 3 2M 1M') == 0
- assert qemu_io_silent(mid_img_path, '-c', 'write -P 3 4M 1M') == 0
+ qemu_io(mid_img_path, '-c', 'write -P 3 2M 1M')
+ qemu_io(mid_img_path, '-c', 'write -P 3 4M 1M')
qemu_img('create', '-f', iotests.imgfmt, '-b', mid_img_path,
'-F', iotests.imgfmt, top_img_path)
- assert qemu_io_silent(top_img_path, '-c', 'write -P 2 1M 1M') == 0
+ qemu_io(top_img_path, '-c', 'write -P 2 1M 1M')
# 0 1 2 3 4
# top 2
@@ -107,10 +107,10 @@ with iotests.FilePath('base.img') as base_img_path, \
# Detach backing to check that we can read the data from the top level now
qemu_img('rebase', '-u', '-b', '', '-f', iotests.imgfmt, top_img_path)
- assert qemu_io_silent(top_img_path, '-c', 'read -P 0 0 1M') == 0
- assert qemu_io_silent(top_img_path, '-c', 'read -P 2 1M 1M') == 0
- assert qemu_io_silent(top_img_path, '-c', 'read -P 3 2M 1M') == 0
- assert qemu_io_silent(top_img_path, '-c', 'read -P 0 3M 1M') == 0
- assert qemu_io_silent(top_img_path, '-c', 'read -P 3 4M 1M') == 0
+ qemu_io(top_img_path, '-c', 'read -P 0 0 1M')
+ qemu_io(top_img_path, '-c', 'read -P 2 1M 1M')
+ qemu_io(top_img_path, '-c', 'read -P 3 2M 1M')
+ qemu_io(top_img_path, '-c', 'read -P 0 3M 1M')
+ qemu_io(top_img_path, '-c', 'read -P 3 4M 1M')
log('Done')
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e903c8ede0..1d103a3872 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -370,22 +370,6 @@ def qemu_io_log(*args: str) -> 'subprocess.CompletedProcess[str]':
log(result.stdout, filters=[filter_testfiles, filter_qemu_io])
return result
-def qemu_io_silent(*args):
- '''Run qemu-io and return the exit code, suppressing stdout'''
- args = qemu_io_wrap_args(args)
- result = subprocess.run(args, stdout=subprocess.DEVNULL, check=False)
- if result.returncode < 0:
- sys.stderr.write('qemu-io received signal %i: %s\n' %
- (-result.returncode, ' '.join(args)))
- return result.returncode
-
-def qemu_io_silent_check(*args):
- '''Run qemu-io and return the true if subprocess returned 0'''
- args = qemu_io_wrap_args(args)
- result = subprocess.run(args, stdout=subprocess.DEVNULL,
- stderr=subprocess.STDOUT, check=False)
- return result.returncode == 0
-
class QemuIoInteractive:
def __init__(self, *args):
self.args = qemu_io_wrap_args(args)
diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index ac749702f8..f6e449d071 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -25,7 +25,7 @@
from subprocess import CalledProcessError
import iotests
-from iotests import log, qemu_img, qemu_io, qemu_io_silent
+from iotests import log, qemu_img, qemu_io
iotests.script_initialize(
supported_fmts=['qcow2'],
@@ -270,7 +270,7 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
for p in overwrite + remainder:
cmd = 'read -P%s %s %s' % p
log(cmd)
- assert qemu_io_silent(base_img_path, '-c', cmd) == 0
+ qemu_io(base_img_path, '-c', cmd)
log('')
log('Done')
diff --git a/tests/qemu-iotests/tests/mirror-ready-cancel-error b/tests/qemu-iotests/tests/mirror-ready-cancel-error
index 1d0e333b5e..01217459b9 100755
--- a/tests/qemu-iotests/tests/mirror-ready-cancel-error
+++ b/tests/qemu-iotests/tests/mirror-ready-cancel-error
@@ -37,7 +37,7 @@ class TestMirrorReadyCancelError(iotests.QMPTestCase):
# Ensure that mirror will copy something before READY so the
# target format layer will forward the pre-READY flush to its
# file child
- assert iotests.qemu_io_silent('-c', 'write -P 1 0 64k', source) == 0
+ iotests.qemu_io('-c', 'write -P 1 0 64k', source)
self.vm = iotests.VM()
self.vm.launch()
diff --git a/tests/qemu-iotests/tests/stream-error-on-reset b/tests/qemu-iotests/tests/stream-error-on-reset
index 389ae822b8..5a8c3a9e8d 100755
--- a/tests/qemu-iotests/tests/stream-error-on-reset
+++ b/tests/qemu-iotests/tests/stream-error-on-reset
@@ -21,7 +21,7 @@
import os
import iotests
-from iotests import imgfmt, qemu_img_create, qemu_io_silent, QMPTestCase
+from iotests import imgfmt, qemu_img_create, qemu_io, QMPTestCase
image_size = 1 * 1024 * 1024
@@ -55,7 +55,7 @@ class TestStreamErrorOnReset(QMPTestCase):
- top image is attached to a virtio-scsi device
"""
qemu_img_create('-f', imgfmt, base, str(image_size))
- assert qemu_io_silent('-c', f'write 0 {data_size}', base) == 0
+ qemu_io('-c', f'write 0 {data_size}', base)
qemu_img_create('-f', imgfmt, top, str(image_size))
self.vm = iotests.VM()
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 17/17] iotests: make qemu_io_log() check return codes by default
2022-03-24 18:30 [PATCH v2 00/17] iotests: add enhanced debugging info to qemu-io failures John Snow
` (15 preceding siblings ...)
2022-03-24 18:30 ` [PATCH v2 16/17] iotests: remove qemu_io_silent() and qemu_io_silent_check() John Snow
@ 2022-03-24 18:30 ` John Snow
16 siblings, 0 replies; 28+ messages in thread
From: John Snow @ 2022-03-24 18:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Eric Blake, Hanna Reitz, John Snow, qemu-block
Just like qemu_img_log(), upgrade qemu_io_log() to enforce a return code
of zero by default.
Tests that use qemu_io_log(): 242 245 255 274 303 307 nbd-reconnect-on-open
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
tests/qemu-iotests/iotests.py | 5 +++--
tests/qemu-iotests/tests/nbd-reconnect-on-open | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1d103a3872..1a3662db0b 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -365,8 +365,9 @@ def qemu_io(*args: str, check: bool = True, combine_stdio: bool = True
return qemu_tool(*qemu_io_wrap_args(args),
check=check, combine_stdio=combine_stdio)
-def qemu_io_log(*args: str) -> 'subprocess.CompletedProcess[str]':
- result = qemu_io(*args, check=False)
+def qemu_io_log(*args: str, check: bool = True
+ ) -> 'subprocess.CompletedProcess[str]':
+ result = qemu_io(*args, check=check)
log(result.stdout, filters=[filter_testfiles, filter_qemu_io])
return result
diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open b/tests/qemu-iotests/tests/nbd-reconnect-on-open
index 8be721a24f..d0b401b060 100755
--- a/tests/qemu-iotests/tests/nbd-reconnect-on-open
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open
@@ -39,7 +39,7 @@ def check_fail_to_connect(open_timeout):
log(f'Check fail to connect with {open_timeout} seconds of timeout')
start_t = time.time()
- qemu_io_log(*create_args(open_timeout))
+ qemu_io_log(*create_args(open_timeout), check=False)
delta_t = time.time() - start_t
max_delta = open_timeout + 0.2
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread