[PATCH v1 0/2] Improve virtio_check_params test

[PATCH v1 0/2] Improve virtio_check_params test

[Denis Plotnikov]

* fixed failing on non-existed machine type removal
* the test refactored to add more parameters to check

Gereral questions left:
   How to restric test for using:
   1. on a set of target OS-es
   2. on a set target architectures
  

Denis Plotnikov (2):
  tests/acceptance/virtio_check_params: remove excluded machine types
    carefully
  tests/acceptance/virtio_check_params: prepare to check different
    params

 tests/acceptance/virtio_check_params.py | 52 ++++++++++++++++---------
 1 file changed, 33 insertions(+), 19 deletions(-)

-- 
2.17.0

[PATCH v1 1/2] tests/acceptance/virtio_check_params: remove excluded machine types carefully

[Denis Plotnikov]

Before, the test failed if an excluded machine type was absent in the machine
types lists.

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
---
 tests/acceptance/virtio_check_params.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/acceptance/virtio_check_params.py b/tests/acceptance/virtio_check_params.py
index 87e6c839d1..deec89bf86 100644
--- a/tests/acceptance/virtio_check_params.py
+++ b/tests/acceptance/virtio_check_params.py
@@ -40,6 +40,8 @@ VM_DEV_PARAMS = {'virtio-scsi-pci': ['-device', 'virtio-scsi-pci,id=scsi0'],
                                     '-drive',
                                     'driver=null-co,id=drive0,if=none']}
 
+EXCLUDED_MACHINES = ['none', 'isapc', 'microvm']
+
 
 class VirtioMaxSegSettingsCheck(Test):
     @staticmethod
@@ -117,6 +119,13 @@ class VirtioMaxSegSettingsCheck(Test):
             return True
         return False
 
+    @staticmethod
+    def filter_machines(machines):
+        for mt in EXCLUDED_MACHINES:
+            if mt in machines:
+                machines.remove(mt)
+        return machines
+
     @skip("break multi-arch CI")
     def test_machine_types(self):
         # collect all machine types except 'none', 'isapc', 'microvm'
@@ -124,9 +133,8 @@ class VirtioMaxSegSettingsCheck(Test):
             vm.launch()
             machines = [m['name'] for m in vm.command('query-machines')]
             vm.shutdown()
-        machines.remove('none')
-        machines.remove('isapc')
-        machines.remove('microvm')
+
+        machines = self.filter_machines(machines)
 
         for dev_type in DEV_TYPES:
             # create the list of machine types and their parameters.
-- 
2.17.0

[PATCH v1 2/2] tests/acceptance/virtio_check_params: prepare to check different params

[Denis Plotnikov]

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
---
 tests/acceptance/virtio_check_params.py | 38 ++++++++++++++-----------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/tests/acceptance/virtio_check_params.py b/tests/acceptance/virtio_check_params.py
index deec89bf86..e578952a97 100644
--- a/tests/acceptance/virtio_check_params.py
+++ b/tests/acceptance/virtio_check_params.py
@@ -43,7 +43,7 @@ VM_DEV_PARAMS = {'virtio-scsi-pci': ['-device', 'virtio-scsi-pci,id=scsi0'],
 EXCLUDED_MACHINES = ['none', 'isapc', 'microvm']
 
 
-class VirtioMaxSegSettingsCheck(Test):
+class VirtioParamsCheck(Test):
     @staticmethod
     def make_pattern(props):
         pattern_items = ['{0} = \w+'.format(prop) for prop in props]
@@ -75,12 +75,12 @@ class VirtioMaxSegSettingsCheck(Test):
                 props[p[0]] = p[1]
         return query_ok, props, error
 
-    def check_mt(self, mt, dev_type_name):
-        mt['device'] = dev_type_name # Only for the debug() call.
+    def check_mt(self, mt, expected_vals, dev_type_name):
+        msg = "mt: %s dev: %s" % (mt, dev_type_name) # For debug() call only.
         logger = logging.getLogger('machine')
-        logger.debug(mt)
+        logger.debug(msg)
         with QEMUMachine(self.qemu_bin) as vm:
-            vm.set_machine(mt["name"])
+            vm.set_machine(mt)
             vm.add_args('-nodefaults')
             for s in VM_DEV_PARAMS[dev_type_name]:
                 vm.add_args(s)
@@ -92,11 +92,15 @@ class VirtioMaxSegSettingsCheck(Test):
                 error = sys.exc_info()[0]
 
         if not query_ok:
-            self.fail('machine type {0}: {1}'.format(mt['name'], error))
+            self.fail('machine type {0}: {1}'.format(mt, error))
 
         for prop_name, prop_val in props.items():
-            expected_val = mt[prop_name]
-            self.assertEqual(expected_val, prop_val)
+            expected_val = expected_vals[prop_name]
+            msg = 'Property value mismatch for (MT: {0}, '\
+                  'property name: {1}): expected value: "{2}" '\
+                  'actual value: "{3}"'\
+                  .format(mt, prop_name, expected_val, prop_val)
+            self.assertEqual(expected_val, prop_val, msg)
 
     @staticmethod
     def seg_max_adjust_enabled(mt):
@@ -128,25 +132,27 @@ class VirtioMaxSegSettingsCheck(Test):
 
     @skip("break multi-arch CI")
     def test_machine_types(self):
-        # collect all machine types except 'none', 'isapc', 'microvm'
+        # collect all machine types
         with QEMUMachine(self.qemu_bin) as vm:
             vm.launch()
             machines = [m['name'] for m in vm.command('query-machines')]
             vm.shutdown()
 
+        # ..and exclude non-relevant ones
         machines = self.filter_machines(machines)
 
         for dev_type in DEV_TYPES:
-            # create the list of machine types and their parameters.
-            mtypes = list()
+            # define expected parameters for each machine type
+            mt_expected_vals = dict()
             for m in machines:
                 if self.seg_max_adjust_enabled(m):
                     enabled = 'true'
                 else:
                     enabled = 'false'
-                mtypes.append({'name': m,
-                               DEV_TYPES[dev_type]['seg_max_adjust']: enabled})
 
-            # test each machine type for a device type
-            for mt in mtypes:
-                self.check_mt(mt, dev_type)
+                mt_expected_vals[m] = {
+                    DEV_TYPES[dev_type]['seg_max_adjust']: enabled }
+
+            # test each machine type
+            for mt in mt_expected_vals:
+                self.check_mt(mt, mt_expected_vals[mt], dev_type)
-- 
2.17.0

Re: [PATCH v1 0/2] Improve virtio_check_params test

[Philippe Mathieu-Daudé]

Hi Denis,

On 2/11/20 3:25 PM, Denis Plotnikov wrote:
> * fixed failing on non-existed machine type removal
> * the test refactored to add more parameters to check
> 
> Gereral questions left:
>     How to restric test for using:
>     1. on a set of target OS-es
>     2. on a set target architectures
>    
> 
> Denis Plotnikov (2):
>    tests/acceptance/virtio_check_params: remove excluded machine types
>      carefully
>    tests/acceptance/virtio_check_params: prepare to check different
>      params
> 
>   tests/acceptance/virtio_check_params.py | 52 ++++++++++++++++---------
>   1 file changed, 33 insertions(+), 19 deletions(-)
> 

Have you noticed my other series suggested by Cornelia?

It runs your test on S390X and PPC:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675092.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675095.html

Re: [PATCH v1 0/2] Improve virtio_check_params test

[Denis Plotnikov]

On 11.02.2020 17:37, Philippe Mathieu-Daudé wrote:
> Hi Denis,
>
> On 2/11/20 3:25 PM, Denis Plotnikov wrote:
>> * fixed failing on non-existed machine type removal
>> * the test refactored to add more parameters to check
>>
>> Gereral questions left:
>>     How to restric test for using:
>>     1. on a set of target OS-es
>>     2. on a set target architectures
>>
>> Denis Plotnikov (2):
>>    tests/acceptance/virtio_check_params: remove excluded machine types
>>      carefully
>>    tests/acceptance/virtio_check_params: prepare to check different
>>      params
>>
>>   tests/acceptance/virtio_check_params.py | 52 ++++++++++++++++---------
>>   1 file changed, 33 insertions(+), 19 deletions(-)
>>
>
> Have you noticed my other series suggested by Cornelia?
>
> It runs your test on S390X and PPC:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg675092.html
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg675095.html
>
> Hi, Philippe
Seems that I've missed them. I just made patches upon the fresh master.
Can I get a git tree which has those patches applied? Or should I wait 
while the patches landed to qemu master and the rebase on them?

Denis