It tests proper seg_max_adjust settings for all machine types except
'none', 'isapc', 'microvm'
Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
---
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)
--
2.17.0
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 <dplotnikov@virtuozzo.com>
> ---
> 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 <hch@lst.de>
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.
Regards,
Phil.
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 <dplotnikov@virtuozzo.com>
>> ---
>> 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 <hch@lst.de>
> 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 <cornelia.huck@de.ibm.com>
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 <cornelia.huck@de.ibm.com>
Message-id: 1444991154-79217-4-git-send-email-cornelia.huck@de.ibm.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
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 =)
On Wed, 22 Jan 2020 22:47:42 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > 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 <hch@lst.de>
> > 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 <cornelia.huck@de.ibm.com>
> 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 <cornelia.huck@de.ibm.com>
> Message-id: 1444991154-79217-4-git-send-email-cornelia.huck@de.ibm.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> 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",\
> + },
This code has changed a lot in the meantime...
>
> #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__?
... so something like the following might do the trick:
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 3e288bfceb7f..d8e30e4895d8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -148,7 +148,8 @@ GlobalProperty hw_compat_2_5[] = {
const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
GlobalProperty hw_compat_2_4[] = {
- { "virtio-blk-device", "scsi", "true" },
+ /* Optional because the 'scsi' property is Linux-only */
+ { "virtio-blk-device", "scsi", "true", .optional = true },
{ "e1000", "extra_mac_registers", "off" },
{ "virtio-pci", "x-disable-pcie", "on" },
{ "virtio-pci", "migrate-extra", "off" },
>
> Probably nobody ran a pre-2.4 machine out of Linux =)
>
Yeah. I'm wondering if there's more compat stuff in there that should
be optional. Devices that simply do not exist are not a problem, but
properties that not always exist are.
On 1/22/20 6: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 <dplotnikov@virtuozzo.com>
>> ---
>> 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:
Kind related...
Just yesterday I found a bug on the test case code itself that makes it
fail on ppc64le (likely to fail on other arches). I'm working on a fix.
- Wainer
>
> 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 <hch@lst.de>
> 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.
>
> Regards,
>
> Phil.
>
>
© 2016 - 2026 Red Hat, Inc.