vif: Resolve a TODO and update another

Once TODO noted that a block could be removed once we bump to libvirt
1.3.8 or greater. We require 3.0.0 now so that's resolved. Another one
looks like it should be resolved in 3.2.0 so the TODO is updated to
highlight this for future reviewers.

Change-Id: I5235751b1dbc77ecc919eec7f3e022cd70085051
Signed-off-by: Stephen Finucane <[email protected]>

Author: stephenfin

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

@@ -975,38 +975,29 @@ def test_generic_driver_bridge(self):
     @mock.patch.object(pci_utils, 'get_vf_num_by_pci_address', return_value=1)
     @mock.patch('nova.privsep.linux_net.set_device_macaddr')
     @mock.patch('nova.privsep.linux_net.set_device_macaddr_and_vlan')
-    def _test_hw_veb_op(self, op, vlan, mock_set_macaddr_and_vlan,
-                        mock_set_macaddr, mock_get_vf_num,
-                        mock_get_ifname):
+    def test_unplug_hw_veb(self, mock_set_macaddr_and_vlan,
+                           mock_set_macaddr, mock_get_vf_num,
+                           mock_get_ifname):
+        d = vif.LibvirtGenericVIFDriver()
         mock_get_ifname.side_effect = ['eth1', 'eth13']
-        port_state = 'up' if vlan > 0 else 'down'
-        mac = ('00:00:00:00:00:00' if op.__name__ == 'unplug'
-               else self.vif_hw_veb_macvtap['address'])
         calls = {
             'get_ifname':
                 [mock.call(self.vif_hw_veb_macvtap['profile']['pci_slot'],
                            pf_interface=True),
                  mock.call(self.vif_hw_veb_macvtap['profile']['pci_slot'])],
             'get_vf_num':
                 [mock.call(self.vif_hw_veb_macvtap['profile']['pci_slot'])],
-            'set_macaddr': [mock.call('eth13', mac, port_state=port_state)]
+            'set_macaddr': [mock.call(
+                'eth13', '00:00:00:00:00:00', port_state='down')]
         }
-        op(self.instance, self.vif_hw_veb_macvtap)
+
+        d.unplug(self.instance, self.vif_hw_veb_macvtap)
+
         mock_get_ifname.assert_has_calls(calls['get_ifname'])
         mock_get_vf_num.assert_has_calls(calls['get_vf_num'])
         mock_set_macaddr.assert_has_calls(calls['set_macaddr'])
         mock_set_macaddr_and_vlan.assert_called_once_with(
-            'eth1', 1, mock.ANY, vlan)
-
-    def test_plug_hw_veb(self):
-        d = vif.LibvirtGenericVIFDriver()
-        self._test_hw_veb_op(
-            d.plug,
-            self.vif_hw_veb_macvtap['details'][network_model.VIF_DETAILS_VLAN])
-
-    def test_unplug_hw_veb(self):
-        d = vif.LibvirtGenericVIFDriver()
-        self._test_hw_veb_op(d.unplug, 0)
+            'eth1', 1, mock.ANY, 0)
 
     @mock.patch('nova.network.linux_net.set_vf_trusted')
     def test_plug_hw_veb_trusted(self, mset_vf_trusted):

nova/virt/libvirt/vif.py

@@ -110,8 +110,9 @@ def set_vf_interface_vlan(pci_addr, mac_addr, vlan=0):
 
     # Bring up/down the VF's interface
     # TODO(edand): The mac is assigned as a workaround for the following issue
-    #              https://bugzilla.redhat.com/show_bug.cgi?id=1372944
-    #              once resolved it will be removed
+    # https://bugzilla.redhat.com/show_bug.cgi?id=1415609 and should be removed
+    # once we bump the libvirt minimum to 3.2.0, as noted in
+    # https://bugzilla.redhat.com/show_bug.cgi?id=1372944
     port_state = 'up' if vlan > 0 else 'down'
     nova.privsep.linux_net.set_device_macaddr(vf_ifname, mac_addr,
                                               port_state=port_state)
@@ -640,15 +641,7 @@ def plug_ib_hostdev(self, instance, vif):
                           instance=instance)
 
     def plug_hw_veb(self, instance, vif):
-        # TODO(vladikr): This code can be removed once the minimum version of
-        # Libvirt is incleased above 1.3.5, as vlan will be set by libvirt
-        if vif['vnic_type'] == network_model.VNIC_TYPE_MACVTAP:
-            set_vf_interface_vlan(
-                vif['profile']['pci_slot'],
-                mac_addr=vif['address'],
-                vlan=vif['details'][network_model.VIF_DETAILS_VLAN])
-
-        elif vif['vnic_type'] == network_model.VNIC_TYPE_DIRECT:
+        if vif['vnic_type'] == network_model.VNIC_TYPE_DIRECT:
             trusted = strutils.bool_from_string(
                 vif['profile'].get('trusted', "False"))
             if trusted: