[Qemu-devel] [PATCH v4 0/7] iotests: Selfish patches

[Qemu-devel] [PATCH v4 0/7] iotests: Selfish patches

[Max Reitz]

Hi,

Again, let me start with a link to an actually explanatory cover letter:
https://lists.nongnu.org/archive/html/qemu-block/2019-06/msg01102.html

v3:
https://lists.nongnu.org/archive/html/qemu-block/2019-08/msg00950.html

v4:
- I merged the old patch 1 in the meantime

- Patch 2: Adjusted the comment to make it more clear that it is
           case_notrun() itself that will not skip the test case, as
           requested by Andrey (I hope it fits what he had in mind, more
           or less); kept the R-bs, because I somehow feel like that’s
           the right thing to do here.

- Patch 3: The func_wrapper returned by the skip_test_decorator has a
           mandatory argument; make that and its required type explicit
           (with an annotation), as suggested by John
           (Kevin made me aware of the fact that annotations exist since
           Python 3.0, it’s just that they didn’t mean anything back
           then (neither do they really now, but whatever, it’s better
           than a comment))

- Patch 4: Resolved a conflict because of the change to patch 3


git-backport-diff against v3:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/7:[----] [--] 'iotests: Prefer null-co over null-aio'
002/7:[0007] [FC] 'iotests: Allow skipping test cases'
003/7:[0008] [FC] 'iotests: Use case_skip() in skip_if_unsupported()'
004/7:[0002] [FC] 'iotests: Let skip_if_unsupported() accept a method'
005/7:[----] [--] 'iotests: Test driver whitelisting in 093'
006/7:[----] [--] 'iotests: Test driver whitelisting in 136'
007/7:[----] [--] 'iotests: Cache supported_formats()'


Max Reitz (7):
  iotests: Prefer null-co over null-aio
  iotests: Allow skipping test cases
  iotests: Use case_skip() in skip_if_unsupported()
  iotests: Let skip_if_unsupported accept a function
  iotests: Test driver whitelisting in 093
  iotests: Test driver whitelisting in 136
  iotests: Cache supported_formats()

 tests/qemu-iotests/093        | 19 ++++++++-----
 tests/qemu-iotests/136        | 14 +++++++---
 tests/qemu-iotests/245        |  2 +-
 tests/qemu-iotests/iotests.py | 52 +++++++++++++++++++++++++++--------
 4 files changed, 63 insertions(+), 24 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v4 1/7] iotests: Prefer null-co over null-aio

[Max Reitz]

We use null-co basically everywhere in the iotests.  Unless we want to
test null-aio specifically, we should use it instead (for consistency).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/093 | 7 +++----
 tests/qemu-iotests/245 | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 3c4f5173ce..50c1e7f2ec 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -267,13 +267,12 @@ class ThrottleTestCoroutine(ThrottleTestCase):
     test_img = "null-co://"
 
 class ThrottleTestGroupNames(iotests.QMPTestCase):
-    test_img = "null-aio://"
     max_drives = 3
 
     def setUp(self):
         self.vm = iotests.VM()
         for i in range(0, self.max_drives):
-            self.vm.add_drive(self.test_img,
+            self.vm.add_drive("null-co://",
                               "throttling.iops-total=100,file.read-zeroes=on")
         self.vm.launch()
 
@@ -376,10 +375,10 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase):
 
     def test_removable_media(self):
         # Add a couple of dummy nodes named cd0 and cd1
-        result = self.vm.qmp("blockdev-add", driver="null-aio",
+        result = self.vm.qmp("blockdev-add", driver="null-co",
                              read_zeroes=True, node_name="cd0")
         self.assert_qmp(result, 'return', {})
-        result = self.vm.qmp("blockdev-add", driver="null-aio",
+        result = self.vm.qmp("blockdev-add", driver="null-co",
                              read_zeroes=True, node_name="cd1")
         self.assert_qmp(result, 'return', {})
 
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 41218d5f1d..e66a23c5f0 100644
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -598,7 +598,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
         ##################
         ###### null ######
         ##################
-        opts = {'driver': 'null-aio', 'node-name': 'root', 'size': 1024}
+        opts = {'driver': 'null-co', 'node-name': 'root', 'size': 1024}
 
         result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
         self.assert_qmp(result, 'return', {})
-- 
2.21.0

Re: [Qemu-devel] [PATCH v4 1/7] iotests: Prefer null-co over null-aio

[Andrey Shinkevich]

On 17/09/2019 12:19, Max Reitz wrote:
> We use null-co basically everywhere in the iotests.  Unless we want to
> test null-aio specifically, we should use it instead (for consistency).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/qemu-iotests/093 | 7 +++----
>   tests/qemu-iotests/245 | 2 +-
>   2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
> index 3c4f5173ce..50c1e7f2ec 100755
> --- a/tests/qemu-iotests/093
> +++ b/tests/qemu-iotests/093
> @@ -267,13 +267,12 @@ class ThrottleTestCoroutine(ThrottleTestCase):
>       test_img = "null-co://"
>   
>   class ThrottleTestGroupNames(iotests.QMPTestCase):
> -    test_img = "null-aio://"
>       max_drives = 3
>   
>       def setUp(self):
>           self.vm = iotests.VM()
>           for i in range(0, self.max_drives):
> -            self.vm.add_drive(self.test_img,
> +            self.vm.add_drive("null-co://",
>                                 "throttling.iops-total=100,file.read-zeroes=on")
>           self.vm.launch()
>   
> @@ -376,10 +375,10 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase):
>   
>       def test_removable_media(self):
>           # Add a couple of dummy nodes named cd0 and cd1
> -        result = self.vm.qmp("blockdev-add", driver="null-aio",
> +        result = self.vm.qmp("blockdev-add", driver="null-co",
>                                read_zeroes=True, node_name="cd0")
>           self.assert_qmp(result, 'return', {})
> -        result = self.vm.qmp("blockdev-add", driver="null-aio",
> +        result = self.vm.qmp("blockdev-add", driver="null-co",
>                                read_zeroes=True, node_name="cd1")
>           self.assert_qmp(result, 'return', {})
>   
> diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
> index 41218d5f1d..e66a23c5f0 100644
> --- a/tests/qemu-iotests/245
> +++ b/tests/qemu-iotests/245
> @@ -598,7 +598,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
>           ##################
>           ###### null ######
>           ##################
> -        opts = {'driver': 'null-aio', 'node-name': 'root', 'size': 1024}
> +        opts = {'driver': 'null-co', 'node-name': 'root', 'size': 1024}
>   
>           result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
>           self.assert_qmp(result, 'return', {})
> 

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-- 
With the best regards,
Andrey Shinkevich

[Qemu-devel] [PATCH v4 2/7] iotests: Allow skipping test cases

[Max Reitz]

case_notrun() does not actually skip the current test case.  It just
adds a "notrun" note and then returns to the caller, who manually has to
skip the test.  Generally, skipping a test case is as simple as
returning from the current function, but not always: For example, this
model does not allow skipping tests already in the setUp() function.

Thus, add a QMPTestCase.case_skip() function that invokes case_notrun()
and then self.skipTest().  To make this work, we need to filter the
information on how many test cases were skipped from the unittest
output.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b26271187c..e46f3b6aed 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -802,6 +802,11 @@ class QMPTestCase(unittest.TestCase):
             return self.pause_wait(job_id)
         return result
 
+    def case_skip(self, reason):
+        '''Skip this test case'''
+        case_notrun(reason)
+        self.skipTest(reason)
+
 
 def notrun(reason):
     '''Skip this test suite'''
@@ -813,7 +818,11 @@ def notrun(reason):
     sys.exit(0)
 
 def case_notrun(reason):
-    '''Skip this test case'''
+    '''Mark this test case as not having been run (without actually
+    skipping it, that is left to the caller).  See
+    QMPTestCase.case_skip() for a variant that actually skips the
+    current test case.'''
+
     # Each test in qemu-iotests has a number ("seq")
     seq = os.path.basename(sys.argv[0])
 
@@ -904,8 +913,15 @@ def execute_unittest(output, verbosity, debug):
         unittest.main(testRunner=runner)
     finally:
         if not debug:
-            sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s',
-                                    r'Ran \1 tests', output.getvalue()))
+            out = output.getvalue()
+            out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
+
+            # Hide skipped tests from the reference output
+            out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
+            out_first_line, out_rest = out.split('\n', 1)
+            out = out_first_line.replace('s', '.') + '\n' + out_rest
+
+            sys.stderr.write(out)
 
 def execute_test(test_function=None,
                  supported_fmts=[], supported_oses=['linux'],
-- 
2.21.0

[Qemu-devel] [PATCH v4 3/7] iotests: Use case_skip() in skip_if_unsupported()

[Max Reitz]

skip_if_unsupported() should use the stronger variant case_skip(),
because this allows it to be used even with setUp() (in a meaningful
way).

In the process, make it explicit what we expect the first argument of
the func_wrapper to be (namely something derived of QMPTestCase).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e46f3b6aed..427c34697a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -893,14 +893,14 @@ 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):
+        def func_wrapper(test_case: QMPTestCase, *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))
+                test_case.case_skip('{}: formats {} are not whitelisted'.format(
+                    test_case, usf_list))
             else:
-                return func(*args, **kwargs)
+                return func(test_case, *args, **kwargs)
         return func_wrapper
     return skip_test_decorator
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH v4 3/7] iotests: Use case_skip() in skip_if_unsupported()

[Andrey Shinkevich]

On 17/09/2019 12:20, Max Reitz wrote:
> skip_if_unsupported() should use the stronger variant case_skip(),
> because this allows it to be used even with setUp() (in a meaningful
> way).
> 
> In the process, make it explicit what we expect the first argument of
> the func_wrapper to be (namely something derived of QMPTestCase).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index e46f3b6aed..427c34697a 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -893,14 +893,14 @@ 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):
> +        def func_wrapper(test_case: QMPTestCase, *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))
> +                test_case.case_skip('{}: formats {} are not whitelisted'.format(
> +                    test_case, usf_list))
>               else:
> -                return func(*args, **kwargs)
> +                return func(test_case, *args, **kwargs)
>           return func_wrapper
>       return skip_test_decorator
>   
> 

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v4 3/7] iotests: Use case_skip() in skip_if_unsupported()

[John Snow]

On 9/17/19 5:20 AM, Max Reitz wrote:
> skip_if_unsupported() should use the stronger variant case_skip(),
> because this allows it to be used even with setUp() (in a meaningful
> way).
> 
> In the process, make it explicit what we expect the first argument of
> the func_wrapper to be (namely something derived of QMPTestCase).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index e46f3b6aed..427c34697a 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -893,14 +893,14 @@ 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):
> +        def func_wrapper(test_case: QMPTestCase, *args, **kwargs):

The dream lives!

>               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))
> +                test_case.case_skip('{}: formats {} are not whitelisted'.format(
> +                    test_case, usf_list))
>               else:
> -                return func(*args, **kwargs)
> +                return func(test_case, *args, **kwargs)
>           return func_wrapper
>       return skip_test_decorator
>   
> 

Reviewed-by: John Snow <jsnow@redhat.com>

[Qemu-devel] [PATCH v4 4/7] iotests: Let skip_if_unsupported accept a function

[Max Reitz]

This lets tests use skip_if_unsupported() with a potentially variable
list of required formats.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 427c34697a..a42702b6b9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -894,8 +894,12 @@ def skip_if_unsupported(required_formats=[], read_only=False):
        Runs the test if all the required formats are whitelisted'''
     def skip_test_decorator(func):
         def func_wrapper(test_case: QMPTestCase, *args, **kwargs):
-            usf_list = list(set(required_formats) -
-                            set(supported_formats(read_only)))
+            if callable(required_formats):
+                fmts = required_formats(test_case)
+            else:
+                fmts = required_formats
+
+            usf_list = list(set(fmts) - set(supported_formats(read_only)))
             if usf_list:
                 test_case.case_skip('{}: formats {} are not whitelisted'.format(
                     test_case, usf_list))
-- 
2.21.0

Re: [Qemu-devel] [PATCH v4 4/7] iotests: Let skip_if_unsupported accept a function

[Andrey Shinkevich]

On 17/09/2019 12:20, Max Reitz wrote:
> This lets tests use skip_if_unsupported() with a potentially variable
> list of required formats.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 427c34697a..a42702b6b9 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -894,8 +894,12 @@ def skip_if_unsupported(required_formats=[], read_only=False):
>          Runs the test if all the required formats are whitelisted'''
>       def skip_test_decorator(func):
>           def func_wrapper(test_case: QMPTestCase, *args, **kwargs):
> -            usf_list = list(set(required_formats) -
> -                            set(supported_formats(read_only)))
> +            if callable(required_formats):
> +                fmts = required_formats(test_case)
> +            else:
> +                fmts = required_formats
> +
> +            usf_list = list(set(fmts) - set(supported_formats(read_only)))
>               if usf_list:
>                   test_case.case_skip('{}: formats {} are not whitelisted'.format(
>                       test_case, usf_list))
> 

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-- 
With the best regards,
Andrey Shinkevich

[Qemu-devel] [PATCH v4 5/7] iotests: Test driver whitelisting in 093

[Max Reitz]

null-aio may not be whitelisted.  Skip all test cases that require it.

(And skip the whole test if null-co is not whitelisted.)

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/093 | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 50c1e7f2ec..f03fa24a07 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -24,7 +24,7 @@ import iotests
 nsec_per_sec = 1000000000
 
 class ThrottleTestCase(iotests.QMPTestCase):
-    test_img = "null-aio://"
+    test_driver = "null-aio"
     max_drives = 3
 
     def blockstats(self, device):
@@ -35,10 +35,14 @@ class ThrottleTestCase(iotests.QMPTestCase):
                 return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
         raise Exception("Device not found for blockstats: %s" % device)
 
+    def required_drivers(self):
+        return [self.test_driver]
+
+    @iotests.skip_if_unsupported(required_drivers)
     def setUp(self):
         self.vm = iotests.VM()
         for i in range(0, self.max_drives):
-            self.vm.add_drive(self.test_img, "file.read-zeroes=on")
+            self.vm.add_drive(self.test_driver + "://", "file.read-zeroes=on")
         self.vm.launch()
 
     def tearDown(self):
@@ -264,7 +268,7 @@ class ThrottleTestCase(iotests.QMPTestCase):
         self.assertEqual(self.blockstats('drive1')[0], 4096)
 
 class ThrottleTestCoroutine(ThrottleTestCase):
-    test_img = "null-co://"
+    test_driver = "null-co"
 
 class ThrottleTestGroupNames(iotests.QMPTestCase):
     max_drives = 3
@@ -425,4 +429,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase):
 
 
 if __name__ == '__main__':
+    if 'null-co' not in iotests.supported_formats():
+        iotests.notrun('null-co driver support missing')
     iotests.main(supported_fmts=["raw"])
-- 
2.21.0

Re: [Qemu-devel] [PATCH v4 5/7] iotests: Test driver whitelisting in 093

[Andrey Shinkevich]

On 17/09/2019 12:20, Max Reitz wrote:
> null-aio may not be whitelisted.  Skip all test cases that require it.
> 
> (And skip the whole test if null-co is not whitelisted.)
This solution has been persisting since v1. Nevertheless, I am still not 
clear why not to run the test if the null-aio is the only whitelisted 
driver? Will the null-aio be supported in future?

Andrey
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/093 | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
> index 50c1e7f2ec..f03fa24a07 100755
> --- a/tests/qemu-iotests/093
> +++ b/tests/qemu-iotests/093
> @@ -24,7 +24,7 @@ import iotests
>   nsec_per_sec = 1000000000
>   
>   class ThrottleTestCase(iotests.QMPTestCase):
> -    test_img = "null-aio://"
> +    test_driver = "null-aio"
>       max_drives = 3
>   
>       def blockstats(self, device):
> @@ -35,10 +35,14 @@ class ThrottleTestCase(iotests.QMPTestCase):
>                   return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
>           raise Exception("Device not found for blockstats: %s" % device)
>   
> +    def required_drivers(self):
> +        return [self.test_driver]
> +
> +    @iotests.skip_if_unsupported(required_drivers)
>       def setUp(self):
>           self.vm = iotests.VM()
>           for i in range(0, self.max_drives):
> -            self.vm.add_drive(self.test_img, "file.read-zeroes=on")
> +            self.vm.add_drive(self.test_driver + "://", "file.read-zeroes=on")
>           self.vm.launch()
>   
>       def tearDown(self):
> @@ -264,7 +268,7 @@ class ThrottleTestCase(iotests.QMPTestCase):
>           self.assertEqual(self.blockstats('drive1')[0], 4096)
>   
>   class ThrottleTestCoroutine(ThrottleTestCase):
> -    test_img = "null-co://"
> +    test_driver = "null-co"
>   
>   class ThrottleTestGroupNames(iotests.QMPTestCase):
>       max_drives = 3
> @@ -425,4 +429,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase):
>   
>   
>   if __name__ == '__main__':
> +    if 'null-co' not in iotests.supported_formats():
> +        iotests.notrun('null-co driver support missing')
>       iotests.main(supported_fmts=["raw"])
> 

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v4 5/7] iotests: Test driver whitelisting in 093

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 642 bytes --]

On 17.09.19 16:25, Andrey Shinkevich wrote:
> 
> 
> On 17/09/2019 12:20, Max Reitz wrote:
>> null-aio may not be whitelisted.  Skip all test cases that require it.
>>
>> (And skip the whole test if null-co is not whitelisted.)
> This solution has been persisting since v1. Nevertheless, I am still not 
> clear why not to run the test if the null-aio is the only whitelisted 
> driver? Will the null-aio be supported in future?

I don’t see a reason why we’d remove it.  But chances are, if any of the
null* drivers is whitelisted, it’ll be null-co (because we barely use
the AIO block driver interface anymore).

Max


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

Re: [Qemu-devel] [PATCH v4 5/7] iotests: Test driver whitelisting in 093

[Andrey Shinkevich]

On 18/09/2019 09:29, Max Reitz wrote:
> On 17.09.19 16:25, Andrey Shinkevich wrote:
>>
>>
>> On 17/09/2019 12:20, Max Reitz wrote:
>>> null-aio may not be whitelisted.  Skip all test cases that require it.
>>>
>>> (And skip the whole test if null-co is not whitelisted.)
>> This solution has been persisting since v1. Nevertheless, I am still not
>> clear why not to run the test if the null-aio is the only whitelisted
>> driver? Will the null-aio be supported in future?
> 
> I don’t see a reason why we’d remove it.  But chances are, if any of the
> null* drivers is whitelisted, it’ll be null-co (because we barely use
> the AIO block driver interface anymore).
> 
> Max
> 

Thank you Max.

Andrey
-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v4 5/7] iotests: Test driver whitelisting in 093

[John Snow]

On 9/17/19 5:20 AM, Max Reitz wrote:
> null-aio may not be whitelisted.  Skip all test cases that require it.
> 
> (And skip the whole test if null-co is not whitelisted.)
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Sorry for the goose chase on v(n-1).

I feel like maybe there's a nicer way to do this, but I can't figure out 
what it is, so ... my mistake.

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>   tests/qemu-iotests/093 | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
> index 50c1e7f2ec..f03fa24a07 100755
> --- a/tests/qemu-iotests/093
> +++ b/tests/qemu-iotests/093
> @@ -24,7 +24,7 @@ import iotests
>   nsec_per_sec = 1000000000
>   
>   class ThrottleTestCase(iotests.QMPTestCase):
> -    test_img = "null-aio://"
> +    test_driver = "null-aio"
>       max_drives = 3
>   
>       def blockstats(self, device):
> @@ -35,10 +35,14 @@ class ThrottleTestCase(iotests.QMPTestCase):
>                   return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
>           raise Exception("Device not found for blockstats: %s" % device)
>   
> +    def required_drivers(self):
> +        return [self.test_driver]
> +
> +    @iotests.skip_if_unsupported(required_drivers)
>       def setUp(self):
>           self.vm = iotests.VM()
>           for i in range(0, self.max_drives):
> -            self.vm.add_drive(self.test_img, "file.read-zeroes=on")
> +            self.vm.add_drive(self.test_driver + "://", "file.read-zeroes=on")
>           self.vm.launch()
>   
>       def tearDown(self):
> @@ -264,7 +268,7 @@ class ThrottleTestCase(iotests.QMPTestCase):
>           self.assertEqual(self.blockstats('drive1')[0], 4096)
>   
>   class ThrottleTestCoroutine(ThrottleTestCase):
> -    test_img = "null-co://"
> +    test_driver = "null-co"
>   
>   class ThrottleTestGroupNames(iotests.QMPTestCase):
>       max_drives = 3
> @@ -425,4 +429,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase):
>   
>   
>   if __name__ == '__main__':
> +    if 'null-co' not in iotests.supported_formats():
> +        iotests.notrun('null-co driver support missing')
>       iotests.main(supported_fmts=["raw"])
> 

-- 
—js

[Qemu-devel] [PATCH v4 6/7] iotests: Test driver whitelisting in 136

[Max Reitz]

null-aio may not be whitelisted.  Skip all test cases that require it.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/136 | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136
index a46a7b7630..012ea111ac 100755
--- a/tests/qemu-iotests/136
+++ b/tests/qemu-iotests/136
@@ -30,7 +30,7 @@ bad_offset = bad_sector * 512
 blkdebug_file = os.path.join(iotests.test_dir, 'blkdebug.conf')
 
 class BlockDeviceStatsTestCase(iotests.QMPTestCase):
-    test_img = "null-aio://"
+    test_driver = "null-aio"
     total_rd_bytes = 0
     total_rd_ops = 0
     total_wr_bytes = 0
@@ -67,6 +67,10 @@ sector = "%d"
 ''' % (bad_sector, bad_sector))
         file.close()
 
+    def required_drivers(self):
+        return [self.test_driver]
+
+    @iotests.skip_if_unsupported(required_drivers)
     def setUp(self):
         drive_args = []
         drive_args.append("stats-intervals.0=%d" % interval_length)
@@ -76,8 +80,8 @@ sector = "%d"
                           (self.account_failed and "on" or "off"))
         drive_args.append("file.image.read-zeroes=on")
         self.create_blkdebug_file()
-        self.vm = iotests.VM().add_drive('blkdebug:%s:%s' %
-                                         (blkdebug_file, self.test_img),
+        self.vm = iotests.VM().add_drive('blkdebug:%s:%s://' %
+                                         (blkdebug_file, self.test_driver),
                                          ','.join(drive_args))
         self.vm.launch()
         # Set an initial value for the clock
@@ -337,7 +341,9 @@ class BlockDeviceStatsTestAccountBoth(BlockDeviceStatsTestCase):
     account_failed = True
 
 class BlockDeviceStatsTestCoroutine(BlockDeviceStatsTestCase):
-    test_img = "null-co://"
+    test_driver = "null-co"
 
 if __name__ == '__main__':
+    if 'null-co' not in iotests.supported_formats():
+        iotests.notrun('null-co driver support missing')
     iotests.main(supported_fmts=["raw"])
-- 
2.21.0

Re: [Qemu-devel] [PATCH v4 6/7] iotests: Test driver whitelisting in 136

[Andrey Shinkevich]

On 17/09/2019 12:20, Max Reitz wrote:
> null-aio may not be whitelisted.  Skip all test cases that require it.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/136 | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136
> index a46a7b7630..012ea111ac 100755
> --- a/tests/qemu-iotests/136
> +++ b/tests/qemu-iotests/136
> @@ -30,7 +30,7 @@ bad_offset = bad_sector * 512
>   blkdebug_file = os.path.join(iotests.test_dir, 'blkdebug.conf')
>   
>   class BlockDeviceStatsTestCase(iotests.QMPTestCase):
> -    test_img = "null-aio://"
> +    test_driver = "null-aio"
>       total_rd_bytes = 0
>       total_rd_ops = 0
>       total_wr_bytes = 0
> @@ -67,6 +67,10 @@ sector = "%d"
>   ''' % (bad_sector, bad_sector))
>           file.close()
>   
> +    def required_drivers(self):
> +        return [self.test_driver]
> +
> +    @iotests.skip_if_unsupported(required_drivers)
>       def setUp(self):
>           drive_args = []
>           drive_args.append("stats-intervals.0=%d" % interval_length)
> @@ -76,8 +80,8 @@ sector = "%d"
>                             (self.account_failed and "on" or "off"))
>           drive_args.append("file.image.read-zeroes=on")
>           self.create_blkdebug_file()
> -        self.vm = iotests.VM().add_drive('blkdebug:%s:%s' %
> -                                         (blkdebug_file, self.test_img),
> +        self.vm = iotests.VM().add_drive('blkdebug:%s:%s://' %
> +                                         (blkdebug_file, self.test_driver),
>                                            ','.join(drive_args))
>           self.vm.launch()
>           # Set an initial value for the clock
> @@ -337,7 +341,9 @@ class BlockDeviceStatsTestAccountBoth(BlockDeviceStatsTestCase):
>       account_failed = True
>   
>   class BlockDeviceStatsTestCoroutine(BlockDeviceStatsTestCase):
> -    test_img = "null-co://"
> +    test_driver = "null-co"
>   
>   if __name__ == '__main__':
> +    if 'null-co' not in iotests.supported_formats():
> +        iotests.notrun('null-co driver support missing')
>       iotests.main(supported_fmts=["raw"])
> 

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v4 6/7] iotests: Test driver whitelisting in 136

[John Snow]

On 9/17/19 5:20 AM, Max Reitz wrote:
> null-aio may not be whitelisted.  Skip all test cases that require it.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>   tests/qemu-iotests/136 | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136
> index a46a7b7630..012ea111ac 100755
> --- a/tests/qemu-iotests/136
> +++ b/tests/qemu-iotests/136
> @@ -30,7 +30,7 @@ bad_offset = bad_sector * 512
>   blkdebug_file = os.path.join(iotests.test_dir, 'blkdebug.conf')
>   
>   class BlockDeviceStatsTestCase(iotests.QMPTestCase):
> -    test_img = "null-aio://"
> +    test_driver = "null-aio"
>       total_rd_bytes = 0
>       total_rd_ops = 0
>       total_wr_bytes = 0
> @@ -67,6 +67,10 @@ sector = "%d"
>   ''' % (bad_sector, bad_sector))
>           file.close()
>   
> +    def required_drivers(self):
> +        return [self.test_driver]
> +
> +    @iotests.skip_if_unsupported(required_drivers)
>       def setUp(self):
>           drive_args = []
>           drive_args.append("stats-intervals.0=%d" % interval_length)
> @@ -76,8 +80,8 @@ sector = "%d"
>                             (self.account_failed and "on" or "off"))
>           drive_args.append("file.image.read-zeroes=on")
>           self.create_blkdebug_file()
> -        self.vm = iotests.VM().add_drive('blkdebug:%s:%s' %
> -                                         (blkdebug_file, self.test_img),
> +        self.vm = iotests.VM().add_drive('blkdebug:%s:%s://' %
> +                                         (blkdebug_file, self.test_driver),
>                                            ','.join(drive_args))
>           self.vm.launch()
>           # Set an initial value for the clock
> @@ -337,7 +341,9 @@ class BlockDeviceStatsTestAccountBoth(BlockDeviceStatsTestCase):
>       account_failed = True
>   
>   class BlockDeviceStatsTestCoroutine(BlockDeviceStatsTestCase):
> -    test_img = "null-co://"
> +    test_driver = "null-co"
>   
>   if __name__ == '__main__':
> +    if 'null-co' not in iotests.supported_formats():
> +        iotests.notrun('null-co driver support missing')
>       iotests.main(supported_fmts=["raw"])
> 

-- 
—js

[Qemu-devel] [PATCH v4 7/7] iotests: Cache supported_formats()

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index a42702b6b9..b30044e1cc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -885,9 +885,17 @@ def qemu_pipe(*args):
 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")
-    line = 1 if read_only else 0
-    return format_message.splitlines()[line].split(":")[1].split()
+
+    if not hasattr(supported_formats, "formats"):
+        supported_formats.formats = {}
+
+    if read_only not in supported_formats.formats:
+        format_message = qemu_pipe("-drive", "format=help")
+        line = 1 if read_only else 0
+        supported_formats.formats[read_only] = \
+            format_message.splitlines()[line].split(":")[1].split()
+
+    return supported_formats.formats[read_only]
 
 def skip_if_unsupported(required_formats=[], read_only=False):
     '''Skip Test Decorator
-- 
2.21.0

Re: [Qemu-devel] [PATCH v4 7/7] iotests: Cache supported_formats()

[Andrey Shinkevich]

On 17/09/2019 12:20, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index a42702b6b9..b30044e1cc 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -885,9 +885,17 @@ def qemu_pipe(*args):
>   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")
> -    line = 1 if read_only else 0
> -    return format_message.splitlines()[line].split(":")[1].split()
> +
> +    if not hasattr(supported_formats, "formats"):
> +        supported_formats.formats = {}
> +
> +    if read_only not in supported_formats.formats:
> +        format_message = qemu_pipe("-drive", "format=help")
> +        line = 1 if read_only else 0
> +        supported_formats.formats[read_only] = \
> +            format_message.splitlines()[line].split(":")[1].split()
> +
> +    return supported_formats.formats[read_only]
>   
>   def skip_if_unsupported(required_formats=[], read_only=False):
>       '''Skip Test Decorator
> 

Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
-- 
With the best regards,
Andrey Shinkevich

Re: [PATCH v4 0/7] iotests: Selfish patches

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 1342 bytes --]

On 17.09.19 11:19, Max Reitz wrote:
> Hi,
> 
> Again, let me start with a link to an actually explanatory cover letter:
> https://lists.nongnu.org/archive/html/qemu-block/2019-06/msg01102.html
> 
> v3:
> https://lists.nongnu.org/archive/html/qemu-block/2019-08/msg00950.html
> 
> v4:
> - I merged the old patch 1 in the meantime
> 
> - Patch 2: Adjusted the comment to make it more clear that it is
>            case_notrun() itself that will not skip the test case, as
>            requested by Andrey (I hope it fits what he had in mind, more
>            or less); kept the R-bs, because I somehow feel like that’s
>            the right thing to do here.
> 
> - Patch 3: The func_wrapper returned by the skip_test_decorator has a
>            mandatory argument; make that and its required type explicit
>            (with an annotation), as suggested by John
>            (Kevin made me aware of the fact that annotations exist since
>            Python 3.0, it’s just that they didn’t mean anything back
>            then (neither do they really now, but whatever, it’s better
>            than a comment))
> 
> - Patch 4: Resolved a conflict because of the change to patch 3

Thanks for the reviews, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max


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

Re: [PATCH v4 0/7] iotests: Selfish patches

[Andrey Shinkevich]

On 14/10/2019 17:59, Max Reitz wrote:
> On 17.09.19 11:19, Max Reitz wrote:
>> Hi,
>>
>> Again, let me start with a link to an actually explanatory cover letter:
>> https://lists.nongnu.org/archive/html/qemu-block/2019-06/msg01102.html
>>
>> v3:
>> https://lists.nongnu.org/archive/html/qemu-block/2019-08/msg00950.html
>>
>> v4:
>> - I merged the old patch 1 in the meantime
>>
>> - Patch 2: Adjusted the comment to make it more clear that it is
>>             case_notrun() itself that will not skip the test case, as
>>             requested by Andrey (I hope it fits what he had in mind, more
>>             or less); kept the R-bs, because I somehow feel like that’s
>>             the right thing to do here.
>>
>> - Patch 3: The func_wrapper returned by the skip_test_decorator has a
>>             mandatory argument; make that and its required type explicit
>>             (with an annotation), as suggested by John
>>             (Kevin made me aware of the fact that annotations exist since
>>             Python 3.0, it’s just that they didn’t mean anything back
>>             then (neither do they really now, but whatever, it’s better
>>             than a comment))
>>
>> - Patch 4: Resolved a conflict because of the change to patch 3
> 
> Thanks for the reviews, applied to my block branch:
> 
> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
> 
> Max
> 

Good
-- 
With the best regards,
Andrey Shinkevich