* [Qemu-devel] [PATCH v4 0/4] iotests: use python logging
@ 2019-09-12 0:16 John Snow
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize John Snow
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: John Snow @ 2019-09-12 0:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, John Snow, qemu-block, Max Reitz
This series uses python logging to enable output conditionally on
iotests.log(). We unify an initialization call (which also enables
debugging output for those tests with -d) and then make the switch
inside of iotests.
It will help alleviate the need to create logged/unlogged versions
of all the various helpers we have made.
V4:
- Rebased on top of kwolf/block at the behest of mreitz
V3:
- Rebased for 4.1+; now based on main branch.
V2:
- Added all of the other python tests I missed to use script_initialize
- Refactored the common setup as per Ehabkost's suggestion
- Added protocol arguments to common initialization,
but this isn't strictly required.
John Snow (4):
iotests: add script_initialize
iotest 258: use script_main
iotests: specify protocol support via initialization info
iotests: use python logging for iotests.log()
tests/qemu-iotests/030 | 4 +-
tests/qemu-iotests/149 | 3 +-
tests/qemu-iotests/194 | 3 +-
tests/qemu-iotests/202 | 3 +-
tests/qemu-iotests/203 | 3 +-
tests/qemu-iotests/206 | 2 +-
tests/qemu-iotests/207 | 4 +-
tests/qemu-iotests/208 | 2 +-
tests/qemu-iotests/209 | 2 +-
tests/qemu-iotests/210 | 4 +-
tests/qemu-iotests/211 | 4 +-
tests/qemu-iotests/212 | 4 +-
tests/qemu-iotests/213 | 4 +-
tests/qemu-iotests/216 | 3 +-
tests/qemu-iotests/218 | 2 +-
tests/qemu-iotests/219 | 2 +-
tests/qemu-iotests/222 | 5 +-
tests/qemu-iotests/224 | 3 +-
tests/qemu-iotests/228 | 3 +-
tests/qemu-iotests/234 | 3 +-
tests/qemu-iotests/235 | 4 +-
tests/qemu-iotests/236 | 2 +-
tests/qemu-iotests/237 | 2 +-
tests/qemu-iotests/238 | 2 +
tests/qemu-iotests/242 | 2 +-
tests/qemu-iotests/245 | 1 +
tests/qemu-iotests/245.out | 24 ++++----
tests/qemu-iotests/246 | 2 +-
tests/qemu-iotests/248 | 2 +-
tests/qemu-iotests/254 | 2 +-
tests/qemu-iotests/255 | 2 +-
tests/qemu-iotests/256 | 2 +-
tests/qemu-iotests/258 | 8 +--
tests/qemu-iotests/262 | 3 +-
tests/qemu-iotests/iotests.py | 108 +++++++++++++++++++++-------------
35 files changed, 123 insertions(+), 106 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-12 0:16 [Qemu-devel] [PATCH v4 0/4] iotests: use python logging John Snow
@ 2019-09-12 0:16 ` John Snow
2019-09-16 14:56 ` Vladimir Sementsov-Ogievskiy
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 2/4] iotest 258: use script_main John Snow
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: John Snow @ 2019-09-12 0:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, John Snow, qemu-block, Max Reitz
Like script_main, but doesn't require a single point of entry.
Replace all existing initialization sections with this drop-in replacement.
This brings debug support to all existing script-style iotests.
Note: supported_oses=['linux'] was omitted, as it is a default argument.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/149 | 3 +-
tests/qemu-iotests/194 | 3 +-
tests/qemu-iotests/202 | 3 +-
tests/qemu-iotests/203 | 3 +-
tests/qemu-iotests/206 | 2 +-
tests/qemu-iotests/207 | 2 +-
tests/qemu-iotests/208 | 2 +-
tests/qemu-iotests/209 | 2 +-
tests/qemu-iotests/210 | 2 +-
tests/qemu-iotests/211 | 2 +-
tests/qemu-iotests/212 | 2 +-
tests/qemu-iotests/213 | 2 +-
tests/qemu-iotests/216 | 3 +-
tests/qemu-iotests/218 | 2 +-
tests/qemu-iotests/219 | 2 +-
tests/qemu-iotests/222 | 5 ++-
tests/qemu-iotests/224 | 3 +-
tests/qemu-iotests/228 | 3 +-
tests/qemu-iotests/234 | 3 +-
tests/qemu-iotests/235 | 4 +--
tests/qemu-iotests/236 | 2 +-
tests/qemu-iotests/237 | 2 +-
tests/qemu-iotests/238 | 2 ++
tests/qemu-iotests/242 | 2 +-
tests/qemu-iotests/246 | 2 +-
tests/qemu-iotests/248 | 2 +-
tests/qemu-iotests/254 | 2 +-
tests/qemu-iotests/255 | 2 +-
tests/qemu-iotests/256 | 2 +-
tests/qemu-iotests/262 | 3 +-
tests/qemu-iotests/iotests.py | 61 +++++++++++++++++++++++------------
31 files changed, 73 insertions(+), 62 deletions(-)
diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 4f363f295f..9fa97966c5 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -383,8 +383,7 @@ def test_once(config, qemu_img=False):
# Obviously we only work with the luks image format
-iotests.verify_image_format(supported_fmts=['luks'])
-iotests.verify_platform()
+iotests.script_initialize(supported_fmts=['luks'])
# We need sudo in order to run cryptsetup to create
# dm-crypt devices. This is safe to use on any
diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index d746ab1e21..c8aeb6d0e4 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -21,8 +21,7 @@
import iotests
-iotests.verify_image_format(supported_fmts=['qcow2', 'qed', 'raw'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2', 'qed', 'raw'])
with iotests.FilePath('source.img') as source_img_path, \
iotests.FilePath('dest.img') as dest_img_path, \
diff --git a/tests/qemu-iotests/202 b/tests/qemu-iotests/202
index 581ca34d79..1271ac9459 100755
--- a/tests/qemu-iotests/202
+++ b/tests/qemu-iotests/202
@@ -24,8 +24,7 @@
import iotests
-iotests.verify_image_format(supported_fmts=['qcow2'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2'])
with iotests.FilePath('disk0.img') as disk0_img_path, \
iotests.FilePath('disk1.img') as disk1_img_path, \
diff --git a/tests/qemu-iotests/203 b/tests/qemu-iotests/203
index 4874a1a0d8..c40fe231ea 100755
--- a/tests/qemu-iotests/203
+++ b/tests/qemu-iotests/203
@@ -24,8 +24,7 @@
import iotests
-iotests.verify_image_format(supported_fmts=['qcow2'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2'])
with iotests.FilePath('disk0.img') as disk0_img_path, \
iotests.FilePath('disk1.img') as disk1_img_path, \
diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
index 5bb738bf23..23ff2f624b 100755
--- a/tests/qemu-iotests/206
+++ b/tests/qemu-iotests/206
@@ -23,7 +23,7 @@
import iotests
from iotests import imgfmt
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
def blockdev_create(vm, options):
result = vm.qmp_log('blockdev-create',
diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207
index ec8c1d06f0..ab9e3b6747 100755
--- a/tests/qemu-iotests/207
+++ b/tests/qemu-iotests/207
@@ -24,7 +24,7 @@ import iotests
import subprocess
import re
-iotests.verify_image_format(supported_fmts=['raw'])
+iotests.script_initialize(supported_fmts=['raw'])
iotests.verify_protocol(supported=['ssh'])
def filter_hash(qmsg):
diff --git a/tests/qemu-iotests/208 b/tests/qemu-iotests/208
index 1e202388dc..dfce6f9fe4 100755
--- a/tests/qemu-iotests/208
+++ b/tests/qemu-iotests/208
@@ -22,7 +22,7 @@
import iotests
-iotests.verify_image_format(supported_fmts=['generic'])
+iotests.script_initialize(supported_fmts=['generic'])
with iotests.FilePath('disk.img') as disk_img_path, \
iotests.FilePath('disk-snapshot.img') as disk_snapshot_img_path, \
diff --git a/tests/qemu-iotests/209 b/tests/qemu-iotests/209
index 259e991ec6..a77f884166 100755
--- a/tests/qemu-iotests/209
+++ b/tests/qemu-iotests/209
@@ -22,7 +22,7 @@ import iotests
from iotests import qemu_img_create, qemu_io, qemu_img_verbose, qemu_nbd, \
file_path
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
disk, nbd_sock = file_path('disk', 'nbd-sock')
nbd_uri = 'nbd+unix:///exp?socket=' + nbd_sock
diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
index 565e3b7b9b..5a7013cd34 100755
--- a/tests/qemu-iotests/210
+++ b/tests/qemu-iotests/210
@@ -23,7 +23,7 @@
import iotests
from iotests import imgfmt
-iotests.verify_image_format(supported_fmts=['luks'])
+iotests.script_initialize(supported_fmts=['luks'])
iotests.verify_protocol(supported=['file'])
def blockdev_create(vm, options):
diff --git a/tests/qemu-iotests/211 b/tests/qemu-iotests/211
index 6afc894f76..4d6aac497f 100755
--- a/tests/qemu-iotests/211
+++ b/tests/qemu-iotests/211
@@ -23,7 +23,7 @@
import iotests
from iotests import imgfmt
-iotests.verify_image_format(supported_fmts=['vdi'])
+iotests.script_initialize(supported_fmts=['vdi'])
iotests.verify_protocol(supported=['file'])
def blockdev_create(vm, options):
diff --git a/tests/qemu-iotests/212 b/tests/qemu-iotests/212
index 42b74f208b..ec35bceb11 100755
--- a/tests/qemu-iotests/212
+++ b/tests/qemu-iotests/212
@@ -23,7 +23,7 @@
import iotests
from iotests import imgfmt
-iotests.verify_image_format(supported_fmts=['parallels'])
+iotests.script_initialize(supported_fmts=['parallels'])
iotests.verify_protocol(supported=['file'])
def blockdev_create(vm, options):
diff --git a/tests/qemu-iotests/213 b/tests/qemu-iotests/213
index 5604f3cebb..3d2c024375 100755
--- a/tests/qemu-iotests/213
+++ b/tests/qemu-iotests/213
@@ -23,7 +23,7 @@
import iotests
from iotests import imgfmt
-iotests.verify_image_format(supported_fmts=['vhdx'])
+iotests.script_initialize(supported_fmts=['vhdx'])
iotests.verify_protocol(supported=['file'])
def blockdev_create(vm, options):
diff --git a/tests/qemu-iotests/216 b/tests/qemu-iotests/216
index 3c0ae54b44..7574bcc09f 100755
--- a/tests/qemu-iotests/216
+++ b/tests/qemu-iotests/216
@@ -23,8 +23,7 @@ import iotests
from iotests import log, qemu_img, qemu_io_silent
# Need backing file support
-iotests.verify_image_format(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk'])
log('')
log('=== Copy-on-read across nodes ===')
diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
index 2554d84581..e18e31076b 100755
--- a/tests/qemu-iotests/218
+++ b/tests/qemu-iotests/218
@@ -29,7 +29,7 @@
import iotests
from iotests import log, qemu_img, qemu_io_silent
-iotests.verify_image_format(supported_fmts=['qcow2', 'raw'])
+iotests.script_initialize(supported_fmts=['qcow2', 'raw'])
# Launches the VM, adds two null-co nodes (source and target), and
diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219
index e0c51662c0..9ae27cb04e 100755
--- a/tests/qemu-iotests/219
+++ b/tests/qemu-iotests/219
@@ -21,7 +21,7 @@
import iotests
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
img_size = 4 * 1024 * 1024
diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/222
index 0ead56d574..6788979ed3 100644
--- a/tests/qemu-iotests/222
+++ b/tests/qemu-iotests/222
@@ -24,9 +24,8 @@
import iotests
from iotests import log, qemu_img, qemu_io, qemu_io_silent
-iotests.verify_platform(['linux'])
-iotests.verify_image_format(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk',
- 'vhdx', 'raw'])
+iotests.script_initialize(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk',
+ 'vhdx', 'raw'])
patterns = [("0x5d", "0", "64k"),
("0xd5", "1M", "64k"),
diff --git a/tests/qemu-iotests/224 b/tests/qemu-iotests/224
index b4dfaa639f..d0d0c44104 100755
--- a/tests/qemu-iotests/224
+++ b/tests/qemu-iotests/224
@@ -26,8 +26,7 @@ from iotests import log, qemu_img, qemu_io_silent, filter_qmp_testfiles, \
import json
# Need backing file support (for arbitrary backing formats)
-iotests.verify_image_format(supported_fmts=['qcow2', 'qcow', 'qed'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2', 'qcow', 'qed'])
# There are two variations of this test:
diff --git a/tests/qemu-iotests/228 b/tests/qemu-iotests/228
index 9a50afd205..9785868ab3 100755
--- a/tests/qemu-iotests/228
+++ b/tests/qemu-iotests/228
@@ -25,8 +25,7 @@ from iotests import log, qemu_img, filter_testfiles, filter_imgfmt, \
filter_qmp_testfiles, filter_qmp_imgfmt
# Need backing file and change-backing-file support
-iotests.verify_image_format(supported_fmts=['qcow2', 'qed'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2', 'qed'])
def log_node_info(node):
diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234
index 34c818c485..3de6ab2341 100755
--- a/tests/qemu-iotests/234
+++ b/tests/qemu-iotests/234
@@ -23,8 +23,7 @@
import iotests
import os
-iotests.verify_image_format(supported_fmts=['qcow2'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2'])
with iotests.FilePath('img') as img_path, \
iotests.FilePath('backing') as backing_path, \
diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
index fedd111fd4..9e88c65b93 100755
--- a/tests/qemu-iotests/235
+++ b/tests/qemu-iotests/235
@@ -27,6 +27,8 @@ sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
from qemu.machine import QEMUMachine
+iotests.script_initialize(supported_fmts=['qcow2'])
+
# Note:
# This test was added to check that mirror dead-lock was fixed (see previous
# commit before this test addition).
@@ -40,8 +42,6 @@ from qemu.machine import QEMUMachine
size = 1 * 1024 * 1024 * 1024
-iotests.verify_image_format(supported_fmts=['qcow2'])
-
disk = file_path('disk')
# prepare source image
diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
index 79a6381f8e..b88779eb0b 100755
--- a/tests/qemu-iotests/236
+++ b/tests/qemu-iotests/236
@@ -22,7 +22,7 @@
import iotests
from iotests import log
-iotests.verify_image_format(supported_fmts=['generic'])
+iotests.script_initialize(supported_fmts=['generic'])
size = 64 * 1024 * 1024
granularity = 64 * 1024
diff --git a/tests/qemu-iotests/237 b/tests/qemu-iotests/237
index 06897f8c87..3758ace0bc 100755
--- a/tests/qemu-iotests/237
+++ b/tests/qemu-iotests/237
@@ -24,7 +24,7 @@ import math
import iotests
from iotests import imgfmt
-iotests.verify_image_format(supported_fmts=['vmdk'])
+iotests.script_initialize(supported_fmts=['vmdk'])
def blockdev_create(vm, options):
result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
diff --git a/tests/qemu-iotests/238 b/tests/qemu-iotests/238
index e5ac2b2ff8..6e27fb40c2 100755
--- a/tests/qemu-iotests/238
+++ b/tests/qemu-iotests/238
@@ -23,6 +23,8 @@ import os
import iotests
from iotests import log
+iotests.script_initialize()
+
virtio_scsi_device = iotests.get_virtio_scsi_device()
vm = iotests.VM()
diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
index c176e92da6..7c2685b4cc 100755
--- a/tests/qemu-iotests/242
+++ b/tests/qemu-iotests/242
@@ -24,7 +24,7 @@ import struct
from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
file_path, img_info_log, log, filter_qemu_io
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
disk = file_path('disk')
chunk = 256 * 1024
diff --git a/tests/qemu-iotests/246 b/tests/qemu-iotests/246
index b0997a392f..1d7747d62d 100755
--- a/tests/qemu-iotests/246
+++ b/tests/qemu-iotests/246
@@ -22,7 +22,7 @@
import iotests
from iotests import log
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
size = 64 * 1024 * 1024 * 1024
gran_small = 32 * 1024
gran_large = 128 * 1024
diff --git a/tests/qemu-iotests/248 b/tests/qemu-iotests/248
index f26b4bb2aa..781b21b227 100755
--- a/tests/qemu-iotests/248
+++ b/tests/qemu-iotests/248
@@ -21,7 +21,7 @@
import iotests
from iotests import qemu_img_create, qemu_io, file_path, filter_qmp_testfiles
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
source, target = file_path('source', 'target')
size = 5 * 1024 * 1024
diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254
index 09584f3f7d..43b40f4f71 100755
--- a/tests/qemu-iotests/254
+++ b/tests/qemu-iotests/254
@@ -21,7 +21,7 @@
import iotests
from iotests import qemu_img_create, file_path, log
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
disk, top = file_path('disk', 'top')
size = 1024 * 1024
diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255
index 3632d507d0..ff16402268 100755
--- a/tests/qemu-iotests/255
+++ b/tests/qemu-iotests/255
@@ -23,7 +23,7 @@
import iotests
from iotests import imgfmt
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
def blockdev_create(vm, options):
result = vm.qmp_log('blockdev-create',
diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
index c594a43205..d2f9212e5a 100755
--- a/tests/qemu-iotests/256
+++ b/tests/qemu-iotests/256
@@ -23,7 +23,7 @@ import os
import iotests
from iotests import log
-iotests.verify_image_format(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2'])
size = 64 * 1024 * 1024
with iotests.FilePath('img0') as img0_path, \
diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
index 398f63587e..f0e9d0f8ac 100755
--- a/tests/qemu-iotests/262
+++ b/tests/qemu-iotests/262
@@ -23,8 +23,7 @@
import iotests
import os
-iotests.verify_image_format(supported_fmts=['qcow2'])
-iotests.verify_platform(['linux'])
+iotests.script_initialize(supported_fmts=['qcow2'])
with iotests.FilePath('img') as img_path, \
iotests.FilePath('mig_fifo') as fifo, \
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b26271187c..92117a64bc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -895,7 +895,20 @@ def skip_if_unsupported(required_formats=[], read_only=False):
return func_wrapper
return skip_test_decorator
-def execute_unittest(output, verbosity, debug):
+def execute_unittest(debug=False):
+ """Executes unittests within the calling module."""
+
+ verbosity = 2 if debug else 1
+
+ if debug:
+ output = sys.stdout
+ elif sys.version_info.major >= 3:
+ output = io.StringIO()
+ else:
+ # io.StringIO is for unicode strings, which is not what
+ # 2.x's test runner emits.
+ output = io.BytesIO()
+
runner = unittest.TextTestRunner(stream=output, descriptions=True,
verbosity=verbosity)
try:
@@ -903,15 +916,21 @@ def execute_unittest(output, verbosity, debug):
# exception
unittest.main(testRunner=runner)
finally:
+ # We need to filter out the time taken from the output so that
+ # qemu-iotest can reliably diff the results against master output.
if not debug:
sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s',
r'Ran \1 tests', output.getvalue()))
-def execute_test(test_function=None,
- supported_fmts=[], supported_oses=['linux'],
- supported_cache_modes=[], unsupported_fmts=[],
- supported_protocols=[], unsupported_protocols=[]):
- """Run either unittest or script-style tests."""
+def execute_setup_common(supported_fmts=[],
+ supported_oses=['linux'],
+ supported_cache_modes=[],
+ unsupported_fmts=[],
+ supported_protocols=[],
+ unsupported_protocols=[]):
+ """
+ Perform necessary setup for either script-style or unittest-style tests.
+ """
# We are using TEST_DIR and QEMU_DEFAULT_MACHINE as proxies to
# indicate that we're not being run via "check". There may be
@@ -921,38 +940,38 @@ def execute_test(test_function=None,
sys.stderr.write('Please run this test via the "check" script\n')
sys.exit(os.EX_USAGE)
- debug = '-d' in sys.argv
- verbosity = 1
verify_image_format(supported_fmts, unsupported_fmts)
verify_protocol(supported_protocols, unsupported_protocols)
verify_platform(supported_oses)
verify_cache_mode(supported_cache_modes)
+ debug = '-d' in sys.argv
if debug:
- output = sys.stdout
- verbosity = 2
sys.argv.remove('-d')
- else:
- # We need to filter out the time taken from the output so that
- # qemu-iotest can reliably diff the results against master output.
- if sys.version_info.major >= 3:
- output = io.StringIO()
- else:
- # io.StringIO is for unicode strings, which is not what
- # 2.x's test runner emits.
- output = io.BytesIO()
-
logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
+ return debug
+
+def execute_test(test_function=None, *args, **kwargs):
+ """Run either unittest or script-style tests."""
+
+ debug = execute_setup_common(*args, **kwargs)
if not test_function:
- execute_unittest(output, verbosity, debug)
+ execute_unittest(debug)
else:
test_function()
+# This is called from script-style iotests without a single point of entry
+def script_initialize(*args, **kwargs):
+ """Initialize script-style tests without running any tests."""
+ execute_setup_common(*args, **kwargs)
+
+# This is called from script-style iotests with a single point of entry
def script_main(test_function, *args, **kwargs):
"""Run script-style tests outside of the unittest framework"""
execute_test(test_function, *args, **kwargs)
+# This is called from unittest style iotests
def main(*args, **kwargs):
"""Run tests using the unittest framework"""
execute_test(None, *args, **kwargs)
--
2.21.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v4 2/4] iotest 258: use script_main
2019-09-12 0:16 [Qemu-devel] [PATCH v4 0/4] iotests: use python logging John Snow
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize John Snow
@ 2019-09-12 0:16 ` John Snow
2019-09-17 9:36 ` Vladimir Sementsov-Ogievskiy
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 3/4] iotests: specify protocol support via initialization info John Snow
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 4/4] iotests: use python logging for iotests.log() John Snow
3 siblings, 1 reply; 19+ messages in thread
From: John Snow @ 2019-09-12 0:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, John Snow, qemu-block, Max Reitz
Since this one is nicely factored to use a single entry point,
use script_main to run the tests.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/258 | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tests/qemu-iotests/258 b/tests/qemu-iotests/258
index b84cf02254..1372522c7a 100755
--- a/tests/qemu-iotests/258
+++ b/tests/qemu-iotests/258
@@ -23,11 +23,6 @@ import iotests
from iotests import log, qemu_img, qemu_io_silent, \
filter_qmp_testfiles, filter_qmp_imgfmt
-# Need backing file and change-backing-file support
-iotests.verify_image_format(supported_fmts=['qcow2', 'qed'])
-iotests.verify_platform(['linux'])
-
-
# Returns a node for blockdev-add
def node(node_name, path, backing=None, fmt=None, throttle=None):
if fmt is None:
@@ -160,4 +155,5 @@ def main():
test_concurrent_finish(False)
if __name__ == '__main__':
- main()
+ # Need backing file and change-backing-file support
+ iotests.script_main(main, supported_fmts=['qcow2', 'qed'])
--
2.21.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v4 3/4] iotests: specify protocol support via initialization info
2019-09-12 0:16 [Qemu-devel] [PATCH v4 0/4] iotests: use python logging John Snow
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize John Snow
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 2/4] iotest 258: use script_main John Snow
@ 2019-09-12 0:16 ` John Snow
2019-09-17 9:51 ` Vladimir Sementsov-Ogievskiy
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 4/4] iotests: use python logging for iotests.log() John Snow
3 siblings, 1 reply; 19+ messages in thread
From: John Snow @ 2019-09-12 0:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, John Snow, qemu-block, Max Reitz
Switch from verify_protocols to any one of:
iotests.main, iotests.script_main, iotests.script_initialize.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/207 | 4 ++--
tests/qemu-iotests/210 | 4 ++--
tests/qemu-iotests/211 | 4 ++--
tests/qemu-iotests/212 | 4 ++--
tests/qemu-iotests/213 | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207
index ab9e3b6747..35d98f2736 100755
--- a/tests/qemu-iotests/207
+++ b/tests/qemu-iotests/207
@@ -24,8 +24,8 @@ import iotests
import subprocess
import re
-iotests.script_initialize(supported_fmts=['raw'])
-iotests.verify_protocol(supported=['ssh'])
+iotests.script_initialize(supported_fmts=['raw'],
+ supported_protocols=['ssh'])
def filter_hash(qmsg):
def _filter(key, value):
diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
index 5a7013cd34..d9fe780c07 100755
--- a/tests/qemu-iotests/210
+++ b/tests/qemu-iotests/210
@@ -23,8 +23,8 @@
import iotests
from iotests import imgfmt
-iotests.script_initialize(supported_fmts=['luks'])
-iotests.verify_protocol(supported=['file'])
+iotests.script_initialize(supported_fmts=['luks'],
+ supported_protocols=['file'])
def blockdev_create(vm, options):
result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
diff --git a/tests/qemu-iotests/211 b/tests/qemu-iotests/211
index 4d6aac497f..155fac4e87 100755
--- a/tests/qemu-iotests/211
+++ b/tests/qemu-iotests/211
@@ -23,8 +23,8 @@
import iotests
from iotests import imgfmt
-iotests.script_initialize(supported_fmts=['vdi'])
-iotests.verify_protocol(supported=['file'])
+iotests.script_initialize(supported_fmts=['vdi'],
+ supported_protocols=['file'])
def blockdev_create(vm, options):
result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
diff --git a/tests/qemu-iotests/212 b/tests/qemu-iotests/212
index ec35bceb11..67e5a1dbb5 100755
--- a/tests/qemu-iotests/212
+++ b/tests/qemu-iotests/212
@@ -23,8 +23,8 @@
import iotests
from iotests import imgfmt
-iotests.script_initialize(supported_fmts=['parallels'])
-iotests.verify_protocol(supported=['file'])
+iotests.script_initialize(supported_fmts=['parallels'],
+ supported_protocols=['file'])
def blockdev_create(vm, options):
result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
diff --git a/tests/qemu-iotests/213 b/tests/qemu-iotests/213
index 3d2c024375..23f387ab63 100755
--- a/tests/qemu-iotests/213
+++ b/tests/qemu-iotests/213
@@ -23,8 +23,8 @@
import iotests
from iotests import imgfmt
-iotests.script_initialize(supported_fmts=['vhdx'])
-iotests.verify_protocol(supported=['file'])
+iotests.script_initialize(supported_fmts=['vhdx'],
+ supported_protocols=['file'])
def blockdev_create(vm, options):
result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
--
2.21.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v4 4/4] iotests: use python logging for iotests.log()
2019-09-12 0:16 [Qemu-devel] [PATCH v4 0/4] iotests: use python logging John Snow
` (2 preceding siblings ...)
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 3/4] iotests: specify protocol support via initialization info John Snow
@ 2019-09-12 0:16 ` John Snow
2019-09-17 11:35 ` Vladimir Sementsov-Ogievskiy
3 siblings, 1 reply; 19+ messages in thread
From: John Snow @ 2019-09-12 0:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, John Snow, qemu-block, Max Reitz
We can turn logging on/off globally instead of per-function.
Remove use_log from run_job, and use python logging to turn on
diffable output when we run through a script entry point.
iotest 245 changes output order due to buffering reasons.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/030 | 4 +--
tests/qemu-iotests/245 | 1 +
tests/qemu-iotests/245.out | 24 +++++++++---------
tests/qemu-iotests/iotests.py | 47 +++++++++++++++++++++--------------
4 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index f3766f2a81..01aa96ed16 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -411,8 +411,8 @@ class TestParallelOps(iotests.QMPTestCase):
result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
self.assert_qmp(result, 'return', {})
- self.vm.run_job(job='drive0', auto_dismiss=True, use_log=False)
- self.vm.run_job(job='node4', auto_dismiss=True, use_log=False)
+ self.vm.run_job(job='drive0', auto_dismiss=True)
+ self.vm.run_job(job='node4', auto_dismiss=True)
self.assert_no_active_block_jobs()
# Test a block-stream and a block-commit job in parallel
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 41218d5f1d..eba2157cff 100644
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -1000,5 +1000,6 @@ class TestBlockdevReopen(iotests.QMPTestCase):
self.reopen(opts, {'backing': 'hd2'})
if __name__ == '__main__':
+ iotests.activate_logging()
iotests.main(supported_fmts=["qcow2"],
supported_protocols=["file"])
diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out
index a19de5214d..15c3630e92 100644
--- a/tests/qemu-iotests/245.out
+++ b/tests/qemu-iotests/245.out
@@ -1,17 +1,17 @@
+{"execute": "job-finalize", "arguments": {"id": "commit0"}}
+{"return": {}}
+{"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"execute": "job-finalize", "arguments": {"id": "stream0"}}
+{"return": {}}
+{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"execute": "job-finalize", "arguments": {"id": "stream0"}}
+{"return": {}}
+{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
..................
----------------------------------------------------------------------
Ran 18 tests
OK
-{"execute": "job-finalize", "arguments": {"id": "commit0"}}
-{"return": {}}
-{"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-{"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-{"execute": "job-finalize", "arguments": {"id": "stream0"}}
-{"return": {}}
-{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-{"execute": "job-finalize", "arguments": {"id": "stream0"}}
-{"return": {}}
-{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 92117a64bc..7e46fb2754 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -35,6 +35,13 @@ from collections import OrderedDict
sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
from qemu import qtest
+# Use this logger for logging messages directly from the iotests module
+logger = logging.getLogger(__name__)
+logger.addHandler(logging.NullHandler())
+
+# Use this logger for messages that ought to be used for diff output.
+test_logger = logging.getLogger('.'.join((__name__, 'iotest')))
+test_logger.addHandler(logging.NullHandler())
# This will not work if arguments contain spaces but is necessary if we
# want to support the override options that ./check supports.
@@ -343,10 +350,10 @@ def log(msg, filters=[], indent=None):
separators = (', ', ': ') if indent is None else (',', ': ')
# Don't sort if it's already sorted
do_sort = not isinstance(msg, OrderedDict)
- print(json.dumps(msg, sort_keys=do_sort,
- indent=indent, separators=separators))
+ test_logger.info(json.dumps(msg, sort_keys=do_sort,
+ indent=indent, separators=separators))
else:
- print(msg)
+ test_logger.info(msg)
class Timeout:
def __init__(self, seconds, errmsg = "Timeout"):
@@ -559,7 +566,7 @@ class VM(qtest.QEMUQtestMachine):
# Returns None on success, and an error string on failure
def run_job(self, job, auto_finalize=True, auto_dismiss=False,
- pre_finalize=None, cancel=False, use_log=True, wait=60.0):
+ pre_finalize=None, cancel=False, wait=60.0):
"""
run_job moves a job from creation through to dismissal.
@@ -572,7 +579,6 @@ class VM(qtest.QEMUQtestMachine):
invoked prior to issuing job-finalize, if any.
:param cancel: Bool. When true, cancels the job after the pre_finalize
callback.
- :param use_log: Bool. When false, does not log QMP messages.
:param wait: Float. Timeout value specifying how long to wait for any
event, in seconds. Defaults to 60.0.
"""
@@ -590,8 +596,7 @@ class VM(qtest.QEMUQtestMachine):
while True:
ev = filter_qmp_event(self.events_wait(events))
if ev['event'] != 'JOB_STATUS_CHANGE':
- if use_log:
- log(ev)
+ log(ev)
continue
status = ev['data']['status']
if status == 'aborting':
@@ -599,24 +604,16 @@ class VM(qtest.QEMUQtestMachine):
for j in result['return']:
if j['id'] == job:
error = j['error']
- if use_log:
- log('Job failed: %s' % (j['error']))
+ log('Job failed: %s' % (j['error']))
elif status == 'pending' and not auto_finalize:
if pre_finalize:
pre_finalize()
- if cancel and use_log:
+ if cancel:
self.qmp_log('job-cancel', id=job)
- elif cancel:
- self.qmp('job-cancel', id=job)
- elif use_log:
+ else:
self.qmp_log('job-finalize', id=job)
- else:
- self.qmp('job-finalize', id=job)
elif status == 'concluded' and not auto_dismiss:
- if use_log:
- self.qmp_log('job-dismiss', id=job)
- else:
- self.qmp('job-dismiss', id=job)
+ self.qmp_log('job-dismiss', id=job)
elif status == 'null':
return error
@@ -949,6 +946,7 @@ def execute_setup_common(supported_fmts=[],
if debug:
sys.argv.remove('-d')
logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
+ logger.debug("iotests debugging messages active")
return debug
@@ -961,14 +959,25 @@ def execute_test(test_function=None, *args, **kwargs):
else:
test_function()
+def activate_logging():
+ """Activate iotests.log() output to stdout for script-style tests."""
+ handler = logging.StreamHandler(stream=sys.stdout)
+ formatter = logging.Formatter('%(message)s')
+ handler.setFormatter(formatter)
+ test_logger.addHandler(handler)
+ test_logger.setLevel(logging.INFO)
+ test_logger.propagate = False
+
# This is called from script-style iotests without a single point of entry
def script_initialize(*args, **kwargs):
"""Initialize script-style tests without running any tests."""
+ activate_logging()
execute_setup_common(*args, **kwargs)
# This is called from script-style iotests with a single point of entry
def script_main(test_function, *args, **kwargs):
"""Run script-style tests outside of the unittest framework"""
+ activate_logging()
execute_test(test_function, *args, **kwargs)
# This is called from unittest style iotests
--
2.21.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize John Snow
@ 2019-09-16 14:56 ` Vladimir Sementsov-Ogievskiy
2019-09-16 16:32 ` John Snow
2019-09-17 22:29 ` John Snow
0 siblings, 2 replies; 19+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 14:56 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz
12.09.2019 3:16, John Snow wrote:
> Like script_main, but doesn't require a single point of entry.
> Replace all existing initialization sections with this drop-in replacement.
>
> This brings debug support to all existing script-style iotests.
>
> Note: supported_oses=['linux'] was omitted, as it is a default argument.
But after this patch all test which didn't check os start to check linux
(as it's default).. So all tests which worked on other platforms will now
be skipped on these other platforms?
Finally do we support something except linux for iotests?
for bash tests _supported_os also used only with "Linux" in 87 tests..
May be we'd better drop both _supported_os and supported_oses alltogether,
and don't care?
Anyway, if we support only linux, any reason to skip almost all tests,
if someone tries to run test on other platform? Let him run what he wants.
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
[..]
> +def execute_test(test_function=None, *args, **kwargs):
> + """Run either unittest or script-style tests."""
> +
> + debug = execute_setup_common(*args, **kwargs)
> if not test_function:
> - execute_unittest(output, verbosity, debug)
> + execute_unittest(debug)
> else:
> test_function()
>
> +# This is called from script-style iotests without a single point of entry
> +def script_initialize(*args, **kwargs):
> + """Initialize script-style tests without running any tests."""
> + execute_setup_common(*args, **kwargs)
> +
> +# This is called from script-style iotests with a single point of entry
> def script_main(test_function, *args, **kwargs):
> """Run script-style tests outside of the unittest framework"""
> execute_test(test_function, *args, **kwargs)
>
> +# This is called from unittest style iotests
> def main(*args, **kwargs):
> """Run tests using the unittest framework"""
Hmm, now two different styles of code documenting used: comment and doc-string,
both containing almost equal meaning.. I don't like it, still don't really mind.
> execute_test(None, *args, **kwargs)
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-16 14:56 ` Vladimir Sementsov-Ogievskiy
@ 2019-09-16 16:32 ` John Snow
2019-09-16 16:39 ` Vladimir Sementsov-Ogievskiy
2019-09-17 22:29 ` John Snow
1 sibling, 1 reply; 19+ messages in thread
From: John Snow @ 2019-09-16 16:32 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: Kevin Wolf, qemu-block, Max Reitz
On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.09.2019 3:16, John Snow wrote:
>> Like script_main, but doesn't require a single point of entry.
>> Replace all existing initialization sections with this drop-in replacement.
>>
>> This brings debug support to all existing script-style iotests.
>>
>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>
> But after this patch all test which didn't check os start to check linux
> (as it's default).. So all tests which worked on other platforms will now
> be skipped on these other platforms?
>
def verify_platform(supported_oses=['linux']):
if True not in [sys.platform.startswith(x) for x in supported_oses]:
notrun('not suitable for this OS: %s' % sys.platform)
It was already the default. I didn't *make* it the default. There is no
change here. Feel free to propose a fix, but I don't think it's within
the scope of this series.
> Finally do we support something except linux for iotests?
> for bash tests _supported_os also used only with "Linux" in 87 tests..
>
Evidently, not really. See bc521696607c5348fcd8a9e57b408d0ac0dbe2f8
> May be we'd better drop both _supported_os and supported_oses alltogether,
> and don't care?
>
Beyond the scope of this series.
> Anyway, if we support only linux, any reason to skip almost all tests,
> if someone tries to run test on other platform? Let him run what he wants.
Maybe true, maybe not.
>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>
> [..]
>
>> +def execute_test(test_function=None, *args, **kwargs):
>> + """Run either unittest or script-style tests."""
>> +
>> + debug = execute_setup_common(*args, **kwargs)
>> if not test_function:
>> - execute_unittest(output, verbosity, debug)
>> + execute_unittest(debug)
>> else:
>> test_function()
>>
>> +# This is called from script-style iotests without a single point of entry
>> +def script_initialize(*args, **kwargs):
>> + """Initialize script-style tests without running any tests."""
>> + execute_setup_common(*args, **kwargs)
>> +
>> +# This is called from script-style iotests with a single point of entry
>> def script_main(test_function, *args, **kwargs):
>> """Run script-style tests outside of the unittest framework"""
>> execute_test(test_function, *args, **kwargs)
>>
>> +# This is called from unittest style iotests
>> def main(*args, **kwargs):
>> """Run tests using the unittest framework"""
>
>
> Hmm, now two different styles of code documenting used: comment and doc-string,
> both containing almost equal meaning.. I don't like it, still don't really mind.
I think I was trying to document what the function /does/ separately
from a note about explaining how and where it is used. It's quite nearly
redundant and if it's distracting I can remove it.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-16 16:32 ` John Snow
@ 2019-09-16 16:39 ` Vladimir Sementsov-Ogievskiy
2019-09-16 17:13 ` John Snow
0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 16:39 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz
16.09.2019 19:32, John Snow wrote:
>
>
> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 12.09.2019 3:16, John Snow wrote:
>>> Like script_main, but doesn't require a single point of entry.
>>> Replace all existing initialization sections with this drop-in replacement.
>>>
>>> This brings debug support to all existing script-style iotests.
>>>
>>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>>
>> But after this patch all test which didn't check os start to check linux
>> (as it's default).. So all tests which worked on other platforms will now
>> be skipped on these other platforms?
>>
>
> def verify_platform(supported_oses=['linux']):
> if True not in [sys.platform.startswith(x) for x in supported_oses]:
> notrun('not suitable for this OS: %s' % sys.platform)
>
>
> It was already the default. I didn't *make* it the default.
Yes. But for some tests, verify_platform wasn't called before this patch at all.
And now it is called and checks "linux". It's the change. Or what I miss?
> There is no
> change here. Feel free to propose a fix, but I don't think it's within
> the scope of this series.
>
>> Finally do we support something except linux for iotests?
>> for bash tests _supported_os also used only with "Linux" in 87 tests..
>>
>
> Evidently, not really. See bc521696607c5348fcd8a9e57b408d0ac0dbe2f8
>
>> May be we'd better drop both _supported_os and supported_oses alltogether,
>> and don't care?
>>
>
> Beyond the scope of this series.
>
>> Anyway, if we support only linux, any reason to skip almost all tests,
>> if someone tries to run test on other platform? Let him run what he wants.
>
> Maybe true, maybe not.
>
>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>
>> [..]
>>
>>> +def execute_test(test_function=None, *args, **kwargs):
>>> + """Run either unittest or script-style tests."""
>>> +
>>> + debug = execute_setup_common(*args, **kwargs)
>>> if not test_function:
>>> - execute_unittest(output, verbosity, debug)
>>> + execute_unittest(debug)
>>> else:
>>> test_function()
>>>
>>> +# This is called from script-style iotests without a single point of entry
>>> +def script_initialize(*args, **kwargs):
>>> + """Initialize script-style tests without running any tests."""
>>> + execute_setup_common(*args, **kwargs)
>>> +
>>> +# This is called from script-style iotests with a single point of entry
>>> def script_main(test_function, *args, **kwargs):
>>> """Run script-style tests outside of the unittest framework"""
>>> execute_test(test_function, *args, **kwargs)
>>>
>>> +# This is called from unittest style iotests
>>> def main(*args, **kwargs):
>>> """Run tests using the unittest framework"""
>>
>>
>> Hmm, now two different styles of code documenting used: comment and doc-string,
>> both containing almost equal meaning.. I don't like it, still don't really mind.
>
> I think I was trying to document what the function /does/ separately
> from a note about explaining how and where it is used. It's quite nearly
> redundant and if it's distracting I can remove it.
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-16 16:39 ` Vladimir Sementsov-Ogievskiy
@ 2019-09-16 17:13 ` John Snow
0 siblings, 0 replies; 19+ messages in thread
From: John Snow @ 2019-09-16 17:13 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, Kevin Wolf
Cc: qemu-block, Max Reitz
On 9/16/19 12:39 PM, Vladimir Sementsov-Ogievskiy wrote:
> 16.09.2019 19:32, John Snow wrote:
>>
>>
>> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 12.09.2019 3:16, John Snow wrote:
>>>> Like script_main, but doesn't require a single point of entry.
>>>> Replace all existing initialization sections with this drop-in replacement.
>>>>
>>>> This brings debug support to all existing script-style iotests.
>>>>
>>>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>>>
>>> But after this patch all test which didn't check os start to check linux
>>> (as it's default).. So all tests which worked on other platforms will now
>>> be skipped on these other platforms?
>>>
>>
>> def verify_platform(supported_oses=['linux']):
>> if True not in [sys.platform.startswith(x) for x in supported_oses]:
>> notrun('not suitable for this OS: %s' % sys.platform)
>>
>>
>> It was already the default. I didn't *make* it the default.
>
> Yes. But for some tests, verify_platform wasn't called before this patch at all.
> And now it is called and checks "linux". It's the change. Or what I miss?
>
I guess there are more than I thought.
(I thought it was just one, based on a discussion with Philippe.)
So, you're right: there are more tests now that will refuse to run on
non-Linux platforms.
Those tests are:
206
207
208
209
210
211
212
213
218
219
235
236
237
238
242
246
248
254
255
256
What sets these apart from the other python-based tests? Nothing in
particular? Just depends on who copied from whom that day. I don't think
there's any rhyme or reason to any of it.
I don't really have a BSD or OSX machine to test which ones should and
should not be limited, either.
Kevin, do you remember why we added these checks?
--js
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/4] iotest 258: use script_main
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 2/4] iotest 258: use script_main John Snow
@ 2019-09-17 9:36 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 19+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-17 9:36 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz
12.09.2019 3:16, John Snow wrote:
> Since this one is nicely factored to use a single entry point,
> use script_main to run the tests.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/258 | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qemu-iotests/258 b/tests/qemu-iotests/258
> index b84cf02254..1372522c7a 100755
> --- a/tests/qemu-iotests/258
> +++ b/tests/qemu-iotests/258
> @@ -23,11 +23,6 @@ import iotests
> from iotests import log, qemu_img, qemu_io_silent, \
> filter_qmp_testfiles, filter_qmp_imgfmt
>
> -# Need backing file and change-backing-file support
> -iotests.verify_image_format(supported_fmts=['qcow2', 'qed'])
> -iotests.verify_platform(['linux'])
> -
> -
> # Returns a node for blockdev-add
> def node(node_name, path, backing=None, fmt=None, throttle=None):
> if fmt is None:
> @@ -160,4 +155,5 @@ def main():
> test_concurrent_finish(False)
>
> if __name__ == '__main__':
> - main()
> + # Need backing file and change-backing-file support
> + iotests.script_main(main, supported_fmts=['qcow2', 'qed'])
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] iotests: specify protocol support via initialization info
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 3/4] iotests: specify protocol support via initialization info John Snow
@ 2019-09-17 9:51 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 19+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-17 9:51 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz
12.09.2019 3:16, John Snow wrote:
> Switch from verify_protocols to any one of:
> iotests.main, iotests.script_main, iotests.script_initialize.
Actually, only script_initialize used in the patch.
Also, I think it all may be safely merged to 01
>
> Signed-off-by: John Snow <jsnow@redhat.com>
Still,
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/207 | 4 ++--
> tests/qemu-iotests/210 | 4 ++--
> tests/qemu-iotests/211 | 4 ++--
> tests/qemu-iotests/212 | 4 ++--
> tests/qemu-iotests/213 | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207
> index ab9e3b6747..35d98f2736 100755
> --- a/tests/qemu-iotests/207
> +++ b/tests/qemu-iotests/207
> @@ -24,8 +24,8 @@ import iotests
> import subprocess
> import re
>
> -iotests.script_initialize(supported_fmts=['raw'])
> -iotests.verify_protocol(supported=['ssh'])
> +iotests.script_initialize(supported_fmts=['raw'],
> + supported_protocols=['ssh'])
>
> def filter_hash(qmsg):
> def _filter(key, value):
> diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
> index 5a7013cd34..d9fe780c07 100755
> --- a/tests/qemu-iotests/210
> +++ b/tests/qemu-iotests/210
> @@ -23,8 +23,8 @@
> import iotests
> from iotests import imgfmt
>
> -iotests.script_initialize(supported_fmts=['luks'])
> -iotests.verify_protocol(supported=['file'])
> +iotests.script_initialize(supported_fmts=['luks'],
> + supported_protocols=['file'])
>
> def blockdev_create(vm, options):
> result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
> diff --git a/tests/qemu-iotests/211 b/tests/qemu-iotests/211
> index 4d6aac497f..155fac4e87 100755
> --- a/tests/qemu-iotests/211
> +++ b/tests/qemu-iotests/211
> @@ -23,8 +23,8 @@
> import iotests
> from iotests import imgfmt
>
> -iotests.script_initialize(supported_fmts=['vdi'])
> -iotests.verify_protocol(supported=['file'])
> +iotests.script_initialize(supported_fmts=['vdi'],
> + supported_protocols=['file'])
>
> def blockdev_create(vm, options):
> result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
> diff --git a/tests/qemu-iotests/212 b/tests/qemu-iotests/212
> index ec35bceb11..67e5a1dbb5 100755
> --- a/tests/qemu-iotests/212
> +++ b/tests/qemu-iotests/212
> @@ -23,8 +23,8 @@
> import iotests
> from iotests import imgfmt
>
> -iotests.script_initialize(supported_fmts=['parallels'])
> -iotests.verify_protocol(supported=['file'])
> +iotests.script_initialize(supported_fmts=['parallels'],
> + supported_protocols=['file'])
>
> def blockdev_create(vm, options):
> result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
> diff --git a/tests/qemu-iotests/213 b/tests/qemu-iotests/213
> index 3d2c024375..23f387ab63 100755
> --- a/tests/qemu-iotests/213
> +++ b/tests/qemu-iotests/213
> @@ -23,8 +23,8 @@
> import iotests
> from iotests import imgfmt
>
> -iotests.script_initialize(supported_fmts=['vhdx'])
> -iotests.verify_protocol(supported=['file'])
> +iotests.script_initialize(supported_fmts=['vhdx'],
> + supported_protocols=['file'])
>
> def blockdev_create(vm, options):
> result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/4] iotests: use python logging for iotests.log()
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 4/4] iotests: use python logging for iotests.log() John Snow
@ 2019-09-17 11:35 ` Vladimir Sementsov-Ogievskiy
2019-09-17 19:46 ` John Snow
0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-17 11:35 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz
12.09.2019 3:16, John Snow wrote:
> We can turn logging on/off globally instead of per-function.
>
> Remove use_log from run_job, and use python logging to turn on
> diffable output when we run through a script entry point.
>
> iotest 245 changes output order due to buffering reasons.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/030 | 4 +--
> tests/qemu-iotests/245 | 1 +
> tests/qemu-iotests/245.out | 24 +++++++++---------
> tests/qemu-iotests/iotests.py | 47 +++++++++++++++++++++--------------
> 4 files changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index f3766f2a81..01aa96ed16 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -411,8 +411,8 @@ class TestParallelOps(iotests.QMPTestCase):
> result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
> self.assert_qmp(result, 'return', {})
>
> - self.vm.run_job(job='drive0', auto_dismiss=True, use_log=False)
> - self.vm.run_job(job='node4', auto_dismiss=True, use_log=False)
> + self.vm.run_job(job='drive0', auto_dismiss=True)
> + self.vm.run_job(job='node4', auto_dismiss=True)
> self.assert_no_active_block_jobs()
>
> # Test a block-stream and a block-commit job in parallel
> diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
> index 41218d5f1d..eba2157cff 100644
> --- a/tests/qemu-iotests/245
> +++ b/tests/qemu-iotests/245
> @@ -1000,5 +1000,6 @@ class TestBlockdevReopen(iotests.QMPTestCase):
> self.reopen(opts, {'backing': 'hd2'})
>
> if __name__ == '__main__':
> + iotests.activate_logging()
or it may be a parameter for main().. But I'm OK with this too, of course.
> iotests.main(supported_fmts=["qcow2"],
> supported_protocols=["file"])
> diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out
> index a19de5214d..15c3630e92 100644
> --- a/tests/qemu-iotests/245.out
> +++ b/tests/qemu-iotests/245.out
> @@ -1,17 +1,17 @@
> +{"execute": "job-finalize", "arguments": {"id": "commit0"}}
> +{"return": {}}
> +{"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> +{"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> +{"execute": "job-finalize", "arguments": {"id": "stream0"}}
> +{"return": {}}
> +{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> +{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> +{"execute": "job-finalize", "arguments": {"id": "stream0"}}
> +{"return": {}}
> +{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> +{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> ..................
> ----------------------------------------------------------------------
> Ran 18 tests
>
> OK
> -{"execute": "job-finalize", "arguments": {"id": "commit0"}}
> -{"return": {}}
> -{"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> -{"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> -{"execute": "job-finalize", "arguments": {"id": "stream0"}}
> -{"return": {}}
> -{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> -{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> -{"execute": "job-finalize", "arguments": {"id": "stream0"}}
> -{"return": {}}
> -{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> -{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 92117a64bc..7e46fb2754 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -35,6 +35,13 @@ from collections import OrderedDict
> sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> from qemu import qtest
>
> +# Use this logger for logging messages directly from the iotests module
> +logger = logging.getLogger(__name__)
> +logger.addHandler(logging.NullHandler())
> +
> +# Use this logger for messages that ought to be used for diff output.
> +test_logger = logging.getLogger('.'.join((__name__, 'iotest')))
> +test_logger.addHandler(logging.NullHandler())
Still, I see one "print" call left in notrun(). Should it be changed to use logger or test_logger?
>
> # This will not work if arguments contain spaces but is necessary if we
> # want to support the override options that ./check supports.
> @@ -343,10 +350,10 @@ def log(msg, filters=[], indent=None):
> separators = (', ', ': ') if indent is None else (',', ': ')
> # Don't sort if it's already sorted
> do_sort = not isinstance(msg, OrderedDict)
> - print(json.dumps(msg, sort_keys=do_sort,
> - indent=indent, separators=separators))
> + test_logger.info(json.dumps(msg, sort_keys=do_sort,
> + indent=indent, separators=separators))
> else:
> - print(msg)
> + test_logger.info(msg)
>
> class Timeout:
> def __init__(self, seconds, errmsg = "Timeout"):
> @@ -559,7 +566,7 @@ class VM(qtest.QEMUQtestMachine):
>
> # Returns None on success, and an error string on failure
> def run_job(self, job, auto_finalize=True, auto_dismiss=False,
> - pre_finalize=None, cancel=False, use_log=True, wait=60.0):
> + pre_finalize=None, cancel=False, wait=60.0):
> """
> run_job moves a job from creation through to dismissal.
>
> @@ -572,7 +579,6 @@ class VM(qtest.QEMUQtestMachine):
> invoked prior to issuing job-finalize, if any.
> :param cancel: Bool. When true, cancels the job after the pre_finalize
> callback.
> - :param use_log: Bool. When false, does not log QMP messages.
> :param wait: Float. Timeout value specifying how long to wait for any
> event, in seconds. Defaults to 60.0.
> """
> @@ -590,8 +596,7 @@ class VM(qtest.QEMUQtestMachine):
> while True:
> ev = filter_qmp_event(self.events_wait(events))
> if ev['event'] != 'JOB_STATUS_CHANGE':
> - if use_log:
> - log(ev)
> + log(ev)
> continue
> status = ev['data']['status']
> if status == 'aborting':
> @@ -599,24 +604,16 @@ class VM(qtest.QEMUQtestMachine):
> for j in result['return']:
> if j['id'] == job:
> error = j['error']
> - if use_log:
> - log('Job failed: %s' % (j['error']))
> + log('Job failed: %s' % (j['error']))
> elif status == 'pending' and not auto_finalize:
> if pre_finalize:
> pre_finalize()
> - if cancel and use_log:
> + if cancel:
> self.qmp_log('job-cancel', id=job)
> - elif cancel:
> - self.qmp('job-cancel', id=job)
> - elif use_log:
> + else:
> self.qmp_log('job-finalize', id=job)
> - else:
> - self.qmp('job-finalize', id=job)
> elif status == 'concluded' and not auto_dismiss:
> - if use_log:
> - self.qmp_log('job-dismiss', id=job)
> - else:
> - self.qmp('job-dismiss', id=job)
> + self.qmp_log('job-dismiss', id=job)
> elif status == 'null':
> return error
I like this change. Interesting, should we just enable logging in qmp() and
drop qmp_log() function, to always manage log/don't-log behavior through
python logger? But this seems to be larger effort, including iotests output
changes, not for these series.
>
> @@ -949,6 +946,7 @@ def execute_setup_common(supported_fmts=[],
> if debug:
> sys.argv.remove('-d')
> logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
Hmm, interesting, am I right, that before this patch, this logging.basicConfig() line
is the only use of logging module, and therefore unused?
Does it affect all existent loggers (i.e. logger and test_logger)? Hmm, seems not:
"Does basic configuration for the logging system by creating a StreamHandler with a default Formatter and adding it to the root logger."
> + logger.debug("iotests debugging messages active")
>
> return debug
>
> @@ -961,14 +959,25 @@ def execute_test(test_function=None, *args, **kwargs):
> else:
> test_function()
>
> +def activate_logging():
> + """Activate iotests.log() output to stdout for script-style tests."""
> + handler = logging.StreamHandler(stream=sys.stdout)
> + formatter = logging.Formatter('%(message)s')
> + handler.setFormatter(formatter)
> + test_logger.addHandler(handler)
> + test_logger.setLevel(logging.INFO)
> + test_logger.propagate = False
> +
> # This is called from script-style iotests without a single point of entry
> def script_initialize(*args, **kwargs):
> """Initialize script-style tests without running any tests."""
> + activate_logging()
> execute_setup_common(*args, **kwargs)
>
> # This is called from script-style iotests with a single point of entry
> def script_main(test_function, *args, **kwargs):
> """Run script-style tests outside of the unittest framework"""
> + activate_logging()
> execute_test(test_function, *args, **kwargs)
>
> # This is called from unittest style iotests
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/4] iotests: use python logging for iotests.log()
2019-09-17 11:35 ` Vladimir Sementsov-Ogievskiy
@ 2019-09-17 19:46 ` John Snow
0 siblings, 0 replies; 19+ messages in thread
From: John Snow @ 2019-09-17 19:46 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: Kevin Wolf, qemu-block, Max Reitz
On 9/17/19 7:35 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.09.2019 3:16, John Snow wrote:
>> We can turn logging on/off globally instead of per-function.
>>
>> Remove use_log from run_job, and use python logging to turn on
>> diffable output when we run through a script entry point.
>>
>> iotest 245 changes output order due to buffering reasons.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> tests/qemu-iotests/030 | 4 +--
>> tests/qemu-iotests/245 | 1 +
>> tests/qemu-iotests/245.out | 24 +++++++++---------
>> tests/qemu-iotests/iotests.py | 47 +++++++++++++++++++++--------------
>> 4 files changed, 43 insertions(+), 33 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
>> index f3766f2a81..01aa96ed16 100755
>> --- a/tests/qemu-iotests/030
>> +++ b/tests/qemu-iotests/030
>> @@ -411,8 +411,8 @@ class TestParallelOps(iotests.QMPTestCase):
>> result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
>> self.assert_qmp(result, 'return', {})
>>
>> - self.vm.run_job(job='drive0', auto_dismiss=True, use_log=False)
>> - self.vm.run_job(job='node4', auto_dismiss=True, use_log=False)
>> + self.vm.run_job(job='drive0', auto_dismiss=True)
>> + self.vm.run_job(job='node4', auto_dismiss=True)
>> self.assert_no_active_block_jobs()
>>
>> # Test a block-stream and a block-commit job in parallel
>> diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
>> index 41218d5f1d..eba2157cff 100644
>> --- a/tests/qemu-iotests/245
>> +++ b/tests/qemu-iotests/245
>> @@ -1000,5 +1000,6 @@ class TestBlockdevReopen(iotests.QMPTestCase):
>> self.reopen(opts, {'backing': 'hd2'})
>>
>> if __name__ == '__main__':
>> + iotests.activate_logging()
>
> or it may be a parameter for main().. But I'm OK with this too, of course.
>
I think most of the unittest style tests don't need or want logged
output in this way, but we can always do it later if we want.
>> iotests.main(supported_fmts=["qcow2"],
>> supported_protocols=["file"])
>> diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out
>> index a19de5214d..15c3630e92 100644
>> --- a/tests/qemu-iotests/245.out
>> +++ b/tests/qemu-iotests/245.out
>> @@ -1,17 +1,17 @@
>> +{"execute": "job-finalize", "arguments": {"id": "commit0"}}
>> +{"return": {}}
>> +{"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> +{"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> +{"execute": "job-finalize", "arguments": {"id": "stream0"}}
>> +{"return": {}}
>> +{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> +{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> +{"execute": "job-finalize", "arguments": {"id": "stream0"}}
>> +{"return": {}}
>> +{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> +{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> ..................
>> ----------------------------------------------------------------------
>> Ran 18 tests
>>
>> OK
>> -{"execute": "job-finalize", "arguments": {"id": "commit0"}}
>> -{"return": {}}
>> -{"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> -{"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> -{"execute": "job-finalize", "arguments": {"id": "stream0"}}
>> -{"return": {}}
>> -{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> -{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> -{"execute": "job-finalize", "arguments": {"id": "stream0"}}
>> -{"return": {}}
>> -{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> -{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 92117a64bc..7e46fb2754 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -35,6 +35,13 @@ from collections import OrderedDict
>> sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>> from qemu import qtest
>>
>> +# Use this logger for logging messages directly from the iotests module
>> +logger = logging.getLogger(__name__)
>> +logger.addHandler(logging.NullHandler())
>> +
>> +# Use this logger for messages that ought to be used for diff output.
>> +test_logger = logging.getLogger('.'.join((__name__, 'iotest')))
>> +test_logger.addHandler(logging.NullHandler())
>
> Still, I see one "print" call left in notrun(). Should it be changed to use logger or test_logger?
>
Good question...
I guess notrun() writes to the .notrun file, which the main test runner
uses to know not to check the output in stdout as a diff against the
reference output.
The 'check' runner then cats the .notrun file to stdout for the user to see.
the print() is discarded (unless we were in debug mode, and then it was
printed to the screen.)
It seems like a control message for the user, so my guess is that it
ought to be sent to the module-level logger and printed to stderr, actually.
maybe warning-level?
>>
>> # This will not work if arguments contain spaces but is necessary if we
>> # want to support the override options that ./check supports.
>> @@ -343,10 +350,10 @@ def log(msg, filters=[], indent=None):
>> separators = (', ', ': ') if indent is None else (',', ': ')
>> # Don't sort if it's already sorted
>> do_sort = not isinstance(msg, OrderedDict)
>> - print(json.dumps(msg, sort_keys=do_sort,
>> - indent=indent, separators=separators))
>> + test_logger.info(json.dumps(msg, sort_keys=do_sort,
>> + indent=indent, separators=separators))
>> else:
>> - print(msg)
>> + test_logger.info(msg)
>>
>> class Timeout:
>> def __init__(self, seconds, errmsg = "Timeout"):
>> @@ -559,7 +566,7 @@ class VM(qtest.QEMUQtestMachine):
>>
>> # Returns None on success, and an error string on failure
>> def run_job(self, job, auto_finalize=True, auto_dismiss=False,
>> - pre_finalize=None, cancel=False, use_log=True, wait=60.0):
>> + pre_finalize=None, cancel=False, wait=60.0):
>> """
>> run_job moves a job from creation through to dismissal.
>>
>> @@ -572,7 +579,6 @@ class VM(qtest.QEMUQtestMachine):
>> invoked prior to issuing job-finalize, if any.
>> :param cancel: Bool. When true, cancels the job after the pre_finalize
>> callback.
>> - :param use_log: Bool. When false, does not log QMP messages.
>> :param wait: Float. Timeout value specifying how long to wait for any
>> event, in seconds. Defaults to 60.0.
>> """
>> @@ -590,8 +596,7 @@ class VM(qtest.QEMUQtestMachine):
>> while True:
>> ev = filter_qmp_event(self.events_wait(events))
>> if ev['event'] != 'JOB_STATUS_CHANGE':
>> - if use_log:
>> - log(ev)
>> + log(ev)
>> continue
>> status = ev['data']['status']
>> if status == 'aborting':
>> @@ -599,24 +604,16 @@ class VM(qtest.QEMUQtestMachine):
>> for j in result['return']:
>> if j['id'] == job:
>> error = j['error']
>> - if use_log:
>> - log('Job failed: %s' % (j['error']))
>> + log('Job failed: %s' % (j['error']))
>> elif status == 'pending' and not auto_finalize:
>> if pre_finalize:
>> pre_finalize()
>> - if cancel and use_log:
>> + if cancel:
>> self.qmp_log('job-cancel', id=job)
>> - elif cancel:
>> - self.qmp('job-cancel', id=job)
>> - elif use_log:
>> + else:
>> self.qmp_log('job-finalize', id=job)
>> - else:
>> - self.qmp('job-finalize', id=job)
>> elif status == 'concluded' and not auto_dismiss:
>> - if use_log:
>> - self.qmp_log('job-dismiss', id=job)
>> - else:
>> - self.qmp('job-dismiss', id=job)
>> + self.qmp_log('job-dismiss', id=job)
>> elif status == 'null':
>> return error
>
> I like this change. Interesting, should we just enable logging in qmp() and
> drop qmp_log() function, to always manage log/don't-log behavior through
> python logger? But this seems to be larger effort, including iotests output
> changes, not for these series.
>
Yes, that's probably a good idea. One function that always logs, and
where that gets routed to and what happens to it is configured elsewhere.
We can work towards a unified logging story over time.
>>
>> @@ -949,6 +946,7 @@ def execute_setup_common(supported_fmts=[],
>> if debug:
>> sys.argv.remove('-d')
>> logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
>
> Hmm, interesting, am I right, that before this patch, this logging.basicConfig() line
> is the only use of logging module, and therefore unused?
>
Oh, I see -- no, actually; this line turns on all of the debugging
statements in included modules too.
basicConfig, when executed *before any other logging statement*, will
create a defaultly-configured handler and logger at the root of the
hierarchy.
When you call basicConfig, you create and enable the root logger. Later,
when qmp.py calls logging.getLogger('QMP'), it attaches itself under the
root logger.
Then, messages will propagate from the QMP logger to the root logger.
So by enabling it here, you actually enable logging everywhere.
> Does it affect all existent loggers (i.e. logger and test_logger)? Hmm, seems not:
> "Does basic configuration for the logging system by creating a StreamHandler with a default Formatter and adding it to the root logger."
>
You're half-right.
If other loggers *already exist*, basicConfig does nothing. If they
don't, it creates a default handler.
By effect, that default handler allows other loggers to propagate their
messages up to the root and in effect, it enables those loggers.
However, all loggers exist in one tree in a python process; so we can
enable and disable them post-hoc to our liking.
>> + logger.debug("iotests debugging messages active")
>>
>> return debug
>>
>> @@ -961,14 +959,25 @@ def execute_test(test_function=None, *args, **kwargs):
>> else:
>> test_function()
>>
>> +def activate_logging():
>> + """Activate iotests.log() output to stdout for script-style tests."""
>> + handler = logging.StreamHandler(stream=sys.stdout)
>> + formatter = logging.Formatter('%(message)s')
>> + handler.setFormatter(formatter)
>> + test_logger.addHandler(handler)
>> + test_logger.setLevel(logging.INFO)
>> + test_logger.propagate = False
>> +
Which is what I am doing here.
I create a formatter that does nothing but print the message as-is, and
a handler that routes the message to stdout.
Then I add the formatter to the handler, and the handler to the logger.
I turn on event processing at the INFO level and then turn off propagation.
>> # This is called from script-style iotests without a single point of entry
>> def script_initialize(*args, **kwargs):
>> """Initialize script-style tests without running any tests."""
>> + activate_logging()
>> execute_setup_common(*args, **kwargs)
>>
>> # This is called from script-style iotests with a single point of entry
>> def script_main(test_function, *args, **kwargs):
>> """Run script-style tests outside of the unittest framework"""
>> + activate_logging()
>> execute_test(test_function, *args, **kwargs)
>>
>> # This is called from unittest style iotests
>>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-16 14:56 ` Vladimir Sementsov-Ogievskiy
2019-09-16 16:32 ` John Snow
@ 2019-09-17 22:29 ` John Snow
2019-09-18 10:30 ` Vladimir Sementsov-Ogievskiy
2019-09-18 13:05 ` Thomas Huth
1 sibling, 2 replies; 19+ messages in thread
From: John Snow @ 2019-09-17 22:29 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: Kevin Wolf, qemu-block, Max Reitz
On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.09.2019 3:16, John Snow wrote:
>> Like script_main, but doesn't require a single point of entry.
>> Replace all existing initialization sections with this drop-in replacement.
>>
>> This brings debug support to all existing script-style iotests.
>>
>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>
> But after this patch all test which didn't check os start to check linux
> (as it's default).. So all tests which worked on other platforms will now
> be skipped on these other platforms?
>
> Finally do we support something except linux for iotests?
> for bash tests _supported_os also used only with "Linux" in 87 tests..
>
> May be we'd better drop both _supported_os and supported_oses alltogether,
> and don't care?
>
> Anyway, if we support only linux, any reason to skip almost all tests,
> if someone tries to run test on other platform? Let him run what he wants.
>
Currently, the following tests are python:
030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147
148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209
210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246
248 254 255 256 257 258 262 266
And as it stands, none of the script-style python tests we've selected
to run in `make check` fail on the FreeBSD platform.
So as an experiment, I lifted the restriction on python tests. I kept
running ./check until I found some invocation that they didn't skip.
Failures: 045 147 149 169 194 199 211
Not too bad...
045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist
149: Wants to use 'sudo', but our freebsd image doesn't have that.
194: +OSError: AF_UNIX path too long
211:
-[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true,
"offset": 1024},
-{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data":
true, "offset": 4096}]
+[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data":
true, "offset": 1024},
+{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data":
true, "offset": 32768}]
149 can probably be fixed, and 194 and 211 I have fail in similar ways
on my own Linux machine, so that's probably not BSD's fault.
Interestingly, 169 and 199, bitmap migration related tests, cause a
SIGSEGV in QEMU ...
169:
+EEEE....EEEE........
+WARNING:qemu.machine:qemu received signal 6:
/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
-chardev
socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock
-mon chardev=mon,mode=control -display none -vga none -qtest
unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock
-machine accel=qtest -nodefaults -display none -machine accel=qtest
-incoming defer -drive
if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback
The common thread in 169 is the +migbitmap trait, which ... makes me a
little nervous, of course!
199:
+WARNING:qemu.machine:qemu received signal 6:
/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
-chardev
socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock
-mon chardev=mon,mode=control -display none -vga none -qtest
unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock
-machine accel=qtest -nodefaults -display none -machine accel=qtest
-drive
if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none
-incoming exec: cat
'/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo'
+E
Vladimir, I was able to provoke this error by editing
./tests/vm/Makefile.include and removing the --snapshot invocation, then
using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd`
to open up a shell.
Then the tricks are the usual ones; navigate to iotests directory,
./check -v -qcow2 169, etc. You'll need to create a build that allows
Python tests to run; do it before you run the snapshot-less build.
aaand lastly, running `make check` doesn't happen to call any of the
tests that appear broken on FreeBSD right now, so I'm just going to go
ahead and say we can open Pandora's box and make the default python test
behavior to run on any OS, and start re-blacklisting the edge-cases as
we find them.
As far as iotests go, there are a few new broken ones that surface, but
they won't gate Peter Maydell's build process because they don't get run
by 'make check'.
I think it's a safe move to make.
--js
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-17 22:29 ` John Snow
@ 2019-09-18 10:30 ` Vladimir Sementsov-Ogievskiy
2019-09-18 16:44 ` Vladimir Sementsov-Ogievskiy
2019-09-18 13:05 ` Thomas Huth
1 sibling, 1 reply; 19+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-18 10:30 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz
18.09.2019 1:29, John Snow wrote:
>
>
> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 12.09.2019 3:16, John Snow wrote:
>>> Like script_main, but doesn't require a single point of entry.
>>> Replace all existing initialization sections with this drop-in replacement.
>>>
>>> This brings debug support to all existing script-style iotests.
>>>
>>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>>
>> But after this patch all test which didn't check os start to check linux
>> (as it's default).. So all tests which worked on other platforms will now
>> be skipped on these other platforms?
>>
>> Finally do we support something except linux for iotests?
>> for bash tests _supported_os also used only with "Linux" in 87 tests..
>>
>> May be we'd better drop both _supported_os and supported_oses alltogether,
>> and don't care?
>>
>> Anyway, if we support only linux, any reason to skip almost all tests,
>> if someone tries to run test on other platform? Let him run what he wants.
>>
>
> Currently, the following tests are python:
>
> 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147
> 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209
> 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246
> 248 254 255 256 257 258 262 266
>
> And as it stands, none of the script-style python tests we've selected
> to run in `make check` fail on the FreeBSD platform.
>
> So as an experiment, I lifted the restriction on python tests. I kept
> running ./check until I found some invocation that they didn't skip.
>
> Failures: 045 147 149 169 194 199 211
>
> Not too bad...
>
> 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist
> 149: Wants to use 'sudo', but our freebsd image doesn't have that.
> 194: +OSError: AF_UNIX path too long
> 211:
> -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true,
> "offset": 1024},
> -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data":
> true, "offset": 4096}]
> +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data":
> true, "offset": 1024},
> +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data":
> true, "offset": 32768}]
>
>
> 149 can probably be fixed, and 194 and 211 I have fail in similar ways
> on my own Linux machine, so that's probably not BSD's fault.
>
> Interestingly, 169 and 199, bitmap migration related tests, cause a
> SIGSEGV in QEMU ...
>
>
> 169:
> +EEEE....EEEE........
> +WARNING:qemu.machine:qemu received signal 6:
> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
> -chardev
> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock
> -mon chardev=mon,mode=control -display none -vga none -qtest
> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock
> -machine accel=qtest -nodefaults -display none -machine accel=qtest
> -incoming defer -drive
> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback
>
> The common thread in 169 is the +migbitmap trait, which ... makes me a
> little nervous, of course!
>
>
> 199:
> +WARNING:qemu.machine:qemu received signal 6:
> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
> -chardev
> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock
> -mon chardev=mon,mode=control -display none -vga none -qtest
> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock
> -machine accel=qtest -nodefaults -display none -machine accel=qtest
> -drive
> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none
> -incoming exec: cat
> '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo'
> +E
>
>
> Vladimir, I was able to provoke this error by editing
> ./tests/vm/Makefile.include and removing the --snapshot invocation, then
> using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd`
> to open up a shell.
>
> Then the tricks are the usual ones; navigate to iotests directory,
> ./check -v -qcow2 169, etc. You'll need to create a build that allows
> Python tests to run; do it before you run the snapshot-less build.
>
>
Interesting, I'll try to reproduce.
>
>
> aaand lastly, running `make check` doesn't happen to call any of the
> tests that appear broken on FreeBSD right now, so I'm just going to go
> ahead and say we can open Pandora's box and make the default python test
> behavior to run on any OS, and start re-blacklisting the edge-cases as
> we find them.
>
> As far as iotests go, there are a few new broken ones that surface, but
> they won't gate Peter Maydell's build process because they don't get run
> by 'make check'.
>
> I think it's a safe move to make.
>
> --js
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-17 22:29 ` John Snow
2019-09-18 10:30 ` Vladimir Sementsov-Ogievskiy
@ 2019-09-18 13:05 ` Thomas Huth
2019-09-18 18:41 ` John Snow
1 sibling, 1 reply; 19+ messages in thread
From: Thomas Huth @ 2019-09-18 13:05 UTC (permalink / raw)
To: John Snow, Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: Kevin Wolf, qemu-block, Max Reitz
On 18/09/2019 00.29, John Snow wrote:
>
>
> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
[...]
>> Finally do we support something except linux for iotests?
>> for bash tests _supported_os also used only with "Linux" in 87 tests..
The iotests in the "auto" group are supposed to work on other OSes
beside Linux, too, since they are run automatically during "make check"
now. You can use github with cirrus-ci to check FreeBSD and macOS, see
e.g.: https://cirrus-ci.com/build/5114679677943808
Travis has support for macOS, too.
And to test them on OpenBSD (or FreeBSD), you can use the VM tests, e.g.
something like this:
make vm-build-openbsd J=8 BUILD_TARGET=check-block \
EXTRA_CONFIGURE_OPTS=--target-list=x86_64-softmmu
> aaand lastly, running `make check` doesn't happen to call any of the
> tests that appear broken on FreeBSD right now, so I'm just going to go
> ahead and say we can open Pandora's box and make the default python test
> behavior to run on any OS, and start re-blacklisting the edge-cases as
> we find them.
Sounds good. If it breaks on FreeBSD or macOS, we'll find out with
cirrus-ci or Travis pretty soon.
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-18 10:30 ` Vladimir Sementsov-Ogievskiy
@ 2019-09-18 16:44 ` Vladimir Sementsov-Ogievskiy
2019-09-18 17:00 ` John Snow
0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-18 16:44 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: Kevin Wolf, qemu-block, Max Reitz
18.09.2019 13:30, Vladimir Sementsov-Ogievskiy wrote:
> 18.09.2019 1:29, John Snow wrote:
>>
>>
>> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 12.09.2019 3:16, John Snow wrote:
>>>> Like script_main, but doesn't require a single point of entry.
>>>> Replace all existing initialization sections with this drop-in replacement.
>>>>
>>>> This brings debug support to all existing script-style iotests.
>>>>
>>>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>>>
>>> But after this patch all test which didn't check os start to check linux
>>> (as it's default).. So all tests which worked on other platforms will now
>>> be skipped on these other platforms?
>>>
>>> Finally do we support something except linux for iotests?
>>> for bash tests _supported_os also used only with "Linux" in 87 tests..
>>>
>>> May be we'd better drop both _supported_os and supported_oses alltogether,
>>> and don't care?
>>>
>>> Anyway, if we support only linux, any reason to skip almost all tests,
>>> if someone tries to run test on other platform? Let him run what he wants.
>>>
>>
>> Currently, the following tests are python:
>>
>> 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147
>> 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209
>> 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246
>> 248 254 255 256 257 258 262 266
>>
>> And as it stands, none of the script-style python tests we've selected
>> to run in `make check` fail on the FreeBSD platform.
>>
>> So as an experiment, I lifted the restriction on python tests. I kept
>> running ./check until I found some invocation that they didn't skip.
>>
>> Failures: 045 147 149 169 194 199 211
>>
>> Not too bad...
>>
>> 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist
>> 149: Wants to use 'sudo', but our freebsd image doesn't have that.
>> 194: +OSError: AF_UNIX path too long
>> 211:
>> -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true,
>> "offset": 1024},
>> -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data":
>> true, "offset": 4096}]
>> +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data":
>> true, "offset": 1024},
>> +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data":
>> true, "offset": 32768}]
>>
>>
>> 149 can probably be fixed, and 194 and 211 I have fail in similar ways
>> on my own Linux machine, so that's probably not BSD's fault.
>>
>> Interestingly, 169 and 199, bitmap migration related tests, cause a
>> SIGSEGV in QEMU ...
>>
>>
>> 169:
>> +EEEE....EEEE........
>> +WARNING:qemu.machine:qemu received signal 6:
>> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
>> -chardev
>> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock
>> -mon chardev=mon,mode=control -display none -vga none -qtest
>> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock
>> -machine accel=qtest -nodefaults -display none -machine accel=qtest
>> -incoming defer -drive
>> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback
>>
>> The common thread in 169 is the +migbitmap trait, which ... makes me a
>> little nervous, of course!
>>
>>
>> 199:
>> +WARNING:qemu.machine:qemu received signal 6:
>> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
>> -chardev
>> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock
>> -mon chardev=mon,mode=control -display none -vga none -qtest
>> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock
>> -machine accel=qtest -nodefaults -display none -machine accel=qtest
>> -drive
>> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none
>> -incoming exec: cat
>> '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo'
>> +E
>>
>>
>> Vladimir, I was able to provoke this error by editing
>> ./tests/vm/Makefile.include and removing the --snapshot invocation, then
>> using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd`
>> to open up a shell.
>>
>> Then the tricks are the usual ones; navigate to iotests directory,
>> ./check -v -qcow2 169, etc. You'll need to create a build that allows
>> Python tests to run; do it before you run the snapshot-less build.
>>
>>
>
> Interesting, I'll try to reproduce.
Could you provide more detailed steps?
# make vm-build-freebsd
VM-IMAGE freebsd
### Downloading install iso ...
### Preparing iso and disk image ...
/root/.cache/qemu-vm/images/freebsd.img.install.iso.xz (1/1)
100 % 595,0 MiB / 851,1 MiB = 0,699 153 MiB/s 0:05
Failed to prepare guest environment
Traceback (most recent call last):
File "/work/src/qemu/master/tests/vm/basevm.py", line 353, in main
return vm.build_image(args.image)
File "/work/src/qemu/master/tests/vm/freebsd", line 86, in build_image
img_tmp, self.size])
File "/usr/lib64/python3.7/subprocess.py", line 342, in check_call
retcode = call(*popenargs, **kwargs)
File "/usr/lib64/python3.7/subprocess.py", line 323, in call
with Popen(*popenargs, **kwargs) as p:
File "/usr/lib64/python3.7/subprocess.py", line 775, in __init__
restore_signals, start_new_session)
File "/usr/lib64/python3.7/subprocess.py", line 1522, in _execute_child
raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'qemu-img': 'qemu-img'
make: *** [/work/src/qemu/master/tests/vm/Makefile.include:47: /root/.cache/qemu-vm/images/freebsd.img] Error 2
# ls qemu-img
qemu-img
What it wants? I've never done such cross-builds before..
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-18 16:44 ` Vladimir Sementsov-Ogievskiy
@ 2019-09-18 17:00 ` John Snow
0 siblings, 0 replies; 19+ messages in thread
From: John Snow @ 2019-09-18 17:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: Kevin Wolf, qemu-block, Max Reitz
On 9/18/19 12:44 PM, Vladimir Sementsov-Ogievskiy wrote:
> 18.09.2019 13:30, Vladimir Sementsov-Ogievskiy wrote:
>> 18.09.2019 1:29, John Snow wrote:
>>>
>>>
>>> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>> 12.09.2019 3:16, John Snow wrote:
>>>>> Like script_main, but doesn't require a single point of entry.
>>>>> Replace all existing initialization sections with this drop-in replacement.
>>>>>
>>>>> This brings debug support to all existing script-style iotests.
>>>>>
>>>>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>>>>
>>>> But after this patch all test which didn't check os start to check linux
>>>> (as it's default).. So all tests which worked on other platforms will now
>>>> be skipped on these other platforms?
>>>>
>>>> Finally do we support something except linux for iotests?
>>>> for bash tests _supported_os also used only with "Linux" in 87 tests..
>>>>
>>>> May be we'd better drop both _supported_os and supported_oses alltogether,
>>>> and don't care?
>>>>
>>>> Anyway, if we support only linux, any reason to skip almost all tests,
>>>> if someone tries to run test on other platform? Let him run what he wants.
>>>>
>>>
>>> Currently, the following tests are python:
>>>
>>> 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147
>>> 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209
>>> 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246
>>> 248 254 255 256 257 258 262 266
>>>
>>> And as it stands, none of the script-style python tests we've selected
>>> to run in `make check` fail on the FreeBSD platform.
>>>
>>> So as an experiment, I lifted the restriction on python tests. I kept
>>> running ./check until I found some invocation that they didn't skip.
>>>
>>> Failures: 045 147 149 169 194 199 211
>>>
>>> Not too bad...
>>>
>>> 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist
>>> 149: Wants to use 'sudo', but our freebsd image doesn't have that.
>>> 194: +OSError: AF_UNIX path too long
>>> 211:
>>> -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true,
>>> "offset": 1024},
>>> -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data":
>>> true, "offset": 4096}]
>>> +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data":
>>> true, "offset": 1024},
>>> +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data":
>>> true, "offset": 32768}]
>>>
>>>
>>> 149 can probably be fixed, and 194 and 211 I have fail in similar ways
>>> on my own Linux machine, so that's probably not BSD's fault.
>>>
>>> Interestingly, 169 and 199, bitmap migration related tests, cause a
>>> SIGSEGV in QEMU ...
>>>
>>>
>>> 169:
>>> +EEEE....EEEE........
>>> +WARNING:qemu.machine:qemu received signal 6:
>>> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
>>> -chardev
>>> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock
>>> -mon chardev=mon,mode=control -display none -vga none -qtest
>>> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock
>>> -machine accel=qtest -nodefaults -display none -machine accel=qtest
>>> -incoming defer -drive
>>> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback
>>>
>>> The common thread in 169 is the +migbitmap trait, which ... makes me a
>>> little nervous, of course!
>>>
>>>
>>> 199:
>>> +WARNING:qemu.machine:qemu received signal 6:
>>> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
>>> -chardev
>>> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock
>>> -mon chardev=mon,mode=control -display none -vga none -qtest
>>> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock
>>> -machine accel=qtest -nodefaults -display none -machine accel=qtest
>>> -drive
>>> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none
>>> -incoming exec: cat
>>> '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo'
>>> +E
>>>
>>>
>>> Vladimir, I was able to provoke this error by editing
>>> ./tests/vm/Makefile.include and removing the --snapshot invocation, then
>>> using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd`
>>> to open up a shell.
>>>
>>> Then the tricks are the usual ones; navigate to iotests directory,
>>> ./check -v -qcow2 169, etc. You'll need to create a build that allows
>>> Python tests to run; do it before you run the snapshot-less build.
>>>
>>>
>>
>> Interesting, I'll try to reproduce.
>
> Could you provide more detailed steps?
>
> # make vm-build-freebsd
> VM-IMAGE freebsd
> ### Downloading install iso ...
> ### Preparing iso and disk image ...
> /root/.cache/qemu-vm/images/freebsd.img.install.iso.xz (1/1)
> 100 % 595,0 MiB / 851,1 MiB = 0,699 153 MiB/s 0:05
> Failed to prepare guest environment
> Traceback (most recent call last):
> File "/work/src/qemu/master/tests/vm/basevm.py", line 353, in main
> return vm.build_image(args.image)
> File "/work/src/qemu/master/tests/vm/freebsd", line 86, in build_image
> img_tmp, self.size])
> File "/usr/lib64/python3.7/subprocess.py", line 342, in check_call
> retcode = call(*popenargs, **kwargs)
> File "/usr/lib64/python3.7/subprocess.py", line 323, in call
> with Popen(*popenargs, **kwargs) as p:
> File "/usr/lib64/python3.7/subprocess.py", line 775, in __init__
> restore_signals, start_new_session)
> File "/usr/lib64/python3.7/subprocess.py", line 1522, in _execute_child
> raise child_exception_type(errno_num, err_msg, err_filename)
> FileNotFoundError: [Errno 2] No such file or directory: 'qemu-img': 'qemu-img'
> make: *** [/work/src/qemu/master/tests/vm/Makefile.include:47: /root/.cache/qemu-vm/images/freebsd.img] Error 2
> # ls qemu-img
> qemu-img
>
> What it wants? I've never done such cross-builds before..
>
uhhh, hm...
No, I really just typed what you had done -- `make vm-build-freebsd` on
my Fedora 30 host. I do happen to have qemu-img installed system-wide,
maybe it's a requirement.
I removed the '--snapshot' argument from the Makefile in that folder so
that once the build finished I could SSH in to the image and poke around
by running iotests manually.
I wouldn't worry about it right now, exactly -- it might be a problem
with the migration specification for "exec cat" that is causing an
issue, if I had to guess. I can try to debug it later, but I won't be
able to get to it too soon.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize
2019-09-18 13:05 ` Thomas Huth
@ 2019-09-18 18:41 ` John Snow
0 siblings, 0 replies; 19+ messages in thread
From: John Snow @ 2019-09-18 18:41 UTC (permalink / raw)
To: Thomas Huth, Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: Kevin Wolf, qemu-block, Max Reitz
On 9/18/19 9:05 AM, Thomas Huth wrote:
> On 18/09/2019 00.29, John Snow wrote:
>>
>>
>> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> [...]
>>> Finally do we support something except linux for iotests?
>>> for bash tests _supported_os also used only with "Linux" in 87 tests..
>
> The iotests in the "auto" group are supposed to work on other OSes
> beside Linux, too, since they are run automatically during "make check"
> now. You can use github with cirrus-ci to check FreeBSD and macOS, see
> e.g.: https://cirrus-ci.com/build/5114679677943808
>
> Travis has support for macOS, too.
>
> And to test them on OpenBSD (or FreeBSD), you can use the VM tests, e.g.
> something like this:
>
> make vm-build-openbsd J=8 BUILD_TARGET=check-block \
> EXTRA_CONFIGURE_OPTS=--target-list=x86_64-softmmu
>
>> aaand lastly, running `make check` doesn't happen to call any of the
>> tests that appear broken on FreeBSD right now, so I'm just going to go
>> ahead and say we can open Pandora's box and make the default python test
>> behavior to run on any OS, and start re-blacklisting the edge-cases as
>> we find them.
>
> Sounds good. If it breaks on FreeBSD or macOS, we'll find out with
> cirrus-ci or Travis pretty soon.
>
Yeah. It's annoying, but genuinely the quickest way to figure it out.
Keep an eye on the v5 of this series for fallout.
--js
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-09-18 18:43 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-12 0:16 [Qemu-devel] [PATCH v4 0/4] iotests: use python logging John Snow
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize John Snow
2019-09-16 14:56 ` Vladimir Sementsov-Ogievskiy
2019-09-16 16:32 ` John Snow
2019-09-16 16:39 ` Vladimir Sementsov-Ogievskiy
2019-09-16 17:13 ` John Snow
2019-09-17 22:29 ` John Snow
2019-09-18 10:30 ` Vladimir Sementsov-Ogievskiy
2019-09-18 16:44 ` Vladimir Sementsov-Ogievskiy
2019-09-18 17:00 ` John Snow
2019-09-18 13:05 ` Thomas Huth
2019-09-18 18:41 ` John Snow
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 2/4] iotest 258: use script_main John Snow
2019-09-17 9:36 ` Vladimir Sementsov-Ogievskiy
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 3/4] iotests: specify protocol support via initialization info John Snow
2019-09-17 9:51 ` Vladimir Sementsov-Ogievskiy
2019-09-12 0:16 ` [Qemu-devel] [PATCH v4 4/4] iotests: use python logging for iotests.log() John Snow
2019-09-17 11:35 ` Vladimir Sementsov-Ogievskiy
2019-09-17 19:46 ` John Snow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).