Code review comment for lp:~yamahata/nova/boot-from-volume-0

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.