Merge changes I3ec871de,I401beb93,Iddcc7cb0,Id581d23d

* changes:
  Fix arg to get_instance_volume_block_device_info().
  Send a full instance in snapshot_instance.
  Send a full instance in set_admin_password.
  Send a full instance in rollback_live_migration_at_destination.

4 files changed, 82 insertions, 59 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index f1de42d61..78d69becc 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -272,7 +272,7 @@ def _get_image_meta(context, image_ref):
 class ComputeManager(manager.SchedulerDependentManager):
     """Manages the running instances from creation to destruction."""
 
-    RPC_API_VERSION = '1.31'
+    RPC_API_VERSION = '1.34'
 
     def __init__(self, compute_driver=None, *args, **kwargs):
         """Load configuration options and connect to the hypervisor."""
@@ -1116,13 +1116,14 @@ class ComputeManager(manager.SchedulerDependentManager):
 
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @wrap_instance_fault
-    def snapshot_instance(self, context, instance_uuid, image_id,
+    def snapshot_instance(self, context, image_id,
                           image_type='snapshot', backup_type=None,
-                          rotation=None):
+                          rotation=None, instance=None, instance_uuid=None):
         """Snapshot an instance on this host.
 
         :param context: security context
-        :param instance_uuid: nova.db.sqlalchemy.models.Instance.Uuid
+        :param instance_uuid: (deprecated) db.sqlalchemy.models.Instance.Uuid
+        :param instance: an Instance dict
         :param image_id: glance.db.sqlalchemy.models.Image.Id
         :param image_type: snapshot | backup
         :param backup_type: daily | weekly
@@ -1130,18 +1131,20 @@ class ComputeManager(manager.SchedulerDependentManager):
             None if rotation shouldn't be used (as in the case of snapshots)
         """
         context = context.elevated()
-        instance_ref = self.db.instance_get_by_uuid(context, instance_uuid)
 
-        current_power_state = self._get_power_state(context, instance_ref)
+        if not instance:
+            instance = self.db.instance_get_by_uuid(context, instance_uuid)
+
+        current_power_state = self._get_power_state(context, instance)
         self._instance_update(context,
-                              instance_ref['uuid'],
+                              instance['uuid'],
                               power_state=current_power_state)
 
         LOG.audit(_('instance snapshotting'), context=context,
                   instance_uuid=instance_uuid)
 
-        if instance_ref['power_state'] != power_state.RUNNING:
-            state = instance_ref['power_state']
+        if instance['power_state'] != power_state.RUNNING:
+            state = instance['power_state']
             running = power_state.RUNNING
             LOG.warn(_('trying to snapshot a non-running '
                        'instance: (state: %(state)s '
@@ -1149,12 +1152,12 @@ class ComputeManager(manager.SchedulerDependentManager):
                      instance_uuid=instance_uuid)
 
         self._notify_about_instance_usage(
-                context, instance_ref, "snapshot.start")
+                context, instance, "snapshot.start")
 
         try:
-            self.driver.snapshot(context, instance_ref, image_id)
+            self.driver.snapshot(context, instance, image_id)
         finally:
-            self._instance_update(context, instance_ref['uuid'],
+            self._instance_update(context, instance['uuid'],
                                   task_state=None)
 
         if image_type == 'snapshot' and rotation:
@@ -1167,7 +1170,7 @@ class ComputeManager(manager.SchedulerDependentManager):
             raise exception.RotationRequiredForBackup()
 
         self._notify_about_instance_usage(
-                context, instance_ref, "snapshot.end")
+                context, instance, "snapshot.end")
 
     @wrap_instance_fault
     def rotate_backups(self, context, instance_uuid, backup_type, rotation):
@@ -1221,7 +1224,8 @@ class ComputeManager(manager.SchedulerDependentManager):
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @checks_instance_lock
     @wrap_instance_fault
-    def set_admin_password(self, context, instance_uuid, new_pass=None):
+    def set_admin_password(self, context, instance=None, instance_uuid=None,
+                           new_pass=None):
         """Set the root/admin password for an instance on this host.
 
         This is generally only called by API password resets after an
@@ -1234,43 +1238,44 @@ class ComputeManager(manager.SchedulerDependentManager):
             # Generate a random password
             new_pass = utils.generate_password(FLAGS.password_length)
 
+        if not instance:
+            instance = self.db.instance_get_by_uuid(context, instance_uuid)
+
         max_tries = 10
 
         for i in xrange(max_tries):
-            instance_ref = self.db.instance_get_by_uuid(context, instance_uuid)
-
-            current_power_state = self._get_power_state(context, instance_ref)
+            current_power_state = self._get_power_state(context, instance)
             expected_state = power_state.RUNNING
 
             if current_power_state != expected_state:
-                self._instance_update(context, instance_ref['uuid'],
+                self._instance_update(context, instance['uuid'],
                                       task_state=None)
                 _msg = _('Failed to set admin password. Instance %s is not'
-                         ' running') % instance_ref["uuid"]
+                         ' running') % instance["uuid"]
                 raise exception.Invalid(_msg)
             else:
                 try:
-                    self.driver.set_admin_password(instance_ref, new_pass)
-                    LOG.audit(_("Root password set"), instance=instance_ref)
+                    self.driver.set_admin_password(instance, new_pass)
+                    LOG.audit(_("Root password set"), instance=instance)
                     self._instance_update(context,
-                                          instance_ref['uuid'],
+                                          instance['uuid'],
                                           task_state=None)
                     break
                 except NotImplementedError:
                     # NOTE(dprince): if the driver doesn't implement
                     # set_admin_password we break to avoid a loop
                     LOG.warn(_('set_admin_password is not implemented '
-                             'by this driver.'), instance=instance_ref)
+                             'by this driver.'), instance=instance)
                     self._instance_update(context,
-                                          instance_ref['uuid'],
+                                          instance['uuid'],
                                           task_state=None)
                     break
                 except Exception, e:
                     # Catch all here because this could be anything.
-                    LOG.exception(e, instance=instance_ref)
+                    LOG.exception(e, instance=instance)
                     if i == max_tries - 1:
                         self._set_instance_error_state(context,
-                                                       instance_ref['uuid'])
+                                                       instance['uuid'])
                         # We create a new exception here so that we won't
                         # potentially reveal password information to the
                         # API caller.  The real exception is logged above
@@ -2407,24 +2412,28 @@ class ComputeManager(manager.SchedulerDependentManager):
             self.compute_rpcapi.rollback_live_migration_at_destination(context,
                     instance_ref, dest)
 
-    def rollback_live_migration_at_destination(self, context, instance_id):
+    def rollback_live_migration_at_destination(self, context, instance=None,
+                                               instance_id=None):
         """ Cleaning up image directory that is created pre_live_migration.
 
         :param context: security context
-        :param instance_id: nova.db.sqlalchemy.models.Instance.Id
+        :param instance_id: (deprecated) nova.db.sqlalchemy.models.Instance.Id
+        :param instance: an Instance dict sent over rpc
         """
-        instance_ref = self.db.instance_get(context, instance_id)
-        network_info = self._get_instance_nw_info(context, instance_ref)
+        if not instance:
+            instance = self.db.instance_get(context, instance_id)
+
+        network_info = self._get_instance_nw_info(context, instance)
 
         # NOTE(tr3buchet): tear down networks on destination host
-        self.network_api.setup_networks_on_host(context, instance_ref,
+        self.network_api.setup_networks_on_host(context, instance,
                                                 self.host, teardown=True)
 
         # NOTE(vish): The mapping is passed in so the driver can disconnect
         #             from remote volumes if necessary
         block_device_info = self._get_instance_volume_block_device_info(
-                            context, instance_id)
-        self.driver.destroy(instance_ref, self._legacy_nw_info(network_info),
+                            context, instance['uuid'])
+        self.driver.destroy(instance, self._legacy_nw_info(network_info),
                             block_device_info)
 
     @manager.periodic_task
diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py
index 107bf4d8d..153348374 100644
--- a/nova/compute/rpcapi.py
+++ b/nova/compute/rpcapi.py
@@ -102,6 +102,12 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy):
         1.29 - Remove instance_uuid, add instance argument to resize_instance()
         1.30 - Remove instance_uuid, add instance argument to resume_instance()
         1.31 - Remove instance_uuid, add instance argument to revert_resize()
+        1.32 - Remove instance_id, add instance argument to
+               rollback_live_migration_at_destination()
+        1.33 - Remove instance_uuid, add instance argument to
+               set_admin_password()
+        1.34 - Remove instance_uuid, add instance argument to
+               snapshot_instance()
     '''
 
     BASE_RPC_API_VERSION = '1.0'
@@ -386,14 +392,18 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy):
                 version='1.31')
 
     def rollback_live_migration_at_destination(self, ctxt, instance, host):
+        instance_p = jsonutils.to_primitive(instance)
         self.cast(ctxt, self.make_msg('rollback_live_migration_at_destination',
-            instance_id=instance['id']),
-            topic=_compute_topic(self.topic, ctxt, host, None))
+            instance=instance_p),
+            topic=_compute_topic(self.topic, ctxt, host, None),
+            version='1.32')
 
     def set_admin_password(self, ctxt, instance, new_pass):
+        instance_p = jsonutils.to_primitive(instance)
         self.cast(ctxt, self.make_msg('set_admin_password',
-                instance_uuid=instance['uuid'], new_pass=new_pass),
-                topic=_compute_topic(self.topic, ctxt, None, instance))
+                instance=instance_p, new_pass=new_pass),
+                topic=_compute_topic(self.topic, ctxt, None, instance),
+                version='1.33')
 
     def set_host_enabled(self, ctxt, enabled, host):
         topic = _compute_topic(self.topic, ctxt, host, None)
@@ -407,11 +417,13 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy):
 
     def snapshot_instance(self, ctxt, instance, image_id, image_type,
             backup_type, rotation):
+        instance_p = jsonutils.to_primitive(instance)
         self.cast(ctxt, self.make_msg('snapshot_instance',
-                instance_uuid=instance['uuid'], image_id=image_id,
+                instance=instance_p, image_id=image_id,
                 image_type=image_type, backup_type=backup_type,
                 rotation=rotation),
-                topic=_compute_topic(self.topic, ctxt, None, instance))
+                topic=_compute_topic(self.topic, ctxt, None, instance),
+                version='1.34')
 
     def start_instance(self, ctxt, instance):
         instance_p = jsonutils.to_primitive(instance)
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index dec61d3ff..4aa6545d1 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -589,7 +589,7 @@ class ComputeTestCase(BaseTestCase):
 
     def test_set_admin_password(self):
         """Ensure instance can have its admin password set"""
-        instance = self._create_fake_instance()
+        instance = jsonutils.to_primitive(self._create_fake_instance())
         self.compute.run_instance(self.context, instance['uuid'])
         db.instance_update(self.context, instance['uuid'],
                            {'task_state': task_states.UPDATING_PASSWORD})
@@ -598,7 +598,7 @@ class ComputeTestCase(BaseTestCase):
         self.assertEqual(inst_ref['vm_state'], vm_states.ACTIVE)
         self.assertEqual(inst_ref['task_state'], task_states.UPDATING_PASSWORD)
 
-        self.compute.set_admin_password(self.context, instance['uuid'])
+        self.compute.set_admin_password(self.context, instance=instance)
 
         inst_ref = db.instance_get_by_uuid(self.context, instance['uuid'])
         self.assertEqual(inst_ref['vm_state'], vm_states.ACTIVE)
@@ -613,7 +613,8 @@ class ComputeTestCase(BaseTestCase):
         db.instance_update(self.context, instance['uuid'], {
             "power_state": power_state.NOSTATE,
         })
-        instance = db.instance_get_by_uuid(self.context, instance['uuid'])
+        instance = jsonutils.to_primitive(db.instance_get_by_uuid(
+                                          self.context, instance['uuid']))
 
         self.assertEqual(instance['power_state'], power_state.NOSTATE)
 
@@ -630,7 +631,7 @@ class ComputeTestCase(BaseTestCase):
         self.assertRaises(exception.Invalid,
                           self.compute.set_admin_password,
                           self.context,
-                          instance['uuid'])
+                          instance=instance)
         self.compute.terminate_instance(self.context, instance['uuid'])
 
     def test_set_admin_password_driver_error(self):
@@ -660,7 +661,8 @@ class ComputeTestCase(BaseTestCase):
         #so a new error is raised
         self.assertRaises(exception.NovaException,
                           self.compute.set_admin_password,
-                          self.context, instance['uuid'])
+                          self.context,
+                          instance=jsonutils.to_primitive(inst_ref))
 
         inst_ref = db.instance_get_by_uuid(self.context, instance['uuid'])
         self.assertEqual(inst_ref['vm_state'], vm_states.ERROR)
@@ -747,11 +749,11 @@ class ComputeTestCase(BaseTestCase):
 
     def test_snapshot(self):
         """Ensure instance can be snapshotted"""
-        instance = self._create_fake_instance()
+        instance = jsonutils.to_primitive(self._create_fake_instance())
         instance_uuid = instance['uuid']
         name = "myfakesnapshot"
         self.compute.run_instance(self.context, instance_uuid)
-        self.compute.snapshot_instance(self.context, instance_uuid, name)
+        self.compute.snapshot_instance(self.context, name, instance=instance)
         self.compute.terminate_instance(self.context, instance_uuid)
 
     def test_snapshot_fails(self):
@@ -761,11 +763,11 @@ class ComputeTestCase(BaseTestCase):
 
         self.stubs.Set(self.compute.driver, 'snapshot', fake_snapshot)
 
-        instance = self._create_fake_instance()
+        instance = jsonutils.to_primitive(self._create_fake_instance())
         self.compute.run_instance(self.context, instance['uuid'])
         self.assertRaises(test.TestingException,
                           self.compute.snapshot_instance,
-                          self.context, instance['uuid'], "failing_snapshot")
+                          self.context, "failing_snapshot", instance=instance)
         self._assert_state({'task_state': None})
         self.compute.terminate_instance(self.context, instance['uuid'])
 
@@ -1625,8 +1627,8 @@ class ComputeTestCase(BaseTestCase):
                  "version": "1.26"}, None)
         rpc.cast(c, topic,
                 {"method": "rollback_live_migration_at_destination",
-                 "args": {'instance_id': inst_id},
-                 "version": "1.0"})
+                 "args": {'instance': rpcinst},
+                 "version": "1.32"})
 
         # start test
         self.mox.ReplayAll()
diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py
index 7a1195fe0..cafd7fd55 100644
--- a/nova/tests/compute/test_rpcapi.py
+++ b/nova/tests/compute/test_rpcapi.py
@@ -60,8 +60,9 @@ class ComputeRpcAPITestCase(test.TestCase):
             'rebuild_instance', 'remove_fixed_ip_from_instance',
             'remove_volume_connection', 'rescue_instance', 'reset_network',
             'resize_instance', 'resume_instance', 'revert_resize',
-            'start_instance', 'stop_instance', 'suspend_instance',
-            'unpause_instance'
+            'rollback_live_migration_at_destination', 'set_admin_password',
+            'snapshot_instance', 'start_instance', 'stop_instance',
+            'suspend_instance', 'unpause_instance'
         ]
 
         if 'rpcapi_class' in kwargs:
@@ -86,10 +87,7 @@ class ComputeRpcAPITestCase(test.TestCase):
                                                    methods_with_instance):
             instance = expected_msg['args']['instance']
             del expected_msg['args']['instance']
-            if method in ['rollback_live_migration_at_destination']:
-                expected_msg['args']['instance_id'] = instance['id']
-            else:
-                expected_msg['args']['instance_uuid'] = instance['uuid']
+            expected_msg['args']['instance_uuid'] = instance['uuid']
         expected_msg['version'] = expected_version
 
         cast_and_call = ['confirm_resize', 'stop_instance']
@@ -291,11 +289,12 @@ class ComputeRpcAPITestCase(test.TestCase):
 
     def test_rollback_live_migration_at_destination(self):
         self._test_compute_api('rollback_live_migration_at_destination',
-                'cast', instance=self.fake_instance, host='host')
+                'cast', instance=self.fake_instance, host='host',
+                version='1.32')
 
     def test_set_admin_password(self):
         self._test_compute_api('set_admin_password', 'cast',
-                instance=self.fake_instance, new_pass='pw')
+                instance=self.fake_instance, new_pass='pw', version='1.33')
 
     def test_set_host_enabled(self):
         self._test_compute_api('set_host_enabled', 'call',
@@ -308,7 +307,8 @@ class ComputeRpcAPITestCase(test.TestCase):
     def test_snapshot_instance(self):
         self._test_compute_api('snapshot_instance', 'cast',
                 instance=self.fake_instance, image_id='id', image_type='type',
-                backup_type='type', rotation='rotation')
+                backup_type='type', rotation='rotation',
+                version='1.34')
 
     def test_start_instance(self):
         self._test_compute_api('start_instance', 'cast',