merge in trunk

27 files changed, 654 insertions, 116 deletions

diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py
index bebcdc18c..ea6dff004 100644
--- a/nova/api/openstack/__init__.py
+++ b/nova/api/openstack/__init__.py
@@ -93,6 +93,8 @@ class APIRouter(wsgi.Router):
             logging.debug("Including admin operations in API.")
             server_members['pause'] = 'POST'
             server_members['unpause'] = 'POST'
+            server_members["diagnostics"] = "GET"
+            server_members["actions"] = "GET"
             server_members['suspend'] = 'POST'
             server_members['resume'] = 'POST'
 
@@ -100,7 +102,7 @@ class APIRouter(wsgi.Router):
                         collection={'detail': 'GET'},
                         member=server_members)
 
-        mapper.resource("backup_schedule", "backup_schedules",
+        mapper.resource("backup_schedule", "backup_schedule",
                         controller=backup_schedules.Controller(),
                         parent_resource=dict(member_name='server',
                         collection_name='servers'))
diff --git a/nova/api/openstack/auth.py b/nova/api/openstack/auth.py
index e24e58fd3..1dfdd5318 100644
--- a/nova/api/openstack/auth.py
+++ b/nova/api/openstack/auth.py
@@ -55,7 +55,8 @@ class AuthMiddleware(wsgi.Middleware):
         if not user:
             return faults.Fault(webob.exc.HTTPUnauthorized())
 
-        req.environ['nova.context'] = context.RequestContext(user, user)
+        project = self.auth.get_project(FLAGS.default_project)
+        req.environ['nova.context'] = context.RequestContext(user, project)
         return self.application
 
     def has_authentication(self, req):
diff --git a/nova/api/openstack/backup_schedules.py b/nova/api/openstack/backup_schedules.py
index fc70b5c6c..fcc07bdd3 100644
--- a/nova/api/openstack/backup_schedules.py
+++ b/nova/api/openstack/backup_schedules.py
@@ -23,13 +23,25 @@ from nova.api.openstack import faults
 import nova.image.service
 
 
+def _translate_keys(inst):
+    """ Coerces the backup schedule into proper dictionary format """
+    return dict(backupSchedule=inst)
+
+
 class Controller(wsgi.Controller):
+    """ The backup schedule API controller for the Openstack API """
+
+    _serialization_metadata = {
+        'application/xml': {
+            'attributes': {
+                'backupSchedule': []}}}
 
     def __init__(self):
         pass
 
     def index(self, req, server_id):
-        return faults.Fault(exc.HTTPNotFound())
+        """ Returns the list of backup schedules for a given instance """
+        return _translate_keys({})
 
     def create(self, req, server_id):
         """ No actual update method required, since the existing API allows
@@ -37,4 +49,5 @@ class Controller(wsgi.Controller):
         return faults.Fault(exc.HTTPNotFound())
 
     def delete(self, req, server_id, id):
+        """ Deletes an existing backup schedule """
         return faults.Fault(exc.HTTPNotFound())
diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py
index ba35fbc78..867ee5a7e 100644
--- a/nova/api/openstack/images.py
+++ b/nova/api/openstack/images.py
@@ -25,7 +25,7 @@ import nova.image.service
 
 from nova.api.openstack import common
 from nova.api.openstack import faults
-
+from nova.compute import api as compute_api
 
 FLAGS = flags.FLAGS
 
@@ -127,9 +127,11 @@ class Controller(wsgi.Controller):
         raise faults.Fault(exc.HTTPNotFound())
 
     def create(self, req):
-        # Only public images are supported for now, so a request to
-        # make a backup of a server cannot be supproted.
-        raise faults.Fault(exc.HTTPNotFound())
+        context = req.environ['nova.context']
+        env = self._deserialize(req.body, req)
+        instance_id = env["image"]["serverId"]
+        name = env["image"]["name"]
+        return compute_api.ComputeAPI().snapshot(context, instance_id, name)
 
     def update(self, req, id):
         # Users may not modify public images, and that's all that
diff --git a/nova/api/openstack/ratelimiting/__init__.py b/nova/api/openstack/ratelimiting/__init__.py
index 91a8b2e55..cbb4b897e 100644
--- a/nova/api/openstack/ratelimiting/__init__.py
+++ b/nova/api/openstack/ratelimiting/__init__.py
@@ -64,9 +64,9 @@ class RateLimitingMiddleware(wsgi.Middleware):
         If the request should be rate limited, return a 413 status with a
         Retry-After header giving the time when the request would succeed.
         """
-        return self.limited_request(req, self.application)
+        return self.rate_limited_request(req, self.application)
 
-    def limited_request(self, req, application):
+    def rate_limited_request(self, req, application):
         """Rate limit the request.
 
         If the request should be rate limited, return a 413 status with a
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index 849d0bb93..15082cb54 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -35,14 +35,11 @@ LOG = logging.getLogger('server')
 LOG.setLevel(logging.DEBUG)
 
 
-def _entity_list(entities):
-    """ Coerces a list of servers into proper dictionary format """
-    return dict(servers=entities)
-
-
-def _entity_detail(inst):
-    """ Maps everything to Rackspace-like attributes for return"""
+def _translate_detail_keys(inst):
+    """ Coerces into dictionary format, mapping everything to Rackspace-like
+    attributes for return"""
     power_mapping = {
+        None: 'build',
         power_state.NOSTATE: 'build',
         power_state.RUNNING: 'active',
         power_state.BLOCKED: 'active',
@@ -67,8 +64,9 @@ def _entity_detail(inst):
     return dict(server=inst_dict)
 
 
-def _entity_inst(inst):
-    """ Filters all model attributes save for id and name """
+def _translate_keys(inst):
+    """ Coerces into dictionary format, excluding all model attributes
+    save for id and name """
     return dict(server=dict(id=inst['internal_id'], name=inst['display_name']))
 
 
@@ -87,29 +85,29 @@ class Controller(wsgi.Controller):
 
     def index(self, req):
         """ Returns a list of server names and ids for a given user """
-        return self._items(req, entity_maker=_entity_inst)
+        return self._items(req, entity_maker=_translate_keys)
 
     def detail(self, req):
         """ Returns a list of server details for a given user """
-        return self._items(req, entity_maker=_entity_detail)
+        return self._items(req, entity_maker=_translate_detail_keys)
 
     def _items(self, req, entity_maker):
         """Returns a list of servers for a given user.
 
-        entity_maker - either _entity_detail or _entity_inst
+        entity_maker - either _translate_detail_keys or _translate_keys
         """
         instance_list = self.compute_api.get_instances(
             req.environ['nova.context'])
         limited_list = common.limited(instance_list, req)
         res = [entity_maker(inst)['server'] for inst in limited_list]
-        return _entity_list(res)
+        return dict(servers=res)
 
     def show(self, req, id):
         """ Returns server details by server id """
         try:
             instance = self.compute_api.get_instance(
                 req.environ['nova.context'], int(id))
-            return _entity_detail(instance)
+            return _translate_detail_keys(instance)
         except exception.NotFound:
             return faults.Fault(exc.HTTPNotFound())
 
@@ -138,7 +136,7 @@ class Controller(wsgi.Controller):
             description=env['server']['name'],
             key_name=key_pair['name'],
             key_data=key_pair['public_key'])
-        return _entity_inst(instances[0])
+        return _translate_keys(instances[0])
 
     def update(self, req, id):
         """ Updates the server name or password """
@@ -153,8 +151,9 @@ class Controller(wsgi.Controller):
             update_dict['display_name'] = inst_dict['server']['name']
 
         try:
-            self.compute_api.update_instance(req.environ['nova.context'],
-                                             instance['id'],
+            ctxt = req.environ['nova.context']
+            self.compute_api.update_instance(ctxt,
+                                             id,
                                              **update_dict)
         except exception.NotFound:
             return faults.Fault(exc.HTTPNotFound())
@@ -221,10 +220,20 @@ class Controller(wsgi.Controller):
         return exc.HTTPAccepted()
 
     def get_ajax_console(self, req, id):
-        """ Returns a url to and ajaxterm instance console. """
+        """ Returns a url to an instance's ajaxterm console. """
         try:
             self.compute_api.get_ajax_console(req.environ['nova.context'],
                 int(id))
         except exception.NotFound:
             return faults.Fault(exc.HTTPNotFound())
         return exc.HTTPAccepted()
+
+    def diagnostics(self, req, id):
+        """Permit Admins to retrieve server diagnostics."""
+        ctxt = req.environ["nova.context"]
+        return self.compute_api.get_diagnostics(ctxt, id)
+
+    def actions(self, req, id):
+        """Permit Admins to retrieve server actions."""
+        ctxt = req.environ["nova.context"]
+        return self.compute_api.get_actions(ctxt, id)
diff --git a/nova/api/openstack/sharedipgroups.py b/nova/api/openstack/sharedipgroups.py
index 75d02905c..845f5bead 100644
--- a/nova/api/openstack/sharedipgroups.py
+++ b/nova/api/openstack/sharedipgroups.py
@@ -15,26 +15,51 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from webob import exc
+
 from nova import wsgi
+from nova.api.openstack import faults
+
+
+def _translate_keys(inst):
+    """ Coerces a shared IP group instance into proper dictionary format """
+    return dict(sharedIpGroup=inst)
+
+
+def _translate_detail_keys(inst):
+    """ Coerces a shared IP group instance into proper dictionary format with
+    correctly mapped attributes """
+    return dict(sharedIpGroup=inst)
 
 
 class Controller(wsgi.Controller):
     """ The Shared IP Groups Controller for the Openstack API """
 
+    _serialization_metadata = {
+        'application/xml': {
+            'attributes': {
+                'sharedIpGroup': []}}}
+
     def index(self, req):
-        raise NotImplementedError
+        """ Returns a list of Shared IP Groups for the user """
+        return dict(sharedIpGroups=[])
 
     def show(self, req, id):
-        raise NotImplementedError
+        """ Shows in-depth information on a specific Shared IP Group """
+        return _translate_keys({})
 
     def update(self, req, id):
-        raise NotImplementedError
+        """ You can't update a Shared IP Group """
+        raise faults.Fault(exc.HTTPNotImplemented())
 
     def delete(self, req, id):
-        raise NotImplementedError
+        """ Deletes a Shared IP Group """
+        raise faults.Fault(exc.HTTPNotFound())
 
-    def detail(self, req):
-        raise NotImplementedError
+    def detail(self, req, id):
+        """ Returns a complete list of Shared IP Groups """
+        return _translate_detail_keys({})
 
     def create(self, req):
-        raise NotImplementedError
+        """ Creates a new Shared IP group """
+        raise faults.Fault(exc.HTTPNotFound())
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 38b99a3e3..0658044d2 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -257,6 +257,15 @@ class ComputeAPI(base.Base):
     def get_instance(self, context, instance_id):
         return self.db.instance_get_by_internal_id(context, instance_id)
 
+    def snapshot(self, context, instance_id, name):
+        """Snapshot the given instance."""
+        instance = self.db.instance_get_by_internal_id(context, instance_id)
+        host = instance['host']
+        rpc.cast(context,
+                 self.db.queue_get_for(context, FLAGS.compute_topic, host),
+                 {"method": "snapshot_instance",
+                  "args": {"instance_id": instance['id'], "name": name}})
+
     def reboot(self, context, instance_id):
         """Reboot the given instance."""
         instance = self.db.instance_get_by_internal_id(context, instance_id)
@@ -284,6 +293,20 @@ class ComputeAPI(base.Base):
                  {"method": "unpause_instance",
                   "args": {"instance_id": instance['id']}})
 
+    def get_diagnostics(self, context, instance_id):
+        """Retrieve diagnostics for the given instance."""
+        instance = self.db.instance_get_by_internal_id(context, instance_id)
+        host = instance["host"]
+        return rpc.call(context,
+            self.db.queue_get_for(context, FLAGS.compute_topic, host),
+            {"method": "get_diagnostics",
+             "args": {"instance_id": instance["id"]}})
+
+    def get_actions(self, context, instance_id):
+        """Retrieve actions for the given instance."""
+        instance = self.db.instance_get_by_internal_id(context, instance_id)
+        return self.db.instance_get_actions(context, instance["id"])
+
     def suspend(self, context, instance_id):
         """suspend the instance with instance_id"""
         instance = self.db.instance_get_by_internal_id(context, instance_id)
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 5f237eda9..882b000ef 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -225,6 +225,27 @@ class ComputeManager(manager.Manager):
         self._update_state(context, instance_id)
 
     @exception.wrap_exception
+    def snapshot_instance(self, context, instance_id, name):
+        """Snapshot an instance on this server."""
+        context = context.elevated()
+        instance_ref = self.db.instance_get(context, instance_id)
+
+        #NOTE(sirp): update_state currently only refreshes the state field
+        # if we add is_snapshotting, we will need this refreshed too,
+        # potentially?
+        self._update_state(context, instance_id)
+
+        logging.debug(_('instance %s: snapshotting'), instance_ref['name'])
+        if instance_ref['state'] != power_state.RUNNING:
+            logging.warn(_('trying to snapshot a non-running '
+                           'instance: %s (state: %s excepted: %s)'),
+                         instance_ref['internal_id'],
+                         instance_ref['state'],
+                         power_state.RUNNING)
+
+        self.driver.snapshot(instance_ref, name)
+
+    @exception.wrap_exception
     def rescue_instance(self, context, instance_id):
         """Rescue an instance on this server."""
         context = context.elevated()
@@ -297,6 +318,15 @@ class ComputeManager(manager.Manager):
                                                        result))
 
     @exception.wrap_exception
+    def get_diagnostics(self, context, instance_id):
+        """Retrieve diagnostics for an instance on this server."""
+        instance_ref = self.db.instance_get(context, instance_id)
+
+        if instance_ref["state"] == power_state.RUNNING:
+            logging.debug(_("instance %s: retrieving diagnostics"),
+                instance_ref["internal_id"])
+            return self.driver.get_diagnostics(instance_ref)
+
     def suspend_instance(self, context, instance_id):
         """suspend the instance with instance_id"""
         context = context.elevated()
diff --git a/nova/db/api.py b/nova/db/api.py
index fde3f0852..127f15478 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -383,6 +383,11 @@ def instance_action_create(context, values):
     return IMPL.instance_action_create(context, values)
 
 
+def instance_get_actions(context, instance_id):
+    """Get instance actions by instance id."""
+    return IMPL.instance_get_actions(context, instance_id)
+
+
 ###################
 
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 7e945e4cb..8e68d12a4 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -856,6 +856,18 @@ def instance_action_create(context, values):
     return action_ref
 
 
+@require_admin_context
+def instance_get_actions(context, instance_id):
+    """Return the actions associated to the given instance id"""
+    session = get_session()
+    actions = {}
+    for action in session.query(models.InstanceActions).\
+        filter_by(instance_id=instance_id).\
+        all():
+        actions[action.action] = action.error
+    return actions
+
+
 ###################
 
 
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 693db8d23..a050aef23 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -22,7 +22,7 @@ SQLAlchemy models for nova data.
 import datetime
 
 from sqlalchemy.orm import relationship, backref, object_mapper
-from sqlalchemy import Column, Integer, Float, String, schema
+from sqlalchemy import Column, Integer, String, schema
 from sqlalchemy import ForeignKey, DateTime, Boolean, Text
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.ext.declarative import declarative_base
@@ -236,21 +236,6 @@ class Instance(BASE, NovaBase):
     #                     'shutdown', 'shutoff', 'crashed'])
 
 
-class InstanceDiagnostics(BASE, NovaBase):
-    """Represents a guest VM's diagnostics"""
-    __tablename__ = "instance_diagnostics"
-    id = Column(Integer, primary_key=True)
-    instance_id = Column(Integer, ForeignKey('instances.id'))
-
-    memory_available = Column(Float)
-    memory_free = Column(Float)
-    cpu_load = Column(Float)
-    disk_read = Column(Float)
-    disk_write = Column(Float)
-    net_tx = Column(Float)
-    net_rx = Column(Float)
-
-
 class InstanceActions(BASE, NovaBase):
     """Represents a guest VM's actions and results"""
     __tablename__ = "instance_actions"
@@ -452,7 +437,7 @@ class AuthToken(BASE, NovaBase):
     """
     __tablename__ = 'auth_tokens'
     token_hash = Column(String(255), primary_key=True)
-    user_id = Column(Integer)
+    user_id = Column(String(255))
     server_manageent_url = Column(String(255))
     storage_url = Column(String(255))
     cdn_management_url = Column(String(255))
@@ -561,7 +546,7 @@ def register_models():
     it will never need to be called explicitly elsewhere.
     """
     from sqlalchemy import create_engine
-    models = (Service, Instance, InstanceDiagnostics, InstanceActions,
+    models = (Service, Instance, InstanceActions,
               Volume, ExportDevice, IscsiTarget, FixedIp, FloatingIp,
               Network, SecurityGroup, SecurityGroupIngressRule,
               SecurityGroupInstanceAssociation, AuthToken, User,
diff --git a/nova/flags.py b/nova/flags.py
index 6f2747fc9..6eb0da3ec 100644
--- a/nova/flags.py
+++ b/nova/flags.py
@@ -212,6 +212,8 @@ DEFINE_list('region_list',
 DEFINE_string('connection_type', 'libvirt', 'libvirt, xenapi or fake')
 DEFINE_string('aws_access_key_id', 'admin', 'AWS Access ID')
 DEFINE_string('aws_secret_access_key', 'admin', 'AWS Access Key')
+DEFINE_integer('glance_port', 9292, 'glance port')
+DEFINE_string('glance_host', utils.get_my_ip(), 'glance host')
 DEFINE_integer('s3_port', 3333, 's3 port')
 DEFINE_string('s3_host', utils.get_my_ip(), 's3 host (for infrastructure)')
 DEFINE_string('s3_dmz', utils.get_my_ip(), 's3 dmz ip (for instances)')
@@ -246,6 +248,7 @@ DEFINE_string('cc_dmz', utils.get_my_ip(), 'internal ip of api server')
 DEFINE_integer('cc_port', 8773, 'cloud controller port')
 DEFINE_string('ec2_suffix', '/services/Cloud', 'suffix for ec2')
 
+DEFINE_string('default_project', 'openstack', 'default project for openstack')
 DEFINE_string('default_image', 'ami-11111',
               'default image to use, testing only')
 DEFINE_string('default_instance_type', 'm1.small',
diff --git a/nova/image/glance.py b/nova/image/glance.py
index cb3936df1..cc3192e7c 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -24,6 +24,7 @@ import urlparse
 
 import webob.exc
 
+from nova.compute import api as compute_api
 from nova import utils
 from nova import flags
 from nova import exception
diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py
index 79663e43a..961431154 100644
--- a/nova/tests/api/openstack/fakes.py
+++ b/nova/tests/api/openstack/fakes.py
@@ -110,6 +110,12 @@ def stub_out_networking(stubs):
     stubs.Set(nova.utils, 'get_my_ip', get_my_ip)
 
 
+def stub_out_compute_api_snapshot(stubs):
+    def snapshot(self, context, instance_id, name):
+        return 123
+    stubs.Set(nova.compute.api.ComputeAPI, 'snapshot', snapshot)
+
+
 def stub_out_glance(stubs, initial_fixtures=[]):
 
     class FakeParallaxClient:
@@ -213,6 +219,9 @@ class FakeAuthManager(object):
                 return v
         return None
 
+    def get_project(self, pid):
+        return None
+
     def get_user_from_access_key(self, key):
         return FakeAuthManager.auth_data.get(key, None)
 
diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py
index 1b4031217..0f274bd15 100644
--- a/nova/tests/api/openstack/test_images.py
+++ b/nova/tests/api/openstack/test_images.py
@@ -50,7 +50,7 @@ class BaseImageServiceTests(object):
                    'updated': None,
                    'created': None,
                    'status': None,
-                   'serverId': None,
+                   'instance_id': None,
                    'progress': None}
 
         num_images = len(self.service.index(self.context))
@@ -67,7 +67,7 @@ class BaseImageServiceTests(object):
                    'updated': None,
                    'created': None,
                    'status': None,
-                   'serverId': None,
+                   'instance_id': None,
                    'progress': None}
 
         num_images = len(self.service.index(self.context))
@@ -87,7 +87,7 @@ class BaseImageServiceTests(object):
                    'updated': None,
                    'created': None,
                    'status': None,
-                   'serverId': None,
+                   'instance_id': None,
                    'progress': None}
 
         id = self.service.create(self.context, fixture)
@@ -105,13 +105,13 @@ class BaseImageServiceTests(object):
                      'updated': None,
                      'created': None,
                      'status': None,
-                     'serverId': None,
+                     'instance_id': None,
                      'progress': None},
                     {'name': 'test image 2',
                      'updated': None,
                      'created': None,
                      'status': None,
-                     'serverId': None,
+                     'instance_id': None,
                      'progress': None}]
 
         num_images = len(self.service.index(self.context))
@@ -155,6 +155,7 @@ class GlanceImageServiceTest(unittest.TestCase,
     def setUp(self):
         self.stubs = stubout.StubOutForTesting()
         fakes.stub_out_glance(self.stubs)
+        fakes.stub_out_compute_api_snapshot(self.stubs)
         service_class = 'nova.image.glance.GlanceImageService'
         self.service = utils.import_object(service_class)
         self.context = context.RequestContext(None, None)
diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py
index 5d23db588..70ff714e6 100644
--- a/nova/tests/api/openstack/test_servers.py
+++ b/nova/tests/api/openstack/test_servers.py
@@ -95,6 +95,10 @@ class ServersTest(unittest.TestCase):
                        fake_compute_api)
         self.stubs.Set(nova.compute.api.ComputeAPI, 'resume',
                        fake_compute_api)
+        self.stubs.Set(nova.compute.api.ComputeAPI, "get_diagnostics",
+                       fake_compute_api)
+        self.stubs.Set(nova.compute.api.ComputeAPI, "get_actions",
+                       fake_compute_api)
         self.allow_admin = FLAGS.allow_admin_api
 
     def tearDown(self):
@@ -274,6 +278,18 @@ class ServersTest(unittest.TestCase):
         res = req.get_response(nova.api.API('os'))
         self.assertEqual(res.status_int, 202)
 
+    def test_server_diagnostics(self):
+        req = webob.Request.blank("/v1.0/servers/1/diagnostics")
+        req.method = "GET"
+        res = req.get_response(nova.api.API("os"))
+        self.assertEqual(res.status_int, 404)
+
+    def test_server_actions(self):
+        req = webob.Request.blank("/v1.0/servers/1/actions")
+        req.method = "GET"
+        res = req.get_response(nova.api.API("os"))
+        self.assertEqual(res.status_int, 404)
+
     def test_server_reboot(self):
         body = dict(server=dict(
             name='server_test', imageId=2, flavorId=2, metadata={},
diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py
index 11be81123..d6cb33fe2 100644
--- a/nova/tests/test_compute.py
+++ b/nova/tests/test_compute.py
@@ -151,6 +151,14 @@ class ComputeTestCase(test.TestCase):
         self.compute.reboot_instance(self.context, instance_id)
         self.compute.terminate_instance(self.context, instance_id)
 
+    def test_snapshot(self):
+        """Ensure instance can be snapshotted"""
+        instance_id = self._create_instance()
+        name = "myfakesnapshot"
+        self.compute.run_instance(self.context, instance_id)
+        self.compute.snapshot_instance(self.context, instance_id, name)
+        self.compute.terminate_instance(self.context, instance_id)
+
     def test_console_output(self):
         """Make sure we can get console output from instance"""
         instance_id = self._create_instance()
diff --git a/nova/tests/test_scheduler.py b/nova/tests/test_scheduler.py
index 91517cc5d..78e4a1c77 100644
--- a/nova/tests/test_scheduler.py
+++ b/nova/tests/test_scheduler.py
@@ -33,6 +33,7 @@ from nova.scheduler import driver
 
 FLAGS = flags.FLAGS
 flags.DECLARE('max_cores', 'nova.scheduler.simple')
+flags.DECLARE('stub_network', 'nova.compute.manager')
 
 
 class TestDriver(driver.Scheduler):
diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index ed2e4ffde..c95a53af3 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -29,9 +29,9 @@ from nova.auth import manager
 from nova.compute import instance_types
 from nova.compute import power_state
 from nova.virt import xenapi_conn
-from nova.virt.xenapi import fake
+from nova.virt.xenapi import fake as xenapi_fake
 from nova.virt.xenapi import volume_utils
-from nova.tests.db import fakes
+from nova.tests.db import fakes as db_fakes
 from nova.tests.xenapi import stubs
 
 FLAGS = flags.FLAGS
@@ -47,9 +47,9 @@ class XenAPIVolumeTestCase(test.TestCase):
         FLAGS.target_host = '127.0.0.1'
         FLAGS.xenapi_connection_url = 'test_url'
         FLAGS.xenapi_connection_password = 'test_pass'
-        fakes.stub_out_db_instance_api(self.stubs)
+        db_fakes.stub_out_db_instance_api(self.stubs)
         stubs.stub_out_get_target(self.stubs)
-        fake.reset()
+        xenapi_fake.reset()
         self.values = {'name': 1, 'id': 1,
                   'project_id': 'fake',
                   'user_id': 'fake',
@@ -83,7 +83,7 @@ class XenAPIVolumeTestCase(test.TestCase):
         label = 'SR-%s' % vol['ec2_id']
         description = 'Test-SR'
         sr_ref = helper.create_iscsi_storage(session, info, label, description)
-        srs = fake.get_all('SR')
+        srs = xenapi_fake.get_all('SR')
         self.assertEqual(sr_ref, srs[0])
         db.volume_destroy(context.get_admin_context(), vol['id'])
 
@@ -107,17 +107,17 @@ class XenAPIVolumeTestCase(test.TestCase):
         conn = xenapi_conn.get_connection(False)
         volume = self._create_volume()
         instance = db.instance_create(self.values)
-        fake.create_vm(instance.name, 'Running')
+        xenapi_fake.create_vm(instance.name, 'Running')
         result = conn.attach_volume(instance.name, volume['ec2_id'],
                                     '/dev/sdc')
 
         def check():
             # check that the VM has a VBD attached to it
             # Get XenAPI reference for the VM
-            vms = fake.get_all('VM')
+            vms = xenapi_fake.get_all('VM')
             # Get XenAPI record for VBD
-            vbds = fake.get_all('VBD')
-            vbd = fake.get_record('VBD', vbds[0])
+            vbds = xenapi_fake.get_all('VBD')
+            vbd = xenapi_fake.get_record('VBD', vbds[0])
             vm_ref = vbd['VM']
             self.assertEqual(vm_ref, vms[0])
 
@@ -130,7 +130,7 @@ class XenAPIVolumeTestCase(test.TestCase):
         conn = xenapi_conn.get_connection(False)
         volume = self._create_volume()
         instance = db.instance_create(self.values)
-        fake.create_vm(instance.name, 'Running')
+        xenapi_fake.create_vm(instance.name, 'Running')
         self.assertRaises(Exception,
                           conn.attach_volume,
                           instance.name,
@@ -156,41 +156,70 @@ class XenAPIVMTestCase(test.TestCase):
         self.stubs = stubout.StubOutForTesting()
         FLAGS.xenapi_connection_url = 'test_url'
         FLAGS.xenapi_connection_password = 'test_pass'
-        fake.reset()
-        fakes.stub_out_db_instance_api(self.stubs)
-        fake.create_network('fake', FLAGS.flat_network_bridge)
+        xenapi_fake.reset()
+        db_fakes.stub_out_db_instance_api(self.stubs)
+        xenapi_fake.create_network('fake', FLAGS.flat_network_bridge)
+        stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
+        self.conn = xenapi_conn.get_connection(False)
 
     def test_list_instances_0(self):
-        stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
-        conn = xenapi_conn.get_connection(False)
-        instances = conn.list_instances()
+        instances = self.conn.list_instances()
         self.assertEquals(instances, [])
 
+    def test_get_diagnostics(self):
+        instance = self._create_instance()
+        self.conn.get_diagnostics(instance)
+
+    def test_instance_snapshot(self):
+        stubs.stubout_instance_snapshot(self.stubs)
+        instance = self._create_instance()
+
+        name = "MySnapshot"
+        template_vm_ref = self.conn.snapshot(instance, name)
+
+        def ensure_vm_was_torn_down():
+            vm_labels = []
+            for vm_ref in xenapi_fake.get_all('VM'):
+                vm_rec = xenapi_fake.get_record('VM', vm_ref)
+                if not vm_rec["is_control_domain"]:
+                    vm_labels.append(vm_rec["name_label"])
+
+            self.assertEquals(vm_labels, [1])
+
+        def ensure_vbd_was_torn_down():
+            vbd_labels = []
+            for vbd_ref in xenapi_fake.get_all('VBD'):
+                vbd_rec = xenapi_fake.get_record('VBD', vbd_ref)
+                vbd_labels.append(vbd_rec["vm_name_label"])
+
+            self.assertEquals(vbd_labels, [1])
+
+        def ensure_vdi_was_torn_down():
+            for vdi_ref in xenapi_fake.get_all('VDI'):
+                vdi_rec = xenapi_fake.get_record('VDI', vdi_ref)
+                name_label = vdi_rec["name_label"]
+                self.assert_(not name_label.endswith('snapshot'))
+
+        def check():
+            ensure_vm_was_torn_down()
+            ensure_vbd_was_torn_down()
+            ensure_vdi_was_torn_down()
+
+        check()
+
     def test_spawn(self):
-        stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
-        values = {'name': 1, 'id': 1,
-                  'project_id': self.project.id,
-                  'user_id': self.user.id,
-                  'image_id': 1,
-                  'kernel_id': 2,
-                  'ramdisk_id': 3,
-                  'instance_type': 'm1.large',
-                  'mac_address': 'aa:bb:cc:dd:ee:ff',
-                  }
-        conn = xenapi_conn.get_connection(False)
-        instance = db.instance_create(values)
-        conn.spawn(instance)
+        instance = self._create_instance()
 
         def check():
-            instances = conn.list_instances()
+            instances = self.conn.list_instances()
             self.assertEquals(instances, [1])
 
             # Get Nova record for VM
-            vm_info = conn.get_info(1)
+            vm_info = self.conn.get_info(1)
 
             # Get XenAPI record for VM
-            vms = fake.get_all('VM')
-            vm = fake.get_record('VM', vms[0])
+            vms = xenapi_fake.get_all('VM')
+            vm = xenapi_fake.get_record('VM', vms[0])
 
             # Check that m1.large above turned into the right thing.
             instance_type = instance_types.INSTANCE_TYPES['m1.large']
@@ -218,3 +247,18 @@ class XenAPIVMTestCase(test.TestCase):
         self.manager.delete_project(self.project)
         self.manager.delete_user(self.user)
         self.stubs.UnsetAll()
+
+    def _create_instance(self):
+        """Creates and spawns a test instance"""
+        values = {'name': 1, 'id': 1,
+                  'project_id': self.project.id,
+                  'user_id': self.user.id,
+                  'image_id': 1,
+                  'kernel_id': 2,
+                  'ramdisk_id': 3,
+                  'instance_type': 'm1.large',
+                  'mac_address': 'aa:bb:cc:dd:ee:ff'
+                  }
+        instance = db.instance_create(values)
+        self.conn.spawn(instance)
+        return instance
diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py
index a7e592fee..55f751f11 100644
--- a/nova/tests/xenapi/stubs.py
+++ b/nova/tests/xenapi/stubs.py
@@ -19,6 +19,54 @@
 from nova.virt import xenapi_conn
 from nova.virt.xenapi import fake
 from nova.virt.xenapi import volume_utils
+from nova.virt.xenapi import vm_utils
+
+
+def stubout_instance_snapshot(stubs):
+    @classmethod
+    def fake_fetch_image(cls, session, instance_id, image, user, project,
+                         type):
+        # Stubout wait_for_task
+        def fake_wait_for_task(self, id, task):
+            class FakeEvent:
+
+                def send(self, value):
+                    self.rv = value
+
+                def wait(self):
+                    return self.rv
+
+            done = FakeEvent()
+            self._poll_task(id, task, done)
+            rv = done.wait()
+            return rv
+
+        stubs.Set(xenapi_conn.XenAPISession, 'wait_for_task',
+                  fake_wait_for_task)
+
+        from nova.virt.xenapi.fake import create_vdi
+        name_label = "instance-%s" % instance_id
+        #TODO: create fake SR record
+        sr_ref = "fakesr"
+        vdi_ref = create_vdi(name_label=name_label, read_only=False,
+                             sr_ref=sr_ref, sharable=False)
+        vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref)
+        vdi_uuid = vdi_rec['uuid']
+        return vdi_uuid
+
+    stubs.Set(vm_utils.VMHelper, 'fetch_image', fake_fetch_image)
+
+    def fake_parse_xmlrpc_value(val):
+        return val
+
+    stubs.Set(xenapi_conn, '_parse_xmlrpc_value', fake_parse_xmlrpc_value)
+
+    def fake_wait_for_vhd_coalesce(session, instance_id, sr_ref, vdi_ref,
+                              original_parent_uuid):
+        #TODO(sirp): Should we actually fake out the data here
+        return "fakeparent"
+
+    stubs.Set(vm_utils, 'wait_for_vhd_coalesce', fake_wait_for_vhd_coalesce)
 
 
 def stubout_session(stubs, cls):
@@ -63,6 +111,24 @@ class FakeSessionForVMTests(fake.SessionBase):
         vm['is_a_template'] = False
         vm['is_control_domain'] = False
 
+    def VM_snapshot(self, session_ref, vm_ref, label):
+        status = "Running"
+        template_vm_ref = fake.create_vm(label, status, is_a_template=True,
+            is_control_domain=False)
+
+        sr_ref = "fakesr"
+        template_vdi_ref = fake.create_vdi(label, read_only=True,
+            sr_ref=sr_ref, sharable=False)
+
+        template_vbd_ref = fake.create_vbd(template_vm_ref, template_vdi_ref)
+        return template_vm_ref
+
+    def VDI_destroy(self, session_ref, vdi_ref):
+        fake.destroy_vdi(vdi_ref)
+
+    def VM_destroy(self, session_ref, vm_ref):
+        fake.destroy_vm(vm_ref)
+
 
 class FakeSessionForVolumeTests(fake.SessionBase):
     """ Stubs out a XenAPISession for Volume tests """
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index a700e35c1..925c32e4d 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -112,6 +112,20 @@ class FakeConnection(object):
         self.instances[instance.name] = fake_instance
         fake_instance._state = power_state.RUNNING
 
+    def snapshot(self, instance, name):
+        """
+        Snapshots the specified instance.
+
+        The given parameter is an instance of nova.compute.service.Instance,
+        and so the instance is being specified as instance.name.
+
+        The second parameter is the name of the snapshot.
+
+        The work will be done asynchronously.  This function returns a
+        Deferred that allows the caller to detect when it is complete.
+        """
+        pass
+
     def reboot(self, instance):
         """
         Reboot the specified instance.
@@ -202,6 +216,9 @@ class FakeConnection(object):
                 'num_cpu': 2,
                 'cpu_time': 0}
 
+    def get_diagnostics(self, instance_name):
+        pass
+
     def list_disks(self, instance_name):
         """
         Return the IDs of all the virtual disks attached to the specified
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index dc878846d..6471e8c0c 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -73,6 +73,9 @@ FLAGS = flags.FLAGS
 flags.DEFINE_string('rescue_image_id', 'ami-rescue', 'Rescue ami image')
 flags.DEFINE_string('rescue_kernel_id', 'aki-rescue', 'Rescue aki image')
 flags.DEFINE_string('rescue_ramdisk_id', 'ari-rescue', 'Rescue ari image')
+flags.DEFINE_string('injected_network_template',
+                    utils.abspath('virt/interfaces.template'),
+                    'Template file for injected network')
 flags.DEFINE_string('libvirt_xml_template',
                     utils.abspath('virt/libvirt.xml.template'),
                     'Libvirt XML Template')
@@ -265,6 +268,13 @@ class LibvirtConnection(object):
         virt_dom.detachDevice(xml)
 
     @exception.wrap_exception
+    def snapshot(self, instance, name):
+        """ Create snapshot from a running VM instance """
+        raise NotImplementedError(
+            _("Instance snapshotting is not supported for libvirt"
+              "at this time"))
+
+    @exception.wrap_exception
     def reboot(self, instance):
         self.destroy(instance, False)
         xml = self.to_xml(instance)
@@ -624,6 +634,9 @@ class LibvirtConnection(object):
                 'num_cpu': num_cpu,
                 'cpu_time': cpu_time}
 
+    def get_diagnostics(self, instance_name):
+        raise exception.APIError("diagnostics are not supported for libvirt")
+
     def get_disks(self, instance_name):
         """
         Note that this function takes an instance name, not an Instance, so
diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py
index 1eaf31c25..aa4026f97 100644
--- a/nova/virt/xenapi/fake.py
+++ b/nova/virt/xenapi/fake.py
@@ -55,6 +55,8 @@ import datetime
 import logging
 import uuid
 
+from pprint import pformat
+
 from nova import exception
 
 
@@ -64,6 +66,10 @@ _CLASSES = ['host', 'network', 'session', 'SR', 'VBD',\
 _db_content = {}
 
 
+def log_db_contents(msg=None):
+    logging.debug(_("%s: _db_content => %s"), msg or "", pformat(_db_content))
+
+
 def reset():
     for c in _CLASSES:
         _db_content[c] = {}
@@ -93,6 +99,24 @@ def create_vm(name_label, status,
         })
 
 
+def destroy_vm(vm_ref):
+    vm_rec = _db_content['VM'][vm_ref]
+
+    vbd_refs = vm_rec['VBDs']
+    for vbd_ref in vbd_refs:
+        destroy_vbd(vbd_ref)
+
+    del _db_content['VM'][vm_ref]
+
+
+def destroy_vbd(vbd_ref):
+    del _db_content['VBD'][vbd_ref]
+
+
+def destroy_vdi(vdi_ref):
+    del _db_content['VDI'][vdi_ref]
+
+
 def create_vdi(name_label, read_only, sr_ref, sharable):
     return _create_object('VDI', {
         'name_label': name_label,
@@ -109,6 +133,23 @@ def create_vdi(name_label, read_only, sr_ref, sharable):
         })
 
 
+def create_vbd(vm_ref, vdi_ref):
+    vbd_rec = {'VM': vm_ref, 'VDI': vdi_ref}
+    vbd_ref = _create_object('VBD', vbd_rec)
+    after_VBD_create(vbd_ref, vbd_rec)
+    return vbd_ref
+
+
+def after_VBD_create(vbd_ref, vbd_rec):
+    """Create backref from VM to VBD when VBD is created"""
+    vm_ref = vbd_rec['VM']
+    vm_rec = _db_content['VM'][vm_ref]
+    vm_rec['VBDs'] = [vbd_ref]
+
+    vm_name_label = _db_content['VM'][vm_ref]['name_label']
+    vbd_rec['vm_name_label'] = vm_name_label
+
+
 def create_pbd(config, sr_ref, attached):
     return _create_object('PBD', {
         'device-config': config,
@@ -277,11 +318,12 @@ class SessionBase(object):
             self._check_arg_count(params, 2)
             return get_record(cls, params[1])
 
-        if (func == 'get_by_name_label' or
-            func == 'get_by_uuid'):
+        if func in ('get_by_name_label', 'get_by_uuid'):
             self._check_arg_count(params, 2)
+            return_singleton = (func == 'get_by_uuid')
             return self._get_by_field(
-                _db_content[cls], func[len('get_by_'):], params[1])
+                _db_content[cls], func[len('get_by_'):], params[1],
+                return_singleton=return_singleton)
 
         if len(params) == 2:
             field = func[len('get_'):]
@@ -324,6 +366,13 @@ class SessionBase(object):
         (cls, _) = name.split('.')
         ref = is_sr_create and \
             _create_sr(cls, params) or _create_object(cls, params[1])
+
+        # Call hook to provide any fixups needed (ex. creating backrefs)
+        try:
+            globals()["after_%s_create" % cls](ref, params[1])
+        except KeyError:
+            pass
+
         obj = get_record(cls, ref)
 
         # Add RO fields
@@ -359,11 +408,18 @@ class SessionBase(object):
             raise Failure(['MESSAGE_PARAMETER_COUNT_MISMATCH',
                                   expected, actual])
 
-    def _get_by_field(self, recs, k, v):
+    def _get_by_field(self, recs, k, v, return_singleton):
         result = []
         for ref, rec in recs.iteritems():
             if rec.get(k) == v:
                 result.append(ref)
+
+        if return_singleton:
+            try:
+                return result[0]
+            except IndexError:
+                return None
+
         return result
 
 
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index 47fb6db53..9d1b51848 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -20,11 +20,14 @@ their attributes like VDIs, VIFs, as well as their lookup functions.
 """
 
 import logging
+import pickle
 import urllib
 from xml.dom import minidom
 
+from eventlet import event
 from nova import exception
 from nova import flags
+from nova import utils
 from nova.auth.manager import AuthManager
 from nova.compute import instance_types
 from nova.compute import power_state
@@ -204,7 +207,54 @@ class VMHelper(HelperBase):
         return vif_ref
 
     @classmethod
-    def fetch_image(cls, session, image, user, project, type):
+    def create_snapshot(cls, session, instance_id, vm_ref, label):
+        """ Creates Snapshot (Template) VM, Snapshot VBD, Snapshot VDI,
+        Snapshot VHD
+        """
+        #TODO(sirp): Add quiesce and VSS locking support when Windows support
+        # is added
+        logging.debug(_("Snapshotting VM %s with label '%s'..."),
+                      vm_ref, label)
+
+        vm_vdi_ref, vm_vdi_rec = get_vdi_for_vm_safely(session, vm_ref)
+        vm_vdi_uuid = vm_vdi_rec["uuid"]
+        sr_ref = vm_vdi_rec["SR"]
+
+        original_parent_uuid = get_vhd_parent_uuid(session, vm_vdi_ref)
+
+        task = session.call_xenapi('Async.VM.snapshot', vm_ref, label)
+        template_vm_ref = session.wait_for_task(instance_id, task)
+        template_vdi_rec = get_vdi_for_vm_safely(session, template_vm_ref)[1]
+        template_vdi_uuid = template_vdi_rec["uuid"]
+
+        logging.debug(_('Created snapshot %s from VM %s.'), template_vm_ref,
+                      vm_ref)
+
+        parent_uuid = wait_for_vhd_coalesce(
+            session, instance_id, sr_ref, vm_vdi_ref, original_parent_uuid)
+
+        #TODO(sirp): we need to assert only one parent, not parents two deep
+        return template_vm_ref, [template_vdi_uuid, parent_uuid]
+
+    @classmethod
+    def upload_image(cls, session, instance_id, vdi_uuids, image_name):
+        """ Requests that the Glance plugin bundle the specified VDIs and
+        push them into Glance using the specified human-friendly name.
+        """
+        logging.debug(_("Asking xapi to upload %s as '%s'"),
+                      vdi_uuids, image_name)
+
+        params = {'vdi_uuids': vdi_uuids,
+                  'image_name': image_name,
+                  'glance_host': FLAGS.glance_host,
+                  'glance_port': FLAGS.glance_port}
+
+        kwargs = {'params': pickle.dumps(params)}
+        task = session.async_call_plugin('glance', 'put_vdis', kwargs)
+        session.wait_for_task(instance_id, task)
+
+    @classmethod
+    def fetch_image(cls, session, instance_id, image, user, project, type):
         """
         type is interpreted as an ImageType instance
         """
@@ -223,9 +273,7 @@ class VMHelper(HelperBase):
             if type == ImageType.DISK_RAW:
                 args['raw'] = 'true'
         task = session.async_call_plugin('objectstore', fn, args)
-        #FIXME(armando): find a solution to missing instance_id
-        #with Josh Kearney
-        uuid = session.wait_for_task(0, task)
+        uuid = session.wait_for_task(instance_id, task)
         return uuid
 
     @classmethod
@@ -299,6 +347,10 @@ class VMHelper(HelperBase):
         try:
             host = session.get_xenapi_host()
             host_ip = session.get_xenapi().host.get_record(host)["address"]
+        except (cls.XenAPI.Failure, KeyError) as e:
+            return {"Unable to retrieve diagnostics": e}
+
+        try:
             diags = {}
             xml = get_rrd(host_ip, record["uuid"])
             if xml:
@@ -325,3 +377,87 @@ def get_rrd(host, uuid):
         return xml.read()
     except IOError:
         return None
+
+
+#TODO(sirp): This code comes from XS5.6 pluginlib.py, we should refactor to
+# use that implmenetation
+def get_vhd_parent(session, vdi_rec):
+    """
+    Returns the VHD parent of the given VDI record, as a (ref, rec) pair.
+    Returns None if we're at the root of the tree.
+    """
+    if 'vhd-parent' in vdi_rec['sm_config']:
+        parent_uuid = vdi_rec['sm_config']['vhd-parent']
+        #NOTE(sirp): changed xenapi -> get_xenapi()
+        parent_ref = session.get_xenapi().VDI.get_by_uuid(parent_uuid)
+        parent_rec = session.get_xenapi().VDI.get_record(parent_ref)
+        #NOTE(sirp): changed log -> logging
+        logging.debug(_("VHD %s has parent %s"), vdi_rec['uuid'], parent_ref)
+        return parent_ref, parent_rec
+    else:
+        return None
+
+
+def get_vhd_parent_uuid(session, vdi_ref):
+    vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref)
+    ret = get_vhd_parent(session, vdi_rec)
+    if ret:
+        parent_ref, parent_rec = ret
+        return parent_rec["uuid"]
+    else:
+        return None
+
+
+def scan_sr(session, instance_id, sr_ref):
+    logging.debug(_("Re-scanning SR %s"), sr_ref)
+    task = session.call_xenapi('Async.SR.scan', sr_ref)
+    session.wait_for_task(instance_id, task)
+
+
+def wait_for_vhd_coalesce(session, instance_id, sr_ref, vdi_ref,
+                          original_parent_uuid):
+    """ Spin until the parent VHD is coalesced into its parent VHD
+
+    Before coalesce:
+        * original_parent_vhd
+            * parent_vhd
+                snapshot
+
+    Atter coalesce:
+        * parent_vhd
+            snapshot
+    """
+    #TODO(sirp): we need to timeout this req after a while
+
+    def _poll_vhds():
+        scan_sr(session, instance_id, sr_ref)
+        parent_uuid = get_vhd_parent_uuid(session, vdi_ref)
+        if original_parent_uuid and (parent_uuid != original_parent_uuid):
+            logging.debug(
+                _("Parent %s doesn't match original parent %s, "
+                  "waiting for coalesce..."),
+                parent_uuid, original_parent_uuid)
+        else:
+            done.send(parent_uuid)
+
+    done = event.Event()
+    loop = utils.LoopingCall(_poll_vhds)
+    loop.start(FLAGS.xenapi_vhd_coalesce_poll_interval, now=True)
+    parent_uuid = done.wait()
+    loop.stop()
+    return parent_uuid
+
+
+def get_vdi_for_vm_safely(session, vm_ref):
+    vdi_refs = VMHelper.lookup_vm_vdis(session, vm_ref)
+    if vdi_refs is None:
+        raise Exception(_("No VDIs found for VM %s") % vm_ref)
+    else:
+        num_vdis = len(vdi_refs)
+        if num_vdis != 1:
+            raise Exception(_("Unexpected number of VDIs (%s) found for "
+                               "VM %s") % (num_vdis, vm_ref))
+
+    vdi_ref = vdi_refs[0]
+    vdi_rec = session.get_xenapi().VDI.get_record(vdi_ref)
+    return vdi_ref, vdi_rec
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index ba502ffa2..76f31635a 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -70,7 +70,7 @@ class VMOps(object):
             disk_image_type = ImageType.DISK
         else:
             disk_image_type = ImageType.DISK_RAW
-        vdi_uuid = VMHelper.fetch_image(self._session,
+        vdi_uuid = VMHelper.fetch_image(self._session, instance.id,
             instance.image_id, user, project, disk_image_type)
         vdi_ref = self._session.call_xenapi('VDI.get_by_uuid', vdi_uuid)
         #Have a look at the VDI and see if it has a PV kernel
@@ -79,11 +79,11 @@ class VMOps(object):
             pv_kernel = VMHelper.lookup_image(self._session, vdi_ref)
         kernel = None
         if instance.kernel_id:
-            kernel = VMHelper.fetch_image(self._session,
+            kernel = VMHelper.fetch_image(self._session, instance.id,
                 instance.kernel_id, user, project, ImageType.KERNEL_RAMDISK)
         ramdisk = None
         if instance.ramdisk_id:
-            ramdisk = VMHelper.fetch_image(self._session,
+            ramdisk = VMHelper.fetch_image(self._session, instance.id,
                 instance.ramdisk_id, user, project, ImageType.KERNEL_RAMDISK)
         vm_ref = VMHelper.create_vm(self._session,
                                           instance, kernel, ramdisk, pv_kernel)
@@ -120,6 +120,52 @@ class VMOps(object):
         timer.f = _wait_for_boot
         return timer.start(interval=0.5, now=True)
 
+    def snapshot(self, instance, name):
+        """ Create snapshot from a running VM instance
+
+        :param instance: instance to be snapshotted
+        :param name: name/label to be given to the snapshot
+
+        Steps involved in a XenServer snapshot:
+
+        1. XAPI-Snapshot: Snapshotting the instance using XenAPI. This
+            creates: Snapshot (Template) VM, Snapshot VBD, Snapshot VDI,
+            Snapshot VHD
+
+        2. Wait-for-coalesce: The Snapshot VDI and Instance VDI both point to
+            a 'base-copy' VDI.  The base_copy is immutable and may be chained
+            with other base_copies.  If chained, the base_copies
+            coalesce together, so, we must wait for this coalescing to occur to
+            get a stable representation of the data on disk.
+
+        3. Push-to-glance: Once coalesced, we call a plugin on the XenServer
+            that will bundle the VHDs together and then push the bundle into
+            Glance.
+        """
+
+        #TODO(sirp): Add quiesce and VSS locking support when Windows support
+        # is added
+
+        logging.debug(_("Starting snapshot for VM %s"), instance)
+        vm_ref = VMHelper.lookup(self._session, instance.name)
+
+        label = "%s-snapshot" % instance.name
+        try:
+            template_vm_ref, template_vdi_uuids = VMHelper.create_snapshot(
+                self._session, instance.id, vm_ref, label)
+        except self.XenAPI.Failure, exc:
+            logging.error(_("Unable to Snapshot %s: %s"), vm_ref, exc)
+            return
+
+        try:
+            # call plugin to ship snapshot off to glance
+            VMHelper.upload_image(
+                self._session, instance.id, template_vdi_uuids, name)
+        finally:
+            self._destroy(instance, template_vm_ref, shutdown=False)
+
+        logging.debug(_("Finished snapshot and upload for VM %s"), instance)
+
     def reboot(self, instance):
         """Reboot VM instance"""
         instance_name = instance.name
@@ -133,31 +179,36 @@ class VMOps(object):
     def destroy(self, instance):
         """Destroy VM instance"""
         vm = VMHelper.lookup(self._session, instance.name)
+        return self._destroy(instance, vm, shutdown=True)
+
+    def _destroy(self, instance, vm, shutdown=True):
+        """ Destroy VM instance """
         if vm is None:
             # Don't complain, just return.  This lets us clean up instances
             # that have already disappeared from the underlying platform.
             return
         # Get the VDIs related to the VM
         vdis = VMHelper.lookup_vm_vdis(self._session, vm)
-        try:
-            task = self._session.call_xenapi('Async.VM.hard_shutdown',
-                                                   vm)
-            self._session.wait_for_task(instance.id, task)
-        except XenAPI.Failure, exc:
-            logging.warn(exc)
+        if shutdown:
+            try:
+                task = self._session.call_xenapi('Async.VM.hard_shutdown', vm)
+                self._session.wait_for_task(instance.id, task)
+            except self.XenAPI.Failure, exc:
+                logging.warn(exc)
+
         # Disk clean-up
         if vdis:
             for vdi in vdis:
                 try:
                     task = self._session.call_xenapi('Async.VDI.destroy', vdi)
                     self._session.wait_for_task(instance.id, task)
-                except XenAPI.Failure, exc:
+                except self.XenAPI.Failure, exc:
                     logging.warn(exc)
         # VM Destroy
         try:
             task = self._session.call_xenapi('Async.VM.destroy', vm)
             self._session.wait_for_task(instance.id, task)
-        except XenAPI.Failure, exc:
+        except self.XenAPI.Failure, exc:
             logging.warn(exc)
 
     def _wait_with_callback(self, instance_id, task, callback):
@@ -217,11 +268,12 @@ class VMOps(object):
         rec = self._session.get_xenapi().VM.get_record(vm)
         return VMHelper.compile_info(rec)
 
-    def get_diagnostics(self, instance_id):
+    def get_diagnostics(self, instance):
         """Return data about VM diagnostics"""
-        vm = VMHelper.lookup(self._session, instance_id)
+        vm = VMHelper.lookup(self._session, instance.name)
         if vm is None:
-            raise exception.NotFound(_("Instance not found %s") % instance_id)
+            raise exception.NotFound(_("Instance not found %s") %
+                instance.name)
         rec = self._session.get_xenapi().VM.get_record(vm)
         return VMHelper.compile_diagnostics(self._session, rec)
 
diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py
index 7f03d6c2b..f17c8f39d 100644
--- a/nova/virt/xenapi_conn.py
+++ b/nova/virt/xenapi_conn.py
@@ -83,6 +83,10 @@ flags.DEFINE_float('xenapi_task_poll_interval',
                    'The interval used for polling of remote tasks '
                    '(Async.VM.start, etc).  Used only if '
                    'connection_type=xenapi.')
+flags.DEFINE_float('xenapi_vhd_coalesce_poll_interval',
+                   5.0,
+                   'The interval used for polling of coalescing vhds.'
+                   '  Used only if connection_type=xenapi.')
 flags.DEFINE_string('target_host',
                     None,
                     'iSCSI Target Host')
@@ -131,6 +135,10 @@ class XenAPIConnection(object):
         """Create VM instance"""
         self._vmops.spawn(instance)
 
+    def snapshot(self, instance, name):
+        """ Create snapshot from a running VM instance """
+        self._vmops.snapshot(instance, name)
+
     def reboot(self, instance):
         """Reboot VM instance"""
         self._vmops.reboot(instance)
@@ -159,9 +167,9 @@ class XenAPIConnection(object):
         """Return data about VM instance"""
         return self._vmops.get_info(instance_id)
 
-    def get_diagnostics(self, instance_id):
+    def get_diagnostics(self, instance):
         """Return data about VM diagnostics"""
-        return self._vmops.get_diagnostics(instance_id)
+        return self._vmops.get_diagnostics(instance)
 
     def get_console_output(self, instance):
         """Return snapshot of console"""
@@ -233,8 +241,8 @@ class XenAPISession(object):
             name = self._session.xenapi.task.get_name_label(task)
             status = self._session.xenapi.task.get_status(task)
             action = dict(
-                id=int(id),
-                action=name,
+                instance_id=int(id),
+                action=name[0:255],  # Ensure action is never > 255
                 error=None)
             if status == "pending":
                 return