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

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',

nova/tests/unit/virt/libvirt/test_vif.py

@@ -491,7 +491,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,
@@ -508,9 +509,11 @@ class LibvirtVifTestCase(test.NoDBTestCase):
 
         conf = self._get_conf()
         hostimpl = host.Host("qemu:///system")
-        nic = driver.get_config(self.instance, vif, image_meta,
+        with mock.patch.object(hostimpl, 'has_min_version',
-                                flavor, CONF.libvirt.virt_type,
+                               return_value=has_min_libvirt_version):
-                                hostimpl)
+            nic = driver.get_config(self.instance, vif, image_meta,
+                                    flavor, CONF.libvirt.virt_type,
+                                    hostimpl)
         conf.add_device(nic)
         return conf.to_xml()
 
@@ -1225,7 +1228,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",
@@ -1352,7 +1357,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)

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,

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