[Qemu-devel] [PATCH 0/2] Fix / skip recent iotests with LUKS driver

[Qemu-devel] [PATCH 0/2] Fix / skip recent iotests with LUKS driver

[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

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

-- 
2.13.5

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

[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 <berrange@redhat.com>
---
 tests/qemu-iotests/192 | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/192 b/tests/qemu-iotests/192
index b50a2c0c8e..595f0d786a 100755
--- a/tests/qemu-iotests/192
+++ b/tests/qemu-iotests/192
@@ -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"
-- 
2.13.5

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

[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 <berrange@redhat.com>
---
 tests/qemu-iotests/194        | 1 +
 tests/qemu-iotests/iotests.py | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index a3e3bad664..bfe1859ecf 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -23,6 +23,7 @@ import os
 import atexit
 import iotests
 
+iotests.verify_image_format(unsupported_fmts=['luks'])
 iotests.verify_platform(['linux'])
 
 img_size = '1G'
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7233983f3c..3e7304b678 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -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]:
-- 
2.13.5

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On 09/01/2017 05:54 AM, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  tests/qemu-iotests/192 | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 

>  
> -{
> -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 <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On 09/01/2017 05:54 AM, Daniel P. Berrange wrote:
> The 194 test has alot of code that assumes a simple image file. Rewriting

s/alot/a lot/

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

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH 0/2] Fix / skip recent iotests with LUKS driver

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On 09/01/2017 05:54 AM, Daniel P. Berrange wrote:
> 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?

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH 0/2] Fix / skip recent iotests with LUKS driver

[Daniel P. Berrange]

On Fri, Sep 01, 2017 at 09:26:30AM -0500, Eric Blake wrote:
> On 09/01/2017 05:54 AM, Daniel P. Berrange wrote:
> > 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
-- 
|: 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 :|

Re: [Qemu-devel] [PATCH 0/2] Fix / skip recent iotests with LUKS driver

[Fam Zheng]

On Fri, 09/01 11:54, Daniel P. Berrange wrote:
> 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 <famz@redhat.com>

Re: [Qemu-devel] [PATCH 0/2] Fix / skip recent iotests with LUKS driver

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

On 09/01/2017 09:33 AM, Daniel P. Berrange wrote:
> On Fri, Sep 01, 2017 at 09:26:30AM -0500, Eric Blake wrote:
>> On 09/01/2017 05:54 AM, Daniel P. Berrange wrote:
>>> 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

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.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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

[Kashyap Chamarthy]

On Fri, Sep 01, 2017 at 11:54:34AM +0100, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  tests/qemu-iotests/194        | 1 +
>  tests/qemu-iotests/iotests.py | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>

> diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
> index a3e3bad664..bfe1859ecf 100755
> --- a/tests/qemu-iotests/194
> +++ b/tests/qemu-iotests/194
> @@ -23,6 +23,7 @@ import os
>  import atexit
>  import iotests
>  
> +iotests.verify_image_format(unsupported_fmts=['luks'])
>  iotests.verify_platform(['linux'])
>  
>  img_size = '1G'
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 7233983f3c..3e7304b678 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -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]:
> -- 
> 2.13.5
> 
> 

-- 
/kashyap

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

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

Am 01.09.2017 um 16:22 hat Eric Blake geschrieben:
> On 09/01/2017 05:54 AM, Daniel P. Berrange wrote:
> > 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 <berrange@redhat.com>
> > ---
> >  tests/qemu-iotests/192 | 23 ++++++++++++++++-------
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> > 
> 
> >  
> > -{
> > -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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Stefan Hajnoczi]

On Fri, Sep 01, 2017 at 11:54:34AM +0100, Daniel P. Berrange wrote:
> 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 <berrange@redhat.com>
> ---
>  tests/qemu-iotests/194        | 1 +
>  tests/qemu-iotests/iotests.py | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>