xenapi: drop deprecated vif_driver config option

The vif_driver option was deprecated in Ocata:

  I599f3449f18d2821403961fb9d52e9a14dd3366b

And can now be removed. The only supported networking
backend is neutron + ovs.

Related to blueprint remove-nova-network

Co-Authored-By: Naichuan Sun <naichuan.sun@citrix.com>

Change-Id: Ia977f115335f00bc36249fa67437b4336d524251

doc/source/admin/configuration/hypervisor-xen-api.rst

@@ -361,7 +361,6 @@ To enable the XenAPI driver, add the following configuration options to
    connection_username = root
    connection_password = your_password
    ovs_integration_bridge = br-int
-   vif_driver = nova.virt.xenapi.vif.XenAPIOpenVswitchDriver
 
 These connection details are used by OpenStack Compute service to contact your
 hypervisor and are the same details you use to connect XenCenter, the XenServer

nova/conf/xenserver.py

@@ -412,40 +412,6 @@ time is involved until the instance(s) can become available and
 instances do not go to running state within this specified wait
 time, the launch expires and the instance(s) are set to 'error'
 state.
-"""),
-    cfg.StrOpt('vif_driver',
-        default='nova.virt.xenapi.vif.XenAPIOpenVswitchDriver',
-        deprecated_for_removal=True,
-        deprecated_since='15.0.0',
-        deprecated_reason="""
-There are only two in-tree vif drivers for XenServer. XenAPIBridgeDriver is for
-nova-network which is deprecated and XenAPIOpenVswitchDriver is for Neutron
-which is the default configuration for Nova since the 15.0.0 Ocata release. In
-the future the "use_neutron" configuration option will be used to determine
-which vif driver to use.
-""",
-        help="""
-The XenAPI VIF driver using XenServer Network APIs.
-
-Provide a string value representing the VIF XenAPI vif driver to use for
-plugging virtual network interfaces.
-
-Xen configuration uses bridging within the backend domain to allow
-all VMs to appear on the network as individual hosts. Bridge
-interfaces are used to create a XenServer VLAN network in which
-the VIFs for the VM instances are plugged. If no VIF bridge driver
-is plugged, the bridge is not made available. This configuration
-option takes in a value for the VIF driver.
-
-Possible values:
-
-* nova.virt.xenapi.vif.XenAPIOpenVswitchDriver (default)
-* nova.virt.xenapi.vif.XenAPIBridgeDriver (deprecated)
-
-Related options:
-
-* ``vlan_interface``
-* ``ovs_integration_bridge``
 """),
     # TODO(dharinic): Make this, a stevedore plugin
     cfg.StrOpt('image_upload_handler',

nova/tests/unit/virt/xenapi/stubs.py

@@ -397,10 +397,6 @@ def get_fake_session(error=None):
 class XenAPITestBase(test.TestCase):
     def setUp(self):
         super(XenAPITestBase, self).setUp()
-        # TODO(mriedem): The tests need to be fixed to work with the
-        # XenAPIOpenVswitchDriver vif driver.
-        self.flags(vif_driver='nova.virt.xenapi.vif.XenAPIBridgeDriver',
-                   group='xenserver')
         self.useFixture(ReplaceModule('XenAPI', fake))
         fake.reset()
 
@@ -408,9 +404,5 @@ class XenAPITestBase(test.TestCase):
 class XenAPITestBaseNoDB(test.NoDBTestCase):
     def setUp(self):
         super(XenAPITestBaseNoDB, self).setUp()
-        # TODO(mriedem): The tests need to be fixed to work with the
-        # XenAPIOpenVswitchDriver vif driver.
-        self.flags(vif_driver='nova.virt.xenapi.vif.XenAPIBridgeDriver',
-                   group='xenserver')
         self.useFixture(ReplaceModule('XenAPI', fake))
         fake.reset()

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

@@ -17,7 +17,6 @@ import mock
 
 from nova.compute import power_state
 from nova import exception
-from nova.network import model
 from nova import test
 from nova.tests.unit.virt.xenapi import stubs
 from nova.virt.xenapi import network_utils
@@ -151,37 +150,6 @@ class XenVIFDriverTestCase(XenVIFDriverTestBase):
                           instance, fake_vif, vm_ref)
 
 
-class XenAPIBridgeDriverTestCase(XenVIFDriverTestBase, object):
-    def setUp(self):
-        super(XenAPIBridgeDriverTestCase, self).setUp()
-        self.bridge_driver = vif.XenAPIBridgeDriver(self._session)
-
-    @mock.patch.object(vif.XenAPIBridgeDriver, '_ensure_vlan_bridge',
-                       return_value='fake_network_ref')
-    @mock.patch.object(vif.XenVIFDriver, '_create_vif',
-                       return_value='fake_vif_ref')
-    def test_plug_create_vlan(self, mock_create_vif, mock_ensure_vlan_bridge):
-        instance = {'name': "fake_instance_name"}
-        network = model.Network()
-        network._set_meta({'should_create_vlan': True})
-        vif = model.VIF()
-        vif._set_meta({'rxtx_cap': 1})
-        vif['network'] = network
-        vif['address'] = "fake_address"
-        vm_ref = "fake_vm_ref"
-        device = 1
-        ret_vif_ref = self.bridge_driver.plug(instance, vif, vm_ref, device)
-        self.assertEqual('fake_vif_ref', ret_vif_ref)
-
-    @mock.patch.object(vif.vm_utils, 'lookup', return_value=None)
-    def test_plug_exception(self, mock_lookup):
-        instance = {'name': "fake_instance_name"}
-        self.assertRaises(exception.VirtualInterfacePlugException,
-                          self.bridge_driver.plug, instance, fake_vif,
-                          vm_ref=None, device=1)
-        mock_lookup.assert_called_once_with(self._session, instance['name'])
-
-
 class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):
     def setUp(self):
         super(XenAPIOpenVswitchDriverTestCase, self).setUp()

nova/tests/unit/virt/xenapi/test_xenapi.py

@@ -45,6 +45,7 @@ from nova import context
 from nova import crypto
 from nova import db
 from nova import exception
+from nova.network import model as network_model
 from nova import objects
 from nova.objects import base
 from nova.objects import fields as obj_fields
@@ -796,10 +797,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase,
                    group='xenserver')
 
         self._test_spawn(IMAGE_IPXE_ISO, None, None)
-
+        self._check_call_plugin_serialized(mock_call_plugin_serialized)
-        # ipxe inject shouldn't be called
-        mock_call_plugin_serialized.assert_called_once_with(
-            'partition_utils.py', 'make_partition', 'fakedev', '2048', '-')
 
     @mock.patch.object(session.XenAPISession, 'call_plugin_serialized')
     def test_spawn_ipxe_iso_no_boot_menu_url(
@@ -809,10 +807,48 @@ class XenAPIVMTestCase(stubs.XenAPITestBase,
                    group='xenserver')
 
         self._test_spawn(IMAGE_IPXE_ISO, None, None)
+        self._check_call_plugin_serialized(mock_call_plugin_serialized)
+
+    def _check_call_plugin_serialized(self, mock_call_plugin_serialized):
+        vifs = xenapi_fake.get_all_records('VIF')
+        iface_id = vifs[list(vifs)[0]]['other_config']['neutron-port-id']
+
+        def _get_qbr_name(iface_id):
+            return ("qbr" + iface_id)[:network_model.NIC_NAME_LEN]
+
+        def _get_veth_pair_names(iface_id):
+            return (("qvb%s" % iface_id)[:network_model.NIC_NAME_LEN],
+                    ("qvo%s" % iface_id)[:network_model.NIC_NAME_LEN])
+
+        def _get_patch_port_pair_names(iface_id):
+            return (("vif%s" % iface_id)[:network_model.NIC_NAME_LEN],
+                    ("tap%s" % iface_id)[:network_model.NIC_NAME_LEN])
 
         # ipxe inject shouldn't be called
-        mock_call_plugin_serialized.assert_called_once_with(
+        call1 = mock.call('partition_utils.py', 'make_partition', 'fakedev',
-            'partition_utils.py', 'make_partition', 'fakedev', '2048', '-')
+                          '2048', '-')
+        linux_br_name = _get_qbr_name(iface_id)
+        qvb_name, qvo_name = _get_veth_pair_names(iface_id)
+        patch_port1, tap_name = _get_patch_port_pair_names(iface_id)
+
+        args = {'cmd': 'ip_link_get_dev',
+                'args': {'device_name': linux_br_name}
+                }
+        call2 = mock.call('xenhost.py', 'network_config', args)
+
+        args = {'cmd': 'ip_link_get_dev',
+                'args': {'device_name': qvo_name}
+                }
+        call3 = mock.call('xenhost.py', 'network_config', args)
+
+        args = {'cmd': 'ip_link_get_dev',
+                'args': {'device_name': tap_name}
+                }
+        call4 = mock.call('xenhost.py', 'network_config', args)
+        mock_call_plugin_serialized.assert_has_calls([call1,
+                                                      call2,
+                                                      call3,
+                                                      call4])
 
     @mock.patch.object(session.XenAPISession, 'call_plugin_serialized')
     def test_spawn_ipxe_iso_unknown_network_name(
@@ -822,10 +858,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase,
                    group='xenserver')
 
         self._test_spawn(IMAGE_IPXE_ISO, None, None)
-
+        self._check_call_plugin_serialized(mock_call_plugin_serialized)
-        # ipxe inject shouldn't be called
-        mock_call_plugin_serialized.assert_called_once_with(
-            'partition_utils.py', 'make_partition', 'fakedev', '2048', '-')
 
     def test_spawn_empty_dns(self):
         # Test spawning with an empty dns list.
@@ -1057,9 +1090,8 @@ class XenAPIVMTestCase(stubs.XenAPITestBase,
         self._create_instance()
         for vif_ref in xenapi_fake.get_all('VIF'):
             vif_rec = xenapi_fake.get_record('VIF', vif_ref)
-            self.assertEqual(vif_rec['qos_algorithm_type'], 'ratelimit')
+            self.assertEqual(vif_rec['qos_algorithm_type'], '')
-            self.assertEqual(vif_rec['qos_algorithm_params']['kbps'],
+            self.assertEqual(vif_rec['qos_algorithm_params'], {})
-                             str(3 * 10 * 1024))
 
     @mock.patch.object(crypto, 'ssh_encrypt_text')
     @mock.patch.object(stubs.FakeSessionForVMTests,

nova/virt/xenapi/fake.py

@@ -760,6 +760,9 @@ class SessionBase(object):
     def _plugin_xenhost_host_uptime(self, method, args):
         return jsonutils.dumps({"uptime": "fake uptime"})
 
+    def _plugin_xenhost_network_config(self, method, args):
+        return pickle.dumps({"fake_network": "fake conf"})
+
     def _plugin_xenhost_get_pci_device_details(self, method, args):
         """Simulate the ouput of three pci devices.
 
@@ -1007,6 +1010,18 @@ class SessionBase(object):
                 _db_content[cls], func[len('get_by_'):], params[1],
                 return_singleton=return_singleton)
 
+        if func == 'get_VIFs':
+            self._check_arg_count(params, 2)
+            # FIXME(mriedem): figure out how to use _get_by_field for VIFs,
+            # or just stop relying on this fake DB and use mock
+            return _db_content['VIF'].keys()
+
+        if func == 'get_bridge':
+            self._check_arg_count(params, 2)
+            # FIXME(mriedem): figure out how to use _get_by_field for bridge,
+            # or just stop relying on this fake DB and use mock
+            return 'fake_bridge'
+
         if len(params) == 2:
             field = func[len('get_'):]
             ref = params[1]

nova/virt/xenapi/vif.py

@@ -131,114 +131,6 @@ class XenVIFDriver(object):
         pass
 
 
-class XenAPIBridgeDriver(XenVIFDriver):
-    """VIF Driver for XenAPI that uses XenAPI to create Networks."""
-
-    # NOTE(huanxie): This driver uses linux bridge as backend for XenServer,
-    # it only supports nova network, for using neutron, you should use
-    # XenAPIOpenVswitchDriver
-
-    def plug(self, instance, vif, vm_ref=None, device=None):
-        if not vm_ref:
-            vm_ref = vm_utils.lookup(self._session, instance['name'])
-        if not vm_ref:
-            raise exception.VirtualInterfacePlugException(
-                "Cannot find instance %s, discard vif plug" % instance['name'])
-
-        # if VIF already exists, return this vif_ref directly
-        vif_ref = self._get_vif_ref(vif, vm_ref)
-        if vif_ref:
-            LOG.debug("VIF %s already exists when plug vif",
-                      vif_ref, instance=instance)
-            return vif_ref
-
-        if not device:
-            device = 0
-
-        if vif['network'].get_meta('should_create_vlan'):
-            network_ref = self._ensure_vlan_bridge(vif['network'])
-        else:
-            network_ref = network_utils.find_network_with_bridge(
-                    self._session, vif['network']['bridge'])
-        vif_rec = {}
-        vif_rec['device'] = str(device)
-        vif_rec['network'] = network_ref
-        vif_rec['VM'] = vm_ref
-        vif_rec['MAC'] = vif['address']
-        vif_rec['MTU'] = '1500'
-        vif_rec['other_config'] = {}
-        if vif.get_meta('rxtx_cap'):
-            vif_rec['qos_algorithm_type'] = 'ratelimit'
-            vif_rec['qos_algorithm_params'] = {'kbps':
-                         str(int(vif.get_meta('rxtx_cap')) * 1024)}
-        else:
-            vif_rec['qos_algorithm_type'] = ''
-            vif_rec['qos_algorithm_params'] = {}
-        return self._create_vif(vif, vif_rec, vm_ref)
-
-    def _ensure_vlan_bridge(self, network):
-        """Ensure that a VLAN bridge exists."""
-
-        vlan_num = network.get_meta('vlan')
-        bridge = network['bridge']
-        bridge_interface = (CONF.vlan_interface or
-                            network.get_meta('bridge_interface'))
-        # Check whether bridge already exists
-        # Retrieve network whose name_label is "bridge"
-        network_ref = network_utils.find_network_with_name_label(
-                                        self._session, bridge)
-        if network_ref is None:
-            # If bridge does not exists
-            # 1 - create network
-            description = 'network for nova bridge %s' % bridge
-            network_rec = {'name_label': bridge,
-                           'name_description': description,
-                           'other_config': {}}
-            network_ref = self._session.call_xenapi('network.create',
-                                                    network_rec)
-            # 2 - find PIF for VLAN NOTE(salvatore-orlando): using double
-            # quotes inside single quotes as xapi filter only support
-            # tokens in double quotes
-            expr = ('field "device" = "%s" and field "VLAN" = "-1"' %
-                    bridge_interface)
-            pifs = self._session.call_xenapi('PIF.get_all_records_where',
-                                             expr)
-
-            # Multiple PIF are ok: we are dealing with a pool
-            if len(pifs) == 0:
-                raise Exception(_('Found no PIF for device %s') %
-                                bridge_interface)
-            for pif_ref in pifs.keys():
-                self._session.call_xenapi('VLAN.create',
-                                          pif_ref,
-                                          str(vlan_num),
-                                          network_ref)
-        else:
-            # Check VLAN tag is appropriate
-            network_rec = self._session.call_xenapi('network.get_record',
-                                                    network_ref)
-            # Retrieve PIFs from network
-            for pif_ref in network_rec['PIFs']:
-                # Retrieve VLAN from PIF
-                pif_rec = self._session.call_xenapi('PIF.get_record',
-                                                    pif_ref)
-                pif_vlan = int(pif_rec['VLAN'])
-                # Raise an exception if VLAN != vlan_num
-                if pif_vlan != vlan_num:
-                    raise Exception(_("PIF %(pif_uuid)s for network "
-                                      "%(bridge)s has VLAN id %(pif_vlan)d. "
-                                      "Expected %(vlan_num)d"),
-                                    {'pif_uuid': pif_rec['uuid'],
-                                     'bridge': bridge,
-                                     'pif_vlan': pif_vlan,
-                                     'vlan_num': vlan_num})
-
-        return network_ref
-
-    def unplug(self, instance, vif, vm_ref):
-        super(XenAPIBridgeDriver, self).unplug(instance, vif, vm_ref)
-
-
 class XenAPIOpenVswitchDriver(XenVIFDriver):
     """VIF driver for Open vSwitch with XenAPI."""
 

nova/virt/xenapi/vmops.py

@@ -58,6 +58,7 @@ from nova.virt import driver as virt_driver
 from nova.virt import firewall
 from nova.virt.xenapi import agent as xapi_agent
 from nova.virt.xenapi.image import utils as image_utils
+from nova.virt.xenapi import vif as xapi_vif
 from nova.virt.xenapi import vm_utils
 from nova.virt.xenapi import volume_utils
 from nova.virt.xenapi import volumeops
@@ -147,8 +148,8 @@ class VMOps(object):
         self.firewall_driver = firewall.load_driver(
             DEFAULT_FIREWALL_DRIVER,
             xenapi_session=self._session)
-        vif_impl = importutils.import_class(CONF.xenserver.vif_driver)
+        self.vif_driver = xapi_vif.XenAPIOpenVswitchDriver(
-        self.vif_driver = vif_impl(xenapi_session=self._session)
+            xenapi_session=self._session)
         self.default_root_dev = '/dev/sda'
 
         image_handler_cfg = CONF.xenserver.image_handler

releasenotes/notes/remove-xenserver-vif-driver-option-850f8dcfe54bca7c.yaml

@@ -0,0 +1,11 @@
+---
+upgrade:
+  - |
+    The ``[xenserver]/vif_driver`` configuration option was deprecated in
+    the 15.0.0 Ocata release and has now been removed. The only supported
+    vif driver is now ``XenAPIOpenVswitchDriver`` used with Neutron as the
+    backend networking service configured to run the
+    ``neutron-openvswitch-agent`` service. See the `XenServer configuration
+    guide`_ for more details on networking setup.
+
+    .. _XenServer configuration guide: https://docs.openstack.org/nova/latest/admin/configuration/hypervisor-xen-api.html