From cc495a24656893c94031f491a3fed2bc94fe1850 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 21 Mar 2017 13:18:08 -0400 Subject: [PATCH] libvirt: conditionally set script path for ethernet vif types Change I4f97c05e2dec610af22a5150dd27696e1d767896 worked around a change introduced in libvirt 1.3.3 where the script path on a LibvirtConfigGuestInterface could not be the emptry string because libvirt would literally take that as the path and couldn't resolve it, when in fact it used to indicate to libvirt that the script path is a noop. This has been fixed in libvirt 3.1. On Ubuntu with libvirt<1.3.3, if the script path is None then it defaults to /etc/qemu-ifup which is blocked by AppArmor. So this change adds a conditional check when setting the script path value based on the libvirt version so we can straddle releases. Change-Id: I192c61b93bd3736fdfe16b6a6906d58997d3eef9 Closes-Bug: #1665698 Related-Bug: #1649527 (cherry picked from commit a41d265a19b7bcb1af8fc179bf864e00023c6cc6) --- nova/tests/unit/virt/libvirt/test_designer.py | 17 +++++++++++++++-- nova/tests/unit/virt/libvirt/test_vif.py | 18 ++++++++++++------ nova/virt/libvirt/designer.py | 15 +++++++++++++-- nova/virt/libvirt/vif.py | 10 +++++----- 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_designer.py b/nova/tests/unit/virt/libvirt/test_designer.py index 9fb9d722830f..7c701eeffc88 100644 --- a/nova/tests/unit/virt/libvirt/test_designer.py +++ b/nova/tests/unit/virt/libvirt/test_designer.py @@ -53,13 +53,26 @@ class DesignerTestCase(test.NoDBTestCase): self.assertEqual('fake-bridge', conf.source_dev) self.assertEqual('fake-tap', conf.target_dev) - def test_set_vif_host_backend_ethernet_config(self): + def test_set_vif_host_backend_ethernet_config_libvirt_1_3_3(self): conf = config.LibvirtConfigGuestInterface() - designer.set_vif_host_backend_ethernet_config(conf, 'fake-tap') + mock_host = mock.Mock(autospec='nova.virt.libvirt.host.Host') + mock_host.has_min_version.return_value = True + designer.set_vif_host_backend_ethernet_config( + conf, 'fake-tap', mock_host) self.assertEqual('ethernet', conf.net_type) self.assertEqual('fake-tap', conf.target_dev) self.assertIsNone(conf.script) + def test_set_vif_host_backend_ethernet_config_libvirt_pre_1_3_3(self): + conf = config.LibvirtConfigGuestInterface() + mock_host = mock.Mock(autospec='nova.virt.libvirt.host.Host') + mock_host.has_min_version.return_value = False + designer.set_vif_host_backend_ethernet_config( + conf, 'fake-tap', mock_host) + self.assertEqual('ethernet', conf.net_type) + self.assertEqual('fake-tap', conf.target_dev) + self.assertEqual('', conf.script) + def test_set_vif_host_backend_802qbg_config(self): conf = config.LibvirtConfigGuestInterface() designer.set_vif_host_backend_802qbg_config(conf, 'fake-devname', diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index 8d3f5e9d7d3b..7892c1f4a71f 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -495,7 +495,8 @@ class LibvirtVifTestCase(test.NoDBTestCase): conf.vcpus = 4 return conf - def _get_instance_xml(self, driver, vif, image_meta=None, flavor=None): + def _get_instance_xml(self, driver, vif, image_meta=None, flavor=None, + has_min_libvirt_version=True): if flavor is None: flavor = objects.Flavor(name='m1.small', memory_mb=128, @@ -512,9 +513,11 @@ class LibvirtVifTestCase(test.NoDBTestCase): conf = self._get_conf() hostimpl = host.Host("qemu:///system") - nic = driver.get_config(self.instance, vif, image_meta, - flavor, CONF.libvirt.virt_type, - hostimpl) + with mock.patch.object(hostimpl, 'has_min_version', + return_value=has_min_libvirt_version): + nic = driver.get_config(self.instance, vif, image_meta, + flavor, CONF.libvirt.virt_type, + hostimpl) conf.add_device(nic) return conf.to_xml() @@ -1227,7 +1230,9 @@ class LibvirtVifTestCase(test.NoDBTestCase): def test_hw_veb_driver_macvtap_pre_vlan_support(self, ver_mock, mock_get_ifname): d = vif.LibvirtGenericVIFDriver() - xml = self._get_instance_xml(d, self.vif_hw_veb_macvtap) + xml = self._get_instance_xml( + d, self.vif_hw_veb_macvtap, + has_min_libvirt_version=ver_mock.return_value) node = self._get_node(xml) self.assertEqual(node.get("type"), "direct") self._assertTypeEquals(node, "direct", "source", @@ -1354,7 +1359,8 @@ class LibvirtVifTestCase(test.NoDBTestCase): image_meta = objects.ImageMeta.from_dict( {'properties': {'hw_vif_model': 'virtio', 'hw_vif_multiqueue_enabled': 'true'}}) - xml = self._get_instance_xml(d, self.vif_vhostuser, image_meta) + xml = self._get_instance_xml(d, self.vif_vhostuser, image_meta, + has_min_libvirt_version=False) node = self._get_node(xml) self.assertEqual(node.get("type"), network_model.VIF_TYPE_VHOSTUSER) diff --git a/nova/virt/libvirt/designer.py b/nova/virt/libvirt/designer.py index 67514c931bb6..847faa65983e 100644 --- a/nova/virt/libvirt/designer.py +++ b/nova/virt/libvirt/designer.py @@ -22,6 +22,8 @@ classes based on common operational needs / policies from nova.pci import utils as pci_utils +MIN_LIBVIRT_ETHERNET_SCRIPT_PATH_NONE = (1, 3, 3) + def set_vif_guest_frontend_config(conf, mac, model, driver, queues=None): """Populate a LibvirtConfigGuestInterface instance @@ -46,7 +48,7 @@ def set_vif_host_backend_bridge_config(conf, brname, tapname=None): conf.target_dev = tapname -def set_vif_host_backend_ethernet_config(conf, tapname): +def set_vif_host_backend_ethernet_config(conf, tapname, host): """Populate a LibvirtConfigGuestInterface instance with host backend details for an externally configured host device. @@ -57,7 +59,16 @@ def set_vif_host_backend_ethernet_config(conf, tapname): conf.net_type = "ethernet" conf.target_dev = tapname - conf.script = None + # NOTE(mriedem): Before libvirt 1.3.3, passing script=None results + # in errors because /etc/qemu-ifup gets run which is blocked by + # AppArmor. Passing script='' between libvirt 1.3.3 and 3.1 will also + # result in errors. So we have to check the libvirt version and set + # the script value accordingly. Libvirt 3.1 allows and properly handles + # both None and '' as no-ops. + if host.has_min_version(MIN_LIBVIRT_ETHERNET_SCRIPT_PATH_NONE): + conf.script = None + else: + conf.script = '' def set_vif_host_backend_802qbg_config(conf, devname, managerid, diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py index e3952634d880..249cde15e3b9 100644 --- a/nova/virt/libvirt/vif.py +++ b/nova/virt/libvirt/vif.py @@ -259,7 +259,7 @@ class LibvirtGenericVIFDriver(object): vif['vnic_type']) dev = self.get_vif_devname(vif) - designer.set_vif_host_backend_ethernet_config(conf, dev) + designer.set_vif_host_backend_ethernet_config(conf, dev, host) return conf @@ -379,7 +379,7 @@ class LibvirtGenericVIFDriver(object): inst_type, virt_type, vif['vnic_type']) dev = self.get_vif_devname(vif) - designer.set_vif_host_backend_ethernet_config(conf, dev) + designer.set_vif_host_backend_ethernet_config(conf, dev, host) designer.set_vif_bandwidth_config(conf, inst_type) @@ -391,7 +391,7 @@ class LibvirtGenericVIFDriver(object): inst_type, virt_type, vif['vnic_type']) dev = self.get_vif_devname(vif) - designer.set_vif_host_backend_ethernet_config(conf, dev) + designer.set_vif_host_backend_ethernet_config(conf, dev, host) return conf @@ -401,7 +401,7 @@ class LibvirtGenericVIFDriver(object): inst_type, virt_type, vif['vnic_type']) dev = self.get_vif_devname(vif) - designer.set_vif_host_backend_ethernet_config(conf, dev) + designer.set_vif_host_backend_ethernet_config(conf, dev, host) return conf @@ -439,7 +439,7 @@ class LibvirtGenericVIFDriver(object): conf = self.get_base_config(instance, vif['address'], image_meta, inst_type, virt_type, vif['vnic_type']) dev = self.get_vif_devname(vif) - designer.set_vif_host_backend_ethernet_config(conf, dev) + designer.set_vif_host_backend_ethernet_config(conf, dev, host) designer.set_vif_bandwidth_config(conf, inst_type) return conf