Merge lp:~yamahata/nova/boot-from-volume-0 into lp:~hudson-openstack/nova/trunk

[Isaku Yamahata]

Implements a portion of ec2 ebs boot.
What's implemented
- block_device_mapping option for run instance with volume
  (ephemeral device and no device isn't supported yet)
- stop/start instance

TODO:
- ephemeral device/no device
- machine image

This branch implements boot from volume.
What can be done with this branch are
- --block-device-mapping for run instance command.
  volume id or snapshot id can be specified.
  ephemeral device/no device isn't supported yet.
- stop/start instance

There are several things left, they will be done as next step.
- machine image to point volume
- ephemeral device/no device

[Brian Waldon (bcwaldon)]

There are a couple of conflicts you might want to look into.

[Isaku Yamahata (yamahata)]

Thank you. Now I fixed it.

On Thu, May 26, 2011 at 02:21:04PM -0000, Brian Waldon wrote:
> There are a couple of conflicts you might want to look into.
> --
> https://code.launchpad.net/~yamahata/nova/boot-from-volume-0/+merge/62419
> You are the owner of lp:~yamahata/nova/boot-from-volume-0.
>

--
yamahata

[Devin Carlen (devcamcar)]

Can you please add some comments to this code block? It's unclear what this change is for.

37 + if len(parts) > 1:
38 + d = args.get(key, {})
39 + args[key] = d
40 + for k in parts[1:-1]:
41 + k = _camelcase_to_underscore(k)
42 + v = d.get(k, {})
43 + d[k] = v
44 + d = v
45 + d[_camelcase_to_underscore(parts[-1])] = value
46 + else:
47 + args[key] = value

Please add a note header to this comment block, as in:

# NOTE(your username):

This helps in tracking down subject matter experts in this large codebase.

60 + # BlockDevicedMapping.<N>.DeviceName
61 + # BlockDevicedMapping.<N>.Ebs.SnapshotId
62 + # BlockDevicedMapping.<N>.Ebs.VolumeSize
63 + # BlockDevicedMapping.<N>.Ebs.DeleteOnTermination
64 + # BlockDevicedMapping.<N>.VirtualName
65 + # => remove .Ebs and allow volume id in SnapshotId

And same thing here:

188 + # tell vm driver to attach volume at boot time by updating
189 + # BlockDeviceMapping

review: Needs Information

[Isaku Yamahata (yamahata)]

On Fri, May 27, 2011 at 09:06:09PM -0000, Devin Carlen wrote:
> Review: Needs Information
> Can you please add some comments to this code block? It's unclear what this change is for.

I added the comment on the code.
That hunk teaches multi dotted argument to ec2 argument parse.
So far only single dot is allowed. But, ec2 block device mapping
uses multi dot separated argument like
BlockDeviceMapping.1.DeviceName=snap-id.

> Please add a note header to this comment block, as in:
> # NOTE(your username):

Okay, I added it to them.
Is this required custom for nova?
"bzr annotation" provides what you want as version control system
is exactly for that purpose. And sprinkling username makes code
ugly.
--
yamahata

[Isaku Yamahata (yamahata)]

Ping?
What can I do to make progress?

[Rick Harris (rconradharris)]

Very impressive work! Just a few small nits:

Received a test failure:

  ======================================================================
  FAIL: test_stop_with_attached_volume (nova.tests.test_cloud.CloudTestCase)
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/home/rick/openstack/nova/boot-from-volume-0/nova/tests/test_cloud.py", line 691, in test_stop_with_attached_volume
      self._assert_volume_attached(vol, instance_id, '/dev/vdc')
    File "/home/rick/openstack/nova/boot-from-volume-0/nova/tests/test_cloud.py", line 582, in _assert_volume_attached
      self.assertEqual(vol['mountpoint'], mountpoint)
  AssertionError: u'\\/dev\\/vdc' != '/dev/vdc'

> 72 + if len(parts) > 1:
> 73 + d = args.get(key, {})
> 74 + args[key] = d
> 75 + for k in parts[1:-1]:
> 76 + k = _camelcase_to_underscore(k)
> 77 + v = d.get(k, {})
> 78 + d[k] = v
> 79 + d = v
> 80 + d[_camelcase_to_underscore(parts[-1])] = value
> 81 + else:
> 82 + args[key] = value

Might be worth breaking this code out into a utility method, something like:
`dict_from_dotted_str`.

> 68 + # EBS boot uses multi dot-separeted arguments like

Typofix. s/separeted/separated/

> 315 + block_device_mapping=[]):

Usually not a good idea to use a list as a default argument. This is because
the list-object is created at /function definition/ time and the same list
object will be re-used on each invocation--probably not what you wanted.

Instead, it's better to default to None and initialize a new list in the
function's body:

  block_device_mapping=None):

    block_device_mapping = block_device_mapping or []

OR....

    if not block_device_mapping:
      block_device_mapping = []

> 393 + if not _is_able_to_shutdown(instance, instance_id):
> 394 + return

Should we log here that we weren't able to shutdown, something like:

  LOG.warn(_("Unable to shutdown server...."))

> 975 === added file 'nova/db/sqlalchemy/migrate_repo/versions/019_add_volume_snapshot_support.py'

Looks like you'll have to renumber these since trunk has already advanced
migration numbers.

review: Needs Fixing

[Matt Dietz (cerberus)]

First of all, great work on this!

I see a few failing tests:

======================================================================
ERROR: test_compute_can_update_available_resource (nova.tests.test_service.ServiceTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/nova/tests/test_service.py", line 334, in test_compute_can_update_available_resource
    {'wait': wait_func})
TypeError: CreateMock() takes exactly 2 arguments (3 given)

======================================================================
ERROR: test_compute_can_update_available_resource (nova.tests.test_service.ServiceTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/nova/test.py", line 94, in tearDown
    self.mox.VerifyAll()
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/.nova-venv/lib/python2.6/site-packages/mox.py", line 197, in VerifyAll
    mock_obj._Verify()
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/.nova-venv/lib/python2.6/site-packages/mox.py", line 344, in _Verify
    raise ExpectedMethodCallsError(self._expected_calls_queue)
ExpectedMethodCallsError: Verify: Expected methods never called:
  0. __call__(new=<IgnoreArg>) -> None

======================================================================
ERROR: test_create (nova.tests.test_service.ServiceTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/nova/tests/test_service.py", line 144, in test_create
    {'wait': wait_func})
TypeError: CreateMock() takes exactly 2 arguments (3 given)

======================================================================
ERROR: test_create (nova.tests.test_service.ServiceTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/nova/test.py", line 94, in tearDown
    self.mox.VerifyAll()
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/.nova-venv/lib/python2.6/site-packages/mox.py", line 197, in VerifyAll
    mock_obj._Verify()
  File "/Users/cerberus/code/python/nova/boot-from-volume-0/.nova-venv/lib/python2.6/site-packages/mox.py", line 344, in _Verify
    raise ExpectedMethodCallsError(self._expected_calls_queue)
ExpectedMethodCallsError: Verify: Expected methods never called:
  0. __call__(new=<IgnoreArg>) -> None

----------------------------------------------------------------------

455 + try:
456 + bdms = self.db.block_device_mapping_get_all_by_instance(
457 + context, instance_id)
458 + except exception.NotFound:
459 + pass

I don't really like throwing away exceptions. It's an explicit exception, sure, but it could mask something important. Seems like this should at least log the error.

476 + assert ((bdm['snapshot_id'] is None) or
477 + (bdm['volume_id'] ...

review: Needs Fixing

[Isaku Yamahata (yamahata)]

On Wed, Jun 08, 2011 at 06:27:26PM -0000, Rick Harris wrote:
> Review: Needs Fixing
> Very impressive work! Just a few small nits:
>
>
> Received a test failure:
>
> ======================================================================
> FAIL: test_stop_with_attached_volume (nova.tests.test_cloud.CloudTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/home/rick/openstack/nova/boot-from-volume-0/nova/tests/test_cloud.py", line 691, in test_stop_with_attached_volume
> self._assert_volume_attached(vol, instance_id, '/dev/vdc')
> File "/home/rick/openstack/nova/boot-from-volume-0/nova/tests/test_cloud.py", line 582, in _assert_volume_attached
> self.assertEqual(vol['mountpoint'], mountpoint)
> AssertionError: u'\\/dev\\/vdc' != '/dev/vdc'

Hmm, the test passes for me. I'm using sqlite3 for unittest.
I can't find the code to escape '/' into '\\/'. MySQL?

> > 315 + block_device_mapping=[]):
>
> Usually not a good idea to use a list as a default argument. This is because
> the list-object is created at /function definition/ time and the same list
> object will be re-used on each invocation--probably not what you wanted.
>
> Instead, it's better to default to None and initialize a new list in the
> function's body:
>
> block_device_mapping=None):
>
> block_device_mapping = block_device_mapping or []
>
> OR....
>
> if not block_device_mapping:
> block_device_mapping = []

Okay, fixed.
During the fixes, I found other suspicious code.
Since I'm not sure they are intentional or not at a glance,
so please review the attached patch.

> > 393 + if not _is_able_to_shutdown(instance, instance_id):
> > 394 + return
>
> Should we log here that we weren't able to shutdown, something like:
>
> LOG.warn(_("Unable to shutdown server...."))

Yes, _is_able_to_shutdown() itself does.

=== modified file 'nova/objectstore/s3server.py'
--- nova/objectstore/s3server.py 2011-03-24 23:38:31 +0000
+++ nova/objectstore/s3server.py 2011-06-15 05:54:21 +0000
@@ -155,7 +155,8 @@ class BaseRequestHandler(wsgi.Controller
         self.finish('<?xml version="1.0" encoding="UTF-8"?>\n' +
                     ''.join(parts))

- def _render_parts(self, value, parts=[]):
+ def _render_parts(self, value, parts=None):
+ parts = parts or []
         if isinstance(value, basestring):
             parts.append(utils.xhtml_escape(value))
         elif isinstance(value, int) or isinstance(value, long):

=== modified file 'tools/ajaxterm/qweb.py'
--- tools/ajaxterm/qweb.py 2010-09-18 02:08:22 +0000
+++ tools/ajaxterm/qweb.py 2011-06-15 05:57:36 +0000
@@ -726,7 +726,7 @@ class QWebHtml(QWebXml):
 #----------------------------------------------------------
 # QWeb Simple Controller
 #----------------------------------------------------------
-def qweb_control(self,jump='main',p=[]):
+def qweb_control(self,jump='main',p=None):
     """ qweb_control(self,jump='main',p=[]):
     A simple function to handle the controler part of your application. It
     dispatch the control to the jump argument, while ensuring th...

[Isaku Yamahata (yamahata)]

On Wed, Jun 08, 2011 at 09:07:25PM -0000, Matt Dietz wrote:
> I see a few failing tests:

I think you're using old mox version.
With revno 1117, mox 0.5.3 is required instead of 0.5.0
Updating your repo and reinstalling evenv will fix it.

> 455 + try:
> 456 + bdms = self.db.block_device_mapping_get_all_by_instance(
> 457 + context, instance_id)
> 458 + except exception.NotFound:
> 459 + pass
>
> I don't really like throwing away exceptions. It's an explicit exception, sure, but it could mask something important. Seems like this should at least log the error.

I see. Given all the caller catch and ignores it, I changed it returns
empty list instead of raising NotFound. Thus I eliminated except ...: pass.

> 476 + assert ((bdm['snapshot_id'] is None) or
> 477 + (bdm['volume_id'] is not None))
>
> It seems like it would be better to raise an explicit exception here, with a message describing exactly why this is a bad state.

Okay.

> 239 + """Stop each instance in instace_id"""
> 245 + """Start each instance in instace_id"""
>
> instace_id should be instance_id
>
> Also, given that instance_id is singular, it should probably say something like:
>
> "Start the instance denoted by instance_id"

Unfortunately instance_id is a list of ec2 instance id like
terminate_instances. Anyway I fixed typo.
--
yamahata

[Matt Dietz (cerberus)]

Thanks for the changes Yamahata!

review: Approve

[Rick Harris (rconradharris)]

> ======================================================================
> FAIL: test_stop_with_attached_volume (nova.tests.test_cloud.CloudTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/home/rick/openstack/nova/boot-from-volume-0/nova/tests/test_cloud.py", line 691, in
> test_stop_with_attached_volume
> self._assert_volume_attached(vol, instance_id, '/dev/vdc')
> File "/home/rick/openstack/nova/boot-from-volume-0/nova/tests/test_cloud.py", line 582, in >_assert_volume_attached
> self.assertEqual(vol['mountpoint'], mountpoint)
> AssertionError: u'\\/dev\\/vdc' != '/dev/vdc'

Turns out this was caused by running an older version of carrot (0.10.3) which didn't handle escaping properly.

Upgrading to 0.10.5 fixed it.

Patch looks great, nice job.

review: Approve

[Rick Harris (rconradharris)]

Looks like Devin's concerns were addressed, setting to Approved.

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~yamahata/nova/boot-from-volume-0 into lp:nova failed. Below is the output from the failed tests.

AccountsTest
    test_account_create OK 0.16
    test_account_delete OK 0.27
    test_account_update OK 0.16
    test_get_account OK 0.16
AdminAPITest
    test_admin_disabled OK 0.11
    test_admin_enabled OK 0.16
APITest
    test_exceptions_are_converted_to_faults OK 0.02
    test_malformed_json OK 0.04
    test_malformed_xml OK 0.04
Test
    test_authorize_project OK 0.21
    test_authorize_token OK 0.05
    test_authorize_user OK 0.03
    test_bad_project OK 0.05
    test_bad_token OK 0.03
    test_bad_user_bad_key OK 0.03
    test_bad_user_good_key OK 0.03
    test_no_user OK 0.03
    test_not_existing_project OK 0.05
    test_token_expiry OK 0.03
TestFunctional
    test_token_doesnotexist OK 0.04
    test_token_expiry OK 0.06
TestLimiter
    test_authorize_token OK 0.05
LimiterTest
    test_limiter_custom_max_limit OK 0.00
    test_limiter_limit_and_offset OK 0.00
    test_limiter_limit_medium OK 0.00
    test_limiter_limit_over_max OK 0.00
    test_limiter_limit_zero OK 0.00
    test_limiter_negative_limit OK 0.00
    test_limiter_negative_offset OK 0.00
    test_limiter_nothing OK 0.00
    test_limiter_offset_bad OK 0.00
    test_limiter_offset_blank OK 0.00
    test_limiter_offset_medium OK 0.00
    test_limiter_offset_over_max OK 0.00
    test_limiter_offset_zero OK 0.00
PaginationParamsTest
    test_invalid_limit OK 0.00
    test_invalid_marker OK 0.00
    test_no_params OK 0.00
    test_valid_limit OK 0.00
    test_valid_marker OK 0.00
ActionExtensionTest
    test_extended_action ...

[Isaku Yamahata (yamahata)]

fixed two errors of pep8.

--
yamahata

[OpenStack Infra (hudson-openstack)]

No proposals found for merge of lp:~morita-kazutaka/nova/clone-volume into lp:nova.

[Vish Ishaya (vishvananda)]

marking as merged. Our dependency checks set it back to needs review if the prereq branch has already merged, even though the merge was successful.

=== modified file 'nova/api/ec2/apirequest.py'
--- nova/api/ec2/apirequest.py	2011-04-18 20:53:09 +0000
+++ nova/api/ec2/apirequest.py	2011-06-17 23:35:54 +0000
@@ -21,22 +21,15 @@
 """
 
 import datetime
-import re
 # TODO(termie): replace minidom with etree
 from xml.dom import minidom
 
 from nova import log as logging
+from nova.api.ec2 import ec2utils
 
 LOG = logging.getLogger("nova.api.request")
 
 
-_c2u = re.compile('(((?<=[a-z])[A-Z])|([A-Z](?![A-Z]|$)))')
-
-
-def _camelcase_to_underscore(str):
-    return _c2u.sub(r'_\1', str).lower().strip('_')
-
-
 def _underscore_to_camelcase(str):
     return ''.join([x[:1].upper() + x[1:] for x in str.split('_')])
 
@@ -51,59 +44,6 @@
     return datetimeobj.strftime("%Y-%m-%dT%H:%M:%SZ")
 
 
-def _try_convert(value):
-    """Return a non-string from a string or unicode, if possible.
-
-    ============= =====================================================
-    When value is returns
-    ============= =====================================================
-    zero-length   ''
-    'None'        None
-    'True'        True
-    'False'       False
-    '0', '-0'     0
-    0xN, -0xN     int from hex (postitive) (N is any number)
-    0bN, -0bN     int from binary (positive) (N is any number)
-    *             try conversion to int, float, complex, fallback value
-
-    """
-    if len(value) == 0:
-        return ''
-    if value == 'None':
-        return None
-    if value == 'True':
-        return True
-    if value == 'False':
-        return False
-    valueneg = value[1:] if value[0] == '-' else value
-    if valueneg == '0':
-        return 0
-    if valueneg == '':
-        return value
-    if valueneg[0] == '0':
-        if valueneg[1] in 'xX':
-            return int(value, 16)
-        elif valueneg[1] in 'bB':
-            return int(value, 2)
-        else:
-            try:
-                return int(value, 8)
-            except ValueError:
-                pass
-    try:
-        return int(value)
-    except ValueError:
-        pass
-    try:
-        return float(value)
-    except ValueError:
-        pass
-    try:
-        return complex(value)
-    except ValueError:
-        return value
-
-
 class APIRequest(object):
     def __init__(self, controller, action, version, args):
         self.controller = controller
@@ -114,7 +54,7 @@
     def invoke(self, context):
         try:
             method = getattr(self.controller,
-                             _camelcase_to_underscore(self.action))
+                             ec2utils.camelcase_to_underscore(self.action))
         except AttributeError:
             controller = self.controller
             action = self.action
@@ -125,19 +65,7 @@
             #       and reraise as 400 error.
             raise Exception(_error)
 
-        args = {}
-        for key, value in self.args.items():
-            parts = key.split(".")
-            key = _camelcase_to_underscore(parts[0])
-            if isinstance(value, str) or isinstance(value, unicode):
-                # NOTE(vish): Automatically convert strings back
-                #             into their respective values
-                value = _try_convert(value)
-            if len(parts) > 1:
-                d = args.get(key, {})
-                d[parts[1]] = value
-                value = d
-            args[key] = value
+        args = ec2utils.dict_from_dotted_str(self.args.items())
 
         for key in args.keys():
             # NOTE(vish): Turn numeric dict keys into lists

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2011-06-17 20:47:23 +0000
+++ nova/api/ec2/cloud.py	2011-06-17 23:35:54 +0000
@@ -909,6 +909,25 @@
         if kwargs.get('ramdisk_id'):
             ramdisk = self._get_image(context, kwargs['ramdisk_id'])
             kwargs['ramdisk_id'] = ramdisk['id']
+        for bdm in kwargs.get('block_device_mapping', []):
+            # NOTE(yamahata)
+            # BlockDevicedMapping.<N>.DeviceName
+            # BlockDevicedMapping.<N>.Ebs.SnapshotId
+            # BlockDevicedMapping.<N>.Ebs.VolumeSize
+            # BlockDevicedMapping.<N>.Ebs.DeleteOnTermination
+            # BlockDevicedMapping.<N>.VirtualName
+            # => remove .Ebs and allow volume id in SnapshotId
+            ebs = bdm.pop('ebs', None)
+            if ebs:
+                ec2_id = ebs.pop('snapshot_id')
+                id = ec2utils.ec2_id_to_id(ec2_id)
+                if ec2_id.startswith('snap-'):
+                    bdm['snapshot_id'] = id
+                elif ec2_id.startswith('vol-'):
+                    bdm['volume_id'] = id
+                ebs.setdefault('delete_on_termination', True)
+                bdm.update(ebs)
+
         image = self._get_image(context, kwargs['image_id'])
 
         if image:
@@ -933,37 +952,54 @@
             user_data=kwargs.get('user_data'),
             security_group=kwargs.get('security_group'),
             availability_zone=kwargs.get('placement', {}).get(
-                                  'AvailabilityZone'))
+                                  'AvailabilityZone'),
+            block_device_mapping=kwargs.get('block_device_mapping', {}))
         return self._format_run_instances(context,
                                           instances[0]['reservation_id'])
 
+    def _do_instance(self, action, context, ec2_id):
+        instance_id = ec2utils.ec2_id_to_id(ec2_id)
+        action(context, instance_id=instance_id)
+
+    def _do_instances(self, action, context, instance_id):
+        for ec2_id in instance_id:
+            self._do_instance(action, context, ec2_id)
+
     def terminate_instances(self, context, instance_id, **kwargs):
         """Terminate each instance in instance_id, which is a list of ec2 ids.
         instance_id is a kwarg so its name cannot be modified."""
         LOG.debug(_("Going to start terminating instances"))
-        for ec2_id in instance_id:
-            instance_id = ec2utils.ec2_id_to_id(ec2_id)
-            self.compute_api.delete(context, instance_id=instance_id)
+        self._do_instances(self.compute_api.delete, context, instance_id)
         return True
 
     def reboot_instances(self, context, instance_id, **kwargs):
         """instance_id is a list of instance ids"""
         LOG.audit(_("Reboot instance %r"), instance_id, context=context)
-        for ec2_id in instance_id:
-            instance_id = ec2utils.ec2_id_to_id(ec2_id)
-            self.compute_api.reboot(context, instance_id=instance_id)
+        self._do_instances(self.compute_api.reboot, context, instance_id)
+        return True
+
+    def stop_instances(self, context, instance_id, **kwargs):
+        """Stop each instances in instance_id.
+        Here instance_id is a list of instance ids"""
+        LOG.debug(_("Going to stop instances"))
+        self._do_instances(self.compute_api.stop, context, instance_id)
+        return True
+
+    def start_instances(self, context, instance_id, **kwargs):
+        """Start each instances in instance_id.
+        Here instance_id is a list of instance ids"""
+        LOG.debug(_("Going to start instances"))
+        self._do_instances(self.compute_api.start, context, instance_id)
         return True
 
     def rescue_instance(self, context, instance_id, **kwargs):
         """This is an extension to the normal ec2_api"""
-        instance_id = ec2utils.ec2_id_to_id(instance_id)
-        self.compute_api.rescue(context, instance_id=instance_id)
+        self._do_instance(self.compute_api.rescue, contect, instnace_id)
         return True
 
     def unrescue_instance(self, context, instance_id, **kwargs):
         """This is an extension to the normal ec2_api"""
-        instance_id = ec2utils.ec2_id_to_id(instance_id)
-        self.compute_api.unrescue(context, instance_id=instance_id)
+        self._do_instance(self.compute_api.unrescue, context, instance_id)
         return True
 
     def update_instance(self, context, instance_id, **kwargs):

=== modified file 'nova/api/ec2/ec2utils.py'
--- nova/api/ec2/ec2utils.py	2011-05-11 18:02:01 +0000
+++ nova/api/ec2/ec2utils.py	2011-06-17 23:35:54 +0000
@@ -16,6 +16,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import re
+
 from nova import exception
 
 
@@ -30,3 +32,95 @@
 def id_to_ec2_id(instance_id, template='i-%08x'):
     """Convert an instance ID (int) to an ec2 ID (i-[base 16 number])"""
     return template % instance_id
+
+
+_c2u = re.compile('(((?<=[a-z])[A-Z])|([A-Z](?![A-Z]|$)))')
+
+
+def camelcase_to_underscore(str):
+    return _c2u.sub(r'_\1', str).lower().strip('_')
+
+
+def _try_convert(value):
+    """Return a non-string from a string or unicode, if possible.
+
+    ============= =====================================================
+    When value is returns
+    ============= =====================================================
+    zero-length   ''
+    'None'        None
+    'True'        True case insensitive
+    'False'       False case insensitive
+    '0', '-0'     0
+    0xN, -0xN     int from hex (postitive) (N is any number)
+    0bN, -0bN     int from binary (positive) (N is any number)
+    *             try conversion to int, float, complex, fallback value
+
+    """
+    if len(value) == 0:
+        return ''
+    if value == 'None':
+        return None
+    lowered_value = value.lower()
+    if lowered_value == 'true':
+        return True
+    if lowered_value == 'false':
+        return False
+    valueneg = value[1:] if value[0] == '-' else value
+    if valueneg == '0':
+        return 0
+    if valueneg == '':
+        return value
+    if valueneg[0] == '0':
+        if valueneg[1] in 'xX':
+            return int(value, 16)
+        elif valueneg[1] in 'bB':
+            return int(value, 2)
+        else:
+            try:
+                return int(value, 8)
+            except ValueError:
+                pass
+    try:
+        return int(value)
+    except ValueError:
+        pass
+    try:
+        return float(value)
+    except ValueError:
+        pass
+    try:
+        return complex(value)
+    except ValueError:
+        return value
+
+
+def dict_from_dotted_str(items):
+    """parse multi dot-separated argument into dict.
+    EBS boot uses multi dot-separeted arguments like
+    BlockDeviceMapping.1.DeviceName=snap-id
+    Convert the above into
+    {'block_device_mapping': {'1': {'device_name': snap-id}}}
+    """
+    args = {}
+    for key, value in items:
+        parts = key.split(".")
+        key = camelcase_to_underscore(parts[0])
+        if isinstance(value, str) or isinstance(value, unicode):
+            # NOTE(vish): Automatically convert strings back
+            #             into their respective values
+            value = _try_convert(value)
+
+            if len(parts) > 1:
+                d = args.get(key, {})
+                args[key] = d
+                for k in parts[1:-1]:
+                    k = camelcase_to_underscore(k)
+                    v = d.get(k, {})
+                    d[k] = v
+                    d = v
+                d[camelcase_to_underscore(parts[-1])] = value
+            else:
+                args[key] = value
+
+    return args

=== modified file 'nova/compute/api.py'
--- nova/compute/api.py	2011-06-17 15:25:23 +0000
+++ nova/compute/api.py	2011-06-17 23:35:54 +0000
@@ -34,6 +34,7 @@
 from nova import volume
 from nova.compute import instance_types
 from nova.compute import power_state
+from nova.compute.utils import terminate_volumes
 from nova.scheduler import api as scheduler_api
 from nova.db import base
 
@@ -52,6 +53,18 @@
     return str(instance_id)
 
 
+def _is_able_to_shutdown(instance, instance_id):
+    states = {'terminating': "Instance %s is already being terminated",
+              'migrating': "Instance %s is being migrated",
+              'stopping': "Instance %s is being stopped"}
+    msg = states.get(instance['state_description'])
+    if msg:
+        LOG.warning(_(msg), instance_id)
+        return False
+
+    return True
+
+
 class API(base.Base):
     """API for interacting with the compute manager."""
 
@@ -235,7 +248,7 @@
         return (num_instances, base_options, security_groups)
 
     def create_db_entry_for_new_instance(self, context, base_options,
-             security_groups, num=1):
+             security_groups, block_device_mapping, num=1):
         """Create an entry in the DB for this new instance,
         including any related table updates (such as security
         groups, MAC address, etc). This will called by create()
@@ -255,6 +268,23 @@
                                                 instance_id,
                                                 security_group_id)
 
+        # NOTE(yamahata)
+        # tell vm driver to attach volume at boot time by updating
+        # BlockDeviceMapping
+        for bdm in block_device_mapping:
+            LOG.debug(_('bdm %s'), bdm)
+            assert 'device_name' in bdm
+            values = {
+                'instance_id': instance_id,
+                'device_name': bdm['device_name'],
+                'delete_on_termination': bdm.get('delete_on_termination'),
+                'virtual_name': bdm.get('virtual_name'),
+                'snapshot_id': bdm.get('snapshot_id'),
+                'volume_id': bdm.get('volume_id'),
+                'volume_size': bdm.get('volume_size'),
+                'no_device': bdm.get('no_device')}
+            self.db.block_device_mapping_create(elevated, values)
+
         # Set sane defaults if not specified
         updates = dict(hostname=self.hostname_factory(instance_id))
         if (not hasattr(instance, 'display_name') or
@@ -339,7 +369,7 @@
                key_name=None, key_data=None, security_group='default',
                availability_zone=None, user_data=None, metadata={},
                injected_files=None, admin_password=None, zone_blob=None,
-               reservation_id=None):
+               reservation_id=None, block_device_mapping=None):
         """
         Provision the instances by sending off a series of single
         instance requests to the Schedulers. This is fine for trival
@@ -360,11 +390,13 @@
                                injected_files, admin_password, zone_blob,
                                reservation_id)
 
+        block_device_mapping = block_device_mapping or []
         instances = []
         LOG.debug(_("Going to run %s instances..."), num_instances)
         for num in range(num_instances):
             instance = self.create_db_entry_for_new_instance(context,
-                                    base_options, security_groups, num=num)
+                                    base_options, security_groups,
+                                    block_device_mapping, num=num)
             instances.append(instance)
             instance_id = instance['id']
 
@@ -474,24 +506,22 @@
         rv = self.db.instance_update(context, instance_id, kwargs)
         return dict(rv.iteritems())
 
+    def _get_instance(self, context, instance_id, action_str):
+        try:
+            return self.get(context, instance_id)
+        except exception.NotFound:
+            LOG.warning(_("Instance %(instance_id)s was not found during "
+                          "%(action_str)s") %
+                        {'instance_id': instance_id, 'action_str': action_str})
+            raise
+
     @scheduler_api.reroute_compute("delete")
     def delete(self, context, instance_id):
         """Terminate an instance."""
         LOG.debug(_("Going to try to terminate %s"), instance_id)
-        try:
-            instance = self.get(context, instance_id)
-        except exception.NotFound:
-            LOG.warning(_("Instance %s was not found during terminate"),
-                        instance_id)
-            raise
-
-        if instance['state_description'] == 'terminating':
-            LOG.warning(_("Instance %s is already being terminated"),
-                        instance_id)
-            return
-
-        if instance['state_description'] == 'migrating':
-            LOG.warning(_("Instance %s is being migrated"), instance_id)
+        instance = self._get_instance(context, instance_id, 'terminating')
+
+        if not _is_able_to_shutdown(instance, instance_id):
             return
 
         self.update(context,
@@ -505,8 +535,48 @@
             self._cast_compute_message('terminate_instance', context,
                     instance_id, host)
         else:
+            terminate_volumes(self.db, context, instance_id)
             self.db.instance_destroy(context, instance_id)
 
+    @scheduler_api.reroute_compute("stop")
+    def stop(self, context, instance_id):
+        """Stop an instance."""
+        LOG.debug(_("Going to try to stop %s"), instance_id)
+
+        instance = self._get_instance(context, instance_id, 'stopping')
+        if not _is_able_to_shutdown(instance, instance_id):
+            return
+
+        self.update(context,
+                    instance['id'],
+                    state_description='stopping',
+                    state=power_state.NOSTATE,
+                    terminated_at=utils.utcnow())
+
+        host = instance['host']
+        if host:
+            self._cast_compute_message('stop_instance', context,
+                    instance_id, host)
+
+    def start(self, context, instance_id):
+        """Start an instance."""
+        LOG.debug(_("Going to try to start %s"), instance_id)
+        instance = self._get_instance(context, instance_id, 'starting')
+        if instance['state_description'] != 'stopped':
+            _state_description = instance['state_description']
+            LOG.warning(_("Instance %(instance_id)s is not "
+                          "stopped(%(_state_description)s)") % locals())
+            return
+
+        # TODO(yamahata): injected_files isn't supported right now.
+        #                 It is used only for osapi. not for ec2 api.
+        #                 availability_zone isn't used by run_instance.
+        rpc.cast(context,
+                 FLAGS.scheduler_topic,
+                 {"method": "start_instance",
+                  "args": {"topic": FLAGS.compute_topic,
+                           "instance_id": instance_id}})
+
     def get(self, context, instance_id):
         """Get a single instance with the given instance_id."""
         rv = self.db.instance_get(context, instance_id)

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2011-06-03 15:11:01 +0000
+++ nova/compute/manager.py	2011-06-17 23:35:54 +0000
@@ -53,6 +53,7 @@
 from nova import utils
 from nova import volume
 from nova.compute import power_state
+from nova.compute.utils import terminate_volumes
 from nova.virt import driver
 
 
@@ -214,8 +215,63 @@
         """
         return self.driver.refresh_security_group_members(security_group_id)
 
-    @exception.wrap_exception
-    def run_instance(self, context, instance_id, **kwargs):
+    def _setup_block_device_mapping(self, context, instance_id):
+        """setup volumes for block device mapping"""
+        self.db.instance_set_state(context,
+                                   instance_id,
+                                   power_state.NOSTATE,
+                                   'block_device_mapping')
+
+        volume_api = volume.API()
+        block_device_mapping = []
+        for bdm in self.db.block_device_mapping_get_all_by_instance(
+            context, instance_id):
+            LOG.debug(_("setting up bdm %s"), bdm)
+            if ((bdm['snapshot_id'] is not None) and
+                (bdm['volume_id'] is None)):
+                # TODO(yamahata): default name and description
+                vol = volume_api.create(context, bdm['volume_size'],
+                                        bdm['snapshot_id'], '', '')
+                # TODO(yamahata): creating volume simultaneously
+                #                 reduces creation time?
+                volume_api.wait_creation(context, vol['id'])
+                self.db.block_device_mapping_update(
+                    context, bdm['id'], {'volume_id': vol['id']})
+                bdm['volume_id'] = vol['id']
+
+            if not ((bdm['snapshot_id'] is None) or
+                    (bdm['volume_id'] is not None)):
+                LOG.error(_('corrupted state of block device mapping '
+                            'id: %(id)s '
+                            'snapshot: %(snapshot_id) volume: %(vollume_id)') %
+                          {'id': bdm['id'],
+                           'snapshot_id': bdm['snapshot'],
+                           'volume_id': bdm['volume_id']})
+                raise exception.ApiError(_('broken block device mapping %d') %
+                                         bdm['id'])
+
+            if bdm['volume_id'] is not None:
+                volume_api.check_attach(context,
+                                        volume_id=bdm['volume_id'])
+                dev_path = self._attach_volume_boot(context, instance_id,
+                                                    bdm['volume_id'],
+                                                    bdm['device_name'])
+                block_device_mapping.append({'device_path': dev_path,
+                                             'mount_device':
+                                             bdm['device_name']})
+            elif bdm['virtual_name'] is not None:
+                # TODO(yamahata): ephemeral/swap device support
+                LOG.debug(_('block_device_mapping: '
+                            'ephemeral device is not supported yet'))
+            else:
+                # TODO(yamahata): NoDevice support
+                assert bdm['no_device']
+                LOG.debug(_('block_device_mapping: '
+                            'no device is not supported yet'))
+
+        return block_device_mapping
+
+    def _run_instance(self, context, instance_id, **kwargs):
         """Launch a new instance with specified options."""
         context = context.elevated()
         instance_ref = self.db.instance_get(context, instance_id)
@@ -249,11 +305,15 @@
             self.network_manager.setup_compute_network(context,
                                                        instance_id)
 
+        block_device_mapping = self._setup_block_device_mapping(context,
+                                                                instance_id)
+
         # TODO(vish) check to make sure the availability zone matches
         self._update_state(context, instance_id, power_state.BUILDING)
 
         try:
-            self.driver.spawn(instance_ref)
+            self.driver.spawn(instance_ref,
+                              block_device_mapping=block_device_mapping)
         except Exception as ex:  # pylint: disable=W0702
             msg = _("Instance '%(instance_id)s' failed to spawn. Is "
                     "virtualization enabled in the BIOS? Details: "
@@ -277,12 +337,24 @@
         self._update_state(context, instance_id)
 
     @exception.wrap_exception
+    def run_instance(self, context, instance_id, **kwargs):
+        self._run_instance(context, instance_id, **kwargs)
+
+    @exception.wrap_exception
     @checks_instance_lock
-    def terminate_instance(self, context, instance_id):
-        """Terminate an instance on this host."""
+    def start_instance(self, context, instance_id):
+        """Starting an instance on this host."""
+        # TODO(yamahata): injected_files isn't supported.
+        #                 Anyway OSAPI doesn't support stop/start yet
+        self._run_instance(context, instance_id)
+
+    def _shutdown_instance(self, context, instance_id, action_str):
+        """Shutdown an instance on this host."""
         context = context.elevated()
         instance_ref = self.db.instance_get(context, instance_id)
-        LOG.audit(_("Terminating instance %s"), instance_id, context=context)
+        LOG.audit(_("%(action_str)s instance %(instance_id)s") %
+                  {'action_str': action_str, 'instance_id': instance_id},
+                  context=context)
 
         fixed_ip = instance_ref.get('fixed_ip')
         if not FLAGS.stub_network and fixed_ip:
@@ -318,18 +390,36 @@
 
         volumes = instance_ref.get('volumes') or []
         for volume in volumes:
-            self.detach_volume(context, instance_id, volume['id'])
-        if instance_ref['state'] == power_state.SHUTOFF:
+            self._detach_volume(context, instance_id, volume['id'], False)
+
+        if (instance_ref['state'] == power_state.SHUTOFF and
+            instance_ref['state_description'] != 'stopped'):
             self.db.instance_destroy(context, instance_id)
             raise exception.Error(_('trying to destroy already destroyed'
                                     ' instance: %s') % instance_id)
         self.driver.destroy(instance_ref)
 
+        if action_str == 'Terminating':
+            terminate_volumes(self.db, context, instance_id)
+
+    @exception.wrap_exception
+    @checks_instance_lock
+    def terminate_instance(self, context, instance_id):
+        """Terminate an instance on this host."""
+        self._shutdown_instance(context, instance_id, 'Terminating')
+
         # TODO(ja): should we keep it in a terminated state for a bit?
         self.db.instance_destroy(context, instance_id)
 
     @exception.wrap_exception
     @checks_instance_lock
+    def stop_instance(self, context, instance_id):
+        """Stopping an instance on this host."""
+        self._shutdown_instance(context, instance_id, 'Stopping')
+        # instance state will be updated to stopped by _poll_instance_states()
+
+    @exception.wrap_exception
+    @checks_instance_lock
     def rebuild_instance(self, context, instance_id, **kwargs):
         """Destroy and re-make this instance.
 
@@ -800,6 +890,22 @@
         instance_ref = self.db.instance_get(context, instance_id)
         return self.driver.get_vnc_console(instance_ref)
 
+    def _attach_volume_boot(self, context, instance_id, volume_id, mountpoint):
+        """Attach a volume to an instance at boot time. So actual attach
+        is done by instance creation"""
+
+        # TODO(yamahata):
+        # should move check_attach to volume manager?
+        volume.API().check_attach(context, volume_id)
+
+        context = context.elevated()
+        LOG.audit(_("instance %(instance_id)s: booting with "
+                    "volume %(volume_id)s at %(mountpoint)s") %
+                  locals(), context=context)
+        dev_path = self.volume_manager.setup_compute_volume(context, volume_id)
+        self.db.volume_attached(context, volume_id, instance_id, mountpoint)
+        return dev_path
+
     @checks_instance_lock
     def attach_volume(self, context, instance_id, volume_id, mountpoint):
         """Attach a volume to an instance."""
@@ -817,6 +923,16 @@
                                     volume_id,
                                     instance_id,
                                     mountpoint)
+            values = {
+                'instance_id': instance_id,
+                'device_name': mountpoint,
+                'delete_on_termination': False,
+                'virtual_name': None,
+                'snapshot_id': None,
+                'volume_id': volume_id,
+                'volume_size': None,
+                'no_device': None}
+            self.db.block_device_mapping_create(context, values)
         except Exception as exc:  # pylint: disable=W0702
             # NOTE(vish): The inline callback eats the exception info so we
             #             log the traceback here and reraise the same
@@ -831,7 +947,7 @@
 
     @exception.wrap_exception
     @checks_instance_lock
-    def detach_volume(self, context, instance_id, volume_id):
+    def _detach_volume(self, context, instance_id, volume_id, destroy_bdm):
         """Detach a volume from an instance."""
         context = context.elevated()
         instance_ref = self.db.instance_get(context, instance_id)
@@ -847,8 +963,15 @@
                                       volume_ref['mountpoint'])
         self.volume_manager.remove_compute_volume(context, volume_id)
         self.db.volume_detached(context, volume_id)
+        if destroy_bdm:
+            self.db.block_device_mapping_destroy_by_instance_and_volume(
+                context, instance_id, volume_id)
         return True
 
+    def detach_volume(self, context, instance_id, volume_id):
+        """Detach a volume from an instance."""
+        return self._detach_volume(context, instance_id, volume_id, True)
+
     def remove_volume(self, context, volume_id):
         """Remove volume on compute host.
 
@@ -1174,11 +1297,14 @@
                                "State=%(db_state)s, so setting state to "
                                "shutoff.") % locals())
                     vm_state = power_state.SHUTOFF
+                    if db_instance['state_description'] == 'stopping':
+                        self.db.instance_stop(context, db_instance['id'])
+                        continue
             else:
                 vm_state = vm_instance.state
                 vms_not_found_in_db.remove(name)
 
-            if db_instance['state_description'] == 'migrating':
+            if (db_instance['state_description'] in ['migrating', 'stopping']):
                 # A situation which db record exists, but no instance"
                 # sometimes occurs while live-migration at src compute,
                 # this case should be ignored.

=== added file 'nova/compute/utils.py'
--- nova/compute/utils.py	1970-01-01 00:00:00 +0000
+++ nova/compute/utils.py	2011-06-17 23:35:54 +0000
@@ -0,0 +1,29 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright (c) 2011 VA Linux Systems Japan K.K
+# Copyright (c) 2011 Isaku Yamahata
+#
+#    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 nova import volume
+
+
+def terminate_volumes(db, context, instance_id):
+    """delete volumes of delete_on_termination=True in block device mapping"""
+    volume_api = volume.API()
+    for bdm in db.block_device_mapping_get_all_by_instance(context,
+                                                           instance_id):
+        #LOG.debug(_("terminating bdm %s") % bdm)
+        if bdm['volume_id'] and bdm['delete_on_termination']:
+            volume_api.delete(context, bdm['volume_id'])
+        db.block_device_mapping_destroy(context, bdm['id'])

=== modified file 'nova/db/api.py'
--- nova/db/api.py	2011-05-27 04:50:20 +0000
+++ nova/db/api.py	2011-06-17 23:35:54 +0000
@@ -414,6 +414,11 @@
     return IMPL.instance_destroy(context, instance_id)
 
 
+def instance_stop(context, instance_id):
+    """Stop the instance or raise if it does not exist."""
+    return IMPL.instance_stop(context, instance_id)
+
+
 def instance_get(context, instance_id):
     """Get an instance or raise if it does not exist."""
     return IMPL.instance_get(context, instance_id)
@@ -920,6 +925,36 @@
 ####################
 
 
+def block_device_mapping_create(context, values):
+    """Create an entry of block device mapping"""
+    return IMPL.block_device_mapping_create(context, values)
+
+
+def block_device_mapping_update(context, bdm_id, values):
+    """Create an entry of block device mapping"""
+    return IMPL.block_device_mapping_update(context, bdm_id, values)
+
+
+def block_device_mapping_get_all_by_instance(context, instance_id):
+    """Get all block device mapping belonging to a instance"""
+    return IMPL.block_device_mapping_get_all_by_instance(context, instance_id)
+
+
+def block_device_mapping_destroy(context, bdm_id):
+    """Destroy the block device mapping."""
+    return IMPL.block_device_mapping_destroy(context, bdm_id)
+
+
+def block_device_mapping_destroy_by_instance_and_volume(context, instance_id,
+                                                        volume_id):
+    """Destroy the block device mapping or raise if it does not exist."""
+    return IMPL.block_device_mapping_destroy_by_instance_and_volume(
+        context, instance_id, volume_id)
+
+
+####################
+
+
 def security_group_get_all(context):
     """Get all security groups."""
     return IMPL.security_group_get_all(context)

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-06-15 21:35:31 +0000
+++ nova/db/sqlalchemy/api.py	2011-06-17 23:35:54 +0000
@@ -840,6 +840,25 @@
 
 
 @require_context
+def instance_stop(context, instance_id):
+    session = get_session()
+    with session.begin():
+        from nova.compute import power_state
+        session.query(models.Instance).\
+                filter_by(id=instance_id).\
+                update({'host': None,
+                        'state': power_state.SHUTOFF,
+                        'state_description': 'stopped',
+                        'updated_at': literal_column('updated_at')})
+        session.query(models.SecurityGroupInstanceAssociation).\
+                filter_by(instance_id=instance_id).\
+                update({'updated_at': literal_column('updated_at')})
+        session.query(models.InstanceMetadata).\
+                filter_by(instance_id=instance_id).\
+                update({'updated_at': literal_column('updated_at')})
+
+
+@require_context
 def instance_get(context, instance_id, session=None):
     if not session:
         session = get_session()
@@ -1883,6 +1902,66 @@
 
 
 @require_context
+def block_device_mapping_create(context, values):
+    bdm_ref = models.BlockDeviceMapping()
+    bdm_ref.update(values)
+
+    session = get_session()
+    with session.begin():
+        bdm_ref.save(session=session)
+
+
+@require_context
+def block_device_mapping_update(context, bdm_id, values):
+    session = get_session()
+    with session.begin():
+        session.query(models.BlockDeviceMapping).\
+                filter_by(id=bdm_id).\
+                filter_by(deleted=False).\
+                update(values)
+
+
+@require_context
+def block_device_mapping_get_all_by_instance(context, instance_id):
+    session = get_session()
+    result = session.query(models.BlockDeviceMapping).\
+             filter_by(instance_id=instance_id).\
+             filter_by(deleted=False).\
+             all()
+    if not result:
+        return []
+    return result
+
+
+@require_context
+def block_device_mapping_destroy(context, bdm_id):
+    session = get_session()
+    with session.begin():
+        session.query(models.BlockDeviceMapping).\
+                filter_by(id=bdm_id).\
+                update({'deleted': True,
+                        'deleted_at': utils.utcnow(),
+                        'updated_at': literal_column('updated_at')})
+
+
+@require_context
+def block_device_mapping_destroy_by_instance_and_volume(context, instance_id,
+                                                        volume_id):
+    session = get_session()
+    with session.begin():
+        session.query(models.BlockDeviceMapping).\
+        filter_by(instance_id=instance_id).\
+        filter_by(volume_id=volume_id).\
+        filter_by(deleted=False).\
+        update({'deleted': True,
+                'deleted_at': utils.utcnow(),
+                'updated_at': literal_column('updated_at')})
+
+
+###################
+
+
+@require_context
 def security_group_get_all(context):
     session = get_session()
     return session.query(models.SecurityGroup).\

=== added file 'nova/db/sqlalchemy/migrate_repo/versions/024_add_block_device_mapping.py'
--- nova/db/sqlalchemy/migrate_repo/versions/024_add_block_device_mapping.py	1970-01-01 00:00:00 +0000
+++ nova/db/sqlalchemy/migrate_repo/versions/024_add_block_device_mapping.py	2011-06-17 23:35:54 +0000
@@ -0,0 +1,87 @@
+# Copyright 2011 OpenStack LLC.
+# Copyright 2011 Isaku Yamahata
+#
+#    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 MetaData, Table, Column
+from sqlalchemy import DateTime, Boolean, Integer, String
+from sqlalchemy import ForeignKey
+from nova import log as logging
+
+meta = MetaData()
+
+# Just for the ForeignKey and column creation to succeed, these are not the
+# actual definitions of instances or services.
+instances = Table('instances', meta,
+        Column('id', Integer(), primary_key=True, nullable=False),
+        )
+
+volumes = Table('volumes', meta,
+        Column('id', Integer(),  primary_key=True, nullable=False),
+        )
+
+snapshots = Table('snapshots', meta,
+        Column('id', Integer(),  primary_key=True, nullable=False),
+        )
+
+
+block_device_mapping = Table('block_device_mapping', meta,
+        Column('created_at', DateTime(timezone=False)),
+        Column('updated_at', DateTime(timezone=False)),
+        Column('deleted_at', DateTime(timezone=False)),
+        Column('deleted', Boolean(create_constraint=True, name=None)),
+        Column('id', Integer(), primary_key=True, autoincrement=True),
+        Column('instance_id',
+               Integer(),
+               ForeignKey('instances.id'),
+               nullable=False),
+        Column('device_name',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False),
+               nullable=False),
+        Column('delete_on_termination',
+               Boolean(create_constraint=True, name=None),
+               default=False),
+        Column('virtual_name',
+               String(length=255, convert_unicode=False, assert_unicode=None,
+                      unicode_error=None, _warn_on_bytestring=False),
+               nullable=True),
+        Column('snapshot_id',
+               Integer(),
+               ForeignKey('snapshots.id'),
+               nullable=True),
+        Column('volume_id', Integer(), ForeignKey('volumes.id'),
+               nullable=True),
+        Column('volume_size', Integer(), nullable=True),
+        Column('no_device',
+               Boolean(create_constraint=True, name=None),
+               nullable=True),
+        )
+
+
+def upgrade(migrate_engine):
+    # Upgrade operations go here. Don't create your own engine;
+    # bind migrate_engine to your metadata
+    meta.bind = migrate_engine
+    try:
+        block_device_mapping.create()
+    except Exception:
+        logging.info(repr(block_device_mapping))
+        logging.exception('Exception while creating table')
+        meta.drop_all(tables=[block_device_mapping])
+        raise
+
+
+def downgrade(migrate_engine):
+    # Operations to reverse the above upgrade go here.
+    block_device_mapping.drop()

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-06-08 15:45:23 +0000
+++ nova/db/sqlalchemy/models.py	2011-06-17 23:35:54 +0000
@@ -357,6 +357,45 @@
     display_description = Column(String(255))
 
 
+class BlockDeviceMapping(BASE, NovaBase):
+    """Represents block device mapping that is defined by EC2"""
+    __tablename__ = "block_device_mapping"
+    id = Column(Integer, primary_key=True, autoincrement=True)
+
+    instance_id = Column(Integer, ForeignKey('instances.id'), nullable=False)
+    instance = relationship(Instance,
+                            backref=backref('balock_device_mapping'),
+                            foreign_keys=instance_id,
+                            primaryjoin='and_(BlockDeviceMapping.instance_id=='
+                                              'Instance.id,'
+                                              'BlockDeviceMapping.deleted=='
+                                              'False)')
+    device_name = Column(String(255), nullable=False)
+
+    # default=False for compatibility of the existing code.
+    # With EC2 API,
+    # default True for ami specified device.
+    # default False for created with other timing.
+    delete_on_termination = Column(Boolean, default=False)
+
+    # for ephemeral device
+    virtual_name = Column(String(255), nullable=True)
+
+    # for snapshot or volume
+    snapshot_id = Column(Integer, ForeignKey('snapshots.id'), nullable=True)
+    # outer join
+    snapshot = relationship(Snapshot,
+                            foreign_keys=snapshot_id)
+
+    volume_id = Column(Integer, ForeignKey('volumes.id'), nullable=True)
+    volume = relationship(Volume,
+                          foreign_keys=volume_id)
+    volume_size = Column(Integer, nullable=True)
+
+    # for no device to suppress devices.
+    no_device = Column(Boolean, nullable=True)
+
+
 class ExportDevice(BASE, NovaBase):
     """Represates a shelf and blade that a volume can be exported on."""
     __tablename__ = 'export_devices'

=== modified file 'nova/scheduler/simple.py'
--- nova/scheduler/simple.py	2011-06-02 21:23:05 +0000
+++ nova/scheduler/simple.py	2011-06-17 23:35:54 +0000
@@ -39,7 +39,7 @@
 class SimpleScheduler(chance.ChanceScheduler):
     """Implements Naive Scheduler that tries to find least loaded host."""
 
-    def schedule_run_instance(self, context, instance_id, *_args, **_kwargs):
+    def _schedule_instance(self, context, instance_id, *_args, **_kwargs):
         """Picks a host that is up and has the fewest running instances."""
         instance_ref = db.instance_get(context, instance_id)
         if (instance_ref['availability_zone']
@@ -75,6 +75,12 @@
                                    " for this request. Is the appropriate"
                                    " service running?"))
 
+    def schedule_run_instance(self, context, instance_id, *_args, **_kwargs):
+        return self._schedule_instance(context, instance_id, *_args, **_kwargs)
+
+    def schedule_start_instance(self, context, instance_id, *_args, **_kwargs):
+        return self._schedule_instance(context, instance_id, *_args, **_kwargs)
+
     def schedule_create_volume(self, context, volume_id, *_args, **_kwargs):
         """Picks a host that is up and has the fewest volumes."""
         volume_ref = db.volume_get(context, volume_id)

=== modified file 'nova/tests/test_api.py'
--- nova/tests/test_api.py	2011-05-23 21:15:10 +0000
+++ nova/tests/test_api.py	2011-06-17 23:35:54 +0000
@@ -89,7 +89,7 @@
 class XmlConversionTestCase(test.TestCase):
     """Unit test api xml conversion"""
     def test_number_conversion(self):
-        conv = apirequest._try_convert
+        conv = ec2utils._try_convert
         self.assertEqual(conv('None'), None)
         self.assertEqual(conv('True'), True)
         self.assertEqual(conv('False'), False)

=== modified file 'nova/tests/test_cloud.py'
--- nova/tests/test_cloud.py	2011-06-17 20:47:23 +0000
+++ nova/tests/test_cloud.py	2011-06-17 23:35:54 +0000
@@ -56,6 +56,7 @@
         self.compute = self.start_service('compute')
         self.scheduter = self.start_service('scheduler')
         self.network = self.start_service('network')
+        self.volume = self.start_service('volume')
         self.image_service = utils.import_object(FLAGS.image_service)
 
         self.manager = manager.AuthManager()
@@ -373,14 +374,21 @@
         self.assertRaises(exception.ImageNotFound, deregister_image,
                           self.context, 'ami-bad001')
 
+    def _run_instance(self, **kwargs):
+        rv = self.cloud.run_instances(self.context, **kwargs)
+        instance_id = rv['instancesSet'][0]['instanceId']
+        return instance_id
+
+    def _run_instance_wait(self, **kwargs):
+        ec2_instance_id = self._run_instance(**kwargs)
+        self._wait_for_running(ec2_instance_id)
+        return ec2_instance_id
+
     def test_console_output(self):
-        instance_type = FLAGS.default_instance_type
-        max_count = 1
-        kwargs = {'image_id': 'ami-1',
-                  'instance_type': instance_type,
-                  'max_count': max_count}
-        rv = self.cloud.run_instances(self.context, **kwargs)
-        instance_id = rv['instancesSet'][0]['instanceId']
+        instance_id = self._run_instance(
+            image_id='ami-1',
+            instance_type=FLAGS.default_instance_type,
+            max_count=1)
         output = self.cloud.get_console_output(context=self.context,
                                                instance_id=[instance_id])
         self.assertEquals(b64decode(output['output']), 'FAKE CONSOLE?OUTPUT')
@@ -389,9 +397,7 @@
         rv = self.cloud.terminate_instances(self.context, [instance_id])
 
     def test_ajax_console(self):
-        kwargs = {'image_id': 'ami-1'}
-        rv = self.cloud.run_instances(self.context, **kwargs)
-        instance_id = rv['instancesSet'][0]['instanceId']
+        instance_id = self._run_instance(image_id='ami-1')
         output = self.cloud.get_ajax_console(context=self.context,
                                              instance_id=[instance_id])
         self.assertEquals(output['url'],
@@ -569,3 +575,299 @@
         vol = db.volume_get(self.context, vol['id'])
         self.assertEqual(None, vol['mountpoint'])
         db.volume_destroy(self.context, vol['id'])
+
+    def _restart_compute_service(self, periodic_interval=None):
+        """restart compute service. NOTE: fake driver forgets all instances."""
+        self.compute.kill()
+        if periodic_interval:
+            self.compute = self.start_service(
+                'compute', periodic_interval=periodic_interval)
+        else:
+            self.compute = self.start_service('compute')
+
+    def _wait_for_state(self, ctxt, instance_id, predicate):
+        """Wait for an stopping instance to be a given state"""
+        id = ec2utils.ec2_id_to_id(instance_id)
+        while True:
+            info = self.cloud.compute_api.get(context=ctxt, instance_id=id)
+            LOG.debug(info)
+            if predicate(info):
+                break
+            greenthread.sleep(1)
+
+    def _wait_for_running(self, instance_id):
+        def is_running(info):
+            return info['state_description'] == 'running'
+        self._wait_for_state(self.context, instance_id, is_running)
+
+    def _wait_for_stopped(self, instance_id):
+        def is_stopped(info):
+            return info['state_description'] == 'stopped'
+        self._wait_for_state(self.context, instance_id, is_stopped)
+
+    def _wait_for_terminate(self, instance_id):
+        def is_deleted(info):
+            return info['deleted']
+        elevated = self.context.elevated(read_deleted=True)
+        self._wait_for_state(elevated, instance_id, is_deleted)
+
+    def test_stop_start_instance(self):
+        """Makes sure stop/start instance works"""
+        # enforce periodic tasks run in short time to avoid wait for 60s.
+        self._restart_compute_service(periodic_interval=0.3)
+
+        kwargs = {'image_id': 'ami-1',
+                  'instance_type': FLAGS.default_instance_type,
+                  'max_count': 1, }
+        instance_id = self._run_instance_wait(**kwargs)
+
+        # a running instance can't be started. It is just ignored.
+        result = self.cloud.start_instances(self.context, [instance_id])
+        greenthread.sleep(0.3)
+        self.assertTrue(result)
+
+        result = self.cloud.stop_instances(self.context, [instance_id])
+        greenthread.sleep(0.3)
+        self.assertTrue(result)
+        self._wait_for_stopped(instance_id)
+
+        result = self.cloud.start_instances(self.context, [instance_id])
+        greenthread.sleep(0.3)
+        self.assertTrue(result)
+        self._wait_for_running(instance_id)
+
+        result = self.cloud.stop_instances(self.context, [instance_id])
+        greenthread.sleep(0.3)
+        self.assertTrue(result)
+        self._wait_for_stopped(instance_id)
+
+        result = self.cloud.terminate_instances(self.context, [instance_id])
+        greenthread.sleep(0.3)
+        self.assertTrue(result)
+
+        self._restart_compute_service()
+
+    def _volume_create(self):
+        kwargs = {'status': 'available',
+                  'host': self.volume.host,
+                  'size': 1,
+                  'attach_status': 'detached', }
+        return db.volume_create(self.context, kwargs)
+
+    def _assert_volume_attached(self, vol, instance_id, mountpoint):
+        self.assertEqual(vol['instance_id'], instance_id)
+        self.assertEqual(vol['mountpoint'], mountpoint)
+        self.assertEqual(vol['status'], "in-use")
+        self.assertEqual(vol['attach_status'], "attached")
+
+    def _assert_volume_detached(self, vol):
+        self.assertEqual(vol['instance_id'], None)
+        self.assertEqual(vol['mountpoint'], None)
+        self.assertEqual(vol['status'], "available")
+        self.assertEqual(vol['attach_status'], "detached")
+
+    def test_stop_start_with_volume(self):
+        """Make sure run instance with block device mapping works"""
+
+        # enforce periodic tasks run in short time to avoid wait for 60s.
+        self._restart_compute_service(periodic_interval=0.3)
+
+        vol1 = self._volume_create()
+        vol2 = self._volume_create()
+        kwargs = {'image_id': 'ami-1',
+                  'instance_type': FLAGS.default_instance_type,
+                  'max_count': 1,
+                  'block_device_mapping': [{'device_name': '/dev/vdb',
+                                            'volume_id': vol1['id'],
+                                            'delete_on_termination': False, },
+                                           {'device_name': '/dev/vdc',
+                                            'volume_id': vol2['id'],
+                                            'delete_on_termination': True, },
+                                           ]}
+        ec2_instance_id = self._run_instance_wait(**kwargs)
+        instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
+
+        vols = db.volume_get_all_by_instance(self.context, instance_id)
+        self.assertEqual(len(vols), 2)
+        for vol in vols:
+            self.assertTrue(vol['id'] == vol1['id'] or vol['id'] == vol2['id'])
+
+        vol = db.volume_get(self.context, vol1['id'])
+        self._assert_volume_attached(vol, instance_id, '/dev/vdb')
+
+        vol = db.volume_get(self.context, vol2['id'])
+        self._assert_volume_attached(vol, instance_id, '/dev/vdc')
+
+        result = self.cloud.stop_instances(self.context, [ec2_instance_id])
+        self.assertTrue(result)
+        self._wait_for_stopped(ec2_instance_id)
+
+        vol = db.volume_get(self.context, vol1['id'])
+        self._assert_volume_detached(vol)
+        vol = db.volume_get(self.context, vol2['id'])
+        self._assert_volume_detached(vol)
+
+        self.cloud.start_instances(self.context, [ec2_instance_id])
+        self._wait_for_running(ec2_instance_id)
+        vols = db.volume_get_all_by_instance(self.context, instance_id)
+        self.assertEqual(len(vols), 2)
+        for vol in vols:
+            self.assertTrue(vol['id'] == vol1['id'] or vol['id'] == vol2['id'])
+            self.assertTrue(vol['mountpoint'] == '/dev/vdb' or
+                            vol['mountpoint'] == '/dev/vdc')
+            self.assertEqual(vol['instance_id'], instance_id)
+            self.assertEqual(vol['status'], "in-use")
+            self.assertEqual(vol['attach_status'], "attached")
+
+        self.cloud.terminate_instances(self.context, [ec2_instance_id])
+        greenthread.sleep(0.3)
+
+        admin_ctxt = context.get_admin_context(read_deleted=False)
+        vol = db.volume_get(admin_ctxt, vol1['id'])
+        self.assertFalse(vol['deleted'])
+        db.volume_destroy(self.context, vol1['id'])
+
+        greenthread.sleep(0.3)
+        admin_ctxt = context.get_admin_context(read_deleted=True)
+        vol = db.volume_get(admin_ctxt, vol2['id'])
+        self.assertTrue(vol['deleted'])
+
+        self._restart_compute_service()
+
+    def test_stop_with_attached_volume(self):
+        """Make sure attach info is reflected to block device mapping"""
+        # enforce periodic tasks run in short time to avoid wait for 60s.
+        self._restart_compute_service(periodic_interval=0.3)
+
+        vol1 = self._volume_create()
+        vol2 = self._volume_create()
+        kwargs = {'image_id': 'ami-1',
+                  'instance_type': FLAGS.default_instance_type,
+                  'max_count': 1,
+                  'block_device_mapping': [{'device_name': '/dev/vdb',
+                                            'volume_id': vol1['id'],
+                                            'delete_on_termination': True}]}
+        ec2_instance_id = self._run_instance_wait(**kwargs)
+        instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
+
+        vols = db.volume_get_all_by_instance(self.context, instance_id)
+        self.assertEqual(len(vols), 1)
+        for vol in vols:
+            self.assertEqual(vol['id'], vol1['id'])
+            self._assert_volume_attached(vol, instance_id, '/dev/vdb')
+
+        vol = db.volume_get(self.context, vol2['id'])
+        self._assert_volume_detached(vol)
+
+        self.cloud.compute_api.attach_volume(self.context,
+                                             instance_id=instance_id,
+                                             volume_id=vol2['id'],
+                                             device='/dev/vdc')
+        greenthread.sleep(0.3)
+        vol = db.volume_get(self.context, vol2['id'])
+        self._assert_volume_attached(vol, instance_id, '/dev/vdc')
+
+        self.cloud.compute_api.detach_volume(self.context,
+                                             volume_id=vol1['id'])
+        greenthread.sleep(0.3)
+        vol = db.volume_get(self.context, vol1['id'])
+        self._assert_volume_detached(vol)
+
+        result = self.cloud.stop_instances(self.context, [ec2_instance_id])
+        self.assertTrue(result)
+        self._wait_for_stopped(ec2_instance_id)
+
+        for vol_id in (vol1['id'], vol2['id']):
+            vol = db.volume_get(self.context, vol_id)
+            self._assert_volume_detached(vol)
+
+        self.cloud.start_instances(self.context, [ec2_instance_id])
+        self._wait_for_running(ec2_instance_id)
+        vols = db.volume_get_all_by_instance(self.context, instance_id)
+        self.assertEqual(len(vols), 1)
+        for vol in vols:
+            self.assertEqual(vol['id'], vol2['id'])
+            self._assert_volume_attached(vol, instance_id, '/dev/vdc')
+
+        vol = db.volume_get(self.context, vol1['id'])
+        self._assert_volume_detached(vol)
+
+        self.cloud.terminate_instances(self.context, [ec2_instance_id])
+        greenthread.sleep(0.3)
+
+        for vol_id in (vol1['id'], vol2['id']):
+            vol = db.volume_get(self.context, vol_id)
+            self.assertEqual(vol['id'], vol_id)
+            self._assert_volume_detached(vol)
+            db.volume_destroy(self.context, vol_id)
+
+        self._restart_compute_service()
+
+    def _create_snapshot(self, ec2_volume_id):
+        result = self.cloud.create_snapshot(self.context,
+                                            volume_id=ec2_volume_id)
+        greenthread.sleep(0.3)
+        return result['snapshotId']
+
+    def test_run_with_snapshot(self):
+        """Makes sure run/stop/start instance with snapshot works."""
+        vol = self._volume_create()
+        ec2_volume_id = ec2utils.id_to_ec2_id(vol['id'], 'vol-%08x')
+
+        ec2_snapshot1_id = self._create_snapshot(ec2_volume_id)
+        snapshot1_id = ec2utils.ec2_id_to_id(ec2_snapshot1_id)
+        ec2_snapshot2_id = self._create_snapshot(ec2_volume_id)
+        snapshot2_id = ec2utils.ec2_id_to_id(ec2_snapshot2_id)
+
+        kwargs = {'image_id': 'ami-1',
+                  'instance_type': FLAGS.default_instance_type,
+                  'max_count': 1,
+                  'block_device_mapping': [{'device_name': '/dev/vdb',
+                                            'snapshot_id': snapshot1_id,
+                                            'delete_on_termination': False, },
+                                           {'device_name': '/dev/vdc',
+                                            'snapshot_id': snapshot2_id,
+                                            'delete_on_termination': True}]}
+        ec2_instance_id = self._run_instance_wait(**kwargs)
+        instance_id = ec2utils.ec2_id_to_id(ec2_instance_id)
+
+        vols = db.volume_get_all_by_instance(self.context, instance_id)
+        self.assertEqual(len(vols), 2)
+        vol1_id = None
+        vol2_id = None
+        for vol in vols:
+            snapshot_id = vol['snapshot_id']
+            if snapshot_id == snapshot1_id:
+                vol1_id = vol['id']
+                mountpoint = '/dev/vdb'
+            elif snapshot_id == snapshot2_id:
+                vol2_id = vol['id']
+                mountpoint = '/dev/vdc'
+            else:
+                self.fail()
+
+            self._assert_volume_attached(vol, instance_id, mountpoint)
+
+        self.assertTrue(vol1_id)
+        self.assertTrue(vol2_id)
+
+        self.cloud.terminate_instances(self.context, [ec2_instance_id])
+        greenthread.sleep(0.3)
+        self._wait_for_terminate(ec2_instance_id)
+
+        greenthread.sleep(0.3)
+        admin_ctxt = context.get_admin_context(read_deleted=False)
+        vol = db.volume_get(admin_ctxt, vol1_id)
+        self._assert_volume_detached(vol)
+        self.assertFalse(vol['deleted'])
+        db.volume_destroy(self.context, vol1_id)
+
+        greenthread.sleep(0.3)
+        admin_ctxt = context.get_admin_context(read_deleted=True)
+        vol = db.volume_get(admin_ctxt, vol2_id)
+        self.assertTrue(vol['deleted'])
+
+        for snapshot_id in (ec2_snapshot1_id, ec2_snapshot2_id):
+            self.cloud.delete_snapshot(self.context, snapshot_id)
+            greenthread.sleep(0.3)
+        db.volume_destroy(self.context, vol['id'])

=== modified file 'nova/tests/test_compute.py'
--- nova/tests/test_compute.py	2011-06-07 17:32:06 +0000
+++ nova/tests/test_compute.py	2011-06-17 23:35:54 +0000
@@ -228,6 +228,21 @@
         self.assert_(instance_ref['launched_at'] < terminate)
         self.assert_(instance_ref['deleted_at'] > terminate)
 
+    def test_stop(self):
+        """Ensure instance can be stopped"""
+        instance_id = self._create_instance()
+        self.compute.run_instance(self.context, instance_id)
+        self.compute.stop_instance(self.context, instance_id)
+        self.compute.terminate_instance(self.context, instance_id)
+
+    def test_start(self):
+        """Ensure instance can be started"""
+        instance_id = self._create_instance()
+        self.compute.run_instance(self.context, instance_id)
+        self.compute.stop_instance(self.context, instance_id)
+        self.compute.start_instance(self.context, instance_id)
+        self.compute.terminate_instance(self.context, instance_id)
+
     def test_pause(self):
         """Ensure instance can be paused"""
         instance_id = self._create_instance()

=== modified file 'nova/virt/driver.py'
--- nova/virt/driver.py	2011-03-30 00:35:24 +0000
+++ nova/virt/driver.py	2011-06-17 23:35:54 +0000
@@ -61,7 +61,7 @@
         """Return a list of InstanceInfo for all registered VMs"""
         raise NotImplementedError()
 
-    def spawn(self, instance, network_info=None):
+    def spawn(self, instance, network_info=None, block_device_mapping=None):
         """Launch a VM for the specified instance"""
         raise NotImplementedError()
 

=== modified file 'nova/virt/fake.py'
--- nova/virt/fake.py	2011-05-17 14:49:12 +0000
+++ nova/virt/fake.py	2011-06-17 23:35:54 +0000
@@ -129,7 +129,7 @@
             info_list.append(self._map_to_instance_info(instance))
         return info_list
 
-    def spawn(self, instance):
+    def spawn(self, instance, network_info=None, block_device_mapping=None):
         """
         Create a new instance/VM/domain on the virtualization platform.
 
@@ -237,6 +237,10 @@
         """
         pass
 
+    def poll_rescued_instances(self, timeout):
+        """Poll for rescued instances"""
+        pass
+
     def migrate_disk_and_power_off(self, instance, dest):
         """
         Transfers the disk of a running instance in multiple phases, turning

=== modified file 'nova/virt/hyperv.py'
--- nova/virt/hyperv.py	2011-05-28 11:49:31 +0000
+++ nova/virt/hyperv.py	2011-06-17 23:35:54 +0000
@@ -139,7 +139,7 @@
 
         return instance_infos
 
-    def spawn(self, instance):
+    def spawn(self, instance, network_info=None, block_device_mapping=None):
         """ Create a new VM and start it."""
         vm = self._lookup(instance.name)
         if vm is not None:

=== modified file 'nova/virt/libvirt.xml.template'
--- nova/virt/libvirt.xml.template	2011-05-31 17:45:26 +0000
+++ nova/virt/libvirt.xml.template	2011-06-17 23:35:54 +0000
@@ -67,11 +67,13 @@
             <target dev='${disk_prefix}b' bus='${disk_bus}'/>
         </disk>
     #else
+        #if not ($getVar('ebs_root', False))
         <disk type='file'>
             <driver type='${driver_type}'/>
             <source file='${basepath}/disk'/>
             <target dev='${disk_prefix}a' bus='${disk_bus}'/>
         </disk>
+        #end if
         #if $getVar('local', False)
             <disk type='file'>
                 <driver type='${driver_type}'/>
@@ -79,6 +81,13 @@
                 <target dev='${disk_prefix}b' bus='${disk_bus}'/>
             </disk>
         #end if
+        #for $vol in $volumes
+            <disk type='block'>
+                <driver type='raw'/>
+                <source dev='${vol.device_path}'/>
+                <target dev='${vol.mount_device}' bus='${disk_bus}'/>
+            </disk>
+        #end for
     #end if
 #end if
 

=== modified file 'nova/virt/libvirt/connection.py'
--- nova/virt/libvirt/connection.py	2011-06-06 15:54:11 +0000
+++ nova/virt/libvirt/connection.py	2011-06-17 23:35:54 +0000
@@ -40,6 +40,7 @@
 import multiprocessing
 import os
 import random
+import re
 import shutil
 import subprocess
 import sys
@@ -148,6 +149,10 @@
         Template = t.Template
 
 
+def _strip_dev(mount_path):
+        return re.sub(r'^/dev/', '', mount_path)
+
+
 class LibvirtConnection(driver.ComputeDriver):
 
     def __init__(self, read_only):
@@ -575,11 +580,14 @@
     # NOTE(ilyaalekseyev): Implementation like in multinics
     # for xenapi(tr3buchet)
     @exception.wrap_exception
-    def spawn(self, instance, network_info=None):
-        xml = self.to_xml(instance, False, network_info)
+    def spawn(self, instance, network_info=None, block_device_mapping=None):
+        xml = self.to_xml(instance, False, network_info=network_info,
+                          block_device_mapping=block_device_mapping)
+        block_device_mapping = block_device_mapping or []
         self.firewall_driver.setup_basic_filtering(instance, network_info)
         self.firewall_driver.prepare_instance_filter(instance, network_info)
-        self._create_image(instance, xml, network_info=network_info)
+        self._create_image(instance, xml, network_info=network_info,
+                           block_device_mapping=block_device_mapping)
         domain = self._create_new_domain(xml)
         LOG.debug(_("instance %s: is running"), instance['name'])
         self.firewall_driver.apply_instance_filter(instance)
@@ -761,7 +769,8 @@
         # TODO(vish): should we format disk by default?
 
     def _create_image(self, inst, libvirt_xml, suffix='', disk_images=None,
-                        network_info=None):
+                        network_info=None, block_device_mapping=None):
+        block_device_mapping = block_device_mapping or []
         if not network_info:
             network_info = netutils.get_network_info(inst)
 
@@ -824,16 +833,19 @@
             size = None
             root_fname += "_sm"
 
-        self._cache_image(fn=self._fetch_image,
-                          target=basepath('disk'),
-                          fname=root_fname,
-                          cow=FLAGS.use_cow_images,
-                          image_id=disk_images['image_id'],
-                          user=user,
-                          project=project,
-                          size=size)
+        if not self._volume_in_mapping(self.root_mount_device,
+                                       block_device_mapping):
+            self._cache_image(fn=self._fetch_image,
+                              target=basepath('disk'),
+                              fname=root_fname,
+                              cow=FLAGS.use_cow_images,
+                              image_id=disk_images['image_id'],
+                              user=user,
+                              project=project,
+                              size=size)
 
-        if inst_type['local_gb']:
+        if inst_type['local_gb'] and not self._volume_in_mapping(
+            self.local_mount_device, block_device_mapping):
             self._cache_image(fn=self._create_local,
                               target=basepath('disk.local'),
                               fname="local_%s" % inst_type['local_gb'],
@@ -948,7 +960,20 @@
 
         return result
 
-    def _prepare_xml_info(self, instance, rescue=False, network_info=None):
+    root_mount_device = 'vda'  # FIXME for now. it's hard coded.
+    local_mount_device = 'vdb'  # FIXME for now. it's hard coded.
+
+    def _volume_in_mapping(self, mount_device, block_device_mapping):
+        mount_device_ = _strip_dev(mount_device)
+        for vol in block_device_mapping:
+            vol_mount_device = _strip_dev(vol['mount_device'])
+            if vol_mount_device == mount_device_:
+                return True
+        return False
+
+    def _prepare_xml_info(self, instance, rescue=False, network_info=None,
+                          block_device_mapping=None):
+        block_device_mapping = block_device_mapping or []
         # TODO(adiantum) remove network_info creation code
         # when multinics will be completed
         if not network_info:
@@ -966,6 +991,16 @@
         else:
             driver_type = 'raw'
 
+        for vol in block_device_mapping:
+            vol['mount_device'] = _strip_dev(vol['mount_device'])
+        ebs_root = self._volume_in_mapping(self.root_mount_device,
+                                           block_device_mapping)
+        if self._volume_in_mapping(self.local_mount_device,
+                                   block_device_mapping):
+            local_gb = False
+        else:
+            local_gb = inst_type['local_gb']
+
         xml_info = {'type': FLAGS.libvirt_type,
                     'name': instance['name'],
                     'basepath': os.path.join(FLAGS.instances_path,
@@ -973,9 +1008,11 @@
                     'memory_kb': inst_type['memory_mb'] * 1024,
                     'vcpus': inst_type['vcpus'],
                     'rescue': rescue,
-                    'local': inst_type['local_gb'],
+                    'local': local_gb,
                     'driver_type': driver_type,
-                    'nics': nics}
+                    'nics': nics,
+                    'ebs_root': ebs_root,
+                    'volumes': block_device_mapping}
 
         if FLAGS.vnc_enabled:
             if FLAGS.libvirt_type != 'lxc':
@@ -991,10 +1028,13 @@
             xml_info['disk'] = xml_info['basepath'] + "/disk"
         return xml_info
 
-    def to_xml(self, instance, rescue=False, network_info=None):
+    def to_xml(self, instance, rescue=False, network_info=None,
+               block_device_mapping=None):
+        block_device_mapping = block_device_mapping or []
         # TODO(termie): cache?
         LOG.debug(_('instance %s: starting toXML method'), instance['name'])
-        xml_info = self._prepare_xml_info(instance, rescue, network_info)
+        xml_info = self._prepare_xml_info(instance, rescue, network_info,
+                                          block_device_mapping)
         xml = str(Template(self.libvirt_xml, searchList=[xml_info]))
         LOG.debug(_('instance %s: finished toXML method'), instance['name'])
         return xml

=== modified file 'nova/virt/vmwareapi_conn.py'
--- nova/virt/vmwareapi_conn.py	2011-04-12 21:43:07 +0000
+++ nova/virt/vmwareapi_conn.py	2011-06-17 23:35:54 +0000
@@ -124,7 +124,7 @@
         """List VM instances."""
         return self._vmops.list_instances()
 
-    def spawn(self, instance):
+    def spawn(self, instance, network_info=None, block_device_mapping=None):
         """Create VM instance."""
         self._vmops.spawn(instance)
 

=== modified file 'nova/virt/xenapi_conn.py'
--- nova/virt/xenapi_conn.py	2011-05-18 16:27:39 +0000
+++ nova/virt/xenapi_conn.py	2011-06-17 23:35:54 +0000
@@ -194,7 +194,7 @@
     def list_instances_detail(self):
         return self._vmops.list_instances_detail()
 
-    def spawn(self, instance):
+    def spawn(self, instance, network_info=None, block_device_mapping=None):
         """Create VM instance"""
         self._vmops.spawn(instance)
 

=== modified file 'nova/volume/api.py'
--- nova/volume/api.py	2011-06-02 21:23:05 +0000
+++ nova/volume/api.py	2011-06-17 23:35:54 +0000
@@ -21,6 +21,9 @@
 """
 
 
+from eventlet import greenthread
+
+from nova import db
 from nova import exception
 from nova import flags
 from nova import log as logging
@@ -44,7 +47,8 @@
             if snapshot['status'] != "available":
                 raise exception.ApiError(
                     _("Snapshot status must be available"))
-            size = snapshot['volume_size']
+            if not size:
+                size = snapshot['volume_size']
 
         if quota.allowed_volumes(context, 1, size) < 1:
             pid = context.project_id
@@ -73,6 +77,14 @@
                            "snapshot_id": snapshot_id}})
         return volume
 
+    # TODO(yamahata): eliminate dumb polling
+    def wait_creation(self, context, volume_id):
+        while True:
+            volume = self.get(context, volume_id)
+            if volume['status'] != 'creating':
+                return
+            greenthread.sleep(1)
+
     def delete(self, context, volume_id):
         volume = self.get(context, volume_id)
         if volume['status'] != "available":

=== modified file 'nova/volume/driver.py'
--- nova/volume/driver.py	2011-05-27 05:13:17 +0000
+++ nova/volume/driver.py	2011-06-17 23:35:54 +0000
@@ -582,6 +582,14 @@
         """No setup necessary in fake mode."""
         pass
 
+    def discover_volume(self, context, volume):
+        """Discover volume on a remote host."""
+        return "/dev/disk/by-path/volume-id-%d" % volume['id']
+
+    def undiscover_volume(self, volume):
+        """Undiscover volume on a remote host."""
+        pass
+
     @staticmethod
     def fake_execute(cmd, *_args, **_kwargs):
         """Execute that simply logs the command."""