Fix / skip recent iotests with LUKS driver

PATCH In review Daniel P. Berrange 6 participants 12 messages Sep 1, 2017 – Sep 5, 2017

lore

Share & tools

lore

Daniel P. Berrange

author

2017-09-01 10:55 UTC #0 lore raw

cover Fix / skip recent iotests with LUKS driver

Two recently added iotests are currently breaking 'check -luks', fix the easy one and disable the hard one.

Daniel P. Berrange (2):
iotests: rewrite 192 to use _launch_qemu to fix LUKS support
iotests: blacklist 194 with the luks driver

 tests/qemu-iotests/192        | 23 ++++++++++++++++-------
 tests/qemu-iotests/194        |  1 +
 tests/qemu-iotests/iotests.py |  4 +++-
 3 files changed, 20 insertions(+), 8 deletions(-)

Signature · 1 line

2.13.5

Daniel P. Berrange

author

2017-09-01 10:55 UTC #1 lore raw

PATCH 1/2 iotests: rewrite 192 to use _launch_qemu to fix LUKS support

The LUKS driver requires extra args to QEMU to setup passwords. The _launch_qemu function takes care of this, so convert the test to use this function and use correct -drive syntax

Signed-off-by Daniel P. Berrange <[email protected]>

---
 tests/qemu-iotests/192 | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

tests/qemu-iotests/192 +16 -7

index b50a2c0c8e..595f0d786a 100755
@@ -37,6 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
+. ./common.qemu
 
 _supported_fmt generic
 _supported_proto file

@@ -49,13 +50,21 @@ fi
 size=64M
 _make_test_img $size
 
-{
-echo "nbd_server_start unix:$TEST_DIR/nbd"
-echo "nbd_server_add -w drive0"
-echo "q"
-} | $QEMU -nodefaults -display none -monitor stdio \
-    -drive format=$IMGFMT,file=$TEST_IMG,if=ide,id=drive0 \
-    -incoming defer 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp
+if test "$IMGOPTSSYNTAX" = "true"
+then
+    DRIVE_ARG=if=ide,id=drive0,$TEST_IMG
+else
+    DRIVE_ARG=if=ide,id=drive0,format=$IMGFMT,file=$TEST_IMG
+fi
+
+qemu_comm_method="monitor"
+_launch_qemu -drive $DRIVE_ARG -incoming defer
+h=$QEMU_HANDLE
+QEMU_COMM_TIMEOUT=1
+
+_send_qemu_cmd $h "nbd_server_start unix:$TEST_DIR/nbd" "(qemu)"
+_send_qemu_cmd $h "nbd_server_add -w drive0" "(qemu)"
+_send_qemu_cmd $h "q" "(qemu)"
 
 # success, all done
 echo "*** done"

Signature · 1 line

2.13.5

Eric Blake

Reviewed-by on v1 1/2

2017-09-01 14:23 UTC #2 lore raw

Re: [Qemu-devel] [PATCH 1/2] iotests: rewrite 192 to use _launch_qemu to fix LUKS support

Quoted 11 lines · Daniel P. Berrange

The LUKS driver requires extra args to QEMU to setup passwords. The _launch_qemu function takes care of this, so convert the test to use this function and use correct -drive syntax
Signed-off-by Daniel P. Berrange <[email protected]>
---
 tests/qemu-iotests/192 | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

tests/qemu-iotests/192 fi · line 58

Quoted 7 lines

tests/qemu-iotests/192 +0 -7

-{
-echo "nbd_server_start unix:$TEST_DIR/nbd"
-echo "nbd_server_add -w drive0"
-echo "q"
-} | $QEMU -nodefaults -display none -monitor stdio \
-    -drive format=$IMGFMT,file=$TEST_IMG,if=ide,id=drive0 \
-    -incoming defer 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp

Can this test use QMP instead of HMP? But that's an independent question.

Reviewed-by Eric Blake <[email protected]>

Signature · 3 lines

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Kevin Wolf

2017-09-05 09:30 UTC #3 lore raw

Re: [Qemu-devel] [PATCH 1/2] iotests: rewrite 192 to use _launch_qemu to fix LUKS support

Am 01.09.2017 um 16:22 hat Eric Blake geschrieben:

Quoted 12 lines · Daniel P. Berrange

Quoted 11 lines
The LUKS driver requires extra args to QEMU to setup passwords. The _launch_qemu function takes care of this, so convert the test to use this function and use correct -drive syntax
Signed-off-by Daniel P. Berrange <[email protected]>
---
 tests/qemu-iotests/192 | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

Quoted 9 lines

tests/qemu-iotests/192 fi · line 58
Quoted 7 lines
tests/qemu-iotests/192 +0 -7
-{
-echo "nbd_server_start unix:$TEST_DIR/nbd"
-echo "nbd_server_add -w drive0"
-echo "q"
-} | $QEMU -nodefaults -display none -monitor stdio \
-    -drive format=$IMGFMT,file=$TEST_IMG,if=ide,id=drive0 \
-    -incoming defer 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp
Can this test use QMP instead of HMP? But that's an independent question.

As far as I am concerned, there is nothing wrong with having HMP tests. HMP commands already call QMP internally (except for the generic QMP code path below the command handlers), so using HMP actually increases the test coverage in most cases. We should try to have a good mix of test cases using QMP and test cases using HMP.

Kevin

Daniel P. Berrange

author

2017-09-01 10:55 UTC #4 lore raw

PATCH 2/2 iotests: blacklist 194 with the luks driver

In reply to Daniel P. Berrange

The 194 test has alot of code that assumes a simple image file. Rewriting this to work with luks is possible, but non-trivial, so blacklist the luks format for now.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 tests/qemu-iotests/194        | 1 +
 tests/qemu-iotests/iotests.py | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

tests/qemu-iotests/194 +1 -0

index a3e3bad664..bfe1859ecf 100755
@@ -23,6 +23,7 @@ import os
 import atexit
 import iotests
 
+iotests.verify_image_format(unsupported_fmts=['luks'])
 iotests.verify_platform(['linux'])
 
 img_size = '1G'

tests/qemu-iotests/iotests.py +3 -1

index 7233983f3c..3e7304b678 100644
@@ -395,9 +395,11 @@ def notrun(reason):
     print '%s not run: %s' % (seq, reason)
     sys.exit(0)
 
-def verify_image_format(supported_fmts=[]):
+def verify_image_format(supported_fmts=[], unsupported_fmts=[]):
     if supported_fmts and (imgfmt not in supported_fmts):
         notrun('not suitable for this image format: %s' % imgfmt)
+    if unsupported_fmts and (imgfmt in unsupported_fmts):
+        notrun('not suitable for this image format: %s' % imgfmt)
 
 def verify_platform(supported_oses=['linux']):
     if True not in [sys.platform.startswith(x) for x in supported_oses]:

Signature · 1 line

2.13.5

Eric Blake

Reviewed-by on v1 2/2

2017-09-01 14:25 UTC #5 lore raw

Re: [Qemu-devel] [PATCH 2/2] iotests: blacklist 194 with the luks driver

Quoted 1 line · Daniel P. Berrange

The 194 test has alot of code that assumes a simple image file. Rewriting

s/alot/a lot/

Quoted 8 lines

this to work with luks is possible, but non-trivial, so blacklist the luks format for now.
Signed-off-by Daniel P. Berrange <[email protected]>
---
 tests/qemu-iotests/194        | 1 +
 tests/qemu-iotests/iotests.py | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by Eric Blake <[email protected]>

Signature · 3 lines

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Kashyap Chamarthy

Reviewed-by on v1 2/2

2017-09-04 09:14 UTC #6 lore raw

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: blacklist 194 with the luks driver

In reply to Daniel P. Berrange

Quoted 9 lines · Daniel P. Berrange

The 194 test has alot of code that assumes a simple image file. Rewriting this to work with luks is possible, but non-trivial, so blacklist the luks format for now.
Signed-off-by Daniel P. Berrange <[email protected]>
---
 tests/qemu-iotests/194        | 1 +
 tests/qemu-iotests/iotests.py | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by Kashyap Chamarthy <[email protected]>

Quoted 33 lines

tests/qemu-iotests/194 +1 -0

index a3e3bad664..bfe1859ecf 100755
@@ -23,6 +23,7 @@ import os
 import atexit
 import iotests
 
+iotests.verify_image_format(unsupported_fmts=['luks'])
 iotests.verify_platform(['linux'])
 
 img_size = '1G'

tests/qemu-iotests/iotests.py +3 -1

index 7233983f3c..3e7304b678 100644
@@ -395,9 +395,11 @@ def notrun(reason):
     print '%s not run: %s' % (seq, reason)
     sys.exit(0)
 
-def verify_image_format(supported_fmts=[]):
+def verify_image_format(supported_fmts=[], unsupported_fmts=[]):
     if supported_fmts and (imgfmt not in supported_fmts):
         notrun('not suitable for this image format: %s' % imgfmt)
+    if unsupported_fmts and (imgfmt in unsupported_fmts):
+        notrun('not suitable for this image format: %s' % imgfmt)
 
 def verify_platform(supported_oses=['linux']):
     if True not in [sys.platform.startswith(x) for x in supported_oses]:

Signature · 1 line

2.13.5

Signature · 1 line

/kashyap

Stefan Hajnoczi

Reviewed-by on v1 2/2

2017-09-05 10:04 UTC #7 lore raw

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: blacklist 194 with the luks driver

In reply to Daniel P. Berrange

Quoted 9 lines · Daniel P. Berrange

The 194 test has alot of code that assumes a simple image file. Rewriting this to work with luks is possible, but non-trivial, so blacklist the luks format for now.
Signed-off-by Daniel P. Berrange <[email protected]>
---
 tests/qemu-iotests/194        | 1 +
 tests/qemu-iotests/iotests.py | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by Stefan Hajnoczi <[email protected]>

Eric Blake

2017-09-01 14:27 UTC #8 lore raw

In reply to Daniel P. Berrange

Quoted 6 lines · Daniel P. Berrange

Two recently added iotests are currently breaking 'check -luks', fix the easy one and disable the hard one.
Daniel P. Berrange (2):
iotests: rewrite 192 to use _launch_qemu to fix LUKS support
iotests: blacklist 194 with the luks driver

Since both tests involve an NBD server, I can take this series through my NBD queue. Or, since both involve LUKS, you can push it in your next crypto pull request. Any preferences?

Signature · 3 lines

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Daniel P. Berrange

author

2017-09-01 14:34 UTC #9 lore raw

Quoted 11 lines · Eric Blake

Quoted 6 lines · Daniel P. Berrange
Two recently added iotests are currently breaking 'check -luks', fix the easy one and disable the hard one.
Daniel P. Berrange (2):
iotests: rewrite 192 to use _launch_qemu to fix LUKS support
iotests: blacklist 194 with the luks driver
Since both tests involve an NBD server, I can take this series through my NBD queue. Or, since both involve LUKS, you can push it in your next crypto pull request. Any preferences?

Best if either you or a block maintainer queue it i think

Regards,
Daniel

Signature · 3 lines

|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Eric Blake

2017-09-01 20:10 UTC #10 lore raw

Quoted 9 lines · Daniel P. Berrange

Quoted 8 lines · Eric Blake
Quoted 6 lines · Daniel P. Berrange
Two recently added iotests are currently breaking 'check -luks', fix the easy one and disable the hard one.
Daniel P. Berrange (2):
iotests: rewrite 192 to use _launch_qemu to fix LUKS support
iotests: blacklist 194 with the luks driver

Quoted 5 lines

Quoted 3 lines
Since both tests involve an NBD server, I can take this series through my NBD queue. Or, since both involve LUKS, you can push it in your next crypto pull request. Any preferences?
Best if either you or a block maintainer queue it i think

Okay, on my NBD queue:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd

I'll wait to see if other NBD patches land in the next few days, but will send a pull request within a week.

Signature · 3 lines

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Fam Zheng

Tested-by on the v1 series

2017-09-01 14:55 UTC #11 lore raw

In reply to Daniel P. Berrange

Quoted 2 lines · Daniel P. Berrange

Two recently added iotests are currently breaking 'check -luks', fix the easy one and disable the hard one.

I can add -luks to the test-block series with this, thanks for the quick fix!

Tested-by Fam Zheng <[email protected]>