[Qemu-devel] [PATCH 0/3] iotests: check whitelisted formats

[Qemu-devel] [PATCH 0/3] iotests: check whitelisted formats

[Andrey Shinkevich]

Some test cases require specific formats to be supported by QEMU. The
list of formats supported by the block layer doesn't take whitelists
into account. This series manages this issue. The method decorator
skip_if_unsupported() checks if requested formats are whitelisted.
The sample output is shown with the sample test #139.

Andrey Shinkevich (3):
  block: iterate_format with account of whitelisting
  iotests: ask QEMU for supported formats
  iotests: check whitelisted formats

 block.c                       | 23 ++++++++++++++++----
 blockdev.c                    |  4 +++-
 include/block/block.h         |  2 +-
 qemu-img.c                    |  2 +-
 tests/qemu-iotests/139        |  2 ++
 tests/qemu-iotests/check      | 16 +++++++++++++-
 tests/qemu-iotests/iotests.py | 50 +++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 91 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] block: iterate_format with account of whitelisting

[Andrey Shinkevich]

bdrv_iterate_format (which is currently only used for printing out the
formats supported by the block layer) doesn't take format whitelisting
into account.

This creates a problem for tests: they enumerate supported formats to
decide which tests to enable, but then discover that QEMU doesn't let
them actually use some of those formats.

To avoid that, exclude formats that are not whitelisted from
enumeration, if whitelisting is in use.  Since we have separate
whitelists for r/w and r/o, take this a parameter to
bdrv_iterate_format, and print two lists of supported formats (r/w and
r/o) in main qemu.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block.c               | 23 +++++++++++++++++++----
 blockdev.c            |  4 +++-
 include/block/block.h |  2 +-
 qemu-img.c            |  2 +-
 4 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 4ad0e90..1f0e23f 100644
--- a/block.c
+++ b/block.c
@@ -395,7 +395,7 @@ BlockDriver *bdrv_find_format(const char *format_name)
     return bdrv_do_find_format(format_name);
 }
 
-int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
+static int bdrv_format_is_whitelisted(const char *format_name, bool read_only)
 {
     static const char *whitelist_rw[] = {
         CONFIG_BDRV_RW_WHITELIST
@@ -410,13 +410,13 @@ int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
     }
 
     for (p = whitelist_rw; *p; p++) {
-        if (!strcmp(drv->format_name, *p)) {
+        if (!strcmp(format_name, *p)) {
             return 1;
         }
     }
     if (read_only) {
         for (p = whitelist_ro; *p; p++) {
-            if (!strcmp(drv->format_name, *p)) {
+            if (!strcmp(format_name, *p)) {
                 return 1;
             }
         }
@@ -424,6 +424,11 @@ int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
     return 0;
 }
 
+int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
+{
+    return bdrv_format_is_whitelisted(drv->format_name, read_only);
+}
+
 bool bdrv_uses_whitelist(void)
 {
     return use_bdrv_whitelist;
@@ -4040,7 +4045,7 @@ static int qsort_strcmp(const void *a, const void *b)
 }
 
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
-                         void *opaque)
+                         void *opaque, bool read_only)
 {
     BlockDriver *drv;
     int count = 0;
@@ -4051,6 +4056,11 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
         if (drv->format_name) {
             bool found = false;
             int i = count;
+
+            if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, read_only)) {
+                continue;
+            }
+
             while (formats && i && !found) {
                 found = !strcmp(formats[--i], drv->format_name);
             }
@@ -4069,6 +4079,11 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
             bool found = false;
             int j = count;
 
+            if (use_bdrv_whitelist &&
+                !bdrv_format_is_whitelisted(format_name, read_only)) {
+                continue;
+            }
+
             while (formats && j && !found) {
                 found = !strcmp(formats[--j], format_name);
             }
diff --git a/blockdev.c b/blockdev.c
index 8714ad2..292e121 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -531,7 +531,9 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
     if ((buf = qemu_opt_get(opts, "format")) != NULL) {
         if (is_help_option(buf)) {
             error_printf("Supported formats:");
-            bdrv_iterate_format(bdrv_format_print, NULL);
+            bdrv_iterate_format(bdrv_format_print, NULL, false);
+            error_printf("\nSupported formats (read-only):");
+            bdrv_iterate_format(bdrv_format_print, NULL, true);
             error_printf("\n");
             goto early_err;
         }
diff --git a/include/block/block.h b/include/block/block.h
index 73357c6..0f98bc9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -472,7 +472,7 @@ void bdrv_next_cleanup(BdrvNextIterator *it);
 BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs);
 bool bdrv_is_encrypted(BlockDriverState *bs);
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
-                         void *opaque);
+                         void *opaque, bool read_only);
 const char *bdrv_get_node_name(const BlockDriverState *bs);
 const char *bdrv_get_device_name(const BlockDriverState *bs);
 const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
diff --git a/qemu-img.c b/qemu-img.c
index 7853935..ccd3ba9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -198,7 +198,7 @@ static void QEMU_NORETURN help(void)
            "  'skip=N' skip N bs-sized blocks at the start of input\n";
 
     printf("%s\nSupported formats:", help_msg);
-    bdrv_iterate_format(format_print, NULL);
+    bdrv_iterate_format(format_print, NULL, false);
     printf("\n\n" QEMU_HELP_BOTTOM "\n");
     exit(EXIT_SUCCESS);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] iotests: ask QEMU for supported formats

[Andrey Shinkevich]

Supported formats listed by 'qemu' may differ from those listed by
'qemu-img' due to whitelists. Some test cases require specific formats
that may be used with qemu. They can be inquired directly by running
'qemu -drive format=help'. The response takes whitelists into account.
The method supported_formats() serves for that. The method decorator
skip_if_unsupported() checks if all requested formats are whitelisted.
If not, the test case will be skipped. That has been implemented in
the 'check' file in the way similar to the 'test notrun' mechanism.

Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/check      | 16 +++++++++++++-
 tests/qemu-iotests/iotests.py | 50 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 895e1e3..b9d539c 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -25,6 +25,7 @@ try=0
 n_bad=0
 bad=""
 notrun=""
+casenotrun=""
 interrupt=true
 
 # by default don't output timestamps
@@ -664,6 +665,11 @@ END        { if (NR > 0) {
             echo "Not run:$notrun"
             echo "Not run:$notrun" >>check.log
         fi
+        if [ ! -z "$casenotrun" ]
+        then
+            echo "Some cases not run in:$casenotrun"
+            echo "Some cases not run in:$casenotrun" >>check.log
+        fi
         if [ ! -z "$n_bad" -a $n_bad != 0 ]
         then
             echo "Failures:$bad"
@@ -743,6 +749,10 @@ do
                 printf "        "        # prettier output with timestamps.
         fi
         rm -f core $seq.notrun
+        if [ -f $seq.casenotrun ]
+        then
+            rm -f $seq.casenotrun
+        fi
 
         start=$(_wallclock)
         $timestamp && printf %s "        [$(date "+%T")]"
@@ -823,7 +833,11 @@ do
                 fi
             fi
         fi
-
+        if [ -f $seq.casenotrun ]
+        then
+            cat $seq.casenotrun
+            casenotrun="$casenotrun $seq"
+        fi
     fi
 
     # come here for each test, except when $showme is true
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b461f53..8fe1620 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -728,6 +728,56 @@ def verify_quorum():
     if not supports_quorum():
         notrun('quorum support missing')
 
+def qemu_pipe(*args):
+    '''Run qemu with an option to print something and exit (e.g. a help option),
+    and return its output'''
+    args = [qemu_prog] + qemu_opts + list(args)
+    subp = subprocess.Popen(args, stdout=subprocess.PIPE,
+                            stderr=subprocess.STDOUT)
+    exitcode = subp.wait()
+    if exitcode < 0:
+        sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
+            ' '.join(args)))
+    return subp.communicate()[0]
+
+def supported_formats(read_only=False):
+    '''Set 'read_only' to True to check ro-whitelist
+       Otherwise, rw-whitelist is checked'''
+    format_message = qemu_pipe("-drive", "format=help")
+    available = []
+
+    if read_only:
+        # Check read-only whitelist
+        available = format_message.splitlines()[1].split(":")[1].split()
+    else:
+        # Check read-write whitelist
+        available = format_message.splitlines()[0].split(":")[1].split()
+
+    return available
+
+def case_notrun(reason):
+    '''Skip this test case'''
+    # Each test in qemu-iotests has a number ("seq")
+    seq = os.path.basename(sys.argv[0])
+
+    open('%s/%s.casenotrun' % (output_dir, seq), 'ab').write(
+        '    [case not run] ' + reason + '\n')
+
+def skip_if_unsupported(required_formats=[], read_only=False):
+    '''Skip Test Decorator
+       Runs the test if all the required formats are whitelisted'''
+    def skip_test_decorator(func):
+        def func_wrapper(*args, **kwargs):
+            usf_list = list(set(required_formats) -
+                set(supported_formats(read_only)))
+            if usf_list:
+                case_notrun('{}: formats {} are not whitelisted'.format(
+                    args[0], usf_list))
+            else:
+                return func(*args, **kwargs)
+        return func_wrapper
+    return skip_test_decorator
+
 def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[],
          unsupported_fmts=[]):
     '''Run tests'''
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] iotests: check whitelisted formats

[Andrey Shinkevich]

Some test cases require specific formats. The method decorator
skip_if_unsupported() checks if requested formats are whitelisted.
The test #139 was selected for a sample output:

137 3s ...
138 0s ...
139 2s ...
    [case not run] testBlkVerify (__main__.TestBlockdevDel): formats ['blkverify'] are not whitelisted
    [case not run] testQuorum (__main__.TestBlockdevDel): formats ['quorum'] are not whitelisted
140 0s ...
Not run: 131 135 136
Some cases not run in: 139
Passed all 137 tests

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/139 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index 62402c1..cb97029 100755
--- a/tests/qemu-iotests/139
+++ b/tests/qemu-iotests/139
@@ -333,6 +333,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
         self.delBlockDriverState('debug0')
         self.checkBlockDriverState('node0', False)
 
+    @iotests.skip_if_unsupported(['blkverify'])
     def testBlkVerify(self):
         self.addBlkVerify('verify0', 'node0', 'node1')
         # We cannot remove the children of a blkverify device
@@ -343,6 +344,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
         self.checkBlockDriverState('node0', False)
         self.checkBlockDriverState('node1', False)
 
+    @iotests.skip_if_unsupported(['quorum'])
     def testQuorum(self):
         if not iotests.supports_quorum():
             return
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/3] iotests: ask QEMU for supported formats

[Kevin Wolf]

Am 04.03.2019 um 11:08 hat Andrey Shinkevich geschrieben:
> Supported formats listed by 'qemu' may differ from those listed by
> 'qemu-img' due to whitelists. Some test cases require specific formats
> that may be used with qemu. They can be inquired directly by running
> 'qemu -drive format=help'. The response takes whitelists into account.
> The method supported_formats() serves for that. The method decorator
> skip_if_unsupported() checks if all requested formats are whitelisted.
> If not, the test case will be skipped. That has been implemented in
> the 'check' file in the way similar to the 'test notrun' mechanism.
> 
> Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  tests/qemu-iotests/check      | 16 +++++++++++++-
>  tests/qemu-iotests/iotests.py | 50 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 895e1e3..b9d539c 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -25,6 +25,7 @@ try=0
>  n_bad=0
>  bad=""
>  notrun=""
> +casenotrun=""
>  interrupt=true
>  
>  # by default don't output timestamps
> @@ -664,6 +665,11 @@ END        { if (NR > 0) {
>              echo "Not run:$notrun"
>              echo "Not run:$notrun" >>check.log
>          fi
> +        if [ ! -z "$casenotrun" ]
> +        then
> +            echo "Some cases not run in:$casenotrun"
> +            echo "Some cases not run in:$casenotrun" >>check.log
> +        fi
>          if [ ! -z "$n_bad" -a $n_bad != 0 ]
>          then
>              echo "Failures:$bad"
> @@ -743,6 +749,10 @@ do
>                  printf "        "        # prettier output with timestamps.
>          fi
>          rm -f core $seq.notrun
> +        if [ -f $seq.casenotrun ]
> +        then
> +            rm -f $seq.casenotrun
> +        fi

The if is unnecessary here, 'rm -f' doesn't print an error if the file
doesn't exist.

>          start=$(_wallclock)
>          $timestamp && printf %s "        [$(date "+%T")]"
> @@ -823,7 +833,11 @@ do
>                  fi
>              fi
>          fi
> -
> +        if [ -f $seq.casenotrun ]
> +        then
> +            cat $seq.casenotrun
> +            casenotrun="$casenotrun $seq"
> +        fi
>      fi
>  
>      # come here for each test, except when $showme is true
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index b461f53..8fe1620 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -728,6 +728,56 @@ def verify_quorum():
>      if not supports_quorum():
>          notrun('quorum support missing')
>  
> +def qemu_pipe(*args):
> +    '''Run qemu with an option to print something and exit (e.g. a help option),
> +    and return its output'''
> +    args = [qemu_prog] + qemu_opts + list(args)
> +    subp = subprocess.Popen(args, stdout=subprocess.PIPE,
> +                            stderr=subprocess.STDOUT)
> +    exitcode = subp.wait()
> +    if exitcode < 0:
> +        sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
> +            ' '.join(args)))
> +    return subp.communicate()[0]
> +
> +def supported_formats(read_only=False):
> +    '''Set 'read_only' to True to check ro-whitelist
> +       Otherwise, rw-whitelist is checked'''
> +    format_message = qemu_pipe("-drive", "format=help")
> +    available = []
> +
> +    if read_only:
> +        # Check read-only whitelist
> +        available = format_message.splitlines()[1].split(":")[1].split()
> +    else:
> +        # Check read-write whitelist
> +        available = format_message.splitlines()[0].split(":")[1].split()
> +
> +    return available

What do you need available for when you only assign it and then
immediately return it without using it before? I think I would write
it much shorter like this:

line = 1 if read_only else 0
return format_message.splitlines()[line].split(":")[1].split()

> +def case_notrun(reason):
> +    '''Skip this test case'''
> +    # Each test in qemu-iotests has a number ("seq")
> +    seq = os.path.basename(sys.argv[0])
> +
> +    open('%s/%s.casenotrun' % (output_dir, seq), 'ab').write(
> +        '    [case not run] ' + reason + '\n')

Maybe move this next to notrun(), which is similar?

Kevin

Re: [Qemu-devel] [PATCH 2/3] iotests: ask QEMU for supported formats

[Kevin Wolf]

Am 04.03.2019 um 11:08 hat Andrey Shinkevich geschrieben:
> Supported formats listed by 'qemu' may differ from those listed by
> 'qemu-img' due to whitelists. Some test cases require specific formats
> that may be used with qemu. They can be inquired directly by running
> 'qemu -drive format=help'. The response takes whitelists into account.
> The method supported_formats() serves for that. The method decorator
> skip_if_unsupported() checks if all requested formats are whitelisted.
> If not, the test case will be skipped. That has been implemented in
> the 'check' file in the way similar to the 'test notrun' mechanism.
> 
> Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  tests/qemu-iotests/check      | 16 +++++++++++++-
>  tests/qemu-iotests/iotests.py | 50 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 895e1e3..b9d539c 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -25,6 +25,7 @@ try=0
>  n_bad=0
>  bad=""
>  notrun=""
> +casenotrun=""
>  interrupt=true
>  
>  # by default don't output timestamps
> @@ -664,6 +665,11 @@ END        { if (NR > 0) {
>              echo "Not run:$notrun"
>              echo "Not run:$notrun" >>check.log
>          fi
> +        if [ ! -z "$casenotrun" ]
> +        then
> +            echo "Some cases not run in:$casenotrun"
> +            echo "Some cases not run in:$casenotrun" >>check.log
> +        fi
>          if [ ! -z "$n_bad" -a $n_bad != 0 ]
>          then
>              echo "Failures:$bad"
> @@ -743,6 +749,10 @@ do
>                  printf "        "        # prettier output with timestamps.
>          fi
>          rm -f core $seq.notrun
> +        if [ -f $seq.casenotrun ]
> +        then
> +            rm -f $seq.casenotrun
> +        fi
>  
>          start=$(_wallclock)
>          $timestamp && printf %s "        [$(date "+%T")]"
> @@ -823,7 +833,11 @@ do
>                  fi
>              fi
>          fi
> -
> +        if [ -f $seq.casenotrun ]
> +        then
> +            cat $seq.casenotrun
> +            casenotrun="$casenotrun $seq"
> +        fi
>      fi
>  
>      # come here for each test, except when $showme is true
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index b461f53..8fe1620 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -728,6 +728,56 @@ def verify_quorum():
>      if not supports_quorum():
>          notrun('quorum support missing')
>  
> +def qemu_pipe(*args):
> +    '''Run qemu with an option to print something and exit (e.g. a help option),
> +    and return its output'''
> +    args = [qemu_prog] + qemu_opts + list(args)
> +    subp = subprocess.Popen(args, stdout=subprocess.PIPE,
> +                            stderr=subprocess.STDOUT)
> +    exitcode = subp.wait()
> +    if exitcode < 0:
> +        sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
> +            ' '.join(args)))
> +    return subp.communicate()[0]
> +
> +def supported_formats(read_only=False):
> +    '''Set 'read_only' to True to check ro-whitelist
> +       Otherwise, rw-whitelist is checked'''
> +    format_message = qemu_pipe("-drive", "format=help")
> +    available = []
> +
> +    if read_only:
> +        # Check read-only whitelist
> +        available = format_message.splitlines()[1].split(":")[1].split()
> +    else:
> +        # Check read-write whitelist
> +        available = format_message.splitlines()[0].split(":")[1].split()
> +
> +    return available

With Python 3:

+======================================================================
+ERROR: testBlkVerify (__main__.TestBlockdevDel)
+----------------------------------------------------------------------
+Traceback (most recent call last):
+  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 800, in func_wrapper
+    set(supported_formats(read_only)))
+  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 782, in supported_formats
+    available = format_message.splitlines()[0].split(":")[1].split()
+TypeError: a bytes-like object is required, not 'str'
+
+======================================================================
+ERROR: testQuorum (__main__.TestBlockdevDel)
+----------------------------------------------------------------------
+Traceback (most recent call last):
+  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 800, in func_wrapper
+    set(supported_formats(read_only)))
+  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 782, in supported_formats
+    available = format_message.splitlines()[0].split(":")[1].split()
+TypeError: a bytes-like object is required, not 'str'
+

This fixes it for me (the first hunk is actually a bonus which fixes a
preexisting bug, which somehow meant that the reason was not printed):

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e6ae4e91b7..b41ac4be51 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -712,7 +712,7 @@ def notrun(reason):
     # Each test in qemu-iotests has a number ("seq")
     seq = os.path.basename(sys.argv[0])

-    open('%s/%s.notrun' % (output_dir, seq), 'wb').write(reason + '\n')
+    open('%s/%s.notrun' % (output_dir, seq), 'wt').write(reason + '\n')
     print('%s not run: %s' % (seq, reason))
     sys.exit(0)

@@ -761,7 +761,8 @@ def qemu_pipe(*args):
     and return its output'''
     args = [qemu_prog] + qemu_opts + list(args)
     subp = subprocess.Popen(args, stdout=subprocess.PIPE,
-                            stderr=subprocess.STDOUT)
+                            stderr=subprocess.STDOUT,
+                            universal_newlines=True)
     exitcode = subp.wait()
     if exitcode < 0:
         sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
@@ -788,7 +789,7 @@ def case_notrun(reason):
     # Each test in qemu-iotests has a number ("seq")
     seq = os.path.basename(sys.argv[0])

-    open('%s/%s.casenotrun' % (output_dir, seq), 'ab').write(
+    open('%s/%s.casenotrun' % (output_dir, seq), 'at').write(
         '    [case not run] ' + reason + '\n')

 def skip_if_unsupported(required_formats=[], read_only=False):

Re: [Qemu-devel] [PATCH 3/3] iotests: check whitelisted formats

[Kevin Wolf]

Am 04.03.2019 um 11:08 hat Andrey Shinkevich geschrieben:
> Some test cases require specific formats. The method decorator
> skip_if_unsupported() checks if requested formats are whitelisted.
> The test #139 was selected for a sample output:
> 
> 137 3s ...
> 138 0s ...
> 139 2s ...
>     [case not run] testBlkVerify (__main__.TestBlockdevDel): formats ['blkverify'] are not whitelisted
>     [case not run] testQuorum (__main__.TestBlockdevDel): formats ['quorum'] are not whitelisted
> 140 0s ...
> Not run: 131 135 136
> Some cases not run in: 139
> Passed all 137 tests
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

Would you mind squashing in this hunk for v3?

Kevin

diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index cb970294d3..933b45121a 100755
--- a/tests/qemu-iotests/139
+++ b/tests/qemu-iotests/139
@@ -325,6 +325,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
         # FIXME mirror0 disappears, drive-mirror doesn't take a reference
         #self.delBlockDriverState('mirror0')
 
+    @iotests.skip_if_unsupported(['blkdebug'])
     def testBlkDebug(self):
         self.addBlkDebug('debug0', 'node0')
         # 'node0' is used by the blkdebug node

Re: [Qemu-devel] [PATCH 3/3] iotests: check whitelisted formats

[Andrey Shinkevich]

On 06/03/2019 17:52, Kevin Wolf wrote:
> Am 04.03.2019 um 11:08 hat Andrey Shinkevich geschrieben:
>> Some test cases require specific formats. The method decorator
>> skip_if_unsupported() checks if requested formats are whitelisted.
>> The test #139 was selected for a sample output:
>>
>> 137 3s ...
>> 138 0s ...
>> 139 2s ...
>>      [case not run] testBlkVerify (__main__.TestBlockdevDel): formats ['blkverify'] are not whitelisted
>>      [case not run] testQuorum (__main__.TestBlockdevDel): formats ['quorum'] are not whitelisted
>> 140 0s ...
>> Not run: 131 135 136
>> Some cases not run in: 139
>> Passed all 137 tests
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> 
> Would you mind squashing in this hunk for v3?
> 
> Kevin
> 

I will squash it in v3.

Andrey

> diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
> index cb970294d3..933b45121a 100755
> --- a/tests/qemu-iotests/139
> +++ b/tests/qemu-iotests/139
> @@ -325,6 +325,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
>           # FIXME mirror0 disappears, drive-mirror doesn't take a reference
>           #self.delBlockDriverState('mirror0')
>   
> +    @iotests.skip_if_unsupported(['blkdebug'])
>       def testBlkDebug(self):
>           self.addBlkDebug('debug0', 'node0')
>           # 'node0' is used by the blkdebug node
> 

Re: [Qemu-devel] [PATCH 2/3] iotests: ask QEMU for supported formats

[Andrey Shinkevich]

On 06/03/2019 17:51, Kevin Wolf wrote:
> Am 04.03.2019 um 11:08 hat Andrey Shinkevich geschrieben:
>> Supported formats listed by 'qemu' may differ from those listed by
>> 'qemu-img' due to whitelists. Some test cases require specific formats
>> that may be used with qemu. They can be inquired directly by running
>> 'qemu -drive format=help'. The response takes whitelists into account.
>> The method supported_formats() serves for that. The method decorator
>> skip_if_unsupported() checks if all requested formats are whitelisted.
>> If not, the test case will be skipped. That has been implemented in
>> the 'check' file in the way similar to the 'test notrun' mechanism.
>>
>> Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
>> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/check      | 16 +++++++++++++-
>>   tests/qemu-iotests/iotests.py | 50 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index 895e1e3..b9d539c 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -25,6 +25,7 @@ try=0
>>   n_bad=0
>>   bad=""
>>   notrun=""
>> +casenotrun=""
>>   interrupt=true
>>   
>>   # by default don't output timestamps
>> @@ -664,6 +665,11 @@ END        { if (NR > 0) {
>>               echo "Not run:$notrun"
>>               echo "Not run:$notrun" >>check.log
>>           fi
>> +        if [ ! -z "$casenotrun" ]
>> +        then
>> +            echo "Some cases not run in:$casenotrun"
>> +            echo "Some cases not run in:$casenotrun" >>check.log
>> +        fi
>>           if [ ! -z "$n_bad" -a $n_bad != 0 ]
>>           then
>>               echo "Failures:$bad"
>> @@ -743,6 +749,10 @@ do
>>                   printf "        "        # prettier output with timestamps.
>>           fi
>>           rm -f core $seq.notrun
>> +        if [ -f $seq.casenotrun ]
>> +        then
>> +            rm -f $seq.casenotrun
>> +        fi
>>   
>>           start=$(_wallclock)
>>           $timestamp && printf %s "        [$(date "+%T")]"
>> @@ -823,7 +833,11 @@ do
>>                   fi
>>               fi
>>           fi
>> -
>> +        if [ -f $seq.casenotrun ]
>> +        then
>> +            cat $seq.casenotrun
>> +            casenotrun="$casenotrun $seq"
>> +        fi
>>       fi
>>   
>>       # come here for each test, except when $showme is true
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index b461f53..8fe1620 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -728,6 +728,56 @@ def verify_quorum():
>>       if not supports_quorum():
>>           notrun('quorum support missing')
>>   
>> +def qemu_pipe(*args):
>> +    '''Run qemu with an option to print something and exit (e.g. a help option),
>> +    and return its output'''
>> +    args = [qemu_prog] + qemu_opts + list(args)
>> +    subp = subprocess.Popen(args, stdout=subprocess.PIPE,
>> +                            stderr=subprocess.STDOUT)
>> +    exitcode = subp.wait()
>> +    if exitcode < 0:
>> +        sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
>> +            ' '.join(args)))
>> +    return subp.communicate()[0]
>> +
>> +def supported_formats(read_only=False):
>> +    '''Set 'read_only' to True to check ro-whitelist
>> +       Otherwise, rw-whitelist is checked'''
>> +    format_message = qemu_pipe("-drive", "format=help")
>> +    available = []
>> +
>> +    if read_only:
>> +        # Check read-only whitelist
>> +        available = format_message.splitlines()[1].split(":")[1].split()
>> +    else:
>> +        # Check read-write whitelist
>> +        available = format_message.splitlines()[0].split(":")[1].split()
>> +
>> +    return available
> 
> With Python 3:
> 
> +======================================================================
> +ERROR: testBlkVerify (__main__.TestBlockdevDel)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 800, in func_wrapper
> +    set(supported_formats(read_only)))
> +  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 782, in supported_formats
> +    available = format_message.splitlines()[0].split(":")[1].split()
> +TypeError: a bytes-like object is required, not 'str'
> +
> +======================================================================
> +ERROR: testQuorum (__main__.TestBlockdevDel)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 800, in func_wrapper
> +    set(supported_formats(read_only)))
> +  File "/home/kwolf/source/qemu/tests/qemu-iotests/iotests.py", line 782, in supported_formats
> +    available = format_message.splitlines()[0].split(":")[1].split()
> +TypeError: a bytes-like object is required, not 'str'
> +
> 
> This fixes it for me (the first hunk is actually a bonus which fixes a
> preexisting bug, which somehow meant that the reason was not printed):
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index e6ae4e91b7..b41ac4be51 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -712,7 +712,7 @@ def notrun(reason):
>       # Each test in qemu-iotests has a number ("seq")
>       seq = os.path.basename(sys.argv[0])
> 
> -    open('%s/%s.notrun' % (output_dir, seq), 'wb').write(reason + '\n')
> +    open('%s/%s.notrun' % (output_dir, seq), 'wt').write(reason + '\n')
>       print('%s not run: %s' % (seq, reason))
>       sys.exit(0)
> 

Thanks a lot. I am going to add the fix above as a separate patch in v3
with "Signed-off-by: Kevin Wolf <kwolf@redhat.com>"

Andrey

> @@ -761,7 +761,8 @@ def qemu_pipe(*args):
>       and return its output'''
>       args = [qemu_prog] + qemu_opts + list(args)
>       subp = subprocess.Popen(args, stdout=subprocess.PIPE,
> -                            stderr=subprocess.STDOUT)
> +                            stderr=subprocess.STDOUT,
> +                            universal_newlines=True)
>       exitcode = subp.wait()
>       if exitcode < 0:
>           sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
> @@ -788,7 +789,7 @@ def case_notrun(reason):
>       # Each test in qemu-iotests has a number ("seq")
>       seq = os.path.basename(sys.argv[0])
> 
> -    open('%s/%s.casenotrun' % (output_dir, seq), 'ab').write(
> +    open('%s/%s.casenotrun' % (output_dir, seq), 'at').write(
>           '    [case not run] ' + reason + '\n')
> 
>   def skip_if_unsupported(required_formats=[], read_only=False):
> 

Re: [Qemu-devel] [PATCH 2/3] iotests: ask QEMU for supported formats

[Andrey Shinkevich]

On 06/03/2019 17:31, Kevin Wolf wrote:
> Am 04.03.2019 um 11:08 hat Andrey Shinkevich geschrieben:
>> Supported formats listed by 'qemu' may differ from those listed by
>> 'qemu-img' due to whitelists. Some test cases require specific formats
>> that may be used with qemu. They can be inquired directly by running
>> 'qemu -drive format=help'. The response takes whitelists into account.
>> The method supported_formats() serves for that. The method decorator
>> skip_if_unsupported() checks if all requested formats are whitelisted.
>> If not, the test case will be skipped. That has been implemented in
>> the 'check' file in the way similar to the 'test notrun' mechanism.
>>
>> Suggested-by: Roman Kagan <rkagan@virtuozzo.com>
>> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/check      | 16 +++++++++++++-
>>   tests/qemu-iotests/iotests.py | 50 +++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index 895e1e3..b9d539c 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -25,6 +25,7 @@ try=0
>>   n_bad=0
>>   bad=""
>>   notrun=""
>> +casenotrun=""
>>   interrupt=true
>>   
>>   # by default don't output timestamps
>> @@ -664,6 +665,11 @@ END        { if (NR > 0) {
>>               echo "Not run:$notrun"
>>               echo "Not run:$notrun" >>check.log
>>           fi
>> +        if [ ! -z "$casenotrun" ]
>> +        then
>> +            echo "Some cases not run in:$casenotrun"
>> +            echo "Some cases not run in:$casenotrun" >>check.log
>> +        fi
>>           if [ ! -z "$n_bad" -a $n_bad != 0 ]
>>           then
>>               echo "Failures:$bad"
>> @@ -743,6 +749,10 @@ do
>>                   printf "        "        # prettier output with timestamps.
>>           fi
>>           rm -f core $seq.notrun
>> +        if [ -f $seq.casenotrun ]
>> +        then
>> +            rm -f $seq.casenotrun
>> +        fi
> 
> The if is unnecessary here, 'rm -f' doesn't print an error if the file
> doesn't exist.
> 
>>           start=$(_wallclock)
>>           $timestamp && printf %s "        [$(date "+%T")]"
>> @@ -823,7 +833,11 @@ do
>>                   fi
>>               fi
>>           fi
>> -
>> +        if [ -f $seq.casenotrun ]
>> +        then
>> +            cat $seq.casenotrun
>> +            casenotrun="$casenotrun $seq"
>> +        fi
>>       fi
>>   
>>       # come here for each test, except when $showme is true
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index b461f53..8fe1620 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -728,6 +728,56 @@ def verify_quorum():
>>       if not supports_quorum():
>>           notrun('quorum support missing')
>>   
>> +def qemu_pipe(*args):
>> +    '''Run qemu with an option to print something and exit (e.g. a help option),
>> +    and return its output'''
>> +    args = [qemu_prog] + qemu_opts + list(args)
>> +    subp = subprocess.Popen(args, stdout=subprocess.PIPE,
>> +                            stderr=subprocess.STDOUT)
>> +    exitcode = subp.wait()
>> +    if exitcode < 0:
>> +        sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
>> +            ' '.join(args)))
>> +    return subp.communicate()[0]
>> +
>> +def supported_formats(read_only=False):
>> +    '''Set 'read_only' to True to check ro-whitelist
>> +       Otherwise, rw-whitelist is checked'''
>> +    format_message = qemu_pipe("-drive", "format=help")
>> +    available = []
>> +
>> +    if read_only:
>> +        # Check read-only whitelist
>> +        available = format_message.splitlines()[1].split(":")[1].split()
>> +    else:
>> +        # Check read-write whitelist
>> +        available = format_message.splitlines()[0].split(":")[1].split()
>> +
>> +    return available
> 
> What do you need available for when you only assign it and then
> immediately return it without using it before? I think I would write
> it much shorter like this:
> 
> line = 1 if read_only else 0
> return format_message.splitlines()[line].split(":")[1].split()
> 

Thank you, Kevin, for your useful review. I will amend the series with
the v3.

Andrey

>> +def case_notrun(reason):
>> +    '''Skip this test case'''
>> +    # Each test in qemu-iotests has a number ("seq")
>> +    seq = os.path.basename(sys.argv[0])
>> +
>> +    open('%s/%s.casenotrun' % (output_dir, seq), 'ab').write(
>> +        '    [case not run] ' + reason + '\n')
> 
> Maybe move this next to notrun(), which is similar?
> 
> Kevin
> 

Re: [Qemu-devel] [PATCH 2/3] iotests: ask QEMU for supported formats

[Kevin Wolf]

Am 06.03.2019 um 18:59 hat Andrey Shinkevich geschrieben:
> On 06/03/2019 17:51, Kevin Wolf wrote:
> > This fixes it for me (the first hunk is actually a bonus which fixes a
> > preexisting bug, which somehow meant that the reason was not printed):
> > 
> > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > index e6ae4e91b7..b41ac4be51 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -712,7 +712,7 @@ def notrun(reason):
> >       # Each test in qemu-iotests has a number ("seq")
> >       seq = os.path.basename(sys.argv[0])
> > 
> > -    open('%s/%s.notrun' % (output_dir, seq), 'wb').write(reason + '\n')
> > +    open('%s/%s.notrun' % (output_dir, seq), 'wt').write(reason + '\n')
> >       print('%s not run: %s' % (seq, reason))
> >       sys.exit(0)
> > 
> 
> Thanks a lot. I am going to add the fix above as a separate patch in v3
> with "Signed-off-by: Kevin Wolf <kwolf@redhat.com>"

Sounds good to me, thanks!

Kevin