From b802f28c0b24a04e7c12f44d18e90792ce9ee13b Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:07:24 +0900 Subject: [PATCH 01/22] api/ec2: make ec2 api accept true/false ec2 block device mapping api uses 'true'/'false', not 'True'/'False'. So teach ec2 api parser case insensitive true/false conversion. --- nova/api/ec2/apirequest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/api/ec2/apirequest.py b/nova/api/ec2/apirequest.py index 6672e60bbf3b..94900dfbdae1 100644 --- a/nova/api/ec2/apirequest.py +++ b/nova/api/ec2/apirequest.py @@ -59,8 +59,8 @@ def _try_convert(value): ============= ===================================================== zero-length '' 'None' None - 'True' True - 'False' False + '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) @@ -71,9 +71,9 @@ def _try_convert(value): return '' if value == 'None': return None - if value == 'True': + if value.lower() == 'true': return True - if value == 'False': + if value.lower() == 'false': return False valueneg = value[1:] if value[0] == '-' else value if valueneg == '0': From 9c3411c13936438964cc8a21b031c819edbd0ed1 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:07:45 +0900 Subject: [PATCH 02/22] teach ec2 parser multi dot-separted argument nova.api.ec2.apirequest.APIRequest knows only single dot-separated arguments. EBS boot uses multi dot-separeted arguments like BlockDeviceMapping.1.DeviceName=snap-id This patch teaches the parser those argument as the preparetion for ebs boot support. --- nova/api/ec2/apirequest.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/nova/api/ec2/apirequest.py b/nova/api/ec2/apirequest.py index 94900dfbdae1..4d6aa7f0ae79 100644 --- a/nova/api/ec2/apirequest.py +++ b/nova/api/ec2/apirequest.py @@ -133,11 +133,18 @@ class APIRequest(object): # 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 + + 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 for key in args.keys(): # NOTE(vish): Turn numeric dict keys into lists From 42272241d24e120398f741e9c8fa7d810b921209 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:08:02 +0900 Subject: [PATCH 03/22] api/ec2: parse ec2 block device mapping and pass it down to compute api teach ec2 api block device mapping. --- nova/api/ec2/cloud.py | 20 +++++++++++++++++++- nova/compute/api.py | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 524da291e8c5..56b958458e39 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -867,6 +867,23 @@ class CloudController(object): 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', []): + # BlockDevicedMapping..DeviceName + # BlockDevicedMapping..Ebs.SnapshotId + # BlockDevicedMapping..Ebs.VolumeSize + # BlockDevicedMapping..Ebs.DeleteOnTermination + # BlockDevicedMapping..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) instances = self.compute_api.create(context, instance_type=instance_types.get_instance_type_by_name( kwargs.get('instance_type', None)), @@ -881,7 +898,8 @@ class CloudController(object): 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']) diff --git a/nova/compute/api.py b/nova/compute/api.py index a12f8d51556b..b809e59e2603 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -134,7 +134,7 @@ class API(base.Base): display_name='', display_description='', key_name=None, key_data=None, security_group='default', availability_zone=None, user_data=None, metadata={}, - injected_files=None): + injected_files=None, block_device_mapping=[]): """Create the number and type of instances requested. Verifies that quota and other arguments are valid. From d9732fa7cbadd8c3d0ed8dbcf24d920f38129fbc Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:08:18 +0900 Subject: [PATCH 04/22] volume/api: allow volume clone from snapshot without size ec2 ebs boot accepts volume-clone from snapshots without size specified. In that case original snapshot size is used. So teach it to volume api create() --- nova/volume/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/volume/api.py b/nova/volume/api.py index 7fa80383bdb1..b48bcb8daab9 100644 --- a/nova/volume/api.py +++ b/nova/volume/api.py @@ -44,7 +44,8 @@ class API(base.Base): snapshot = self.get_snapshot(context, snapshot_id) 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 From 79779eae788dee00af8523a1f6a0fb9dce07a68e Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:09:20 +0900 Subject: [PATCH 05/22] db: add a table for block device mapping This patch adds a new table for block device mapping and helper APIs for it. --- nova/db/api.py | 28 ++++++ nova/db/sqlalchemy/api.py | 56 ++++++++++++ .../versions/021_add_block_device_mapping.py | 85 +++++++++++++++++++ nova/db/sqlalchemy/models.py | 40 +++++++++ 4 files changed, 209 insertions(+) create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py diff --git a/nova/db/api.py b/nova/db/api.py index 3597732b995e..858c8eb10d83 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -920,6 +920,34 @@ def snapshot_update(context, snapshot_id, values): #################### +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 blonging 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) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index cf8fc62dbff7..e0904b3c660d 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1864,6 +1864,62 @@ def snapshot_update(context, snapshot_id, values): ################### +@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: + raise exception.NotFound() + 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': 1, + 'deleted_at': datetime.datetime.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': 1, + 'deleted_at': datetime.datetime.utcnow(), + 'updated_at': literal_column('updated_at')}) + + +################### + + @require_context def security_group_get_all(context): session = get_session() diff --git a/nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py b/nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py new file mode 100644 index 000000000000..ed429633458c --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py @@ -0,0 +1,85 @@ +# 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() diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 32838d386646..a74db811f166 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -355,6 +355,46 @@ class Snapshot(BASE, NovaBase): display_name = Column(String(255)) 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 compatilibity 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 supress 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' From 945d566c10877aeaf4d75bc26e161ba3a5f9c4b9 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:10:03 +0900 Subject: [PATCH 06/22] compute, virt: support boot-from-volume without ephemeral device and no device This patch implements basic ebs boot support. ephemeral device/no device isn't supported yet. --- nova/compute/api.py | 54 ++++++++++++++----- nova/compute/manager.py | 96 ++++++++++++++++++++++++++++++++-- nova/compute/utils.py | 34 ++++++++++++ nova/virt/driver.py | 2 +- nova/virt/fake.py | 2 +- nova/virt/hyperv.py | 2 +- nova/virt/libvirt.xml.template | 9 ++++ nova/virt/libvirt_conn.py | 69 +++++++++++++++++------- nova/virt/vmwareapi_conn.py | 2 +- nova/virt/xenapi_conn.py | 2 +- nova/volume/api.py | 10 ++++ 11 files changed, 243 insertions(+), 39 deletions(-) create mode 100644 nova/compute/utils.py diff --git a/nova/compute/api.py b/nova/compute/api.py index b809e59e2603..af2f237246ea 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -34,6 +34,7 @@ from nova import utils 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 @@ def generate_default_hostname(instance_id): 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.""" @@ -238,6 +251,22 @@ class API(base.Base): instance_id, security_group_id) + # tell vm driver to attach volume at boot time by updating + # BlockDeviceMapping + for bdm in block_device_mapping: + LOG.debug(_('bdm %s'), bdm) + assert bdm.has_key('device_name') + 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 @@ -365,24 +394,22 @@ class API(base.Base): 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 + instance = self._get_instance(context, instance_id, 'terminating') - 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) + if not _is_able_to_shutdown(instance, instance_id): return self.update(context, @@ -396,6 +423,7 @@ class API(base.Base): 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) def get(self, context, instance_id): diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 11565c25e954..86dac0ff2d83 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -54,6 +54,7 @@ from nova import rpc 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 @@ -215,7 +216,59 @@ class ComputeManager(manager.SchedulerDependentManager): """ return self.driver.refresh_security_group_members(security_group_id) - @exception.wrap_exception + 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') + + block_device_mapping = [] + try: + bdms = self.db.block_device_mapping_get_all_by_instance( + context, instance_id) + except exception.NotFound: + pass + else: + volume_api = volume.API() + for bdm in bdms: + 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): creatning volume simulteneously + # reduce 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'] + + assert ((bdm['snapshot_id'] is None) or + (bdm['volume_id'] is not None)) + + 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) + LOG.debug(_('block_device_mapping: ' + 'ephemeral device is not supported yet')) + else: + # TODO(yamahata) + 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() @@ -249,11 +302,15 @@ class ComputeManager(manager.SchedulerDependentManager): 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: " @@ -786,6 +843,22 @@ class ComputeManager(manager.SchedulerDependentManager): 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 instnace 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.""" @@ -803,6 +876,16 @@ class ComputeManager(manager.SchedulerDependentManager): 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 @@ -817,7 +900,7 @@ class ComputeManager(manager.SchedulerDependentManager): @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) @@ -833,8 +916,15 @@ class ComputeManager(manager.SchedulerDependentManager): 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. diff --git a/nova/compute/utils.py b/nova/compute/utils.py new file mode 100644 index 000000000000..f0a3ab59b763 --- /dev/null +++ b/nova/compute/utils.py @@ -0,0 +1,34 @@ +# 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 exception +from nova import volume + +def terminate_volumes(db, context, instance_id): + """delete volumes of delete_on_termination=True in block device mapping""" + try: + bdms = db.block_device_mapping_get_all_by_instance( + context, instance_id) + except exception.NotFound: + pass + else: + volume_api = volume.API() + 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']) + db.block_device_mapping_destroy(context, bdm['id']) diff --git a/nova/virt/driver.py b/nova/virt/driver.py index eb9626d0879e..773d608559ab 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -61,7 +61,7 @@ class ComputeDriver(object): """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=[]): """Launch a VM for the specified instance""" raise NotImplementedError() diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 5ac376e4634b..093c0f5d2cf7 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -114,7 +114,7 @@ class FakeConnection(driver.ComputeDriver): 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=[]): """ Create a new instance/VM/domain on the virtualization platform. diff --git a/nova/virt/hyperv.py b/nova/virt/hyperv.py index 1142e97a4610..216e90016bc7 100644 --- a/nova/virt/hyperv.py +++ b/nova/virt/hyperv.py @@ -139,7 +139,7 @@ class HyperVConnection(driver.ComputeDriver): return instance_infos - def spawn(self, instance): + def spawn(self, instance, network_info=None, block_device_mapping=[]): """ Create a new VM and start it.""" vm = self._lookup(instance.name) if vm is not None: diff --git a/nova/virt/libvirt.xml.template b/nova/virt/libvirt.xml.template index de2497a762e8..f7d699a15747 100644 --- a/nova/virt/libvirt.xml.template +++ b/nova/virt/libvirt.xml.template @@ -67,11 +67,13 @@ #else + #if not ($getVar('ebs_root', False)) + #end if #if $getVar('local', False) @@ -79,6 +81,13 @@ #end if + #for $vol in $volumes + + + + + + #end for #end if #end if diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index fa918b0a33cf..3290be603c59 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -39,6 +39,7 @@ Supports KVM, LXC, QEMU, UML, and XEN. import multiprocessing import os import random +import re import shutil import subprocess import sys @@ -207,6 +208,8 @@ def _get_network_info(instance): network_info.append((network, mapping)) return network_info +def _strip_dev(mount_path): + return re.sub(r'^/dev/', '', mount_path) class LibvirtConnection(driver.ComputeDriver): @@ -619,11 +622,13 @@ class LibvirtConnection(driver.ComputeDriver): # 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=[]): + xml = self.to_xml(instance, False, network_info=network_info, + block_device_mapping=block_device_mapping) 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) @@ -805,7 +810,7 @@ class LibvirtConnection(driver.ComputeDriver): # 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=[]): if not network_info: network_info = _get_network_info(inst) @@ -868,16 +873,19 @@ class LibvirtConnection(driver.ComputeDriver): 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'], @@ -992,7 +1000,18 @@ class LibvirtConnection(driver.ComputeDriver): 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=[]): # TODO(adiantum) remove network_info creation code # when multinics will be completed if not network_info: @@ -1010,6 +1029,16 @@ class LibvirtConnection(driver.ComputeDriver): 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, @@ -1017,9 +1046,11 @@ class LibvirtConnection(driver.ComputeDriver): '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': @@ -1034,10 +1065,12 @@ class LibvirtConnection(driver.ComputeDriver): 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=[]): # 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 diff --git a/nova/virt/vmwareapi_conn.py b/nova/virt/vmwareapi_conn.py index 1c6d2572db9c..a36b45d80142 100644 --- a/nova/virt/vmwareapi_conn.py +++ b/nova/virt/vmwareapi_conn.py @@ -124,7 +124,7 @@ class VMWareESXConnection(driver.ComputeDriver): """List VM instances.""" return self._vmops.list_instances() - def spawn(self, instance): + def spawn(self, instance, network_info=None, block_device_mapping=[]): """Create VM instance.""" self._vmops.spawn(instance) diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index 6d828e10997c..8c5e2201e09e 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -194,7 +194,7 @@ class XenAPIConnection(driver.ComputeDriver): 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=[]): """Create VM instance""" self._vmops.spawn(instance) diff --git a/nova/volume/api.py b/nova/volume/api.py index b48bcb8daab9..0d2fcc1f2999 100644 --- a/nova/volume/api.py +++ b/nova/volume/api.py @@ -22,6 +22,8 @@ Handles all requests relating to volumes. import datetime +from eventlet import greenthread + from nova import db from nova import exception from nova import flags @@ -74,6 +76,14 @@ class API(base.Base): "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": From ab938bf376efe7a93b54e4ca595d3102d04b0080 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:10:24 +0900 Subject: [PATCH 07/22] compute: implement ec2 stop/start instances This patch implements ec2 stop/start instances with block device mapping support. --- nova/api/ec2/cloud.py | 34 +++++++++++++++++++-------- nova/compute/api.py | 39 +++++++++++++++++++++++++++++++ nova/compute/manager.py | 48 +++++++++++++++++++++++++++++++++------ nova/db/api.py | 5 ++++ nova/db/sqlalchemy/api.py | 18 +++++++++++++++ nova/scheduler/simple.py | 8 ++++++- 6 files changed, 134 insertions(+), 18 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 56b958458e39..0989a4f405fb 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -903,33 +903,47 @@ class CloudController(object): 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 instance in instace_id""" + LOG.debug(_("Going to stop instnces")) + self._do_instances(self.compute_api.stop, context, instance_id) + return True + + def start_instances(self, context, instance_id, **kwargs): + """Start each instance in instace_id""" + LOG.debug(_("Going to start instnces")) + 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): diff --git a/nova/compute/api.py b/nova/compute/api.py index af2f237246ea..6b38e4eb5d9f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -426,6 +426,45 @@ class API(base.Base): 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 instnace.""" + 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=datetime.datetime.utcnow()) + + host = instance['host'] + if host: + self._cast_compute_message('stop_instance', context, + instance_id, host) + + def start(self, context, instance_id): + """Start an instnace.""" + 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) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 86dac0ff2d83..5e8429e8824f 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -269,7 +269,7 @@ class ComputeManager(manager.SchedulerDependentManager): return block_device_mapping - def run_instance(self, context, instance_id, **kwargs): + 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) @@ -333,13 +333,25 @@ class ComputeManager(manager.SchedulerDependentManager): self._update_launched_at(context, instance_id) 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: @@ -375,16 +387,34 @@ class ComputeManager(manager.SchedulerDependentManager): 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_istance_states() + @exception.wrap_exception @checks_instance_lock def rebuild_instance(self, context, instance_id, image_id): @@ -1250,11 +1280,15 @@ class ComputeManager(manager.SchedulerDependentManager): "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. diff --git a/nova/db/api.py b/nova/db/api.py index 858c8eb10d83..7fb7f336fa81 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -414,6 +414,11 @@ def instance_destroy(context, instance_id): 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) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index e0904b3c660d..be47e2c2eb71 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -831,6 +831,24 @@ def instance_destroy(context, instance_id): 'deleted_at': datetime.datetime.utcnow(), 'updated_at': literal_column('updated_at')}) +@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): diff --git a/nova/scheduler/simple.py b/nova/scheduler/simple.py index dd568d2c6380..ccbc79a36a90 100644 --- a/nova/scheduler/simple.py +++ b/nova/scheduler/simple.py @@ -40,7 +40,7 @@ flags.DEFINE_integer("max_networks", 1000, 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'] @@ -76,6 +76,12 @@ class SimpleScheduler(chance.ChanceScheduler): " 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) From 402ccd6d7c68697a3feb454c42207251fb08a64c Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:11:06 +0900 Subject: [PATCH 08/22] unittest: tests for boot from volume and stop/start instances --- nova/tests/test_cloud.py | 322 +++++++++++++++++++++++++++++++++++-- nova/tests/test_compute.py | 15 ++ 2 files changed, 327 insertions(+), 10 deletions(-) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 8c7520fe8dba..331fadaa0a1b 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -63,6 +63,7 @@ class CloudTestCase(test.TestCase): 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() @@ -85,6 +86,7 @@ class CloudTestCase(test.TestCase): db.network_disassociate(self.context, network_ref['id']) self.manager.delete_project(self.project) self.manager.delete_user(self.user) + self.volume.kill() self.compute.kill() self.network.kill() super(CloudTestCase, self).tearDown() @@ -364,15 +366,22 @@ class CloudTestCase(test.TestCase): self.assertRaises(exception.ImageNotFound, deregister_image, self.context, 'ami-bad001') - 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} + def _run_instance(self, **kwargs): rv = self.cloud.run_instances(self.context, **kwargs) greenthread.sleep(0.3) 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_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') @@ -383,10 +392,7 @@ class CloudTestCase(test.TestCase): greenthread.sleep(0.3) def test_ajax_console(self): - kwargs = {'image_id': 'ami-1'} - rv = self.cloud.run_instances(self.context, **kwargs) - instance_id = rv['instancesSet'][0]['instanceId'] - greenthread.sleep(0.3) + 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'], @@ -470,3 +476,299 @@ class CloudTestCase(test.TestCase): 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 instnace 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']) diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 9170837b6ebd..f35f9ce7359a 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -229,6 +229,21 @@ class ComputeTestCase(test.TestCase): 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() From 25104b5e8570003f82ee1b4d2e5678715cdcc551 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 27 May 2011 11:11:30 +0900 Subject: [PATCH 09/22] unittest: make unit tests happy various fixes to make unit tests pass. --- nova/virt/fake.py | 4 ++++ nova/volume/driver.py | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 093c0f5d2cf7..498c5ecbb4f9 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -222,6 +222,10 @@ class FakeConnection(driver.ComputeDriver): """ 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 diff --git a/nova/volume/driver.py b/nova/volume/driver.py index aade3c194889..fe571bb58135 100644 --- a/nova/volume/driver.py +++ b/nova/volume/driver.py @@ -582,6 +582,14 @@ class FakeISCSIDriver(ISCSIDriver): """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.""" From 4171160aa24d2e055da8b33c90c77c5b75c26fd9 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Sun, 29 May 2011 22:45:58 +0900 Subject: [PATCH 10/22] boot-from-volume: some comments and NOTE(user name) --- nova/api/ec2/apirequest.py | 6 ++++++ nova/api/ec2/cloud.py | 1 + nova/compute/api.py | 1 + 3 files changed, 8 insertions(+) diff --git a/nova/api/ec2/apirequest.py b/nova/api/ec2/apirequest.py index 4d6aa7f0ae79..368d925d838c 100644 --- a/nova/api/ec2/apirequest.py +++ b/nova/api/ec2/apirequest.py @@ -134,6 +134,12 @@ class APIRequest(object): # into their respective values value = _try_convert(value) + # NOTE(yamahata) + # parse multi dot-separted argument. + # 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}}} if len(parts) > 1: d = args.get(key, {}) args[key] = d diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index caefe6ff3da4..cff459cad2a1 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -891,6 +891,7 @@ class CloudController(object): ramdisk = self._get_image(context, kwargs['ramdisk_id']) kwargs['ramdisk_id'] = ramdisk['id'] for bdm in kwargs.get('block_device_mapping', []): + # NOTE(yamahata) # BlockDevicedMapping..DeviceName # BlockDevicedMapping..Ebs.SnapshotId # BlockDevicedMapping..Ebs.VolumeSize diff --git a/nova/compute/api.py b/nova/compute/api.py index 455d6d3baa4f..a211bd4bbee5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -258,6 +258,7 @@ class API(base.Base): 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: From 06372798edf744ba28612e2bda688ba3b5f30bb3 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 15 Jun 2011 14:41:29 +0900 Subject: [PATCH 11/22] api/ec2: make the parameter parser an independent method Following the review, make the parser of argument items an independent method for readability. --- nova/api/ec2/apirequest.py | 91 ++----------------------------------- nova/api/ec2/ec2utils.py | 93 ++++++++++++++++++++++++++++++++++++++ nova/tests/test_api.py | 2 +- 3 files changed, 97 insertions(+), 89 deletions(-) diff --git a/nova/api/ec2/apirequest.py b/nova/api/ec2/apirequest.py index 368d925d838c..7d78c5cfada5 100644 --- a/nova/api/ec2/apirequest.py +++ b/nova/api/ec2/apirequest.py @@ -21,22 +21,15 @@ APIRequest class """ 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 @@ def _database_to_isoformat(datetimeobj): 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 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 - if value.lower() == 'true': - return True - if value.lower() == '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 @@ class APIRequest(object): 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,32 +65,7 @@ class APIRequest(object): # 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) - - # NOTE(yamahata) - # parse multi dot-separted argument. - # 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}}} - 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 + args = ec2utils.dict_from_dotted_str(self.args.items()) for key in args.keys(): # NOTE(vish): Turn numeric dict keys into lists diff --git a/nova/api/ec2/ec2utils.py b/nova/api/ec2/ec2utils.py index 163aa4ed2d64..3d523016e201 100644 --- a/nova/api/ec2/ec2utils.py +++ b/nova/api/ec2/ec2utils.py @@ -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,94 @@ def ec2_id_to_id(ec2_id): 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 + if value.lower() == 'true': + return True + if value.lower() == '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 diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index 7c0331eff46f..20b20fcbf48d 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -89,7 +89,7 @@ class FakeHttplibConnection(object): 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) From c7e8f9ff46758ddbeecd049083bece013301bb59 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 15 Jun 2011 14:47:18 +0900 Subject: [PATCH 12/22] block_device_mapping: don't use [] as default argument --- nova/compute/api.py | 3 ++- nova/virt/driver.py | 2 +- nova/virt/fake.py | 2 +- nova/virt/hyperv.py | 2 +- nova/virt/libvirt/connection.py | 12 ++++++++---- nova/virt/vmwareapi_conn.py | 2 +- nova/virt/xenapi_conn.py | 2 +- 7 files changed, 15 insertions(+), 10 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index a211bd4bbee5..3b3bea41b23f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -150,12 +150,13 @@ class API(base.Base): availability_zone=None, user_data=None, metadata={}, injected_files=None, admin_password=None, - block_device_mapping=[]): + block_device_mapping=None): """Create the number and type of instances requested. Verifies that quota and other arguments are valid. """ + block_device_mapping = block_device_mapping or [] if not instance_type: instance_type = instance_types.get_default_instance_type() diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 773d608559ab..6341e81d2067 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -61,7 +61,7 @@ class ComputeDriver(object): """Return a list of InstanceInfo for all registered VMs""" raise NotImplementedError() - def spawn(self, instance, network_info=None, block_device_mapping=[]): + def spawn(self, instance, network_info=None, block_device_mapping=None): """Launch a VM for the specified instance""" raise NotImplementedError() diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 498c5ecbb4f9..ab85de2e4663 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -114,7 +114,7 @@ class FakeConnection(driver.ComputeDriver): info_list.append(self._map_to_instance_info(instance)) return info_list - def spawn(self, instance, network_info=None, block_device_mapping=[]): + def spawn(self, instance, network_info=None, block_device_mapping=None): """ Create a new instance/VM/domain on the virtualization platform. diff --git a/nova/virt/hyperv.py b/nova/virt/hyperv.py index 216e90016bc7..23e25e45727f 100644 --- a/nova/virt/hyperv.py +++ b/nova/virt/hyperv.py @@ -139,7 +139,7 @@ class HyperVConnection(driver.ComputeDriver): return instance_infos - def spawn(self, instance, network_info=None, block_device_mapping=[]): + 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: diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py index 0d22d6c413c9..6300d00a8cf1 100644 --- a/nova/virt/libvirt/connection.py +++ b/nova/virt/libvirt/connection.py @@ -562,9 +562,10 @@ class LibvirtConnection(driver.ComputeDriver): # NOTE(ilyaalekseyev): Implementation like in multinics # for xenapi(tr3buchet) @exception.wrap_exception - def spawn(self, instance, network_info=None, block_device_mapping=[]): + 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, @@ -750,7 +751,8 @@ class LibvirtConnection(driver.ComputeDriver): # TODO(vish): should we format disk by default? def _create_image(self, inst, libvirt_xml, suffix='', disk_images=None, - network_info=None, block_device_mapping=[]): + 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) @@ -951,7 +953,8 @@ class LibvirtConnection(driver.ComputeDriver): return False def _prepare_xml_info(self, instance, rescue=False, network_info=None, - block_device_mapping=[]): + 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: @@ -1006,7 +1009,8 @@ class LibvirtConnection(driver.ComputeDriver): return xml_info def to_xml(self, instance, rescue=False, network_info=None, - block_device_mapping=[]): + 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, diff --git a/nova/virt/vmwareapi_conn.py b/nova/virt/vmwareapi_conn.py index a36b45d80142..3c6345ec8508 100644 --- a/nova/virt/vmwareapi_conn.py +++ b/nova/virt/vmwareapi_conn.py @@ -124,7 +124,7 @@ class VMWareESXConnection(driver.ComputeDriver): """List VM instances.""" return self._vmops.list_instances() - def spawn(self, instance, network_info=None, block_device_mapping=[]): + def spawn(self, instance, network_info=None, block_device_mapping=None): """Create VM instance.""" self._vmops.spawn(instance) diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index 8c5e2201e09e..5fcec1715c70 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -194,7 +194,7 @@ class XenAPIConnection(driver.ComputeDriver): def list_instances_detail(self): return self._vmops.list_instances_detail() - def spawn(self, instance, network_info=None, block_device_mapping=[]): + def spawn(self, instance, network_info=None, block_device_mapping=None): """Create VM instance""" self._vmops.spawn(instance) From b3af5e4d5a623cf10828f4724f29dd4475120b70 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 15 Jun 2011 15:08:23 +0900 Subject: [PATCH 13/22] ec2utils: minor optimize _try_convert() don't call lower() twice. --- nova/api/ec2/ec2utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/api/ec2/ec2utils.py b/nova/api/ec2/ec2utils.py index 3d523016e201..222e1de1efba 100644 --- a/nova/api/ec2/ec2utils.py +++ b/nova/api/ec2/ec2utils.py @@ -61,9 +61,10 @@ def _try_convert(value): return '' if value == 'None': return None - if value.lower() == 'true': + lowered_value = value.lower() + if lowered_value == 'true': return True - if value.lower() == 'false': + if lowered_value == 'false': return False valueneg = value[1:] if value[0] == '-' else value if valueneg == '0': From b0fdb4a2326f6e7c92bba80e6b80857ba2a61612 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 15 Jun 2011 22:58:22 +0900 Subject: [PATCH 14/22] typo --- nova/api/ec2/cloud.py | 10 ++++++---- nova/compute/api.py | 4 ++-- nova/compute/manager.py | 12 ++++++------ nova/db/api.py | 2 +- nova/db/sqlalchemy/models.py | 4 ++-- nova/tests/test_cloud.py | 2 +- 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index cff459cad2a1..637ea46d415b 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -949,14 +949,16 @@ class CloudController(object): return True def stop_instances(self, context, instance_id, **kwargs): - """Stop each instance in instace_id""" - LOG.debug(_("Going to stop instnces")) + """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 instance in instace_id""" - LOG.debug(_("Going to start instnces")) + """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 diff --git a/nova/compute/api.py b/nova/compute/api.py index 3b3bea41b23f..66edf9faaaf9 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -445,7 +445,7 @@ class API(base.Base): @scheduler_api.reroute_compute("stop") def stop(self, context, instance_id): - """Stop an instnace.""" + """Stop an instance.""" LOG.debug(_("Going to try to stop %s"), instance_id) instance = self._get_instance(context, instance_id, 'stopping') @@ -464,7 +464,7 @@ class API(base.Base): instance_id, host) def start(self, context, instance_id): - """Start an instnace.""" + """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': diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 1281449426af..2e130110a8da 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -238,8 +238,8 @@ class ComputeManager(manager.SchedulerDependentManager): # TODO(yamahata): default name and description vol = volume_api.create(context, bdm['volume_size'], bdm['snapshot_id'], '', '') - # TODO(yamahata): creatning volume simulteneously - # reduce creation time? + # 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']}) @@ -258,11 +258,11 @@ class ComputeManager(manager.SchedulerDependentManager): 'mount_device': bdm['device_name']}) elif bdm['virtual_name'] is not None: - # TODO(yamahata) + # TODO(yamahata): ephemeral/swap device support LOG.debug(_('block_device_mapping: ' 'ephemeral device is not supported yet')) else: - # TODO(yamahata) + # TODO(yamahata): NoDevice support assert bdm['no_device'] LOG.debug(_('block_device_mapping: ' 'no device is not supported yet')) @@ -414,7 +414,7 @@ class ComputeManager(manager.SchedulerDependentManager): 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_istance_states() + # instance state will be updated to stopped by _poll_instance_states() @exception.wrap_exception @checks_instance_lock @@ -887,7 +887,7 @@ class ComputeManager(manager.SchedulerDependentManager): return self.driver.get_vnc_console(instance_ref) def _attach_volume_boot(self, context, instance_id, volume_id, mountpoint): - """Attach a volume to an instnace at boot time. So actual attach + """Attach a volume to an instance at boot time. So actual attach is done by instance creation""" # TODO(yamahata): diff --git a/nova/db/api.py b/nova/db/api.py index 215fa500967c..3f5e9a54ac0a 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -934,7 +934,7 @@ def block_device_mapping_update(context, bdm_id, values): 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 blonging to a instance""" + """Get all block device mapping belonging to a instance""" return IMPL.block_device_mapping_get_all_by_instance(context, instance_id) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index a74db811f166..0c3ec40c7321 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -371,7 +371,7 @@ class BlockDeviceMapping(BASE, NovaBase): 'False)') device_name = Column(String(255), nullable=False) - # default=False for compatilibity of the existing code. + # default=False for compatibility of the existing code. # With EC2 API, # default True for ami specified device. # default False for created with other timing. @@ -391,7 +391,7 @@ class BlockDeviceMapping(BASE, NovaBase): foreign_keys=volume_id) volume_size = Column(Integer, nullable=True) - # for no device to supress devices. + # for no device to suppress devices. no_device = Column(Boolean, nullable=True) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index c9b75f966777..1e8b8e8463bf 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -535,7 +535,7 @@ class CloudTestCase(test.TestCase): self._wait_for_state(elevated, instance_id, is_deleted) def test_stop_start_instance(self): - """Makes sure stop/start instnace works""" + """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) From f48f35183f6bc30c0e053ea9569f5348799ed451 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Wed, 15 Jun 2011 23:11:03 +0900 Subject: [PATCH 15/22] pep8 --- nova/api/ec2/cloud.py | 2 +- nova/compute/api.py | 4 +- nova/compute/manager.py | 1 - nova/compute/utils.py | 1 + nova/db/api.py | 2 + nova/db/sqlalchemy/api.py | 5 +++ .../versions/021_add_block_device_mapping.py | 6 ++- nova/db/sqlalchemy/models.py | 4 +- nova/tests/test_cloud.py | 40 +++++++++---------- nova/virt/libvirt/connection.py | 5 ++- 10 files changed, 40 insertions(+), 30 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 637ea46d415b..b370635757de 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -934,7 +934,7 @@ class CloudController(object): 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.""" diff --git a/nova/compute/api.py b/nova/compute/api.py index 66edf9faaaf9..40ad8cd9c28c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -264,7 +264,7 @@ class API(base.Base): # BlockDeviceMapping for bdm in block_device_mapping: LOG.debug(_('bdm %s'), bdm) - assert bdm.has_key('device_name') + assert 'device_name' in bdm values = { 'instance_id': instance_id, 'device_name': bdm['device_name'], @@ -451,7 +451,7 @@ class API(base.Base): 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', diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 2e130110a8da..62e5ce243433 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1300,7 +1300,6 @@ class ComputeManager(manager.SchedulerDependentManager): vm_state = vm_instance.state vms_not_found_in_db.remove(name) - 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, diff --git a/nova/compute/utils.py b/nova/compute/utils.py index f0a3ab59b763..b99ef1357b5a 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -18,6 +18,7 @@ from nova import exception from nova import volume + def terminate_volumes(db, context, instance_id): """delete volumes of delete_on_termination=True in block device mapping""" try: diff --git a/nova/db/api.py b/nova/db/api.py index 3f5e9a54ac0a..cf58e87e5020 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -929,10 +929,12 @@ 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) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9385519df0c0..915b3401627c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -832,6 +832,7 @@ def instance_destroy(context, instance_id): 'deleted_at': datetime.datetime.utcnow(), 'updated_at': literal_column('updated_at')}) + @require_context def instance_stop(context, instance_id): session = get_session() @@ -1892,6 +1893,7 @@ def block_device_mapping_create(context, values): with session.begin(): bdm_ref.save(session=session) + @require_context def block_device_mapping_update(context, bdm_id, values): session = get_session() @@ -1901,6 +1903,7 @@ def block_device_mapping_update(context, bdm_id, values): filter_by(deleted=False).\ update(values) + @require_context def block_device_mapping_get_all_by_instance(context, instance_id): session = get_session() @@ -1912,6 +1915,7 @@ def block_device_mapping_get_all_by_instance(context, instance_id): raise exception.NotFound() return result + @require_context def block_device_mapping_destroy(context, bdm_id): session = get_session() @@ -1922,6 +1926,7 @@ def block_device_mapping_destroy(context, bdm_id): 'deleted_at': datetime.datetime.utcnow(), 'updated_at': literal_column('updated_at')}) + @require_context def block_device_mapping_destroy_by_instance_and_volume(context, instance_id, volume_id): diff --git a/nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py b/nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py index ed429633458c..6e9b806cbdd6 100644 --- a/nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py +++ b/nova/db/sqlalchemy/migrate_repo/versions/021_add_block_device_mapping.py @@ -60,7 +60,8 @@ block_device_mapping = Table('block_device_mapping', meta, Integer(), ForeignKey('snapshots.id'), nullable=True), - Column('volume_id', Integer(), ForeignKey('volumes.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), @@ -79,7 +80,8 @@ def upgrade(migrate_engine): 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() diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 0c3ec40c7321..104f311430fd 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -385,12 +385,12 @@ class BlockDeviceMapping(BASE, NovaBase): # 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) diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 1e8b8e8463bf..3d91eb2b8a8d 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -541,7 +541,7 @@ class CloudTestCase(test.TestCase): kwargs = {'image_id': 'ami-1', 'instance_type': FLAGS.default_instance_type, - 'max_count': 1,} + 'max_count': 1, } instance_id = self._run_instance_wait(**kwargs) # a running instance can't be started. It is just ignored. @@ -553,7 +553,7 @@ class CloudTestCase(test.TestCase): 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) @@ -563,18 +563,18 @@ class CloudTestCase(test.TestCase): 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',} + 'attach_status': 'detached', } return db.volume_create(self.context, kwargs) def _assert_volume_attached(self, vol, instance_id, mountpoint): @@ -582,7 +582,7 @@ class CloudTestCase(test.TestCase): 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) @@ -604,8 +604,8 @@ class CloudTestCase(test.TestCase): 'volume_id': vol1['id'], 'delete_on_termination': False,}, {'device_name': '/dev/vdc', - 'volume_id': vol2['id'], - 'delete_on_termination': True,}, + '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) @@ -629,7 +629,7 @@ class CloudTestCase(test.TestCase): 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) @@ -654,7 +654,7 @@ class CloudTestCase(test.TestCase): 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): @@ -669,7 +669,7 @@ class CloudTestCase(test.TestCase): 'max_count': 1, 'block_device_mapping': [{'device_name': '/dev/vdb', 'volume_id': vol1['id'], - 'delete_on_termination': True,},]} + 'delete_on_termination': True}]} ec2_instance_id = self._run_instance_wait(**kwargs) instance_id = ec2utils.ec2_id_to_id(ec2_instance_id) @@ -695,7 +695,7 @@ class CloudTestCase(test.TestCase): 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) @@ -703,7 +703,7 @@ class CloudTestCase(test.TestCase): 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) @@ -723,15 +723,15 @@ class CloudTestCase(test.TestCase): 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() @@ -747,10 +747,10 @@ class CloudTestCase(test.TestCase): 'max_count': 1, 'block_device_mapping': [{'device_name': '/dev/vdb', 'snapshot_id': snapshot1_id, - 'delete_on_termination': False,}, + 'delete_on_termination': False, }, {'device_name': '/dev/vdc', 'snapshot_id': snapshot2_id, - 'delete_on_termination': True,},],} + 'delete_on_termination': True}]} ec2_instance_id = self._run_instance_wait(**kwargs) instance_id = ec2utils.ec2_id_to_id(ec2_instance_id) @@ -778,14 +778,14 @@ class CloudTestCase(test.TestCase): greenthread.sleep(0.3) self._wait_for_terminate(ec2_instance_id) - greenthread.sleep(0.3) + 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) + 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']) diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py index 6300d00a8cf1..a877749e9b7f 100644 --- a/nova/virt/libvirt/connection.py +++ b/nova/virt/libvirt/connection.py @@ -942,8 +942,9 @@ class LibvirtConnection(driver.ComputeDriver): return result - root_mount_device = 'vda' # FIXME for now. it's hard coded. - local_mount_device = 'vdb' # FIXME for now. it's hard coded. + 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: From a9800765f7eb8430c67d15953bed202752c2e199 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 00:32:03 +0900 Subject: [PATCH 16/22] db/block_device_mapping_get_all_by_instance: don't raise. Make db.block_device_mapping_get_all_by_instance() not raise, but returns empty list when no column found as all users catch notfound exception and ignore it. Thus ugly 'catch NotFound: pass' is eliminated. --- nova/compute/manager.py | 75 ++++++++++++++++++--------------------- nova/compute/utils.py | 20 ++++------- nova/db/sqlalchemy/api.py | 2 +- 3 files changed, 43 insertions(+), 54 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 62e5ce243433..7f7f0fa650d9 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -223,49 +223,44 @@ class ComputeManager(manager.SchedulerDependentManager): power_state.NOSTATE, 'block_device_mapping') + volume_api = volume.API() block_device_mapping = [] - try: - bdms = self.db.block_device_mapping_get_all_by_instance( - context, instance_id) - except exception.NotFound: - pass - else: - volume_api = volume.API() - for bdm in bdms: - 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'] + 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'] - assert ((bdm['snapshot_id'] is None) or - (bdm['volume_id'] is not None)) + assert ((bdm['snapshot_id'] is None) or + (bdm['volume_id'] is not None)) - 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')) + 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 diff --git a/nova/compute/utils.py b/nova/compute/utils.py index b99ef1357b5a..c8cb9bab8dbf 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -15,21 +15,15 @@ # License for the specific language governing permissions and limitations # under the License. -from nova import exception from nova import volume def terminate_volumes(db, context, instance_id): """delete volumes of delete_on_termination=True in block device mapping""" - try: - bdms = db.block_device_mapping_get_all_by_instance( - context, instance_id) - except exception.NotFound: - pass - else: - volume_api = volume.API() - 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']) - db.block_device_mapping_destroy(context, bdm['id']) + 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']) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 915b3401627c..ddaae8ca5b24 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1912,7 +1912,7 @@ def block_device_mapping_get_all_by_instance(context, instance_id): filter_by(deleted=False).\ all() if not result: - raise exception.NotFound() + return [] return result From 31ae0515fbf2dd8f931fb53ef08b40fe0206fc92 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 00:48:25 +0900 Subject: [PATCH 17/22] _setup_block_device_mapping: raise ApiError when db inconsistency found Instead of assert, log it and raise ApiError if db inconsistency in block_device_mapping table is found. --- nova/compute/manager.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 7f7f0fa650d9..1c5d2c858f7a 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -240,8 +240,16 @@ class ComputeManager(manager.SchedulerDependentManager): context, bdm['id'], {'volume_id': vol['id']}) bdm['volume_id'] = vol['id'] - assert ((bdm['snapshot_id'] is None) or - (bdm['volume_id'] is not None)) + 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, From ab7c45a9be39def2d6cbb74084abc6fa42f593dd Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 16 Jun 2011 02:10:28 +0900 Subject: [PATCH 18/22] fix mismerge. --- nova/compute/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 6f7192ae7036..18363ace07a0 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -553,7 +553,7 @@ class API(base.Base): instance['id'], state_description='stopping', state=power_state.NOSTATE, - terminated_at=datetime.datetime.utcnow()) + terminated_at=utils.utcnow()) host = instance['host'] if host: From e35878070ce594d3c9db2f433dcf3f3f1441a497 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 15 Jun 2011 15:28:39 -0400 Subject: [PATCH 19/22] adding server existence check to server metadata resource --- nova/api/openstack/server_metadata.py | 15 +++++- .../api/openstack/test_server_metadata.py | 50 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/server_metadata.py b/nova/api/openstack/server_metadata.py index 57666f6b79c7..ec9e10496709 100644 --- a/nova/api/openstack/server_metadata.py +++ b/nova/api/openstack/server_metadata.py @@ -18,9 +18,10 @@ from webob import exc from nova import compute -from nova import quota from nova.api.openstack import faults from nova.api.openstack import wsgi +from nova import exception +from nova import quota class Controller(object): @@ -42,14 +43,23 @@ class Controller(object): expl = _('No Request Body') raise exc.HTTPBadRequest(explanation=expl) + def _check_server_exists(self, context, server_id): + try: + self.compute_api.routing_get(context, server_id) + except exception.InstanceNotFound: + msg = _('Server does not exist') + raise exc.HTTPNotFound(explanation=msg) + def index(self, req, server_id): """ Returns the list of metadata for a given instance """ context = req.environ['nova.context'] + self._check_server_exists(context, server_id) return self._get_metadata(context, server_id) def create(self, req, server_id, body): self._check_body(body) context = req.environ['nova.context'] + self._check_server_exists(context, server_id) metadata = body.get('metadata') try: self.compute_api.update_or_create_instance_metadata(context, @@ -62,6 +72,7 @@ class Controller(object): def update(self, req, server_id, id, body): self._check_body(body) context = req.environ['nova.context'] + self._check_server_exists(context, server_id) if not id in body: expl = _('Request body and URI mismatch') raise exc.HTTPBadRequest(explanation=expl) @@ -80,6 +91,7 @@ class Controller(object): def show(self, req, server_id, id): """ Return a single metadata item """ context = req.environ['nova.context'] + self._check_server_exists(context, server_id) data = self._get_metadata(context, server_id) if id in data['metadata']: return {id: data['metadata'][id]} @@ -89,6 +101,7 @@ class Controller(object): def delete(self, req, server_id, id): """ Deletes an existing metadata """ context = req.environ['nova.context'] + self._check_server_exists(context, server_id) self.compute_api.delete_instance_metadata(context, server_id, id) def _handle_quota_error(self, error): diff --git a/nova/tests/api/openstack/test_server_metadata.py b/nova/tests/api/openstack/test_server_metadata.py index b583d40fea5a..b484ad6a1881 100644 --- a/nova/tests/api/openstack/test_server_metadata.py +++ b/nova/tests/api/openstack/test_server_metadata.py @@ -21,6 +21,7 @@ import unittest import webob +from nova import exception from nova import flags from nova.api import openstack from nova.tests.api.openstack import fakes @@ -66,6 +67,12 @@ def stub_max_server_metadata(): metadata['metadata']['key%i' % num] = "blah" return metadata +def return_server(context, server_id): + return {'id': server_id} + +def return_server_nonexistant(context, server_id): + raise exception.InstanceNotFound() + class ServerMetaDataTest(unittest.TestCase): @@ -76,6 +83,7 @@ class ServerMetaDataTest(unittest.TestCase): fakes.FakeAuthDatabase.data = {} fakes.stub_out_auth(self.stubs) fakes.stub_out_key_pair_funcs(self.stubs) + self.stubs.Set(nova.db.api, 'instance_get', return_server) def tearDown(self): self.stubs.UnsetAll() @@ -92,6 +100,13 @@ class ServerMetaDataTest(unittest.TestCase): self.assertEqual('application/json', res.headers['Content-Type']) self.assertEqual('value1', res_dict['metadata']['key1']) + def test_index_nonexistant_server(self): + self.stubs.Set(nova.db.api, 'instance_get', return_server_nonexistant) + req = webob.Request.blank('/v1.1/servers/1/meta') + req.environ['api.version'] = '1.1' + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(404, res.status_int) + def test_index_no_data(self): self.stubs.Set(nova.db.api, 'instance_metadata_get', return_empty_server_metadata) @@ -114,6 +129,13 @@ class ServerMetaDataTest(unittest.TestCase): self.assertEqual('application/json', res.headers['Content-Type']) self.assertEqual('value5', res_dict['key5']) + def test_show_nonexistant_server(self): + self.stubs.Set(nova.db.api, 'instance_get', return_server_nonexistant) + req = webob.Request.blank('/v1.1/servers/1/meta/key5') + req.environ['api.version'] = '1.1' + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(404, res.status_int) + def test_show_meta_not_found(self): self.stubs.Set(nova.db.api, 'instance_metadata_get', return_empty_server_metadata) @@ -132,6 +154,14 @@ class ServerMetaDataTest(unittest.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(200, res.status_int) + def test_delete_nonexistant_server(self): + self.stubs.Set(nova.db.api, 'instance_get', return_server_nonexistant) + req = webob.Request.blank('/v1.1/servers/1/meta/key5') + req.environ['api.version'] = '1.1' + req.method = 'DELETE' + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(404, res.status_int) + def test_create(self): self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create', return_create_instance_metadata) @@ -156,6 +186,16 @@ class ServerMetaDataTest(unittest.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(400, res.status_int) + def test_create_nonexistant_server(self): + self.stubs.Set(nova.db.api, 'instance_get', return_server_nonexistant) + req = webob.Request.blank('/v1.1/servers/100/meta') + req.environ['api.version'] = '1.1' + req.method = 'POST' + req.body = '{"metadata": {"key1": "value1"}}' + req.headers["content-type"] = "application/json" + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(404, res.status_int) + def test_update_item(self): self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create', return_create_instance_metadata) @@ -170,6 +210,16 @@ class ServerMetaDataTest(unittest.TestCase): res_dict = json.loads(res.body) self.assertEqual('value1', res_dict['key1']) + def test_update_item_nonexistant_server(self): + self.stubs.Set(nova.db.api, 'instance_get', return_server_nonexistant) + req = webob.Request.blank('/v1.1/servers/asdf/100/key1') + req.environ['api.version'] = '1.1' + req.method = 'PUT' + req.body = '{"key1": "value1"}' + req.headers["content-type"] = "application/json" + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(404, res.status_int) + def test_update_item_empty_body(self): self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create', return_create_instance_metadata) From 86e8463319f55f4d7d82ab89d876a00e1c3b5508 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 15 Jun 2011 15:35:08 -0400 Subject: [PATCH 20/22] pep8 --- nova/tests/api/openstack/test_server_metadata.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nova/tests/api/openstack/test_server_metadata.py b/nova/tests/api/openstack/test_server_metadata.py index b484ad6a1881..8c7b48fedadd 100644 --- a/nova/tests/api/openstack/test_server_metadata.py +++ b/nova/tests/api/openstack/test_server_metadata.py @@ -67,9 +67,11 @@ def stub_max_server_metadata(): metadata['metadata']['key%i' % num] = "blah" return metadata + def return_server(context, server_id): return {'id': server_id} + def return_server_nonexistant(context, server_id): raise exception.InstanceNotFound() From e2fa70fb9d2b6684823328a491e18c0f98184665 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Fri, 17 Jun 2011 17:40:48 -0400 Subject: [PATCH 21/22] moving instance existance logic down to api layer --- nova/api/openstack/server_metadata.py | 47 ++++++++++++------- nova/db/sqlalchemy/api.py | 16 ++++++- .../api/openstack/test_server_metadata.py | 3 +- 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/nova/api/openstack/server_metadata.py b/nova/api/openstack/server_metadata.py index ec9e10496709..8a314de2245f 100644 --- a/nova/api/openstack/server_metadata.py +++ b/nova/api/openstack/server_metadata.py @@ -43,36 +43,35 @@ class Controller(object): expl = _('No Request Body') raise exc.HTTPBadRequest(explanation=expl) - def _check_server_exists(self, context, server_id): - try: - self.compute_api.routing_get(context, server_id) - except exception.InstanceNotFound: - msg = _('Server does not exist') - raise exc.HTTPNotFound(explanation=msg) - def index(self, req, server_id): """ Returns the list of metadata for a given instance """ context = req.environ['nova.context'] - self._check_server_exists(context, server_id) - return self._get_metadata(context, server_id) + try: + return self._get_metadata(context, server_id) + except exception.InstanceNotFound: + msg = _('Server %(server_id)s does not exist') % locals() + raise exc.HTTPNotFound(explanation=msg) def create(self, req, server_id, body): self._check_body(body) context = req.environ['nova.context'] - self._check_server_exists(context, server_id) metadata = body.get('metadata') try: self.compute_api.update_or_create_instance_metadata(context, server_id, metadata) + except exception.InstanceNotFound: + msg = _('Server %(server_id)s does not exist') % locals() + raise exc.HTTPNotFound(explanation=msg) + except quota.QuotaError as error: self._handle_quota_error(error) + return body def update(self, req, server_id, id, body): self._check_body(body) context = req.environ['nova.context'] - self._check_server_exists(context, server_id) if not id in body: expl = _('Request body and URI mismatch') raise exc.HTTPBadRequest(explanation=expl) @@ -83,6 +82,10 @@ class Controller(object): self.compute_api.update_or_create_instance_metadata(context, server_id, body) + except exception.InstanceNotFound: + msg = _('Server %(server_id)s does not exist') % locals() + raise exc.HTTPNotFound(explanation=msg) + except quota.QuotaError as error: self._handle_quota_error(error) @@ -91,18 +94,26 @@ class Controller(object): def show(self, req, server_id, id): """ Return a single metadata item """ context = req.environ['nova.context'] - self._check_server_exists(context, server_id) - data = self._get_metadata(context, server_id) - if id in data['metadata']: + try: + data = self._get_metadata(context, server_id) + except exception.InstanceNotFound: + msg = _('Server %(server_id)s does not exist') % locals() + raise exc.HTTPNotFound(explanation=msg) + + try: return {id: data['metadata'][id]} - else: - return faults.Fault(exc.HTTPNotFound()) + except KeyError: + msg = _("metadata item %s was not found" % (id)) + raise exc.HTTPNotFound(explanation=msg) def delete(self, req, server_id, id): """ Deletes an existing metadata """ context = req.environ['nova.context'] - self._check_server_exists(context, server_id) - self.compute_api.delete_instance_metadata(context, server_id, id) + try: + self.compute_api.delete_instance_metadata(context, server_id, id) + except exception.InstanceNotFound: + msg = _('Server %(server_id)s does not exist') % locals() + raise exc.HTTPNotFound(explanation=msg) def _handle_quota_error(self, error): """Reraise quota errors as api-specific http exceptions.""" diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 73870d2f3a1a..7cea3ba70353 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -18,7 +18,7 @@ """ Implementation of SQLAlchemy backend. """ - +import traceback import warnings from nova import db @@ -2609,7 +2609,17 @@ def zone_get_all(context): #################### + +def require_instance_exists(func): + def new_func(context, instance_id, *args, **kwargs): + db.api.instance_get(context, instance_id) + return func(context, instance_id, *args, **kwargs) + new_func.__name__ = func.__name__ + return new_func + + @require_context +@require_instance_exists def instance_metadata_get(context, instance_id): session = get_session() @@ -2625,6 +2635,7 @@ def instance_metadata_get(context, instance_id): @require_context +@require_instance_exists def instance_metadata_delete(context, instance_id, key): session = get_session() session.query(models.InstanceMetadata).\ @@ -2637,6 +2648,7 @@ def instance_metadata_delete(context, instance_id, key): @require_context +@require_instance_exists def instance_metadata_delete_all(context, instance_id): session = get_session() session.query(models.InstanceMetadata).\ @@ -2648,6 +2660,7 @@ def instance_metadata_delete_all(context, instance_id): @require_context +@require_instance_exists def instance_metadata_get_item(context, instance_id, key): session = get_session() @@ -2664,6 +2677,7 @@ def instance_metadata_get_item(context, instance_id, key): @require_context +@require_instance_exists def instance_metadata_update_or_create(context, instance_id, metadata): session = get_session() diff --git a/nova/tests/api/openstack/test_server_metadata.py b/nova/tests/api/openstack/test_server_metadata.py index 8c7b48fedadd..0431e68d2227 100644 --- a/nova/tests/api/openstack/test_server_metadata.py +++ b/nova/tests/api/openstack/test_server_metadata.py @@ -144,7 +144,6 @@ class ServerMetaDataTest(unittest.TestCase): req = webob.Request.blank('/v1.1/servers/1/meta/key6') req.environ['api.version'] = '1.1' res = req.get_response(fakes.wsgi_app()) - res_dict = json.loads(res.body) self.assertEqual(404, res.status_int) def test_delete(self): @@ -173,8 +172,8 @@ class ServerMetaDataTest(unittest.TestCase): req.body = '{"metadata": {"key1": "value1"}}' req.headers["content-type"] = "application/json" res = req.get_response(fakes.wsgi_app()) - res_dict = json.loads(res.body) self.assertEqual(200, res.status_int) + res_dict = json.loads(res.body) self.assertEqual('application/json', res.headers['Content-Type']) self.assertEqual('value1', res_dict['metadata']['key1']) From 74be8d2791a5579725fa07c8403545ede3354b2e Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Sat, 18 Jun 2011 08:34:20 +0900 Subject: [PATCH 22/22] pep8: white space/blank lines --- nova/compute/api.py | 2 -- nova/tests/test_cloud.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 18363ace07a0..979a29de6cf5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -298,8 +298,6 @@ class API(base.Base): return instance - - def _ask_scheduler_to_create_instance(self, context, base_options, instance_type, zone_blob, availability_zone, injected_files, diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 6a6256c204c7..09e26df4c60c 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -671,7 +671,7 @@ class CloudTestCase(test.TestCase): 'max_count': 1, 'block_device_mapping': [{'device_name': '/dev/vdb', 'volume_id': vol1['id'], - 'delete_on_termination': False,}, + 'delete_on_termination': False, }, {'device_name': '/dev/vdc', 'volume_id': vol2['id'], 'delete_on_termination': True, },