Merge lp:~cerberus/nova/xs_resize into lp:~hudson-openstack/nova/trunk

[Matt Dietz]

Updates the previously merged xs_migration functionality to allow upsizing of the RAM and disk quotas for a XenServer instance.

[Rick Harris (rconradharris)]

Good stuff, just some small nits:

Test failure for me: http://paste.openstack.org/show/928/

> 19 + try:
> 20 + migration = db.migration_get_by_instance_and_status(ctxt,
> 21 + inst['id'], 'finished')
> 22 + inst_dict['status'] = 'resize-confirm'
> 23 + except Exception, e:
> 24 + inst_dict['status'] = power_mapping[inst_dict['status']]

It's a little unclear what's going on here: what exception is being raised when we ask for a 'finished' migration? It *seems* like it would be clearer as something like:

  migration = db.migration_get_by_instance_and_status(ctxt, inst['id'], 'finished')
  if len(migration) > 1:
    log.warn("More than one finished migration, something bad happened")
  elif migration:
    inst_dict['status'] = 'resize-confirm'
  else:
    inst_dict['status'] = power_mapping[inst_dict['status']]

Do we let customers migrate servers by resizing to the same flavor. If that
isn't currently supported, perhaps we need a check at the API layer to assert
that the new flavor isn't the same as the old flavor.

> 187 + # FIXME: is there reload functionality?

FIXME(mdietz)

> 225 +# Copyright 2010 OpenStack LLC.

2011

> 261 +def upgrade(migrate_engine):

No downgrade()? Are not supporting rolling back migrations at this point?

I'm assuming auto-confirm will be tackled in a future bp.

review: Needs Fixing

[Matt Dietz (cerberus)]

Thanks for the review Rick.

I'll take a look at the test failure

In that block, your addendum makes sense. As for the catch, the migration_get DB call raises if none are found, which seems to be the convention in the DB API

Good point on the same flavor resize. I forgot about that one.

I hadn't previously seen any downgrade functionality. I'm assuming that backported to most of the existing db migrations after I wrote that originally?

I'll address the rest.

And yes, auto-confirm will have to be addressed later. AFAIK, we haven't discussed any sort of auto-maintenance routines via cron or something else. Please correct me if I'm wrong.

[Matt Dietz (cerberus)]

Rick: Looks like jsb is adding periodic tasks handling in his ~justin-fathomdb/nova/refresh-instance-state branch. Presumably the auto-confirm functionality can be added some time in Diablo after this branch lands.

[Sandy Walsh (sandy-walsh)]

Looks good. No major complaints.

Minor niggly things:

Shouldn't be db operations in api.openstack ... they should be moved into <service>/api.py in the event another API wanted to offer resize.
37 - except: not allowed (see HACKING). Should be except Exception: at the least
80 - perhaps a hint about the flavor not found?
144-145,192-193,etc - space after #? Dunno the pep8 rule there?

review: Needs Fixing

[Matt Dietz (cerberus)]

sandy: thanks for the review dude. I think I've covered everything you mentioned.

[Sandy Walsh (sandy-walsh)]

lgtm

review: Approve

[Rick Harris (rconradharris)]

Thanks for the fixes, Matt. Just a few more minor nits:

> 55 + except Exception, e:
> 56 + return False

Should probably be changed to:

    except exception.NotFound:
        return False

Wouldn't, for example, want MySQL-Gone-Away errors getting caught here.

> 97 + if current_instance_type['memory_mb'] >= \
> 98 + new_instance_type['memory_mb']:
> 99 + raise exception.ApiError(_("Invalid flavor: cannot downsize"
> 100 + "instances"))

I think this could be a little clearer, to distinguish between the resize
down/same-size cases. Something like:

    cur_mem_mb = current_instance_type['memory_mb']
    new_mem_mb = new_instance_type['memory_mb']
    if cur_mem_mb == new_mem_mb:
        raise exception.ApiError(_("Invalid flavor: cannot use same size"))
    elif cur_mem_mb < new_mem_mb:
        raise exception.ApiError(_("Invalid flavor: cannot downsize"))

> 409 + def test_resize_down_fails(self):
> 410 + """Ensure invalid flavors raise"""

Doc-string doesn't match.

Also, might be a good idea to include:

    def test_resize_same_size_fails(self):
        """Customers cannot resize to the same size. Admins however, can use
        resize to same size for purposes of migrations
        """

review: Needs Fixing

[Rick Harris (rconradharris)]

Thanks for the fixes, looks great!

review: Approve

[Josh Kearney (jk0)]

No big deal, but there is an extra space here on #104:

103 + raise exception.ApiError(_("Invalid flavor: cannot use"
104 + "the same flavor. "))

Also not a big deal, but will this pass pep8.py v0.5.x (current version included in pip-requires)?

149 + 'instance_id': instance_id, },
150 + })

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-03-17 22:24:08 +0000
+++ nova/api/openstack/servers.py	2011-03-23 21:12:17 +0000
@@ -22,6 +22,7 @@
 from webob import exc
 
 from nova import compute
+from nova import context
 from nova import exception
 from nova import flags
 from nova import log as logging

=== modified file 'nova/api/openstack/views/servers.py'
--- nova/api/openstack/views/servers.py	2011-03-15 23:55:13 +0000
+++ nova/api/openstack/views/servers.py	2011-03-23 21:12:17 +0000
@@ -16,7 +16,10 @@
 #    under the License.
 
 import hashlib
+
 from nova.compute import power_state
+import nova.compute
+import nova.context
 from nova.api.openstack import common
 from nova.api.openstack.views import addresses as addresses_view
 from nova.api.openstack.views import flavors as flavors_view
@@ -84,7 +87,12 @@
         for k, v in mapped_keys.iteritems():
             inst_dict[k] = inst[v]
 
+        ctxt = nova.context.get_admin_context()
         inst_dict['status'] = power_mapping[inst_dict['status']]
+        compute_api = nova.compute.API()
+        if compute_api.has_finished_migration(ctxt, inst['id']):
+            inst_dict['status'] = 'resize-confirm'
+
         inst_dict['addresses'] = self.addresses_builder.build(inst)
 
         # Return the metadata as a dictionary

=== modified file 'nova/compute/api.py'
--- nova/compute/api.py	2011-03-15 21:56:00 +0000
+++ nova/compute/api.py	2011-03-23 21:12:17 +0000
@@ -253,6 +253,16 @@
 
         return [dict(x.iteritems()) for x in instances]
 
+    def has_finished_migration(self, context, instance_id):
+        """Retrieves whether or not a finished migration exists for
+        an instance"""
+        try:
+            db.migration_get_by_instance_and_status(context, instance_id,
+                    'finished')
+            return True
+        except exception.NotFound:
+            return False
+
     def ensure_default_security_group(self, context):
         """ Create security group for the security context if it
         does not already exist
@@ -464,6 +474,8 @@
         params = {'migration_id': migration_ref['id']}
         self._cast_compute_message('revert_resize', context, instance_id,
                 migration_ref['dest_compute'], params=params)
+        self.db.migration_update(context, migration_ref['id'],
+                {'status': 'reverted'})
 
     def confirm_resize(self, context, instance_id):
         """Confirms a migration/resize, deleting the 'old' instance in the
@@ -479,17 +491,41 @@
         self._cast_compute_message('confirm_resize', context, instance_id,
                 migration_ref['source_compute'], params=params)
 
-        self.db.migration_update(context, migration_id,
+        self.db.migration_update(context, migration_ref['id'],
                 {'status': 'confirmed'})
         self.db.instance_update(context, instance_id,
                 {'host': migration_ref['dest_compute'], })
 
-    def resize(self, context, instance_id, flavor):
+    def resize(self, context, instance_id, flavor_id):
         """Resize a running instance."""
+        instance = self.db.instance_get(context, instance_id)
+        current_instance_type = self.db.instance_type_get_by_name(
+            context, instance['instance_type'])
+
+        new_instance_type = self.db.instance_type_get_by_flavor_id(
+                context, flavor_id)
+        current_instance_type_name = current_instance_type['name']
+        new_instance_type_name = new_instance_type['name']
+        LOG.debug(_("Old instance type %(current_instance_type_name)s, "
+                " new instance type %(new_instance_type_name)s") % locals())
+        if not new_instance_type:
+            raise exception.ApiError(_("Requested flavor %(flavor_id)d "
+                    "does not exist") % locals())
+
+        current_memory_mb = current_instance_type['memory_mb']
+        new_memory_mb = new_instance_type['memory_mb']
+        if current_memory_mb > new_memory_mb:
+            raise exception.ApiError(_("Invalid flavor: cannot downsize"
+                    "instances"))
+        if current_memory_mb == new_memory_mb:
+            raise exception.ApiError(_("Invalid flavor: cannot use"
+                    "the same flavor. "))
+
         self._cast_scheduler_message(context,
                     {"method": "prep_resize",
                      "args": {"topic": FLAGS.compute_topic,
-                              "instance_id": instance_id, }},)
+                              "instance_id": instance_id,
+                              "flavor_id": flavor_id}})
 
     def pause(self, context, instance_id):
         """Pause the given instance."""

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2011-03-21 15:41:03 +0000
+++ nova/compute/manager.py	2011-03-23 21:12:17 +0000
@@ -437,25 +437,41 @@
         instance_ref = self.db.instance_get(context, instance_id)
         migration_ref = self.db.migration_get(context, migration_id)
 
-        #TODO(mdietz): we may want to split these into separate methods.
-        if migration_ref['source_compute'] == FLAGS.host:
-            self.driver._start(instance_ref)
-            self.db.migration_update(context, migration_id,
-                    {'status': 'reverted'})
-        else:
-            self.driver.destroy(instance_ref)
-            topic = self.db.queue_get_for(context, FLAGS.compute_topic,
-                    instance_ref['host'])
-            rpc.cast(context, topic,
-                    {'method': 'revert_resize',
-                     'args': {
-                           'migration_id': migration_ref['id'],
-                           'instance_id': instance_id, },
-                    })
-
-    @exception.wrap_exception
-    @checks_instance_lock
-    def prep_resize(self, context, instance_id):
+        self.driver.destroy(instance_ref)
+        topic = self.db.queue_get_for(context, FLAGS.compute_topic,
+                instance_ref['host'])
+        rpc.cast(context, topic,
+                {'method': 'finish_revert_resize',
+                 'args': {
+                       'migration_id': migration_ref['id'],
+                       'instance_id': instance_id, },
+                })
+
+    @exception.wrap_exception
+    @checks_instance_lock
+    def finish_revert_resize(self, context, instance_id, migration_id):
+        """Finishes the second half of reverting a resize, powering back on
+        the source instance and reverting the resized attributes in the
+        database"""
+        instance_ref = self.db.instance_get(context, instance_id)
+        migration_ref = self.db.migration_get(context, migration_id)
+        instance_type = self.db.instance_type_get_by_flavor_id(context,
+                migration_ref['old_flavor_id'])
+
+        # Just roll back the record. There's no need to resize down since
+        # the 'old' VM already has the preferred attributes
+        self.db.instance_update(context, instance_id,
+           dict(memory_mb=instance_type['memory_mb'],
+                vcpus=instance_type['vcpus'],
+                local_gb=instance_type['local_gb']))
+
+        self.driver.revert_resize(instance_ref)
+        self.db.migration_update(context, migration_id,
+                {'status': 'reverted'})
+
+    @exception.wrap_exception
+    @checks_instance_lock
+    def prep_resize(self, context, instance_id, flavor_id):
         """Initiates the process of moving a running instance to another
         host, possibly changing the RAM and disk size in the process"""
         context = context.elevated()
@@ -464,12 +480,17 @@
             raise exception.Error(_(
                     'Migration error: destination same as source!'))
 
+        instance_type = self.db.instance_type_get_by_flavor_id(context,
+                flavor_id)
         migration_ref = self.db.migration_create(context,
                 {'instance_id': instance_id,
                  'source_compute': instance_ref['host'],
                  'dest_compute': FLAGS.host,
                  'dest_host':   self.driver.get_host_ip_addr(),
+                 'old_flavor_id': instance_type['flavorid'],
+                 'new_flavor_id': flavor_id,
                  'status':      'pre-migrating'})
+
         LOG.audit(_('instance %s: migrating to '), instance_id,
                 context=context)
         topic = self.db.queue_get_for(context, FLAGS.compute_topic,
@@ -495,8 +516,6 @@
         self.db.migration_update(context, migration_id,
                 {'status': 'post-migrating', })
 
-        #TODO(mdietz): This is where we would update the VM record
-        #after resizing
         service = self.db.service_get_by_host_and_topic(context,
                 migration_ref['dest_compute'], FLAGS.compute_topic)
         topic = self.db.queue_get_for(context, FLAGS.compute_topic,
@@ -517,7 +536,19 @@
         migration_ref = self.db.migration_get(context, migration_id)
         instance_ref = self.db.instance_get(context,
                 migration_ref['instance_id'])
+        # TODO(mdietz): apply the rest of the instance_type attributes going
+        # after they're supported
+        instance_type = self.db.instance_type_get_by_flavor_id(context,
+                migration_ref['new_flavor_id'])
+        self.db.instance_update(context, instance_id,
+               dict(instance_type=instance_type['name'],
+                    memory_mb=instance_type['memory_mb'],
+                    vcpus=instance_type['vcpus'],
+                    local_gb=instance_type['local_gb']))
 
+        # reload the updated instance ref
+        # FIXME(mdietz): is there reload functionality?
+        instance_ref = self.db.instance_get(context, instance_id)
         self.driver.finish_resize(instance_ref, disk_info)
 
         self.db.migration_update(context, migration_id,

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-03-23 05:29:32 +0000
+++ nova/db/sqlalchemy/api.py	2011-03-23 21:12:17 +0000
@@ -2220,8 +2220,8 @@
                      filter_by(instance_id=instance_id).\
                      filter_by(status=status).first()
     if not result:
-        raise exception.NotFound(_("No migration found with instance id %s")
-                % migration_id)
+        raise exception.NotFound(_("No migration found for instance "
+                "%(instance_id)s with status %(status)s") % locals())
     return result
 
 
@@ -2336,8 +2336,8 @@
         instance_type_ref = models.InstanceTypes()
         instance_type_ref.update(values)
         instance_type_ref.save()
-    except:
-        raise exception.DBError
+    except Exception, e:
+        raise exception.DBError(e)
     return instance_type_ref
 
 

=== added file 'nova/db/sqlalchemy/migrate_repo/versions/013_add_flavors_to_migrations.py'
--- nova/db/sqlalchemy/migrate_repo/versions/013_add_flavors_to_migrations.py	1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/013_add_flavors_to_migrations.py	2011-03-23 21:12:17 +0000
@@ -0,0 +1,50 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.from sqlalchemy import *
+
+from sqlalchemy import *
+from migrate import *
+
+from nova import log as logging
+
+
+meta = MetaData()
+
+migrations = Table('migrations', meta,
+        Column('id', Integer(), primary_key=True, nullable=False),
+        )
+
+#
+# Tables to alter
+#
+#
+
+old_flavor_id = Column('old_flavor_id', Integer())
+new_flavor_id = Column('new_flavor_id', Integer())
+
+
+def upgrade(migrate_engine):
+    # Upgrade operations go here. Don't create your own engine;
+    # bind migrate_engine to your metadata
+    meta.bind = migrate_engine
+    migrations.create_column(old_flavor_id)
+    migrations.create_column(new_flavor_id)
+
+
+def downgrade(migrate_engine):
+    meta.bind = migrate_engine
+    migrations.drop_column(old_flavor_id)
+    migrations.drop_column(new_flavor_id)

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-03-23 05:29:32 +0000
+++ nova/db/sqlalchemy/models.py	2011-03-23 21:12:17 +0000
@@ -436,6 +436,8 @@
     source_compute = Column(String(255))
     dest_compute = Column(String(255))
     dest_host = Column(String(255))
+    old_flavor_id = Column(Integer())
+    new_flavor_id = Column(Integer())
     instance_id = Column(Integer, ForeignKey('instances.id'), nullable=True)
     #TODO(_cerberus_): enum
     status = Column(String(255))

=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2011-03-20 19:06:22 +0000
+++ nova/tests/api/openstack/test_servers.py	2011-03-23 21:12:17 +0000
@@ -524,16 +524,6 @@
         req.body = json.dumps(body)
         res = req.get_response(fakes.wsgi_app())
 
-    def test_server_resize(self):
-        body = dict(server=dict(
-            name='server_test', imageId=2, flavorId=2, metadata={},
-            personality={}))
-        req = webob.Request.blank('/v1.0/servers/1/action')
-        req.method = 'POST'
-        req.content_type = 'application/json'
-        req.body = json.dumps(body)
-        res = req.get_response(fakes.wsgi_app())
-
     def test_delete_server_instance(self):
         req = webob.Request.blank('/v1.0/servers/1')
         req.method = 'DELETE'
@@ -589,6 +579,18 @@
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(res.status_int, 400)
 
+    def test_resized_server_has_correct_status(self):
+        req = self.webreq('/1', 'GET', dict(resize=dict(flavorId=3)))
+
+        def fake_migration_get(*args):
+            return {}
+
+        self.stubs.Set(nova.db, 'migration_get_by_instance_and_status',
+                fake_migration_get)
+        res = req.get_response(fakes.wsgi_app())
+        body = json.loads(res.body)
+        self.assertEqual(body['server']['status'], 'resize-confirm')
+
     def test_confirm_resize_server(self):
         req = self.webreq('/1/action', 'POST', dict(confirmResize=None))
 

=== modified file 'nova/tests/test_compute.py'
--- nova/tests/test_compute.py	2011-03-14 22:17:00 +0000
+++ nova/tests/test_compute.py	2011-03-23 21:12:17 +0000
@@ -82,6 +82,21 @@
         inst.update(params)
         return db.instance_create(self.context, inst)['id']
 
+    def _create_instance_type(self, params={}):
+        """Create a test instance"""
+        context = self.context.elevated()
+        inst = {}
+        inst['name'] = 'm1.small'
+        inst['memory_mb'] = '1024'
+        inst['vcpus'] = '1'
+        inst['local_gb'] = '20'
+        inst['flavorid'] = '1'
+        inst['swap'] = '2048'
+        inst['rxtx_quota'] = 100
+        inst['rxtx_cap'] = 200
+        inst.update(params)
+        return db.instance_type_create(context, inst)['id']
+
     def _create_group(self):
         values = {'name': 'testgroup',
                   'description': 'testgroup',
@@ -299,15 +314,53 @@
         """Ensure instance can be migrated/resized"""
         instance_id = self._create_instance()
         context = self.context.elevated()
+
         self.compute.run_instance(self.context, instance_id)
         db.instance_update(self.context, instance_id, {'host': 'foo'})
-        self.compute.prep_resize(context, instance_id)
+        self.compute.prep_resize(context, instance_id, 1)
         migration_ref = db.migration_get_by_instance_and_status(context,
                 instance_id, 'pre-migrating')
         self.compute.resize_instance(context, instance_id,
                 migration_ref['id'])
         self.compute.terminate_instance(context, instance_id)
 
+    def test_resize_invalid_flavor_fails(self):
+        """Ensure invalid flavors raise"""
+        instance_id = self._create_instance()
+        context = self.context.elevated()
+        self.compute.run_instance(self.context, instance_id)
+
+        self.assertRaises(exception.NotFound, self.compute_api.resize,
+                context, instance_id, 200)
+
+        self.compute.terminate_instance(context, instance_id)
+
+    def test_resize_down_fails(self):
+        """Ensure resizing down raises and fails"""
+        context = self.context.elevated()
+        instance_id = self._create_instance()
+
+        self.compute.run_instance(self.context, instance_id)
+        db.instance_update(self.context, instance_id,
+                {'instance_type': 'm1.xlarge'})
+
+        self.assertRaises(exception.ApiError, self.compute_api.resize,
+                context, instance_id, 1)
+
+        self.compute.terminate_instance(context, instance_id)
+
+    def test_resize_same_size_fails(self):
+        """Ensure invalid flavors raise"""
+        context = self.context.elevated()
+        instance_id = self._create_instance()
+
+        self.compute.run_instance(self.context, instance_id)
+
+        self.assertRaises(exception.ApiError, self.compute_api.resize,
+                context, instance_id, 1)
+
+        self.compute.terminate_instance(context, instance_id)
+
     def test_get_by_flavor_id(self):
         type = instance_types.get_by_flavor_id(1)
         self.assertEqual(type, 'm1.tiny')
@@ -318,10 +371,8 @@
         instance_id = self._create_instance()
         self.compute.run_instance(self.context, instance_id)
         self.assertRaises(exception.Error, self.compute.prep_resize,
-                self.context, instance_id)
+                self.context, instance_id, 1)
         self.compute.terminate_instance(self.context, instance_id)
-        type = instance_types.get_by_flavor_id("1")
-        self.assertEqual(type, 'm1.tiny')
 
     def _setup_other_managers(self):
         self.volume_manager = utils.import_object(FLAGS.volume_manager)

=== modified file 'nova/tests/test_localization.py'
--- nova/tests/test_localization.py	2011-02-21 07:16:10 +0000
+++ nova/tests/test_localization.py	2011-03-23 21:12:17 +0000
@@ -21,9 +21,10 @@
 import unittest
 
 import nova
-
-
-class LocalizationTestCase(unittest.TestCase):
+from nova import test
+
+
+class LocalizationTestCase(test.TestCase):
     def test_multiple_positional_format_placeholders(self):
         pat = re.compile("\W_\(")
         single_pat = re.compile("\W%\W")

=== modified file 'nova/tests/xenapi/stubs.py'
--- nova/tests/xenapi/stubs.py	2011-03-08 23:24:19 +0000
+++ nova/tests/xenapi/stubs.py	2011-03-23 21:12:17 +0000
@@ -228,6 +228,9 @@
     def VDI_get_by_uuid(*args):
         return 'hurr'
 
+    def VDI_resize_online(*args):
+        pass
+
     def VM_start(self, _1, ref, _2, _3):
         vm = fake.get_record('VM', ref)
         if vm['power_state'] != 'Halted':
@@ -240,7 +243,7 @@
 
 def stub_out_migration_methods(stubs):
     def fake_get_snapshot(self, instance):
-        return 'foo', 'bar'
+        return 'vm_ref', dict(image='foo', snap='bar')
 
     @classmethod
     def fake_get_vdi(cls, session, vm_ref):
@@ -249,7 +252,7 @@
         vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref)
         return vdi_ref, {'uuid': vdi_rec['uuid'], }
 
-    def fake_shutdown(self, inst, vm, method='clean'):
+    def fake_shutdown(self, inst, vm, hard=True):
         pass
 
     @classmethod

=== modified file 'nova/virt/xenapi/vmops.py'
--- nova/virt/xenapi/vmops.py	2011-03-23 12:53:10 +0000
+++ nova/virt/xenapi/vmops.py	2011-03-23 21:12:17 +0000
@@ -63,6 +63,17 @@
                 vm_refs.append(vm_rec["name_label"])
         return vm_refs
 
+    def revert_resize(self, instance):
+        vm_ref = VMHelper.lookup(self._session, instance.name)
+        self._start(instance, vm_ref)
+
+    def finish_resize(self, instance, disk_info):
+        vdi_uuid = self.link_disks(instance, disk_info['base_copy'],
+                disk_info['cow'])
+        vm_ref = self._create_vm(instance, vdi_uuid)
+        self.resize_instance(instance, vdi_uuid)
+        self._spawn(instance, vm_ref)
+
     def _start(self, instance, vm_ref=None):
         """Power on a VM instance"""
         if not vm_ref:
@@ -73,7 +84,7 @@
         LOG.debug(_("Starting instance %s"), instance.name)
         self._session.call_xenapi('VM.start', vm_ref, False, False)
 
-    def create_disk(self, instance):
+    def _create_disk(self, instance):
         user = AuthManager().get_user(instance.user_id)
         project = AuthManager().get_project(instance.project_id)
         disk_image_type = VMHelper.determine_disk_image_type(instance)
@@ -82,10 +93,11 @@
         return vdi_uuid
 
     def spawn(self, instance, network_info=None):
-        vdi_uuid = self.create_disk(instance)
-        self._spawn_with_disk(instance, vdi_uuid, network_info)
+        vdi_uuid = self._create_disk(instance)
+        vm_ref = self._create_vm(instance, vdi_uuid, network_info)
+        self._spawn(instance, vm_ref)
 
-    def _spawn_with_disk(self, instance, vdi_uuid, network_info=None):
+    def _create_vm(self, instance, vdi_uuid, network_info=None):
         """Create VM instance"""
         instance_name = instance.name
         vm_ref = VMHelper.lookup(self._session, instance_name)
@@ -128,16 +140,19 @@
         VMHelper.create_vbd(session=self._session, vm_ref=vm_ref,
                 vdi_ref=vdi_ref, userdevice=0, bootable=True)
 
-        # inject_network_info and create vifs
         # TODO(tr3buchet) - check to make sure we have network info, otherwise
         # create it now. This goes away once nova-multi-nic hits.
         if network_info is None:
             network_info = self._get_network_info(instance)
         self.create_vifs(vm_ref, network_info)
         self.inject_network_info(instance, vm_ref, network_info)
+        return vm_ref
 
+    def _spawn(self, instance, vm_ref):
+        """Spawn a new instance"""
         LOG.debug(_('Starting VM %s...'), vm_ref)
         self._start(instance, vm_ref)
+        instance_name = instance.name
         LOG.info(_('Spawning VM %(instance_name)s created %(vm_ref)s.')
                  % locals())
 
@@ -310,7 +325,7 @@
         try:
             # transfer the base copy
             template_vm_ref, template_vdi_uuids = self._get_snapshot(instance)
-            base_copy_uuid = template_vdi_uuids[1]
+            base_copy_uuid = template_vdi_uuids['image']
             vdi_ref, vm_vdi_rec = \
                     VMHelper.get_vdi_for_vm_safely(self._session, vm_ref)
             cow_uuid = vm_vdi_rec['uuid']
@@ -325,7 +340,7 @@
             self._session.wait_for_task(task, instance.id)
 
             # Now power down the instance and transfer the COW VHD
-            self._shutdown(instance, vm_ref, method='clean')
+            self._shutdown(instance, vm_ref, hard=False)
 
             params = {'host': dest,
                       'vdi_uuid': cow_uuid,
@@ -345,7 +360,7 @@
         # sensible so we don't need to blindly pass around dictionaries
         return {'base_copy': base_copy_uuid, 'cow': cow_uuid}
 
-    def attach_disk(self, instance, base_copy_uuid, cow_uuid):
+    def link_disks(self, instance, base_copy_uuid, cow_uuid):
         """Links the base copy VHD to the COW via the XAPI plugin"""
         vm_ref = VMHelper.lookup(self._session, instance.name)
         new_base_copy_uuid = str(uuid.uuid4())
@@ -366,9 +381,19 @@
 
         return new_cow_uuid
 
-    def resize(self, instance, flavor):
+    def resize_instance(self, instance, vdi_uuid):
         """Resize a running instance by changing it's RAM and disk size """
-        raise NotImplementedError()
+        #TODO(mdietz): this will need to be adjusted for swap later
+        #The new disk size must be in bytes
+
+        new_disk_size = str(instance.local_gb * 1024 * 1024 * 1024)
+        instance_name = instance.name
+        instance_local_gb = instance.local_gb
+        LOG.debug(_("Resizing VDI %(vdi_uuid)s for instance %(instance_name)s."
+                " Expanding to %(instance_local_gb)d GB") % locals())
+        vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', vdi_uuid)
+        self._session.call_xenapi('VDI.resize_online', vdi_ref, new_disk_size)
+        LOG.debug(_("Resize instance %s complete") % (instance.name))
 
     def reboot(self, instance):
         """Reboot VM instance"""
@@ -443,8 +468,9 @@
         """Shutdown an instance"""
         state = self.get_info(instance['name'])['state']
         if state == power_state.SHUTDOWN:
-            LOG.warn(_("VM %(vm)s already halted, skipping shutdown...") %
-                     locals())
+            instance_name = instance.name
+            LOG.warn(_("VM %(instance_name)s already halted,"
+                    "skipping shutdown...") % locals())
             return
 
         instance_id = instance.id

=== modified file 'nova/virt/xenapi_conn.py'
--- nova/virt/xenapi_conn.py	2011-03-14 17:59:41 +0000
+++ nova/virt/xenapi_conn.py	2011-03-23 21:12:17 +0000
@@ -164,20 +164,18 @@
         """Create VM instance"""
         self._vmops.spawn(instance)
 
+    def revert_resize(self, instance):
+        """Reverts a resize, powering back on the instance"""
+        self._vmops.revert_resize(instance)
+
     def finish_resize(self, instance, disk_info):
         """Completes a resize, turning on the migrated instance"""
-        vdi_uuid = self._vmops.attach_disk(instance, disk_info['base_copy'],
-                disk_info['cow'])
-        self._vmops._spawn_with_disk(instance, vdi_uuid)
+        self._vmops.finish_resize(instance, disk_info)
 
     def snapshot(self, instance, image_id):
         """ Create snapshot from a running VM instance """
         self._vmops.snapshot(instance, image_id)
 
-    def resize(self, instance, flavor):
-        """Resize a VM instance"""
-        raise NotImplementedError()
-
     def reboot(self, instance):
         """Reboot VM instance"""
         self._vmops.reboot(instance)

=== modified file 'plugins/xenserver/xenapi/etc/xapi.d/plugins/migration'
--- plugins/xenserver/xenapi/etc/xapi.d/plugins/migration	2011-02-28 18:30:02 +0000
+++ plugins/xenserver/xenapi/etc/xapi.d/plugins/migration	2011-03-23 21:12:17 +0000
@@ -22,6 +22,7 @@
 import os
 import os.path
 import pickle
+import shlex
 import shutil
 import subprocess
 
@@ -97,7 +98,7 @@
     logging.debug("Preparing to transmit %s to %s" % (source_path,
             dest_path))
 
-    ssh_cmd = 'ssh -o StrictHostKeyChecking=no'
+    ssh_cmd = '\"ssh -o StrictHostKeyChecking=no\"'
 
     rsync_args = shlex.split('nohup /usr/bin/rsync -av --progress -e %s %s %s'
             % (ssh_cmd, source_path, dest_path))