* [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent @ 2019-12-16 10:04 Denis Plotnikov 2019-12-16 10:04 ` [PATCH v4 1/2] " Denis Plotnikov ` (3 more replies) 0 siblings, 4 replies; 7+ messages in thread From: Denis Plotnikov @ 2019-12-16 10:04 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, fam, ehabkost, mst, mreitz, stefanha, pbonzini, den v4: * rebased on 4.2 [MST] v3: * add property to set in machine type [MST] * add min queue size check [Stefan] * add avocado based test [Max, Stefan, Eduardo, Cleber] v2: * the standalone patch to make seg_max virtqueue size dependent * other patches are postponed v1: the initial series Denis Plotnikov (2): virtio: make seg_max virtqueue size dependent tests: add virtio-scsi and virtio-blk seg_max_adjust test hw/block/virtio-blk.c | 9 +- hw/core/machine.c | 3 + hw/scsi/vhost-scsi.c | 2 + hw/scsi/virtio-scsi.c | 10 +- include/hw/virtio/virtio-blk.h | 1 + include/hw/virtio/virtio-scsi.h | 1 + tests/acceptance/virtio_seg_max_adjust.py | 135 ++++++++++++++++++++++ 7 files changed, 159 insertions(+), 2 deletions(-) create mode 100755 tests/acceptance/virtio_seg_max_adjust.py -- 2.17.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/2] virtio: make seg_max virtqueue size dependent 2019-12-16 10:04 [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Denis Plotnikov @ 2019-12-16 10:04 ` Denis Plotnikov 2019-12-19 12:27 ` Michael S. Tsirkin 2019-12-16 10:04 ` [PATCH v4 2/2] tests: add virtio-scsi and virtio-blk seg_max_adjust test Denis Plotnikov ` (2 subsequent siblings) 3 siblings, 1 reply; 7+ messages in thread From: Denis Plotnikov @ 2019-12-16 10:04 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, fam, ehabkost, mst, mreitz, stefanha, pbonzini, den Before the patch, seg_max parameter was immutable and hardcoded to 126 (128 - 2) without respect to queue size. This has two negative effects: 1. when queue size is < 128, we have Virtio 1.1 specfication violation: (2.6.5.3.1 Driver Requirements) seq_max must be <= queue_size. This violation affects the old Linux guests (ver < 4.14). These guests crash on these queue_size setups. 2. when queue_size > 128, as was pointed out by Denis Lunev <den@virtuozzo.com>, seg_max restrics guest's block request length which affects guests' performance making them issues more block request than needed. https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html To mitigate this two effects, the patch adds the property adjusting seg_max to queue size automaticaly. Since seg_max is a guest visible parameter, the property is machine type managable and allows to choose between old (seg_max = 126 always) and new (seg_max = queue_size - 2) behaviors. Not to change the behavior of the older VMs, prevent setting the default seg_max_adjust value for older machine types. Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> --- hw/block/virtio-blk.c | 9 ++++++++- hw/core/machine.c | 3 +++ hw/scsi/vhost-scsi.c | 2 ++ hw/scsi/virtio-scsi.c | 10 +++++++++- include/hw/virtio/virtio-blk.h | 1 + include/hw/virtio/virtio-scsi.h | 1 + 6 files changed, 24 insertions(+), 2 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index d62e6377c2..0f6f8113b7 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -908,7 +908,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) blk_get_geometry(s->blk, &capacity); memset(&blkcfg, 0, sizeof(blkcfg)); virtio_stq_p(vdev, &blkcfg.capacity, capacity); - virtio_stl_p(vdev, &blkcfg.seg_max, 128 - 2); + virtio_stl_p(vdev, &blkcfg.seg_max, + s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2); virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls); virtio_stl_p(vdev, &blkcfg.blk_size, blk_size); virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size); @@ -1133,6 +1134,11 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) error_setg(errp, "num-queues property must be larger than 0"); return; } + if (conf->queue_size <= 2) { + error_setg(errp, "invalid queue-size property (%" PRIu16 "), " + "must be > 2", conf->queue_size); + return; + } if (!is_power_of_2(conf->queue_size) || conf->queue_size > VIRTQUEUE_MAX_SIZE) { error_setg(errp, "invalid queue-size property (%" PRIu16 "), " @@ -1262,6 +1268,7 @@ static Property virtio_blk_properties[] = { true), DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1), DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128), + DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true), DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD, IOThread *), DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, diff --git a/hw/core/machine.c b/hw/core/machine.c index 023548b4f3..bfa320387e 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -29,6 +29,9 @@ GlobalProperty hw_compat_4_2[] = { { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" }, + { "virtio-blk-device", "seg-max-adjust", "off"}, + { "virtio-scsi-device", "seg_max_adjust", "off"}, + { "vhost-blk-device", "seg_max_adjust", "off"}, }; const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2); diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index c693fc748a..26f710d3ec 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -275,6 +275,8 @@ static Property vhost_scsi_properties[] = { DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1), DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size, 128), + DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSICommon, conf.seg_max_adjust, + true), DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors, 0xFFFF), DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128), diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index e8b2b64d09..405cb6c953 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -654,7 +654,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev, VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(vdev); virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues); - virtio_stl_p(vdev, &scsiconf->seg_max, 128 - 2); + virtio_stl_p(vdev, &scsiconf->seg_max, + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2); virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors); virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun); virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent)); @@ -893,6 +894,11 @@ void virtio_scsi_common_realize(DeviceState *dev, virtio_cleanup(vdev); return; } + if (s->conf.virtqueue_size <= 2) { + error_setg(errp, "invalid virtqueue_size property (= %" PRIu16 "), " + "must be > 2", s->conf.virtqueue_size); + return; + } s->cmd_vqs = g_new0(VirtQueue *, s->conf.num_queues); s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE; s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE; @@ -949,6 +955,8 @@ static Property virtio_scsi_properties[] = { DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1), DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI, parent_obj.conf.virtqueue_size, 128), + DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI, + parent_obj.conf.seg_max_adjust, true), DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors, 0xFFFF), DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun, diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 9c19f5b634..1e62f869b2 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -38,6 +38,7 @@ struct VirtIOBlkConf uint32_t request_merging; uint16_t num_queues; uint16_t queue_size; + bool seg_max_adjust; uint32_t max_discard_sectors; uint32_t max_write_zeroes_sectors; bool x_enable_wce_if_config_wce; diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index 122f7c4b6f..24e768909d 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -48,6 +48,7 @@ typedef struct virtio_scsi_config VirtIOSCSIConfig; struct VirtIOSCSIConf { uint32_t num_queues; uint32_t virtqueue_size; + bool seg_max_adjust; uint32_t max_sectors; uint32_t cmd_per_lun; #ifdef CONFIG_VHOST_SCSI -- 2.17.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] virtio: make seg_max virtqueue size dependent 2019-12-16 10:04 ` [PATCH v4 1/2] " Denis Plotnikov @ 2019-12-19 12:27 ` Michael S. Tsirkin 0 siblings, 0 replies; 7+ messages in thread From: Michael S. Tsirkin @ 2019-12-19 12:27 UTC (permalink / raw) To: Denis Plotnikov Cc: kwolf, fam, ehabkost, qemu-devel, mreitz, stefanha, pbonzini, den On Mon, Dec 16, 2019 at 01:04:50PM +0300, Denis Plotnikov wrote: > Before the patch, seg_max parameter was immutable and hardcoded > to 126 (128 - 2) without respect to queue size. This has two negative effects: > > 1. when queue size is < 128, we have Virtio 1.1 specfication violation: > (2.6.5.3.1 Driver Requirements) seq_max must be <= queue_size. > This violation affects the old Linux guests (ver < 4.14). These guests > crash on these queue_size setups. > > 2. when queue_size > 128, as was pointed out by Denis Lunev <den@virtuozzo.com>, > seg_max restrics guest's block request length which affects guests' > performance making them issues more block request than needed. > https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html > > To mitigate this two effects, the patch adds the property adjusting seg_max > to queue size automaticaly. Since seg_max is a guest visible parameter, > the property is machine type managable and allows to choose between > old (seg_max = 126 always) and new (seg_max = queue_size - 2) behaviors. > > Not to change the behavior of the older VMs, prevent setting the default > seg_max_adjust value for older machine types. > > Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> Stefan I think you acked this already? Could you ack? Denis could you rebase on latest master pls so I can apply? Thanks! > --- > hw/block/virtio-blk.c | 9 ++++++++- > hw/core/machine.c | 3 +++ > hw/scsi/vhost-scsi.c | 2 ++ > hw/scsi/virtio-scsi.c | 10 +++++++++- > include/hw/virtio/virtio-blk.h | 1 + > include/hw/virtio/virtio-scsi.h | 1 + > 6 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index d62e6377c2..0f6f8113b7 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -908,7 +908,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) > blk_get_geometry(s->blk, &capacity); > memset(&blkcfg, 0, sizeof(blkcfg)); > virtio_stq_p(vdev, &blkcfg.capacity, capacity); > - virtio_stl_p(vdev, &blkcfg.seg_max, 128 - 2); > + virtio_stl_p(vdev, &blkcfg.seg_max, > + s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2); > virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls); > virtio_stl_p(vdev, &blkcfg.blk_size, blk_size); > virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size); > @@ -1133,6 +1134,11 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) > error_setg(errp, "num-queues property must be larger than 0"); > return; > } > + if (conf->queue_size <= 2) { > + error_setg(errp, "invalid queue-size property (%" PRIu16 "), " > + "must be > 2", conf->queue_size); > + return; > + } > if (!is_power_of_2(conf->queue_size) || > conf->queue_size > VIRTQUEUE_MAX_SIZE) { > error_setg(errp, "invalid queue-size property (%" PRIu16 "), " > @@ -1262,6 +1268,7 @@ static Property virtio_blk_properties[] = { > true), > DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1), > DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128), > + DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true), > DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD, > IOThread *), > DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 023548b4f3..bfa320387e 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -29,6 +29,9 @@ > > GlobalProperty hw_compat_4_2[] = { > { "virtio-blk-device", "x-enable-wce-if-config-wce", "off" }, > + { "virtio-blk-device", "seg-max-adjust", "off"}, > + { "virtio-scsi-device", "seg_max_adjust", "off"}, > + { "vhost-blk-device", "seg_max_adjust", "off"}, > }; > const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2); > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c > index c693fc748a..26f710d3ec 100644 > --- a/hw/scsi/vhost-scsi.c > +++ b/hw/scsi/vhost-scsi.c > @@ -275,6 +275,8 @@ static Property vhost_scsi_properties[] = { > DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1), > DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size, > 128), > + DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSICommon, conf.seg_max_adjust, > + true), > DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors, > 0xFFFF), > DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun, 128), > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index e8b2b64d09..405cb6c953 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -654,7 +654,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev, > VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(vdev); > > virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues); > - virtio_stl_p(vdev, &scsiconf->seg_max, 128 - 2); > + virtio_stl_p(vdev, &scsiconf->seg_max, > + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2); > virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors); > virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun); > virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent)); > @@ -893,6 +894,11 @@ void virtio_scsi_common_realize(DeviceState *dev, > virtio_cleanup(vdev); > return; > } > + if (s->conf.virtqueue_size <= 2) { > + error_setg(errp, "invalid virtqueue_size property (= %" PRIu16 "), " > + "must be > 2", s->conf.virtqueue_size); > + return; > + } > s->cmd_vqs = g_new0(VirtQueue *, s->conf.num_queues); > s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE; > s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE; > @@ -949,6 +955,8 @@ static Property virtio_scsi_properties[] = { > DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1), > DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI, > parent_obj.conf.virtqueue_size, 128), > + DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI, > + parent_obj.conf.seg_max_adjust, true), > DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors, > 0xFFFF), > DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun, > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index 9c19f5b634..1e62f869b2 100644 > --- a/include/hw/virtio/virtio-blk.h > +++ b/include/hw/virtio/virtio-blk.h > @@ -38,6 +38,7 @@ struct VirtIOBlkConf > uint32_t request_merging; > uint16_t num_queues; > uint16_t queue_size; > + bool seg_max_adjust; > uint32_t max_discard_sectors; > uint32_t max_write_zeroes_sectors; > bool x_enable_wce_if_config_wce; > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h > index 122f7c4b6f..24e768909d 100644 > --- a/include/hw/virtio/virtio-scsi.h > +++ b/include/hw/virtio/virtio-scsi.h > @@ -48,6 +48,7 @@ typedef struct virtio_scsi_config VirtIOSCSIConfig; > struct VirtIOSCSIConf { > uint32_t num_queues; > uint32_t virtqueue_size; > + bool seg_max_adjust; > uint32_t max_sectors; > uint32_t cmd_per_lun; > #ifdef CONFIG_VHOST_SCSI > -- > 2.17.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] tests: add virtio-scsi and virtio-blk seg_max_adjust test 2019-12-16 10:04 [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Denis Plotnikov 2019-12-16 10:04 ` [PATCH v4 1/2] " Denis Plotnikov @ 2019-12-16 10:04 ` Denis Plotnikov 2019-12-16 11:04 ` [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Michael S. Tsirkin 2019-12-19 16:23 ` Stefan Hajnoczi 3 siblings, 0 replies; 7+ messages in thread From: Denis Plotnikov @ 2019-12-16 10:04 UTC (permalink / raw) To: qemu-devel; +Cc: kwolf, fam, ehabkost, mst, mreitz, stefanha, pbonzini, den 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 | 135 ++++++++++++++++++++++ 1 file changed, 135 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..00cf2565d9 --- /dev/null +++ b/tests/acceptance/virtio_seg_max_adjust.py @@ -0,0 +1,135 @@ +#!/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 > 4.2 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("."); + + # all versions greater than 4.2 goes with seg_max_adjust enabled + major = int(ver[0]) + minor = int(ver[1]) + + if major > 4 or (major == 4 and minor > 2): + 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent 2019-12-16 10:04 [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Denis Plotnikov 2019-12-16 10:04 ` [PATCH v4 1/2] " Denis Plotnikov 2019-12-16 10:04 ` [PATCH v4 2/2] tests: add virtio-scsi and virtio-blk seg_max_adjust test Denis Plotnikov @ 2019-12-16 11:04 ` Michael S. Tsirkin 2019-12-19 16:23 ` Stefan Hajnoczi 2019-12-19 16:23 ` Stefan Hajnoczi 3 siblings, 1 reply; 7+ messages in thread From: Michael S. Tsirkin @ 2019-12-16 11:04 UTC (permalink / raw) To: Denis Plotnikov Cc: kwolf, fam, ehabkost, qemu-devel, mreitz, stefanha, pbonzini, den On Mon, Dec 16, 2019 at 01:04:49PM +0300, Denis Plotnikov wrote: > v4: > * rebased on 4.2 [MST] Looks good. Can I get some acks from storage guys pls? > v3: > * add property to set in machine type [MST] > * add min queue size check [Stefan] > * add avocado based test [Max, Stefan, Eduardo, Cleber] > > v2: > * the standalone patch to make seg_max virtqueue size dependent > * other patches are postponed > > v1: > the initial series > > Denis Plotnikov (2): > virtio: make seg_max virtqueue size dependent > tests: add virtio-scsi and virtio-blk seg_max_adjust test > > hw/block/virtio-blk.c | 9 +- > hw/core/machine.c | 3 + > hw/scsi/vhost-scsi.c | 2 + > hw/scsi/virtio-scsi.c | 10 +- > include/hw/virtio/virtio-blk.h | 1 + > include/hw/virtio/virtio-scsi.h | 1 + > tests/acceptance/virtio_seg_max_adjust.py | 135 ++++++++++++++++++++++ > 7 files changed, 159 insertions(+), 2 deletions(-) > create mode 100755 tests/acceptance/virtio_seg_max_adjust.py > > -- > 2.17.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent 2019-12-16 11:04 ` [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Michael S. Tsirkin @ 2019-12-19 16:23 ` Stefan Hajnoczi 0 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2019-12-19 16:23 UTC (permalink / raw) To: Michael S. Tsirkin Cc: kwolf, fam, ehabkost, qemu-devel, mreitz, Denis Plotnikov, stefanha, pbonzini, den [-- Attachment #1: Type: text/plain, Size: 273 bytes --] On Mon, Dec 16, 2019 at 06:04:21AM -0500, Michael S. Tsirkin wrote: > On Mon, Dec 16, 2019 at 01:04:49PM +0300, Denis Plotnikov wrote: > > v4: > > * rebased on 4.2 [MST] > > > Looks good. Can I get some acks from storage guys pls? Looks good to me. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent 2019-12-16 10:04 [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Denis Plotnikov ` (2 preceding siblings ...) 2019-12-16 11:04 ` [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Michael S. Tsirkin @ 2019-12-19 16:23 ` Stefan Hajnoczi 3 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2019-12-19 16:23 UTC (permalink / raw) To: Denis Plotnikov Cc: kwolf, fam, ehabkost, mst, qemu-devel, mreitz, stefanha, pbonzini, den [-- Attachment #1: Type: text/plain, Size: 1175 bytes --] On Mon, Dec 16, 2019 at 01:04:49PM +0300, Denis Plotnikov wrote: > v4: > * rebased on 4.2 [MST] > > v3: > * add property to set in machine type [MST] > * add min queue size check [Stefan] > * add avocado based test [Max, Stefan, Eduardo, Cleber] > > v2: > * the standalone patch to make seg_max virtqueue size dependent > * other patches are postponed > > v1: > the initial series > > Denis Plotnikov (2): > virtio: make seg_max virtqueue size dependent > tests: add virtio-scsi and virtio-blk seg_max_adjust test > > hw/block/virtio-blk.c | 9 +- > hw/core/machine.c | 3 + > hw/scsi/vhost-scsi.c | 2 + > hw/scsi/virtio-scsi.c | 10 +- > include/hw/virtio/virtio-blk.h | 1 + > include/hw/virtio/virtio-scsi.h | 1 + > tests/acceptance/virtio_seg_max_adjust.py | 135 ++++++++++++++++++++++ > 7 files changed, 159 insertions(+), 2 deletions(-) > create mode 100755 tests/acceptance/virtio_seg_max_adjust.py Bellissimo! Thanks for doing this :) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-19 16:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-16 10:04 [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Denis Plotnikov 2019-12-16 10:04 ` [PATCH v4 1/2] " Denis Plotnikov 2019-12-19 12:27 ` Michael S. Tsirkin 2019-12-16 10:04 ` [PATCH v4 2/2] tests: add virtio-scsi and virtio-blk seg_max_adjust test Denis Plotnikov 2019-12-16 11:04 ` [PATCH v4 0/2] virtio: make seg_max virtqueue size dependent Michael S. Tsirkin 2019-12-19 16:23 ` Stefan Hajnoczi 2019-12-19 16:23 ` Stefan Hajnoczi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.