Merge "xenapi: Avoid hotplugging volumes on resize."

6 files changed, 119 insertions, 28 deletions

diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index f2799b8f3..3ca69dc4c 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -911,7 +911,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase):
             def __init__(self):
                 self.finish_revert_migration_called = False
 
-            def finish_revert_migration(self, instance):
+            def finish_revert_migration(self, instance, block_info):
                 self.finish_revert_migration_called = True
 
         conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False)
diff --git a/nova/tests/virt/xenapi/test_volumeops.py b/nova/tests/virt/xenapi/test_volumeops.py
index bc84386c4..5b5c38139 100644
--- a/nova/tests/virt/xenapi/test_volumeops.py
+++ b/nova/tests/virt/xenapi/test_volumeops.py
@@ -15,11 +15,12 @@
 #    under the License.
 
 from nova import test
+from nova.tests.xenapi import stubs
 from nova.virt.xenapi import volumeops
 
 
 class VolumeAttachTestCase(test.TestCase):
-    def test_connect_volume_call(self):
+    def test_attach_volume_call(self):
         ops = volumeops.VolumeOps('session')
         self.mox.StubOutWithMock(ops, 'connect_volume')
         self.mox.StubOutWithMock(volumeops.vm_utils, 'vm_ref_or_raise')
@@ -31,9 +32,79 @@ class VolumeAttachTestCase(test.TestCase):
         volumeops.volume_utils.get_device_number('mountpoint').AndReturn(
             'devnumber')
 
-        ops.connect_volume('conn_data', 'devnumber', 'instance_1', 'vmref')
+        ops.connect_volume(
+            'conn_data', 'devnumber', 'instance_1', 'vmref', hotplug=True)
 
         self.mox.ReplayAll()
         ops.attach_volume(
             dict(driver_volume_type='iscsi', data='conn_data'),
             'instance_1', 'mountpoint')
+
+    def test_attach_volume_no_hotplug(self):
+        ops = volumeops.VolumeOps('session')
+        self.mox.StubOutWithMock(ops, 'connect_volume')
+        self.mox.StubOutWithMock(volumeops.vm_utils, 'vm_ref_or_raise')
+        self.mox.StubOutWithMock(volumeops.volume_utils, 'get_device_number')
+
+        volumeops.vm_utils.vm_ref_or_raise('session', 'instance_1').AndReturn(
+            'vmref')
+
+        volumeops.volume_utils.get_device_number('mountpoint').AndReturn(
+            'devnumber')
+
+        ops.connect_volume(
+            'conn_data', 'devnumber', 'instance_1', 'vmref', hotplug=False)
+
+        self.mox.ReplayAll()
+        ops.attach_volume(
+            dict(driver_volume_type='iscsi', data='conn_data'),
+            'instance_1', 'mountpoint', hotplug=False)
+
+    def test_connect_volume_no_hotplug(self):
+        session = stubs.FakeSessionForVolumeTests('fake_uri')
+        ops = volumeops.VolumeOps(session)
+        instance_name = 'instance_1'
+        sr_uuid = '1'
+        sr_label = 'Disk-for:%s' % instance_name
+        sr_params = ''
+        sr_ref = 'sr_ref'
+        vdi_uuid = '2'
+        vdi_ref = 'vdi_ref'
+        vbd_ref = 'vbd_ref'
+        connection_data = {'vdi_uuid': vdi_uuid}
+        vm_ref = 'vm_ref'
+        dev_number = 1
+
+        called = {'xenapi': False}
+
+        def fake_call_xenapi(self, *args, **kwargs):
+            # Only used for VBD.plug in this code path.
+            called['xenapi'] = True
+            raise Exception()
+
+        self.stubs.Set(ops._session, 'call_xenapi', fake_call_xenapi)
+
+        self.mox.StubOutWithMock(volumeops.volume_utils, 'parse_sr_info')
+        self.mox.StubOutWithMock(ops, 'introduce_sr')
+        self.mox.StubOutWithMock(volumeops.volume_utils, 'introduce_vdi')
+        self.mox.StubOutWithMock(volumeops.vm_utils, 'create_vbd')
+
+        volumeops.volume_utils.parse_sr_info(
+            connection_data, sr_label).AndReturn(
+                tuple([sr_uuid, sr_label, sr_params]))
+
+        ops.introduce_sr(sr_uuid, sr_label, sr_params).AndReturn(sr_ref)
+
+        volumeops.volume_utils.introduce_vdi(
+            session, sr_ref, vdi_uuid, None).AndReturn(vdi_ref)
+
+        volumeops.vm_utils.create_vbd(
+            session, vm_ref, vdi_ref, dev_number,
+            bootable=False, osvol=True).AndReturn(vbd_ref)
+
+        self.mox.ReplayAll()
+
+        ops.connect_volume(connection_data, dev_number, instance_name,
+                           vm_ref, hotplug=False)
+
+        self.assertEquals(False, called['xenapi'])
diff --git a/nova/virt/xenapi/driver.py b/nova/virt/xenapi/driver.py
index 7bf27b7e0..10bc99fef 100644
--- a/nova/virt/xenapi/driver.py
+++ b/nova/virt/xenapi/driver.py
@@ -178,25 +178,15 @@ class XenAPIDriver(driver.ComputeDriver):
                                 block_device_info=None):
         """Finish reverting a resize, powering back on the instance"""
         # NOTE(vish): Xen currently does not use network info.
-        self._vmops.finish_revert_migration(instance)
-        self._attach_mapped_block_devices(instance, block_device_info)
+        self._vmops.finish_revert_migration(instance, block_device_info)
 
     def finish_migration(self, context, migration, instance, disk_info,
                          network_info, image_meta, resize_instance=False,
                          block_device_info=None):
         """Completes a resize, turning on the migrated instance"""
         self._vmops.finish_migration(context, migration, instance, disk_info,
-                                     network_info, image_meta, resize_instance)
-        self._attach_mapped_block_devices(instance, block_device_info)
-
-    def _attach_mapped_block_devices(self, instance, block_device_info):
-        block_device_mapping = driver.block_device_info_get_mapping(
-                block_device_info)
-        for vol in block_device_mapping:
-            connection_info = vol['connection_info']
-            mount_device = vol['mount_device'].rpartition("/")[2]
-            self.attach_volume(connection_info,
-                    instance['name'], mount_device)
+                                     network_info, image_meta, resize_instance,
+                                     block_device_info)
 
     def snapshot(self, context, instance, image_id):
         """ Create snapshot from a running VM instance """
diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py
index 9af8a9f41..666e46754 100644
--- a/nova/virt/xenapi/fake.py
+++ b/nova/virt/xenapi/fake.py
@@ -727,6 +727,8 @@ class SessionBase(object):
             return lambda *params: self._create(name, params)
         elif self._is_destroy(name):
             return lambda *params: self._destroy(name, params)
+        elif name == 'XenAPI':
+            return FakeXenAPI()
         else:
             return None
 
@@ -890,6 +892,11 @@ class SessionBase(object):
         return result
 
 
+class FakeXenAPI(object):
+    def __init__(self):
+        self.Failure = Failure
+
+
 # Based upon _Method from xmlrpclib.
 class _Dispatcher:
     def __init__(self, send, name):
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 8d4687fe8..a96a90f0e 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -39,11 +39,13 @@ from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import timeutils
 from nova import utils
+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 import pool_states
 from nova.virt.xenapi import vm_utils
 from nova.virt.xenapi import volume_utils
+from nova.virt.xenapi import volumeops
 
 
 LOG = logging.getLogger(__name__)
@@ -147,6 +149,7 @@ class VMOps(object):
         self.compute_api = compute.API()
         self._session = session
         self._virtapi = virtapi
+        self._volumeops = volumeops.VolumeOps(self._session)
         self.firewall_driver = firewall.load_driver(
             DEFAULT_FIREWALL_DRIVER,
             self._virtapi,
@@ -179,7 +182,20 @@ class VMOps(object):
         vm_ref = vm_utils.lookup(self._session, name_label)
         return self._destroy(instance, vm_ref, network_info)
 
-    def finish_revert_migration(self, instance):
+    def _attach_mapped_block_devices(self, instance, block_device_info):
+        # We are attaching these volumes before start (no hotplugging)
+        # because some guests (windows) don't load PV drivers quickly
+        block_device_mapping = virt_driver.block_device_info_get_mapping(
+                block_device_info)
+        for vol in block_device_mapping:
+            connection_info = vol['connection_info']
+            mount_device = vol['mount_device'].rpartition("/")[2]
+            self._volumeops.attach_volume(connection_info,
+                                          instance['name'],
+                                          mount_device,
+                                          hotplug=False)
+
+    def finish_revert_migration(self, instance, block_device_info=None):
         # NOTE(sirp): the original vm was suffixed with '-orig'; find it using
         # the old suffix, remove the suffix, then power it back on.
         name_label = self._get_orig_vm_name_label(instance)
@@ -190,6 +206,8 @@ class VMOps(object):
         name_label = instance['name']
         vm_utils.set_vm_name_label(self._session, vm_ref, name_label)
 
+        self._attach_mapped_block_devices(instance, block_device_info)
+
         self._start(instance, vm_ref)
 
     def finish_migration(self, context, migration, instance, disk_info,
@@ -221,6 +239,9 @@ class VMOps(object):
                                  {'root': root_vdi},
                                  disk_image_type, network_info, kernel_file,
                                  ramdisk_file)
+
+        self._attach_mapped_block_devices(instance, block_device_info)
+
         # 5. Start VM
         self._start(instance, vm_ref=vm_ref)
         self._update_instance_progress(context, instance,
diff --git a/nova/virt/xenapi/volumeops.py b/nova/virt/xenapi/volumeops.py
index d17adeba6..056313478 100644
--- a/nova/virt/xenapi/volumeops.py
+++ b/nova/virt/xenapi/volumeops.py
@@ -105,7 +105,8 @@ class VolumeOps(object):
             LOG.exception(exc)
             raise exception.NovaException(_('Could not forget SR'))
 
-    def attach_volume(self, connection_info, instance_name, mountpoint):
+    def attach_volume(self, connection_info, instance_name, mountpoint,
+                      hotplug=True):
         """Attach volume storage to VM instance"""
 
         vm_ref = vm_utils.vm_ref_or_raise(self._session, instance_name)
@@ -121,14 +122,14 @@ class VolumeOps(object):
         connection_data = connection_info['data']
         dev_number = volume_utils.get_device_number(mountpoint)
 
-        self.connect_volume(
-            connection_data, dev_number, instance_name, vm_ref)
+        self.connect_volume(connection_data, dev_number, instance_name,
+                            vm_ref, hotplug=hotplug)
 
         LOG.info(_('Mountpoint %(mountpoint)s attached to'
                 ' instance %(instance_name)s') % locals())
 
     def connect_volume(self, connection_data, dev_number, instance_name,
-                       vm_ref):
+                       vm_ref, hotplug=True):
 
         description = 'Disk-for:%s' % instance_name
         uuid, label, sr_params = volume_utils.parse_sr_info(connection_data,
@@ -172,13 +173,14 @@ class VolumeOps(object):
             raise Exception(_('Unable to use SR %(sr_ref)s for'
                               ' instance %(instance_name)s') % locals())
 
-        try:
-            self._session.call_xenapi("VBD.plug", vbd_ref)
-        except self._session.XenAPI.Failure, exc:
-            LOG.exception(exc)
-            self.forget_sr(uuid)
-            raise Exception(_('Unable to attach volume to instance %s')
-                            % instance_name)
+        if hotplug:
+            try:
+                self._session.call_xenapi("VBD.plug", vbd_ref)
+            except self._session.XenAPI.Failure, exc:
+                LOG.exception(exc)
+                self.forget_sr(uuid)
+                raise Exception(_('Unable to attach volume to instance %s')
+                                % instance_name)
 
     def detach_volume(self, connection_info, instance_name, mountpoint):
         """Detach volume storage to VM instance"""