From 622ebf2fab0a9bf75ee12437bef28f60e083f849 Mon Sep 17 00:00:00 2001 From: Moshe Levi Date: Thu, 23 Aug 2018 14:25:04 +0300 Subject: [PATCH] libvirt: skip setting rx/tx queue sizes for not virto interfaces It seem that if driver name is None nova try to set rx_queue_size tx_queue_size config. (they define the virtio-net rx/tx queue sizes). Direct/Physical Direct vnic_types are not vritio so this kind of config is invalid and causing booting guest to failed. To avoid issues we skip such configuration these vnic_types Closes-Bug: #1789074 Change-Id: I45532896690ad9505f2b09c98d8d86b61bcfef2b --- nova/tests/unit/virt/libvirt/test_vif.py | 42 +++++++++++++++++++++--- nova/virt/libvirt/vif.py | 42 +++++++++++++----------- 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index e69171484633..f472a0cbbce7 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -668,13 +668,14 @@ class LibvirtVifTestCase(test.NoDBTestCase): self.assertIsNone(conf.vhost_queues) self.assertIsNone(conf.driver_name) - def _test_virtio_config_queue_sizes(self): + def _test_virtio_config_queue_sizes( + self, vnic_type=network_model.VNIC_TYPE_NORMAL): self.flags(rx_queue_size=512, group='libvirt') self.flags(tx_queue_size=1024, group='libvirt') hostimpl = host.Host("qemu:///system") v = vif.LibvirtGenericVIFDriver() conf = v.get_base_config( - None, 'ca:fe:de:ad:be:ef', {}, objects.Flavor(), 'kvm', 'normal', + None, 'ca:fe:de:ad:be:ef', {}, objects.Flavor(), 'kvm', vnic_type, hostimpl) return hostimpl, v, conf @@ -684,6 +685,36 @@ class LibvirtVifTestCase(test.NoDBTestCase): self.assertEqual(512, conf.vhost_rx_queue_size) self.assertIsNone(conf.vhost_tx_queue_size) + @mock.patch.object(host.Host, "has_min_version", return_value=True) + def test_virtio_vhost_queue_sizes_vnic_type_direct(self, has_min_version): + _, _, conf = self._test_virtio_config_queue_sizes( + vnic_type=network_model.VNIC_TYPE_DIRECT) + self.assertIsNone(conf.vhost_rx_queue_size) + self.assertIsNone(conf.vhost_tx_queue_size) + + @mock.patch.object(host.Host, "has_min_version", return_value=True) + def test_virtio_vhost_queue_sizes_vnic_type_direct_physical( + self, has_min_version): + _, _, conf = self._test_virtio_config_queue_sizes( + vnic_type=network_model.VNIC_TYPE_DIRECT_PHYSICAL) + self.assertIsNone(conf.vhost_rx_queue_size) + self.assertIsNone(conf.vhost_tx_queue_size) + + @mock.patch.object(host.Host, "has_min_version", return_value=True) + def test_virtio_vhost_queue_sizes_vnic_type_macvtap(self, has_min_version): + _, _, conf = self._test_virtio_config_queue_sizes( + vnic_type=network_model.VNIC_TYPE_MACVTAP) + self.assertEqual(512, conf.vhost_rx_queue_size) + self.assertIsNone(conf.vhost_tx_queue_size) + + @mock.patch.object(host.Host, "has_min_version", return_value=True) + def test_virtio_vhost_queue_sizes_vnic_type_virtio_forwarder( + self, has_min_version): + _, _, conf = self._test_virtio_config_queue_sizes( + vnic_type=network_model.VNIC_TYPE_VIRTIO_FORWARDER) + self.assertEqual(512, conf.vhost_rx_queue_size) + self.assertIsNone(conf.vhost_tx_queue_size) + @mock.patch.object(host.Host, "has_min_version", return_value=False) def test_virtio_vhost_queue_sizes_nover(self, has_min_version): _, _, conf = self._test_virtio_config_queue_sizes() @@ -817,7 +848,7 @@ class LibvirtVifTestCase(test.NoDBTestCase): 'virtio', None, None, None) @mock.patch.object(vif.designer, 'set_vif_guest_frontend_config') - def test_model_sriov_multi_queue_not_set(self, mock_set): + def test_model_sriov_direct_multi_queue_not_set(self, mock_set): self.flags(use_virtio_for_bridges=True, virt_type='kvm', group='libvirt') @@ -829,9 +860,10 @@ class LibvirtVifTestCase(test.NoDBTestCase): image_meta = {'properties': {'os_name': 'fedora22'}} image_meta = objects.ImageMeta.from_dict(image_meta) conf = d.get_base_config(None, 'ca:fe:de:ad:be:ef', image_meta, - None, 'kvm', 'direct', hostimpl) + None, 'kvm', network_model.VNIC_TYPE_DIRECT, + hostimpl) mock_set.assert_called_once_with(mock.ANY, 'ca:fe:de:ad:be:ef', - 'virtio', None, None, None) + None, None, None, None) self.assertIsNone(conf.vhost_queues) self.assertIsNone(conf.driver_name) diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py index 32d09e052d04..b1c6b3e3f1c9 100644 --- a/nova/virt/libvirt/vif.py +++ b/nova/virt/libvirt/vif.py @@ -134,12 +134,15 @@ class LibvirtGenericVIFDriver(object): if image_meta: model = osinfo.HardwareProperties(image_meta).network_model - # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio - # according to the global config parameter - if (model is None and - virt_type in ('kvm', 'qemu', 'parallels') and - CONF.libvirt.use_virtio_for_bridges): - model = network_model.VIF_MODEL_VIRTIO + # Note(moshele): Skip passthough vnic_types as they don't support + # virtio model. + if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH: + # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio + # according to the global config parameter + if (model is None and + virt_type in ('kvm', 'qemu', 'parallels') and + CONF.libvirt.use_virtio_for_bridges): + model = network_model.VIF_MODEL_VIRTIO # Workaround libvirt bug, where it mistakenly # enables vhost mode, even for non-KVM guests @@ -152,8 +155,7 @@ class LibvirtGenericVIFDriver(object): raise exception.UnsupportedHardware(model=model, virt=virt_type) if (virt_type in ('kvm', 'parallels') and - model == network_model.VIF_MODEL_VIRTIO and - vnic_type not in network_model.VNIC_TYPES_SRIOV): + model == network_model.VIF_MODEL_VIRTIO): vhost_drv, vhost_queues = self._get_virtio_mq_settings(image_meta, inst_type) # TODO(sahid): It seems that we return driver 'vhost' even @@ -166,17 +168,19 @@ class LibvirtGenericVIFDriver(object): driver = vhost_drv or driver rx_queue_size = None - if driver == 'vhost' or driver is None: - # vhost backend only supports update of RX queue size - rx_queue_size, _ = self._get_virtio_queue_sizes(host) - if rx_queue_size: - # TODO(sahid): Specifically force driver to be vhost - # that because if None we don't generate the XML - # driver element needed to set the queue size - # attribute. This can be removed when get_base_config - # will be fixed and rewrite to set the correct - # backend. - driver = 'vhost' + # Note(moshele): rx_queue_size is support only for virtio model + if model == network_model.VIF_MODEL_VIRTIO: + if driver == 'vhost' or driver is None: + # vhost backend only supports update of RX queue size + rx_queue_size, _ = self._get_virtio_queue_sizes(host) + if rx_queue_size: + # TODO(sahid): Specifically force driver to be vhost + # that because if None we don't generate the XML + # driver element needed to set the queue size + # attribute. This can be removed when get_base_config + # will be fixed and rewrite to set the correct + # backend. + driver = 'vhost' designer.set_vif_guest_frontend_config( conf, mac, model, driver, vhost_queues, rx_queue_size)