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)

diff --git a/nova/tests/unit/virt/libvirt/test_designer.py b/nova/tests/unit/virt/libvirt/test_designer.py
index 9fb9d722830f772bdb8f4fe303cf14b5c47aad4f..7c701eeffc88e8f175ede09eb35021a34c8a1f79 100644 (file)
--- 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 8d3f5e9d7d3b269038642e5f903b0a3240d6067e..7892c1f4a71f491550c9286574f3b602c6130ee1 100644 (file)
--- 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 67514c931bb6786c5a0bf87b1ab0ebdda5215cd7..847faa65983ea20d5d4a7a7a1b423631db01865c 100644 (file)
--- 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 e3952634d880404a1b0ba29ad45ed8a530da2bdf..249cde15e3b99f7e068c521f809f83b3ad9d86a0 100644 (file)
--- 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