From f651ebc0ceeca70aba8c0f7e5054f598e491c848 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 6 Feb 2019 14:00:51 +0000 Subject: [PATCH] Don't set bandwidth limits for vhostuser, hostdev interfaces 'tc' [1] is used for rate limiting in libvirt. This is suitable for tap devices but not for the sockets used by vhostuser interface or for physical devices; rate limiting for these interfaces must be done in the vSwitch or via the interface itself. For this reason, libvirt explicitly marks these interfaces, along with some other interfaces types, as not supporting bandwidth limiting [2]. Given that there is no reason to be setting the bandwidth-related attributes on vhostuser or hostdev interfaces, we should stop doing it. [1] http://man7.org/linux/man-pages/man8/tc.8.html [2] https://github.com/libvirt/libvirt/blob/568a41722/src/conf/netdev_bandwidth_conf.h#L38 Change-Id: Id57e4c149616fe0896e80fca322ece79ec3bfd69 Closes-Bug: #1814882 --- nova/tests/unit/virt/libvirt/test_vif.py | 23 +++++++++++++++++++++++ nova/virt/libvirt/vif.py | 6 +++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index 260148188ce3..46c9f65b6d65 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -576,6 +576,8 @@ class LibvirtVifTestCase(test.NoDBTestCase): self.assertEqual(pci_slot, pci_slot_want) def _assertXmlEqual(self, expectedXmlstr, actualXmlstr): + if not isinstance(actualXmlstr, six.string_types): + actualXmlstr = etree.tostring(actualXmlstr, pretty_print=True) self.assertThat(actualXmlstr, matchers.XMLMatches(expectedXmlstr)) def _get_conf(self): @@ -610,6 +612,8 @@ class LibvirtVifTestCase(test.NoDBTestCase): nic = driver.get_config(self.instance, vif, image_meta, flavor, CONF.libvirt.virt_type, hostimpl) + # TODO(stephenfin): There doesn't appear to be any reason we should do + # this: just return 'nic.to_xml()' and remove '_get_node' conf.add_device(nic) return conf.to_xml() @@ -1630,6 +1634,25 @@ class LibvirtVifTestCase(test.NoDBTestCase): """, cfg.to_xml()) + @mock.patch("nova.network.os_vif_util.nova_to_osvif_instance") + @mock.patch("nova.network.os_vif_util.nova_to_osvif_vif") + def test_config_os_vif_vhostuser(self, mock_convert_vif, + mock_convert_inst): + mock_convert_vif.return_value = self.os_vif_vhostuser + mock_convert_inst.return_value = self.os_vif_inst_info + + d = vif.LibvirtGenericVIFDriver() + xml = self._get_instance_xml(d, self.vif_vhostuser) + node = self._get_node(xml) + + self._assertXmlEqual(""" + + + + + """, node) + @mock.patch("nova.network.os_vif_util.nova_to_osvif_instance") @mock.patch("nova.network.os_vif_util.nova_to_osvif_vif") def test_config_os_vif_agilio_ovs_fallthrough(self, mock_convert_vif, diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py index 4b846b6c2bb9..c5b5a308e1b4 100644 --- a/nova/virt/libvirt/vif.py +++ b/nova/virt/libvirt/vif.py @@ -573,7 +573,11 @@ class LibvirtGenericVIFDriver(object): {'obj': vif.obj_name(), 'func': viffunc}) func(instance, vif, conf, host) - designer.set_vif_bandwidth_config(conf, inst_type) + # not all VIF types support bandwidth configuration + # https://github.com/libvirt/libvirt/blob/568a41722/src/conf/netdev_bandwidth_conf.h#L38 + if vif.obj_name() not in ('VIFVHostUser', 'VIFHostDevice'): + designer.set_vif_bandwidth_config(conf, inst_type) + if ('network' in vif and 'mtu' in vif.network and self._has_min_version_for_mtu(host)): designer.set_vif_mtu_config(conf, vif.network.mtu)