[PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings

Denis Plotnikov posted 4 patches 6 years, 3 months ago
There is a newer version of this series
[PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings
Posted by Denis Plotnikov 6 years, 3 months ago
It tests proper queue size settings for all available machine types.

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
---
 tests/qemu-iotests/267     | 154 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/267.out |   1 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 156 insertions(+)
 create mode 100755 tests/qemu-iotests/267
 create mode 100644 tests/qemu-iotests/267.out

diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
new file mode 100755
index 0000000000..6d3cc574b3
--- /dev/null
+++ b/tests/qemu-iotests/267
@@ -0,0 +1,154 @@
+#!/usr/bin/env python
+#
+# Test virtio-scsi and virtio-blk queue settings for all machine types
+#
+# Copyright (c) 2019 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import sys
+import os
+import re
+import iotests
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts'))
+import qemu
+
+# list of device types and virtqueue properties to test
+# this is a generalized approach
+# for now we just check queue length
+# more properties can be added in each list
+virtio_scsi_props = {'vq_size': 'virtqueue_size'}
+virtio_blk_props = {'vq_size': 'queue-size'}
+
+dev_types = {'virtio-scsi-pci': virtio_scsi_props,
+             'virtio-blk-pci': virtio_blk_props}
+
+vm_dev_params = {'virtio-scsi-pci': ['-device', 'virtio-scsi-pci,id=scsi0'],
+                 'virtio-blk-pci': ['-device',
+                                    'virtio-blk-pci,id=scsi0,drive=drive0',
+                                    '-drive',
+                                    'driver=null-co,id=drive0,if=none']}
+
+def make_pattern(props):
+     pattern_items = ['{0} = \d+'.format(prop) for prop in props]
+     return '|'.join(pattern_items)
+
+
+def query_virtqueue_props(vm, dev_type_name):
+    output = vm.qmp('human-monitor-command', command_line='info qtree')
+    output = output['return']
+
+    props_list = dev_types[dev_type_name].values();
+
+    pattern = make_pattern(props_list)
+
+    res = re.findall(pattern, output)
+
+    if len(res) != len(props_list):
+        not_found = props_list.difference(set(res))
+
+        ret = (0, '({0}): The following properties not found: {1}'
+                  .format(dev_type_name, ', '.join(not_found)))
+    else:
+        props = dict()
+        for prop in res:
+            p = prop.split(' = ')
+            props[p[0]] = int(p[1])
+        ret = (1, props)
+
+    return ret
+
+
+def check_mt(mt, dev_type_name):
+    vm_params = ['-machine', mt['name']] + vm_dev_params[dev_type_name]
+
+    vm = qemu.QEMUMachine(iotests.qemu_prog, vm_params)
+    vm.launch()
+    ret = query_virtqueue_props(vm, dev_type_name)
+    vm.shutdown()
+
+    if ret[0] == 0:
+        print('Error ({0}): {1}'.format(mt['name'], ret[1]))
+        return 1
+
+    errors = 0
+    props = ret[1]
+
+    for prop_name, prop_val in props.items():
+        if mt[prop_name] != prop_val:
+            print('Error [{0}, {1}]: {2}={3} (expected {4})'.
+                  format(mt['name'], dev_type_name, prop_name, prop_val,
+                         mt[prop_name]))
+            errors += 1
+
+    return errors
+
+def is_256_virtqueue_size_mt(mt):
+    mt = mt.split("-")
+
+    # machine types like pc-x.x
+    if len(mt) == 2:
+        return False
+
+    # machine types like pc-<chip_name>-x.x[.x]
+    ver = mt[2]
+
+    ver = ver.split(".");
+
+    # all versions greater than 4.0.1 goes with 256 queue size
+    if int(ver[0]) >= 4:
+        major = int(ver[1])
+        minor = 0
+        if len(ver) > 2:
+            minor = int(ver[2])
+
+        if major > 0 or minor > 1:
+             return True
+
+    return False
+
+
+# collect all machine types except 'none'
+vm = iotests.VM()
+vm.launch()
+machines = [m['name'] for m in vm.qmp('query-machines')['return']]
+vm.shutdown()
+machines.remove('none')
+machines.remove('isapc')
+
+failed = 0
+
+for dev_type in dev_types:
+    # create a list of machine types and their parameters
+    # machine types vz8.X.X must have virtqueue_length=256
+    # others must have virtqueue_length=128
+    mtypes = list()
+    for m in machines:
+        if is_256_virtqueue_size_mt(m):
+            vq_size = 256
+        else:
+            vq_size = 128
+
+        mtypes.append({'name': m, dev_types[dev_type]['vq_size']: vq_size})
+
+    # test each machine type
+    for mt in mtypes:
+        failed += check_mt(mt, dev_type)
+
+if failed > 0:
+    print('Failed')
+else:
+    print('Success')
diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
new file mode 100644
index 0000000000..35821117c8
--- /dev/null
+++ b/tests/qemu-iotests/267.out
@@ -0,0 +1 @@
+Success
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 3605796bb2..ab8523ad60 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -252,3 +252,4 @@
 255 rw auto quick
 256 rw auto quick
 266 rw quick
+267 auto quick
-- 
2.17.0


Re: [PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings
Posted by Stefan Hajnoczi 6 years, 3 months ago
On Tue, Nov 05, 2019 at 07:11:05PM +0300, Denis Plotnikov wrote:
> It tests proper queue size settings for all available machine types.
> 
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> ---
>  tests/qemu-iotests/267     | 154 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/267.out |   1 +
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 156 insertions(+)
>  create mode 100755 tests/qemu-iotests/267
>  create mode 100644 tests/qemu-iotests/267.out

The qemu-iotests maintainers might prefer for this to be at the
top-level in tests/ since it's not really an iotest, but the code itself
looks fine to me:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings
Posted by Max Reitz 6 years, 3 months ago
On 06.11.19 10:24, Stefan Hajnoczi wrote:
> On Tue, Nov 05, 2019 at 07:11:05PM +0300, Denis Plotnikov wrote:
>> It tests proper queue size settings for all available machine types.
>>
>> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>> ---
>>  tests/qemu-iotests/267     | 154 +++++++++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/267.out |   1 +
>>  tests/qemu-iotests/group   |   1 +
>>  3 files changed, 156 insertions(+)
>>  create mode 100755 tests/qemu-iotests/267
>>  create mode 100644 tests/qemu-iotests/267.out
> 
> The qemu-iotests maintainers might prefer for this to be at the
> top-level in tests/ since it's not really an iotest, but the code itself
> looks fine to me:
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Good question.  I don’t really mind, but it would be weird if started
adding all kinds of “external” qemu tests (i.e. that use QMP) in the
iotests directory.

What is the alternative?  Just putting it in a different directory
doesn’t sound that appealing to me either, because it would still depend
on the iotests infrastructure, right?  (i.e., iotests.py and check)

Max

Re: [PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings
Posted by Eduardo Habkost 6 years, 3 months ago
On Wed, Nov 06, 2019 at 11:04:16AM +0100, Max Reitz wrote:
> On 06.11.19 10:24, Stefan Hajnoczi wrote:
> > On Tue, Nov 05, 2019 at 07:11:05PM +0300, Denis Plotnikov wrote:
> >> It tests proper queue size settings for all available machine types.
> >>
> >> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> >> ---
> >>  tests/qemu-iotests/267     | 154 +++++++++++++++++++++++++++++++++++++
> >>  tests/qemu-iotests/267.out |   1 +
> >>  tests/qemu-iotests/group   |   1 +
> >>  3 files changed, 156 insertions(+)
> >>  create mode 100755 tests/qemu-iotests/267
> >>  create mode 100644 tests/qemu-iotests/267.out
> > 
> > The qemu-iotests maintainers might prefer for this to be at the
> > top-level in tests/ since it's not really an iotest, but the code itself
> > looks fine to me:
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Good question.  I don’t really mind, but it would be weird if started
> adding all kinds of “external” qemu tests (i.e. that use QMP) in the
> iotests directory.
> 
> What is the alternative?  Just putting it in a different directory
> doesn’t sound that appealing to me either, because it would still depend
> on the iotests infrastructure, right?  (i.e., iotests.py and check)

We do have tests/acceptance for simple test cases written in
Python.  What's the reason for this test case to depend on the
iotests infrastructure?

-- 
Eduardo


Re: [PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings
Posted by Cleber Rosa 6 years, 3 months ago
On Wed, Nov 06, 2019 at 04:26:41PM -0300, Eduardo Habkost wrote:
> On Wed, Nov 06, 2019 at 11:04:16AM +0100, Max Reitz wrote:
> > On 06.11.19 10:24, Stefan Hajnoczi wrote:
> > > On Tue, Nov 05, 2019 at 07:11:05PM +0300, Denis Plotnikov wrote:
> > >> It tests proper queue size settings for all available machine types.
> > >>
> > >> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> > >> ---
> > >>  tests/qemu-iotests/267     | 154 +++++++++++++++++++++++++++++++++++++
> > >>  tests/qemu-iotests/267.out |   1 +
> > >>  tests/qemu-iotests/group   |   1 +
> > >>  3 files changed, 156 insertions(+)
> > >>  create mode 100755 tests/qemu-iotests/267
> > >>  create mode 100644 tests/qemu-iotests/267.out
> > > 
> > > The qemu-iotests maintainers might prefer for this to be at the
> > > top-level in tests/ since it's not really an iotest, but the code itself
> > > looks fine to me:
> > > 
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > Good question.  I don’t really mind, but it would be weird if started
> > adding all kinds of “external” qemu tests (i.e. that use QMP) in the
> > iotests directory.
> > 
> > What is the alternative?  Just putting it in a different directory
> > doesn’t sound that appealing to me either, because it would still depend
> > on the iotests infrastructure, right?  (i.e., iotests.py and check)
> 
> We do have tests/acceptance for simple test cases written in
> Python.  What's the reason for this test case to depend on the
> iotests infrastructure?
> 
> -- 
> Eduardo

This test does look similar in spirit to "tests/acceptance/virtio_version.py".

Denis,

If you think this is more of a generic test than an IO test, and would
rather want to have it a more agnostic location, I can provide you
with tips (or a patch) to do so.

Cheers,
- Cleber.


Re: [PATCH v1 4/4] iotests: add test for virtio-scsi and virtio-blk machine type settings
Posted by Denis Plotnikov 6 years, 3 months ago
On 07.11.2019 19:30, Cleber Rosa wrote:
> On Wed, Nov 06, 2019 at 04:26:41PM -0300, Eduardo Habkost wrote:
>> On Wed, Nov 06, 2019 at 11:04:16AM +0100, Max Reitz wrote:
>>> On 06.11.19 10:24, Stefan Hajnoczi wrote:
>>>> On Tue, Nov 05, 2019 at 07:11:05PM +0300, Denis Plotnikov wrote:
>>>>> It tests proper queue size settings for all available machine types.
>>>>>
>>>>> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>>>>> ---
>>>>>   tests/qemu-iotests/267     | 154 +++++++++++++++++++++++++++++++++++++
>>>>>   tests/qemu-iotests/267.out |   1 +
>>>>>   tests/qemu-iotests/group   |   1 +
>>>>>   3 files changed, 156 insertions(+)
>>>>>   create mode 100755 tests/qemu-iotests/267
>>>>>   create mode 100644 tests/qemu-iotests/267.out
>>>> The qemu-iotests maintainers might prefer for this to be at the
>>>> top-level in tests/ since it's not really an iotest, but the code itself
>>>> looks fine to me:
>>>>
>>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> Good question.  I don’t really mind, but it would be weird if started
>>> adding all kinds of “external” qemu tests (i.e. that use QMP) in the
>>> iotests directory.
>>>
>>> What is the alternative?  Just putting it in a different directory
>>> doesn’t sound that appealing to me either, because it would still depend
>>> on the iotests infrastructure, right?  (i.e., iotests.py and check)
>> We do have tests/acceptance for simple test cases written in
>> Python.  What's the reason for this test case to depend on the
>> iotests infrastructure?
>>
>> -- 
>> Eduardo
> This test does look similar in spirit to "tests/acceptance/virtio_version.py".
>
> Denis,
>
> If you think this is more of a generic test than an IO test, and would
> rather want to have it a more agnostic location, I can provide you
> with tips (or a patch) to do so.

It would be great! Thanks!

Denis

>
> Cheers,
> - Cleber.
>