Skip to content

Commit

Permalink
Migrate block_device_mapping to use instance uuids.
Browse files Browse the repository at this point in the history
This started out as wanting cleanup_volumes to take a UUID instead
of an instance ID, and ended up as a wander through the joys of
schema updates. I am assuming that we actually want to transition
these tables across to using the instance UUID instead of just the
ID.

This is my first attempt at a schema update, so please review this
patch with skepticism. Resolves bug 977975. Partially resolves
blueprint finish-uuid-conversion.

Change-Id: Ib5a6f8a872ea0530e201c70e9ac01cd14f82c557
  • Loading branch information
mikalstill committed Apr 30, 2012
1 parent 62bcf37 commit 58af96d
Show file tree
Hide file tree
Showing 15 changed files with 390 additions and 102 deletions.
8 changes: 4 additions & 4 deletions nova/api/ec2/cloud.py
Expand Up @@ -998,7 +998,7 @@ def _unsupported_attribute(instance, result):
def _format_attr_block_device_mapping(instance, result):
tmp = {}
self._format_instance_root_device_name(instance, tmp)
self._format_instance_bdm(context, instance_id,
self._format_instance_bdm(context, instance['uuid'],
tmp['rootDeviceName'], result)

def _format_attr_disable_api_termination(instance, result):
Expand Down Expand Up @@ -1098,13 +1098,13 @@ def _format_terminate_instances(self, context, instance_id,
instances_set.append(i)
return {'instancesSet': instances_set}

def _format_instance_bdm(self, context, instance_id, root_device_name,
def _format_instance_bdm(self, context, instance_uuid, root_device_name,
result):
"""Format InstanceBlockDeviceMappingResponseItemType"""
root_device_type = 'instance-store'
mapping = []
for bdm in db.block_device_mapping_get_all_by_instance(context,
instance_id):
instance_uuid):
volume_id = bdm['volume_id']
if (volume_id is None or bdm['no_device']):
continue
Expand Down Expand Up @@ -1220,7 +1220,7 @@ def _format_instances(self, context, instance_id=None, use_v6=False,
i['launchTime'] = instance['created_at']
i['amiLaunchIndex'] = instance['launch_index']
self._format_instance_root_device_name(instance, i)
self._format_instance_bdm(context, instance_id,
self._format_instance_bdm(context, instance['uuid'],
i['rootDeviceName'], i)
host = instance['host']
services = db.service_get_all_by_host(context.elevated(), host)
Expand Down
2 changes: 1 addition & 1 deletion nova/api/metadata/handler.py
Expand Up @@ -99,7 +99,7 @@ def _format_instance_mapping(self, ctxt, instance_ref):

# 'ephemeralN', 'swap' and ebs
for bdm in db.block_device_mapping_get_all_by_instance(
ctxt, instance_ref['id']):
ctxt, instance_ref['uuid']):
if bdm['no_device']:
continue

Expand Down
17 changes: 9 additions & 8 deletions nova/compute/api.py
Expand Up @@ -484,7 +484,7 @@ def _volume_size(instance_type, virtual_name):
return size

def _update_image_block_device_mapping(self, elevated_context,
instance_type, instance_id,
instance_type, instance_uuid,
mappings):
"""tell vm driver to create ephemeral/swap device at boot time by
updating BlockDeviceMapping
Expand All @@ -507,15 +507,15 @@ def _update_image_block_device_mapping(self, elevated_context,
continue

values = {
'instance_id': instance_id,
'instance_uuid': instance_uuid,
'device_name': bdm['device'],
'virtual_name': virtual_name,
'volume_size': size}
self.db.block_device_mapping_update_or_create(elevated_context,
values)

def _update_block_device_mapping(self, elevated_context,
instance_type, instance_id,
instance_type, instance_uuid,
block_device_mapping):
"""tell vm driver to attach volume at boot time by updating
BlockDeviceMapping
Expand All @@ -524,7 +524,7 @@ def _update_block_device_mapping(self, elevated_context,
for bdm in block_device_mapping:
assert 'device_name' in bdm

values = {'instance_id': instance_id}
values = {'instance_uuid': instance_uuid}
for key in ('device_name', 'delete_on_termination', 'virtual_name',
'snapshot_id', 'volume_id', 'volume_size',
'no_device'):
Expand Down Expand Up @@ -587,12 +587,13 @@ def create_db_entry_for_new_instance(self, context, instance_type, image,

# BlockDeviceMapping table
self._update_image_block_device_mapping(elevated, instance_type,
instance_id, image['properties'].get('mappings', []))
self._update_block_device_mapping(elevated, instance_type, instance_id,
instance_uuid, image['properties'].get('mappings', []))
self._update_block_device_mapping(elevated, instance_type,
instance_uuid,
image['properties'].get('block_device_mapping', []))
# override via command line option
self._update_block_device_mapping(elevated, instance_type, instance_id,
block_device_mapping)
self._update_block_device_mapping(elevated, instance_type,
instance_uuid, block_device_mapping)

# Set sane defaults if not specified
updates = {}
Expand Down
50 changes: 24 additions & 26 deletions nova/compute/manager.py
Expand Up @@ -354,7 +354,7 @@ def _setup_block_device_mapping(self, context, instance):
swap = None
ephemerals = []
for bdm in self.db.block_device_mapping_get_all_by_instance(
context, instance['id']):
context, instance['uuid']):
LOG.debug(_('Setting up bdm %s'), bdm, instance=instance)

if bdm['no_device']:
Expand Down Expand Up @@ -616,22 +616,22 @@ def _deallocate_network(self, context, instance):
instance=instance)
self.network_api.deallocate_for_instance(context, instance)

def _get_instance_volume_bdms(self, context, instance_id):
def _get_instance_volume_bdms(self, context, instance_uuid):
bdms = self.db.block_device_mapping_get_all_by_instance(context,
instance_id)
instance_uuid)
return [bdm for bdm in bdms if bdm['volume_id']]

def _get_instance_volume_bdm(self, context, instance_id, volume_id):
bdms = self._get_instance_volume_bdms(context, instance_id)
def _get_instance_volume_bdm(self, context, instance_uuid, volume_id):
bdms = self._get_instance_volume_bdms(context, instance_uuid)
for bdm in bdms:
# NOTE(vish): Comparing as strings because the os_api doesn't
# convert to integer and we may wish to support uuids
# in the future.
if str(bdm['volume_id']) == str(volume_id):
return bdm

def _get_instance_volume_block_device_info(self, context, instance_id):
bdms = self._get_instance_volume_bdms(context, instance_id)
def _get_instance_volume_block_device_info(self, context, instance_uuid):
bdms = self._get_instance_volume_bdms(context, instance_uuid)
block_device_mapping = []
for bdm in bdms:
cinfo = utils.loads(bdm['connection_info'])
Expand Down Expand Up @@ -667,8 +667,6 @@ def do_start_instance():
def _shutdown_instance(self, context, instance, action_str):
"""Shutdown an instance on this host."""
context = context.elevated()
instance_id = instance['id']
instance_uuid = instance['uuid']
LOG.audit(_('%(action_str)s instance') % {'action_str': action_str},
context=context, instance=instance)

Expand All @@ -680,9 +678,9 @@ def _shutdown_instance(self, context, instance, action_str):
self._deallocate_network(context, instance)

# NOTE(vish) get bdms before destroying the instance
bdms = self._get_instance_volume_bdms(context, instance_id)
bdms = self._get_instance_volume_bdms(context, instance['uuid'])
block_device_info = self._get_instance_volume_block_device_info(
context, instance_id)
context, instance['uuid'])
self.driver.destroy(instance, self._legacy_nw_info(network_info),
block_device_info)
for bdm in bdms:
Expand All @@ -701,11 +699,12 @@ def _shutdown_instance(self, context, instance, action_str):

self._notify_about_instance_usage(context, instance, "shutdown.end")

def _cleanup_volumes(self, context, instance_id):
def _cleanup_volumes(self, context, instance_uuid):
bdms = self.db.block_device_mapping_get_all_by_instance(context,
instance_id)
instance_uuid)
for bdm in bdms:
LOG.debug(_("terminating bdm %s") % bdm)
LOG.debug(_("terminating bdm %s") % bdm,
instance_uuid=instance_uuid)
if bdm['volume_id'] and bdm['delete_on_termination']:
volume = self.volume_api.get(context, bdm['volume_id'])
self.volume_api.delete(context, volume)
Expand All @@ -716,7 +715,7 @@ def _delete_instance(self, context, instance):
instance_id = instance['id']
self._notify_about_instance_usage(context, instance, "delete.start")
self._shutdown_instance(context, instance, 'Terminating')
self._cleanup_volumes(context, instance_id)
self._cleanup_volumes(context, instance['uuid'])
instance = self._instance_update(context,
instance_id,
vm_state=vm_states.DELETED,
Expand Down Expand Up @@ -1711,7 +1710,6 @@ def attach_volume(self, context, instance_uuid, volume_id, mountpoint):
volume = self.volume_api.get(context, volume_id)
context = context.elevated()
instance_ref = self.db.instance_get_by_uuid(context, instance_uuid)
instance_id = instance_ref['id']
LOG.audit(_('Attaching volume %(volume_id)s to %(mountpoint)s'),
locals(), context=context, instance=instance_ref)
try:
Expand Down Expand Up @@ -1740,9 +1738,9 @@ def attach_volume(self, context, instance_uuid, volume_id, mountpoint):
volume,
connector)

self.volume_api.attach(context, volume, instance_id, mountpoint)
self.volume_api.attach(context, volume, instance_ref['id'], mountpoint)
values = {
'instance_id': instance_id,
'instance_uuid': instance_ref['uuid'],
'connection_info': utils.dumps(connection_info),
'device_name': mountpoint,
'delete_on_termination': False,
Expand Down Expand Up @@ -1777,15 +1775,14 @@ def _detach_volume(self, context, instance, bdm):
def detach_volume(self, context, instance_uuid, volume_id):
"""Detach a volume from an instance."""
instance_ref = self.db.instance_get_by_uuid(context, instance_uuid)
instance_id = instance_ref['id']
bdm = self._get_instance_volume_bdm(context, instance_id, volume_id)
bdm = self._get_instance_volume_bdm(context, instance_uuid, volume_id)
self._detach_volume(context, instance_ref, bdm)
volume = self.volume_api.get(context, volume_id)
connector = self.driver.get_volume_connector(instance_ref)
self.volume_api.terminate_connection(context, volume, connector)
self.volume_api.detach(context.elevated(), volume)
self.db.block_device_mapping_destroy_by_instance_and_volume(
context, instance_id, volume_id)
context, instance_uuid, volume_id)
return True

@exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
Expand Down Expand Up @@ -1888,7 +1885,7 @@ def pre_live_migration(self, context, instance_id,

# If any volume is mounted, prepare here.
block_device_info = self._get_instance_volume_block_device_info(
context, instance_id)
context, instance_ref['uuid'])
if not block_device_info['block_device_mapping']:
LOG.info(_('Instance has no volume.'), instance=instance_ref)

Expand Down Expand Up @@ -1958,7 +1955,7 @@ def live_migration(self, context, instance_id,
try:
# Checking volume node is working correctly when any volumes
# are attached to instances.
if self._get_instance_volume_bdms(context, instance_id):
if self._get_instance_volume_bdms(context, instance_ref['uuid']):
rpc.call(context,
FLAGS.volume_topic,
{'method': 'check_for_export',
Expand Down Expand Up @@ -2010,7 +2007,7 @@ def post_live_migration(self, ctxt, instance_ref,
instance=instance_ref)

# Detaching volumes.
for bdm in self._get_instance_volume_bdms(ctxt, instance_ref['id']):
for bdm in self._get_instance_volume_bdms(ctxt, instance_ref['uuid']):
# NOTE(vish): We don't want to actually mark the volume
# detached, or delete the bdm, just remove the
# connection from this host.
Expand Down Expand Up @@ -2146,7 +2143,8 @@ def rollback_live_migration(self, context, instance_ref,
self.network_api.setup_networks_on_host(context, instance_ref,
self.host)

for bdm in self._get_instance_volume_bdms(context, instance_ref['id']):
for bdm in self._get_instance_volume_bdms(context,
instance_ref['uuid']):
volume_id = bdm['volume_id']
volume = self.volume_api.get(context, volume_id)
self.volume_api.update(context, volume, {'status': 'in-use'})
Expand Down Expand Up @@ -2477,7 +2475,7 @@ def _cleanup_running_deleted_instances(self, context):
"DELETED but still present on host."),
locals(), instance=instance)
self._shutdown_instance(context, instance, 'Terminating')
self._cleanup_volumes(context, instance['id'])
self._cleanup_volumes(context, instance['uuid'])
else:
raise Exception(_("Unrecognized value '%(action)s'"
" for FLAGS.running_deleted_"
Expand Down
9 changes: 5 additions & 4 deletions nova/db/api.py
Expand Up @@ -1086,21 +1086,22 @@ def block_device_mapping_update_or_create(context, values):
return IMPL.block_device_mapping_update_or_create(context, values)


def block_device_mapping_get_all_by_instance(context, instance_id):
def block_device_mapping_get_all_by_instance(context, instance_uuid):
"""Get all block device mapping belonging to a instance"""
return IMPL.block_device_mapping_get_all_by_instance(context, instance_id)
return IMPL.block_device_mapping_get_all_by_instance(context,
instance_uuid)


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,
def block_device_mapping_destroy_by_instance_and_volume(context, instance_uuid,
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)
context, instance_uuid, volume_id)


####################
Expand Down
14 changes: 7 additions & 7 deletions nova/db/sqlalchemy/api.py
Expand Up @@ -1316,7 +1316,7 @@ def instance_destroy(context, instance_id):
'deleted_at': utils.utcnow(),
'updated_at': literal_column('updated_at')})
session.query(models.BlockDeviceMapping).\
filter_by(instance_id=instance_id).\
filter_by(instance_uuid=instance_ref['uuid']).\
update({'deleted': True,
'deleted_at': utils.utcnow(),
'updated_at': literal_column('updated_at')})
Expand Down Expand Up @@ -2726,7 +2726,7 @@ def block_device_mapping_update_or_create(context, values):
session = get_session()
with session.begin():
result = _block_device_mapping_get_query(context, session=session).\
filter_by(instance_id=values['instance_id']).\
filter_by(instance_uuid=values['instance_uuid']).\
filter_by(device_name=values['device_name']).\
first()
if not result:
Expand All @@ -2742,7 +2742,7 @@ def block_device_mapping_update_or_create(context, values):
if (virtual_name is not None and
block_device.is_swap_or_ephemeral(virtual_name)):
session.query(models.BlockDeviceMapping).\
filter_by(instance_id=values['instance_id']).\
filter_by(instance_uuid=values['instance_uuid']).\
filter_by(virtual_name=virtual_name).\
filter(models.BlockDeviceMapping.device_name !=
values['device_name']).\
Expand All @@ -2752,9 +2752,9 @@ def block_device_mapping_update_or_create(context, values):


@require_context
def block_device_mapping_get_all_by_instance(context, instance_id):
def block_device_mapping_get_all_by_instance(context, instance_uuid):
return _block_device_mapping_get_query(context).\
filter_by(instance_id=instance_id).\
filter_by(instance_uuid=instance_uuid).\
all()


Expand All @@ -2770,12 +2770,12 @@ def block_device_mapping_destroy(context, bdm_id):


@require_context
def block_device_mapping_destroy_by_instance_and_volume(context, instance_id,
def block_device_mapping_destroy_by_instance_and_volume(context, instance_uuid,
volume_id):
session = get_session()
with session.begin():
_block_device_mapping_get_query(context, session=session).\
filter_by(instance_id=instance_id).\
filter_by(instance_uuid=instance_uuid).\
filter_by(volume_id=volume_id).\
update({'deleted': True,
'deleted_at': utils.utcnow(),
Expand Down

0 comments on commit 58af96d

Please sign in to comment.