qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 2/2] tests: add virtio-scsi and virtio-blk seg_max_adjust


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v5 2/2] tests: add virtio-scsi and virtio-blk seg_max_adjust test
Date: Wed, 22 Jan 2020 22:47:42 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2

Cc'ing Cornelia now ...

On 1/22/20 9:56 PM, Philippe Mathieu-Daudé wrote:
Hello,

On 12/20/19 3:09 PM, Denis Plotnikov wrote:
It tests proper seg_max_adjust settings for all machine types except
'none', 'isapc', 'microvm'

Signed-off-by: Denis Plotnikov <address@hidden>
---
  tests/acceptance/virtio_seg_max_adjust.py | 134 ++++++++++++++++++++++
  1 file changed, 134 insertions(+)
  create mode 100755 tests/acceptance/virtio_seg_max_adjust.py

diff --git a/tests/acceptance/virtio_seg_max_adjust.py b/tests/acceptance/virtio_seg_max_adjust.py
new file mode 100755
index 0000000000..5458573138
--- /dev/null
+++ b/tests/acceptance/virtio_seg_max_adjust.py
@@ -0,0 +1,134 @@
+#!/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
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.machine import QEMUMachine
+from avocado_qemu import Test
+
+#list of machine types and virtqueue properties to test
+VIRTIO_SCSI_PROPS = {'seg_max_adjust': 'seg_max_adjust'}
+VIRTIO_BLK_PROPS = {'seg_max_adjust': 'seg-max-adjust'}
+
+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']}
+
+
+class VirtioMaxSegSettingsCheck(Test):
+    @staticmethod
+    def make_pattern(props):
+        pattern_items = ['{0} = \w+'.format(prop) for prop in props]
+        return '|'.join(pattern_items)
+
+    def query_virtqueue(self, vm, dev_type_name):
+        query_ok = False
+        error = None
+        props = None
+
+        output = vm.command('human-monitor-command',
+                            command_line = 'info qtree')
+        props_list = DEV_TYPES[dev_type_name].values();
+        pattern = self.make_pattern(props_list)
+        res = re.findall(pattern, output)
+
+        if len(res) != len(props_list):
+            props_list = set(props_list)
+            res = set(res)
+            not_found = props_list.difference(res)
+            not_found = ', '.join(not_found)
+            error = '({0}): The following properties not found: {1}'\
+                     .format(dev_type_name, not_found)
+        else:
+            query_ok = True
+            props = dict()
+            for prop in res:
+                p = prop.split(' = ')
+                props[p[0]] = p[1]
+        return query_ok, props, error
+
+    def check_mt(self, mt, dev_type_name):
+        with QEMUMachine(self.qemu_bin) as vm:
+            vm.set_machine(mt["name"])
+            for s in VM_DEV_PARAMS[dev_type_name]:
+                vm.add_args(s)
+            vm.launch()
+            query_ok, props, error = self.query_virtqueue(vm, dev_type_name)
+
+        if not query_ok:
+            self.fail('machine type {0}: {1}'.format(mt['name'], error))
+
+        for prop_name, prop_val in props.items():
+            expected_val = mt[prop_name]
+            self.assertEqual(expected_val, prop_val)
+
+    @staticmethod
+    def seg_max_adjust_enabled(mt):
+        # machine types >= 5.0 should have seg_max_adjust = true
+        # others seg_max_adjust = false
+        mt = mt.split("-")
+
+        # machine types with one line name and name 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(".");
+
+        # versions >= 5.0 goes with seg_max_adjust enabled
+        major = int(ver[0])
+
+        if major >= 5:
+            return True
+        return False
+
+    def test_machine_types(self):
+        # collect all machine types except 'none', 'isapc', 'microvm'
+        with QEMUMachine(self.qemu_bin) as vm:
+            vm.launch()
+            machines = [m['name'] for m in vm.command('query-machines')]
+            vm.shutdown()
+        machines.remove('none')
+        machines.remove('isapc')
+        machines.remove('microvm')
+
+        for dev_type in DEV_TYPES:
+            # create the list of machine types and their parameters.
+            mtypes = list()
+            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)

This test is failing on OSX:

TestFail: machine type pc-i440fx-2.0: <class 'TypeError'>

Looking at my job-results/job-2020-01-22T17.54-92b7fae/job.log:

Unexpected error in object_property_find() at qom/object.c:1201:
qemu-system-x86_64: -device virtio-blk-pci,id=scsi0,drive=drive0: can't apply global virtio-blk-device.scsi=true: Property '.scsi' not found

Which makes sense looking at hw/block/virtio-blk.c:

1261 static Property virtio_blk_properties[] = {
1262     DEFINE_BLOCK_PROPERTIES(VirtIOBlock, conf.conf),
...
1268 #ifdef __linux__
1269     DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features,
1270                       VIRTIO_BLK_F_SCSI, false),
1271 #endif

Except code moved around, origin is:

$ git show 1063b8b15
commit 1063b8b15fb49fcf88ffa282b19aaaf7ca9c678c
Author: Christoph Hellwig <address@hidden>
Date:   Mon Apr 27 10:29:14 2009 +0200

     virtio-blk: add SGI_IO passthru support

     Add support for SG_IO passthru (packet commands) to the virtio-blk
     backend.  Conceptually based on an older patch from Hannes Reinecke
     but largely rewritten to match the code structure and layering in
     virtio-blk.

     Note that currently we issue the hose SG_IO synchronously.  We could
     easily switch to async I/O, but that would required either bloating
     the VirtIOBlockReq by the size of struct sg_io_hdr or an additional
     memory allocation for each SG_IO request.

I'm not sure what is the correct way to fix this.

... because of:

$ git show ed65fd1a27
commit ed65fd1a2750d24290354cc7ea49caec7c13e30b
Author: Cornelia Huck <address@hidden>
Date:   Fri Oct 16 12:25:54 2015 +0200

    virtio-blk: switch off scsi-passthrough by default

    Devices that are compliant with virtio-1 do not support scsi
    passthrough any more (and it has not been a recommended setup
    anyway for quite some time). To avoid having to switch it off
    explicitly in newer qemus that turn on virtio-1 by default, let's
    switch the default to scsi=false for 2.5.

    Signed-off-by: Cornelia Huck <address@hidden>
    Message-id: address@hidden
    Signed-off-by: Stefan Hajnoczi <address@hidden>

diff --git a/include/hw/compat.h b/include/hw/compat.h
index 095de5d12f..93e71afb4a 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,11 @@
 #define HW_COMPAT_H

 #define HW_COMPAT_2_4 \
-        /* empty */
+        {\
+            .driver   = "virtio-blk-device",\
+            .property = "scsi",\
+            .value    = "true",\
+        },

 #define HW_COMPAT_2_3 \
         {\
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 3e230debb8..45a24e4fa6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -972,7 +972,7 @@ static Property virtio_blk_properties[] = {
     DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),
     DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true),
 #ifdef __linux__
-    DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, true),
+    DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false),
 #endif
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
                     true),

Should this HW_COMPAT_2_4 entry be guarded with ifdef __linux__?

Probably nobody ran a pre-2.4 machine out of Linux =)




reply via email to

[Prev in Thread] Current Thread [Next in Thread]