xenapi: support the hotplug of a neutron port

Nova already has support for hotplugging neutorn ports in the libvirt
driver. This extends the support to the XenAPI driver, implement
attaching/detaching VIFs

I have made a patch to run releated testcase in xenserver CI
https://review.openstack.org/#/c/416797/

Change-Id: I22f3fe52d07100592015007653c7f8c47c25d22c
Implements: blueprint xenapi-vif-hotplug

doc/source/support-matrix.ini

@@ -138,7 +138,7 @@ notes=The attach interface operation provides a means to hotplug
   In a cloud model it would be more typical to just spin up a
   new instance with more interfaces.
 cli=nova interface-attach <server>
-driver-impl-xenserver=missing
+driver-impl-xenserver=complete
 driver-impl-libvirt-kvm-x86=complete
 driver-impl-libvirt-kvm-ppc64=complete
 driver-impl-libvirt-kvm-s390x=complete

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

@@ -200,3 +200,18 @@ class XenAPIDriverTestCase(stubs.XenAPITestBaseNoDB):
 
         self.assertTrue(mock_cleanup.called)
         self.assertTrue(mock_ensure.called)
+
+    @mock.patch.object(xenapi_driver.vmops.VMOps, 'attach_interface')
+    def test_attach_interface(self, mock_attach_interface):
+        driver = self._get_driver()
+        driver.attach_interface('fake_context', 'fake_instance',
+                                'fake_image_meta', 'fake_vif')
+        mock_attach_interface.assert_called_once_with('fake_instance',
+                                                      'fake_vif')
+
+    @mock.patch.object(xenapi_driver.vmops.VMOps, 'detach_interface')
+    def test_detach_interface(self, mock_detach_interface):
+        driver = self._get_driver()
+        driver.detach_interface('fake_context', 'fake_instance', 'fake_vif')
+        mock_detach_interface.assert_called_once_with('fake_instance',
+                                                      'fake_vif')

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

@@ -15,12 +15,14 @@
 
 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
 from nova.virt.xenapi import vif
+from nova.virt.xenapi import vm_utils
 
 
 fake_vif = {
@@ -126,14 +128,17 @@ class XenVIFDriverTestCase(XenVIFDriverTestBase):
                           self.base_driver._create_vif,
                           "fake_vif", "missing_vif_rec", "fake_vm_ref")
 
+    @mock.patch.object(vif.XenVIFDriver, 'hot_unplug')
     @mock.patch.object(vif.XenVIFDriver, '_get_vif_ref',
                        return_value='fake_vif_ref')
-    def test_unplug(self, mock_get_vif_ref):
+    def test_unplug(self, mock_get_vif_ref, mock_hot_unplug):
         instance = {'name': "fake_instance"}
         vm_ref = "fake_vm_ref"
         self.base_driver.unplug(instance, fake_vif, vm_ref)
         expected = [mock.call('VIF.destroy', 'fake_vif_ref')]
         self.assertEqual(expected, self._session.call_xenapi.call_args_list)
+        mock_hot_unplug.assert_called_once_with(
+            fake_vif, instance, 'fake_vm_ref', 'fake_vif_ref')
 
     @mock.patch.object(vif.XenVIFDriver, '_get_vif_ref',
                        return_value='missing_vif_ref')
@@ -173,6 +178,7 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):
         super(XenAPIOpenVswitchDriverTestCase, self).setUp()
         self.ovs_driver = vif.XenAPIOpenVswitchDriver(self._session)
 
+    @mock.patch.object(vif.XenAPIOpenVswitchDriver, 'hot_plug')
     @mock.patch.object(vif.XenVIFDriver, '_create_vif',
                        return_value='fake_vif_ref')
     @mock.patch.object(vif.XenAPIOpenVswitchDriver,
@@ -181,7 +187,7 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):
     @mock.patch.object(vif.vm_utils, 'lookup', return_value='fake_vm_ref')
     def test_plug(self, mock_lookup, mock_get_vif_ref,
                   mock_create_vif_interim_network,
-                  mock_create_vif):
+                  mock_create_vif, mock_hot_plug):
         instance = {'name': "fake_instance_name"}
         ret_vif_ref = self.ovs_driver.plug(
             instance, fake_vif, vm_ref=None, device=1)
@@ -190,6 +196,8 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):
         self.assertTrue(mock_create_vif_interim_network.called)
         self.assertTrue(mock_create_vif.called)
         self.assertEqual('fake_vif_ref', ret_vif_ref)
+        mock_hot_plug.assert_called_once_with(fake_vif, instance,
+                                              'fake_vm_ref', 'fake_vif_ref')
 
     @mock.patch.object(vif.XenAPIOpenVswitchDriver, '_delete_linux_bridge')
     @mock.patch.object(vif.XenAPIOpenVswitchDriver, '_delete_linux_port')
@@ -299,3 +307,56 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase):
         network_ref = self.ovs_driver.create_vif_interim_network(fake_vif)
         self.assertTrue(mock_network_create.called)
         self.assertEqual(network_ref, 'new_network_ref')
+
+    @mock.patch.object(vif.XenAPIOpenVswitchDriver, 'post_start_actions')
+    @mock.patch.object(vm_utils, 'get_power_state')
+    def test_hot_plug_power_on(self, mock_get_power_state,
+                               mock_post_start_actions):
+        vif_ref = "fake_vif_ref"
+        vif = "fake_vif"
+        instance = "fake_instance"
+        vm_ref = "fake_vm_ref"
+        mock_get_power_state.return_value = power_state.RUNNING
+        mock_VIF_plug = self.mock_patch_object(
+                                self._session.VIF, 'plug', return_val=None)
+        self.ovs_driver.hot_plug(vif, instance, vm_ref, vif_ref)
+        mock_VIF_plug.assert_called_once_with(vif_ref)
+        mock_post_start_actions.assert_called_once_with(instance, vif_ref)
+        mock_get_power_state.assert_called_once_with(self._session, vm_ref)
+
+    @mock.patch.object(vm_utils, 'get_power_state')
+    def test_hot_plug_power_off(self, mock_get_power_state):
+        vif_ref = "fake_vif_ref"
+        vif = "fake_vif"
+        instance = "fake_instance"
+        vm_ref = "fake_vm_ref"
+        mock_get_power_state.return_value = power_state.SHUTDOWN
+        mock_VIF_plug = self.mock_patch_object(
+                                self._session.VIF, 'plug', return_val=None)
+        self.ovs_driver.hot_plug(vif, instance, vm_ref, vif_ref)
+        mock_VIF_plug.assert_not_called()
+        mock_get_power_state.assert_called_once_with(self._session, vm_ref)
+
+    @mock.patch.object(vm_utils, 'get_power_state')
+    def test_hot_unplug_power_on(self, mock_get_power_state):
+        vm_ref = 'fake_vm_ref'
+        vif_ref = 'fake_vif_ref'
+        instance = 'fake_instance'
+        mock_get_power_state.return_value = power_state.RUNNING
+        mock_VIF_unplug = self.mock_patch_object(
+                                self._session.VIF, 'unplug', return_val=None)
+        self.ovs_driver.hot_unplug(fake_vif, instance, vm_ref, vif_ref)
+        mock_VIF_unplug.assert_called_once_with(vif_ref)
+        mock_get_power_state.assert_called_once_with(self._session, vm_ref)
+
+    @mock.patch.object(vm_utils, 'get_power_state')
+    def test_hot_unplug_power_off(self, mock_get_power_state):
+        vm_ref = 'fake_vm_ref'
+        vif_ref = 'fake_vif_ref'
+        instance = 'fake_instance'
+        mock_get_power_state.return_value = power_state.SHUTDOWN
+        mock_VIF_unplug = self.mock_patch_object(
+                                self._session.VIF, 'unplug', return_val=None)
+        self.ovs_driver.hot_unplug(fake_vif, instance, vm_ref, vif_ref)
+        mock_VIF_unplug.assert_not_called()
+        mock_get_power_state.assert_called_once_with(self._session, vm_ref)

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

@@ -23,6 +23,7 @@ except ImportError:
 from eventlet import greenthread
 import mock
 from os_xenapi.client import session as xenapi_session
+import six
 
 from nova.compute import power_state
 from nova.compute import task_states
@@ -1726,3 +1727,92 @@ class GetVdisForInstanceTestCase(VMOpsTestBase):
             # our stub method is called which asserts the password is scrubbed
             self.assertTrue(debug_mock.called)
             self.assertTrue(fake_debug.matched)
+
+
+class AttachInterfaceTestCase(VMOpsTestBase):
+    """Test VIF hot plug/unplug"""
+    def setUp(self):
+        super(AttachInterfaceTestCase, self).setUp()
+        self.vmops.vif_driver = mock.Mock()
+        self.fake_vif = {'id': '12345'}
+        self.fake_instance = mock.Mock()
+        self.fake_instance.uuid = '6478'
+
+    @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref')
+    def test_attach_interface(self, mock_get_vm_opaque_ref):
+        mock_get_vm_opaque_ref.return_value = 'fake_vm_ref'
+        with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\
+            as fake_devices:
+            fake_devices.return_value = [2, 3, 4]
+            self.vmops.attach_interface(self.fake_instance, self.fake_vif)
+            fake_devices.assert_called_once_with('fake_vm_ref')
+            mock_get_vm_opaque_ref.assert_called_once_with(self.fake_instance)
+            self.vmops.vif_driver.plug.assert_called_once_with(
+                self.fake_instance, self.fake_vif, vm_ref='fake_vm_ref',
+                device=2)
+
+    @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref')
+    def test_attach_interface_no_devices(self, mock_get_vm_opaque_ref):
+        mock_get_vm_opaque_ref.return_value = 'fake_vm_ref'
+        with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\
+            as fake_devices:
+            fake_devices.return_value = []
+            self.assertRaises(exception.InterfaceAttachFailed,
+                              self.vmops.attach_interface,
+                              self.fake_instance, self.fake_vif)
+
+    @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref')
+    def test_attach_interface_plug_failed(self, mock_get_vm_opaque_ref):
+        mock_get_vm_opaque_ref.return_value = 'fake_vm_ref'
+        with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\
+            as fake_devices:
+            fake_devices.return_value = [2, 3, 4]
+            self.vmops.vif_driver.plug.side_effect =\
+                exception.VirtualInterfacePlugException('Failed to plug VIF')
+            self.assertRaises(exception.VirtualInterfacePlugException,
+                              self.vmops.attach_interface,
+                              self.fake_instance, self.fake_vif)
+            self.vmops.vif_driver.plug.assert_called_once_with(
+                self.fake_instance, self.fake_vif, vm_ref='fake_vm_ref',
+                device=2)
+            self.vmops.vif_driver.unplug.assert_called_once_with(
+                self.fake_instance, self.fake_vif, 'fake_vm_ref')
+
+    @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref')
+    def test_attach_interface_reraise_exception(self, mock_get_vm_opaque_ref):
+        mock_get_vm_opaque_ref.return_value = 'fake_vm_ref'
+        with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\
+            as fake_devices:
+            fake_devices.return_value = [2, 3, 4]
+            self.vmops.vif_driver.plug.side_effect =\
+                exception.VirtualInterfacePlugException('Failed to plug VIF')
+            self.vmops.vif_driver.unplug.side_effect =\
+                exception.VirtualInterfaceUnplugException(
+                    'Failed to unplug VIF')
+            ex = self.assertRaises(exception.VirtualInterfacePlugException,
+                                   self.vmops.attach_interface,
+                                   self.fake_instance, self.fake_vif)
+            self.assertEqual('Failed to plug VIF', six.text_type(ex))
+            self.vmops.vif_driver.plug.assert_called_once_with(
+                self.fake_instance, self.fake_vif, vm_ref='fake_vm_ref',
+                device=2)
+            self.vmops.vif_driver.unplug.assert_called_once_with(
+                self.fake_instance, self.fake_vif, 'fake_vm_ref')
+
+    @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref')
+    def test_detach_interface(self, mock_get_vm_opaque_ref):
+        mock_get_vm_opaque_ref.return_value = 'fake_vm_ref'
+        self.vmops.detach_interface(self.fake_instance, self.fake_vif)
+        mock_get_vm_opaque_ref.assert_called_once_with(self.fake_instance)
+        self.vmops.vif_driver.unplug.assert_called_once_with(
+            self.fake_instance, self.fake_vif, 'fake_vm_ref')
+
+    @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref')
+    def test_detach_interface_exception(self, mock_get_vm_opaque_ref):
+        mock_get_vm_opaque_ref.return_value = 'fake_vm_ref'
+        self.vmops.vif_driver.unplug.side_effect =\
+            exception.VirtualInterfaceUnplugException('Failed to unplug VIF')
+
+        self.assertRaises(exception.VirtualInterfaceUnplugException,
+                          self.vmops.detach_interface,
+                          self.fake_instance, self.fake_vif)

nova/virt/xenapi/driver.py

@@ -66,6 +66,13 @@ def invalid_option(option_name, recommended_value):
 
 class XenAPIDriver(driver.ComputeDriver):
     """A connection to XenServer or Xen Cloud Platform."""
+    capabilities = {
+        "has_imagecache": False,
+        "supports_recreate": False,
+        "supports_migrate_to_same_host": False,
+        "supports_attach_interface": True,
+        "supports_device_tagging": False,
+    }
 
     def __init__(self, virtapi, read_only=False):
         super(XenAPIDriver, self).__init__(virtapi)
@@ -654,3 +661,39 @@ class XenAPIDriver(driver.ComputeDriver):
         :returns: dict of  nova uuid => dict of usage info
         """
         return self._vmops.get_per_instance_usage()
+
+    def attach_interface(self, context, instance, image_meta, vif):
+        """Use hotplug to add a network interface to a running instance.
+
+        The counter action to this is :func:`detach_interface`.
+
+        :param context: The request context.
+        :param nova.objects.instance.Instance instance:
+            The instance which will get an additional network interface.
+        :param nova.objects.ImageMeta image_meta:
+            The metadata of the image of the instance.
+        :param nova.network.model.VIF vif:
+            The object which has the information about the interface to attach.
+
+        :raise nova.exception.NovaException: If the attach fails.
+
+        :return: None
+        """
+        self._vmops.attach_interface(instance, vif)
+
+    def detach_interface(self, context, instance, vif):
+        """Use hotunplug to remove a network interface from a running instance.
+
+        The counter action to this is :func:`attach_interface`.
+
+        :param context: The request context.
+        :param nova.objects.instance.Instance instance:
+            The instance which gets a network interface removed.
+        :param nova.network.model.VIF vif:
+            The object which has the information about the interface to detach.
+
+        :raise nova.exception.NovaException: If the detach fails.
+
+        :return: None
+        """
+        self._vmops.detach_interface(instance, vif)

nova/virt/xenapi/vif.py

@@ -19,6 +19,7 @@
 
 from oslo_log import log as logging
 
+from nova.compute import power_state
 import nova.conf
 from nova import exception
 from nova.i18n import _
@@ -72,6 +73,8 @@ class XenVIFDriver(object):
                 LOG.debug("vif didn't exist, no need to unplug vif %s",
                         vif, instance=instance)
                 return
+            # hot unplug the VIF first
+            self.hot_unplug(vif, instance, vm_ref, vif_ref)
             self._session.call_xenapi('VIF.destroy', vif_ref)
         except Exception as e:
             LOG.warning(
@@ -80,6 +83,44 @@ class XenVIFDriver(object):
             raise exception.NovaException(
                 reason=_("Failed to unplug vif %s") % vif)
 
+    def hot_plug(self, vif, instance, vm_ref, vif_ref):
+        """hotplug virtual interface to running instance.
+        :param nova.network.model.VIF vif:
+            The object which has the information about the interface to attach.
+        :param nova.objects.instance.Instance instance:
+            The instance which will get an additional network interface.
+        :param string vm_ref:
+            The instance's reference from hypervisor's point of view.
+        :param string vif_ref:
+            The interface's reference from hypervisor's point of view.
+        :return: None
+        """
+        pass
+
+    def hot_unplug(self, vif, instance, vm_ref, vif_ref):
+        """hot unplug virtual interface from running instance.
+        :param nova.network.model.VIF vif:
+            The object which has the information about the interface to detach.
+        :param nova.objects.instance.Instance instance:
+            The instance which will remove additional network interface.
+        :param string vm_ref:
+            The instance's reference from hypervisor's point of view.
+        :param string vif_ref:
+            The interface's reference from hypervisor's point of view.
+        :return: None
+        """
+        pass
+
+    def post_start_actions(self, instance, vif_ref):
+        """post actions when the instance is power on.
+        :param nova.objects.instance.Instance instance:
+            The instance which will execute extra actions after power on
+        :param string vif_ref:
+            The interface's reference from hypervisor's point of view.
+        :return: None
+        """
+        pass
+
 
 class XenAPIBridgeDriver(XenVIFDriver):
     """VIF Driver for XenAPI that uses XenAPI to create Networks."""
@@ -181,10 +222,6 @@ class XenAPIBridgeDriver(XenVIFDriver):
     def unplug(self, instance, vif, vm_ref):
         super(XenAPIBridgeDriver, self).unplug(instance, vif, vm_ref)
 
-    def post_start_actions(self, instance, vif_ref):
-        """no further actions needed for this driver type"""
-        pass
-
 
 class XenAPIOpenVswitchDriver(XenVIFDriver):
     """VIF driver for Open vSwitch with XenAPI."""
@@ -221,7 +258,12 @@ class XenAPIOpenVswitchDriver(XenVIFDriver):
         # OVS on the hypervisor monitors this key and uses it to
         # set the iface-id attribute
         vif_rec['other_config'] = {'nicira-iface-id': vif['id']}
-        return self._create_vif(vif, vif_rec, vm_ref)
+        vif_ref = self._create_vif(vif, vif_rec, vm_ref)
+
+        # call XenAPI to plug vif
+        self.hot_plug(vif, instance, vm_ref, vif_ref)
+
+        return vif_ref
 
     def unplug(self, instance, vif, vm_ref):
         """unplug vif:
@@ -289,6 +331,29 @@ class XenAPIOpenVswitchDriver(XenVIFDriver):
             raise exception.VirtualInterfaceUnplugException(
                 reason=_("Failed to delete bridge"))
 
+    def hot_plug(self, vif, instance, vm_ref, vif_ref):
+        # hot plug vif only when VM's power state is running
+        LOG.debug("Hot plug vif, vif: %s", vif, instance=instance)
+        state = vm_utils.get_power_state(self._session, vm_ref)
+        if state != power_state.RUNNING:
+            LOG.debug("Skip hot plug VIF, VM is not running, vif: %s", vif,
+                      instance=instance)
+            return
+
+        self._session.VIF.plug(vif_ref)
+        self.post_start_actions(instance, vif_ref)
+
+    def hot_unplug(self, vif, instance, vm_ref, vif_ref):
+        # hot unplug vif only when VM's power state is running
+        LOG.debug("Hot unplug vif, vif: %s", vif, instance=instance)
+        state = vm_utils.get_power_state(self._session, vm_ref)
+        if state != power_state.RUNNING:
+            LOG.debug("Skip hot unplug VIF, VM is not running, vif: %s", vif,
+                      instance=instance)
+            return
+
+        self._session.VIF.unplug(vif_ref)
+
     def _get_qbr_name(self, iface_id):
         return ("qbr" + iface_id)[:network_model.NIC_NAME_LEN]
 

nova/virt/xenapi/vmops.py

@@ -2484,3 +2484,47 @@ class VMOps(object):
                     volume_utils.forget_sr(self._session, sr_ref)
 
         return sr_uuid_map
+
+    def attach_interface(self, instance, vif):
+        LOG.debug("Attach interface, vif info: %s", vif, instance=instance)
+        vm_ref = self._get_vm_opaque_ref(instance)
+
+        @utils.synchronized('xenapi-vif-' + vm_ref)
+        def _attach_interface(instance, vm_ref, vif):
+            # find device for use with XenAPI
+            allowed_devices = self._session.VM.get_allowed_VIF_devices(vm_ref)
+            if allowed_devices is None or len(allowed_devices) == 0:
+                raise exception.InterfaceAttachFailed(
+                    _('attach network interface %(vif_id)s to instance '
+                      '%(instance_uuid)s failed, no allowed devices.'),
+                    vif_id=vif['id'], instance_uuid=instance.uuid)
+            device = allowed_devices[0]
+            try:
+                # plug VIF
+                self.vif_driver.plug(instance, vif, vm_ref=vm_ref,
+                                     device=device)
+                # set firewall filtering
+                self.firewall_driver.setup_basic_filtering(instance, [vif])
+            except exception.NovaException:
+                with excutils.save_and_reraise_exception():
+                    LOG.exception(_LE('attach network interface %s failed.'),
+                                  vif['id'], instance=instance)
+                    try:
+                        self.vif_driver.unplug(instance, vif, vm_ref)
+                    except exception.NovaException:
+                        # if unplug failed, no need to raise exception
+                        LOG.warning(_LW('Unplug VIF %s failed.'),
+                                    vif['id'], instance=instance)
+
+        _attach_interface(instance, vm_ref, vif)
+
+    def detach_interface(self, instance, vif):
+        LOG.debug("Detach interface, vif info: %s", vif, instance=instance)
+
+        try:
+            vm_ref = self._get_vm_opaque_ref(instance)
+            self.vif_driver.unplug(instance, vif, vm_ref)
+        except exception.NovaException:
+            with excutils.save_and_reraise_exception():
+                LOG.exception(_LE('detach network interface %s failed.'),
+                              vif['id'], instance=instance)

releasenotes/notes/bp-xenapi-vif-hotplug-2a2b913c49123fe0.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - The XenServer compute driver now supports hot-plugging
+    virtual network interfaces.