Skip to content

Commit

Permalink
Refactors handling of detach volume
Browse files Browse the repository at this point in the history 
 * removes unnecessary flags in detach_volume call
 * stops double detach reported in bug 887402
 * moves volume.API() into init

Change-Id: I65332cabedf2edb88acb48b3293cba291d440238
  • Loading branch information
@vishvananda
vishvananda committed Dec 15, 2011
1 parent 369050f commit 31a7924
Showing 1 changed file with 72 additions and 70 deletions.
142 changes: 72 additions & 70 deletions nova/compute/manager.py
View file
Expand Up @@ -33,7 +33,6 @@
"""

import datetime
import functools
import os
import socket
Expand Down Expand Up @@ -146,6 +145,7 @@ def __init__(self, compute_driver=None, *args, **kwargs):
sys.exit(1)

self.network_api = network.API()
self.volume_api = volume.API()
self.network_manager = utils.import_object(FLAGS.network_manager)
self._last_host_check = 0
self._last_bw_usage_poll = 0
Expand Down Expand Up @@ -245,7 +245,6 @@ def _get_instance_nw_info(self, context, instance):

def _setup_block_device_mapping(self, context, instance):
"""setup volumes for block device mapping"""
volume_api = volume.API()
block_device_mapping = []
swap = None
ephemerals = []
Expand Down Expand Up @@ -273,18 +272,18 @@ def _setup_block_device_mapping(self, context, instance):
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'], '', '')
vol = self.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.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 bdm['volume_id'] is not None:
volume_api.check_attach(context,
volume_id=bdm['volume_id'])
self.volume_api.check_attach(context,
volume_id=bdm['volume_id'])
cinfo = self._attach_volume_boot(context, instance,
bdm['volume_id'],
bdm['device_name'])
Expand Down Expand Up @@ -507,6 +506,15 @@ def _get_instance_volume_bdms(self, context, instance_id):
instance_id)
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)
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)
block_device_mapping = []
Expand All @@ -515,8 +523,8 @@ def _get_instance_volume_block_device_info(self, context, instance_id):
block_device_mapping.append({'connection_info': cinfo,
'mount_device':
bdm['device_name']})
## NOTE(vish): The mapping is passed in so the driver can disconnect
## from remote volumes if necessary
# NOTE(vish): The mapping is passed in so the driver can disconnect
# from remote volumes if necessary
return {'block_device_mapping': block_device_mapping}

@exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
Expand Down Expand Up @@ -546,29 +554,33 @@ def _shutdown_instance(self, context, instance, action_str, cleanup):
if not FLAGS.stub_network:
self.network_api.deallocate_for_instance(context, instance)

for bdm in self._get_instance_volume_bdms(context, instance_id):
volume_id = bdm['volume_id']
try:
self._detach_volume(context, instance_uuid, volume_id)
except exception.DiskNotFound as exc:
LOG.warn(_("Ignoring DiskNotFound: %s") % exc)

if instance['power_state'] == power_state.SHUTOFF:
self.db.instance_destroy(context, instance_id)
raise exception.Error(_('trying to destroy already destroyed'
' instance: %s') % instance_uuid)
# NOTE(vish) get bdms before destroying the instance
bdms = self._get_instance_volume_bdms(context, instance_id)
block_device_info = self._get_instance_volume_block_device_info(
context, instance_id)
self.driver.destroy(instance, network_info, block_device_info, cleanup)
for bdm in bdms:
try:
# NOTE(vish): actual driver detach done in driver.destroy, so
# just tell nova-volume that we are done with it.
self.volume_api.terminate_connection(context,
bdm['volume_id'],
FLAGS.my_ip)
self.volume_api.detach(context, bdm['volume_id'])
except exception.DiskNotFound as exc:
LOG.warn(_("Ignoring DiskNotFound: %s") % exc)

def _cleanup_volumes(self, context, instance_id):
volume_api = volume.API()
bdms = self.db.block_device_mapping_get_all_by_instance(context,
instance_id)
for bdm in bdms:
LOG.debug(_("terminating bdm %s") % bdm)
if bdm['volume_id'] and bdm['delete_on_termination']:
volume_api.delete(context, bdm['volume_id'])
self.volume_api.delete(context, bdm['volume_id'])
# NOTE(vish): bdms will be deleted on instance destroy

def _delete_instance(self, context, instance):
Expand Down Expand Up @@ -1400,13 +1412,13 @@ def _attach_volume_boot(self, context, instance, volume_id, mountpoint):
"volume %(volume_id)s at %(mountpoint)s") %
locals(), context=context)
address = FLAGS.my_ip
volume_api = volume.API()
connection_info = volume_api.initialize_connection(context,
volume_id,
address)
volume_api.attach(context, volume_id, instance_id, mountpoint)
connection_info = self.volume_api.initialize_connection(context,
volume_id,
address)
self.volume_api.attach(context, volume_id, instance_id, mountpoint)
return connection_info

@exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
@checks_instance_lock
def attach_volume(self, context, instance_uuid, volume_id, mountpoint):
"""Attach a volume to an instance."""
Expand All @@ -1416,11 +1428,10 @@ def attach_volume(self, context, instance_uuid, volume_id, mountpoint):
LOG.audit(
_("instance %(instance_uuid)s: attaching volume %(volume_id)s"
" to %(mountpoint)s") % locals(), context=context)
volume_api = volume.API()
address = FLAGS.my_ip
connection_info = volume_api.initialize_connection(context,
volume_id,
address)
connection_info = self.volume_api.initialize_connection(context,
volume_id,
address)
try:
self.driver.attach_volume(connection_info,
instance_ref['name'],
Expand All @@ -1432,10 +1443,10 @@ def attach_volume(self, context, instance_uuid, volume_id, mountpoint):
# ecxception below.
LOG.exception(_("instance %(instance_uuid)s: attach failed"
" %(mountpoint)s, removing") % locals(), context=context)
volume_api.terminate_connection(context, volume_id, address)
self.volume_api.terminate_connection(context, volume_id, address)
raise exc

volume_api.attach(context, volume_id, instance_id, mountpoint)
self.volume_api.attach(context, volume_id, instance_id, mountpoint)
values = {
'instance_id': instance_id,
'connection_info': utils.dumps(connection_info),
Expand All @@ -1449,60 +1460,51 @@ def attach_volume(self, context, instance_uuid, volume_id, mountpoint):
self.db.block_device_mapping_create(context, values)
return True

@exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
@checks_instance_lock
def _detach_volume(self, context, instance_uuid, volume_id,
destroy_bdm=False, mark_detached=True,
force_detach=False):
"""Detach a volume from an instance."""
context = context.elevated()
instance_ref = self.db.instance_get_by_uuid(context, instance_uuid)
instance_id = instance_ref['id']
bdms = self.db.block_device_mapping_get_all_by_instance(
context, instance_id)
for item 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(item['volume_id']) == str(volume_id):
bdm = item
break
def _detach_volume(self, context, instance_name, bdm):
"""Do the actual driver detach using block device mapping."""
mp = bdm['device_name']
volume_id = bdm['volume_id']

LOG.audit(_("Detach volume %(volume_id)s from mountpoint %(mp)s"
" on instance %(instance_id)s") % locals(), context=context)
volume_api = volume.API()
if (instance_ref['name'] not in self.driver.list_instances() and
not force_detach):
" on instance %(instance_name)s") % locals(), context=context)

if instance_name not in self.driver.list_instances():
LOG.warn(_("Detaching volume from unknown instance %s"),
instance_id, context=context)
else:
self.driver.detach_volume(utils.loads(bdm['connection_info']),
instance_ref['name'],
bdm['device_name'])
address = FLAGS.my_ip
volume_api.terminate_connection(context, volume_id, address)
if mark_detached:
volume_api.detach(context, volume_id)
if destroy_bdm:
self.db.block_device_mapping_destroy_by_instance_and_volume(
context, instance_id, volume_id)
return True
instance_name, context=context)
self.driver.detach_volume(utils.loads(bdm['connection_info']),
instance_name,
mp)

@exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
@checks_instance_lock
def detach_volume(self, context, instance_uuid, volume_id):
"""Detach a volume from an instance."""
return self._detach_volume(context, instance_uuid, volume_id, True)
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)
self._detach_volume(context, instance_ref['name'], bdm)
self.volume_api.terminate_connection(context, volume_id, FLAGS.my_ip)
self.volume_api.detach(context, volume_id)
self.db.block_device_mapping_destroy_by_instance_and_volume(
context, instance_id, volume_id)
return True

@exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
def remove_volume_connection(self, context, instance_id, volume_id):
"""Detach a volume from an instance.,"""
"""Remove a volume connection using the volume api"""
# NOTE(vish): We don't want to actually mark the volume
# detached, or delete the bdm, just remove the
# connection from this host.
try:
instance_ref = self.db.instance_get(context, instance_id)
self._detach_volume(context, instance_ref['uuid'], volume_id,
False, False, True)
bdm = self._get_instance_volume_bdm(context,
instance_id,
volume_id)
self._detach_volume(context, instance_ref['name'],
bdm['volume_id'], bdm['device_name'])
self.volume_api.terminate_connection(context,
volume_id,
FLAGS.my_ip)
except exception.NotFound:
pass

Expand Down Expand Up @@ -1821,8 +1823,8 @@ def rollback_live_migration(self, context, instance_ref,
for bdm in self._get_instance_volume_bdms(context, instance_ref['id']):
volume_id = bdm['volume_id']
self.db.volume_update(context, volume_id, {'status': 'in-use'})
volume.API().remove_from_compute(context, instance_ref['id'],
volume_id, dest)
self.volume_api.remove_from_compute(context, instance_ref['id'],
volume_id, dest)

# Block migration needs empty image at destination host
# before migration starts, so if any failure occurs,
Expand Down

0 comments on commit 31a7924

Please sign in to comment.