Convert instancetype.flavorid to string

Fixes bug 861666. This also removes some direct database access
in favor of using nova.compute.instance_types throughout the code.

Change-Id: I572cc19454fa76f435f5672d3d6e7ed55c8817da

3 files changed, 85 insertions, 74 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index a39d35006..ae8e95cc5 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -905,7 +905,7 @@ class API(base.Base):
 
     def get_instance_type(self, context, instance_type_id):
         """Get an instance type by instance type id."""
-        return self.db.instance_type_get(context, instance_type_id)
+        return instance_types.get_instance_type(instance_type_id)
 
     def get(self, context, instance_id):
         """Get a single instance with the given instance_id."""
@@ -948,8 +948,8 @@ class API(base.Base):
         filters = {}
 
         def _remap_flavor_filter(flavor_id):
-            instance_type = self.db.instance_type_get_by_flavor_id(
-                    context, flavor_id)
+            instance_type = instance_types.get_instance_type_by_flavor_id(
+                    flavor_id)
             filters['instance_type_id'] = instance_type['id']
 
         def _remap_fixed_ip_filter(fixed_ip):
@@ -1258,8 +1258,8 @@ class API(base.Base):
             LOG.debug(_("flavor_id is None. Assuming migration."))
             new_instance_type = current_instance_type
         else:
-            new_instance_type = self.db.instance_type_get_by_flavor_id(
-                    context, flavor_id)
+            new_instance_type = instance_types.get_instance_type_by_flavor_id(
+                    flavor_id)
 
         current_instance_type_name = current_instance_type['name']
         new_instance_type_name = new_instance_type['name']
diff --git a/nova/compute/instance_types.py b/nova/compute/instance_types.py
index ffbefb24c..b727de2cc 100644
--- a/nova/compute/instance_types.py
+++ b/nova/compute/instance_types.py
@@ -33,66 +33,72 @@ LOG = logging.getLogger('nova.instance_types')
 def create(name, memory, vcpus, local_gb, flavorid, swap=0,
            rxtx_quota=0, rxtx_cap=0):
     """Creates instance types."""
-    for option in [memory, vcpus, local_gb, flavorid]:
+    kwargs = {
+        'memory_mb': memory,
+        'vcpus': vcpus,
+        'local_gb': local_gb,
+        'swap': swap,
+        'rxtx_quota': rxtx_quota,
+        'rxtx_cap': rxtx_cap,
+    }
+
+    # ensure some attributes are integers and greater than or equal to 0
+    for option in kwargs:
         try:
-            int(option)
-        except ValueError:
-            raise exception.InvalidInput(reason=_("create arguments must "
-                                                  "be positive integers"))
-    if (int(memory) <= 0) or (int(vcpus) <= 0) or (int(local_gb) < 0):
-        raise exception.InvalidInput(reason=_("create arguments must "
-                                              "be positive integers"))
+            kwargs[option] = int(kwargs[option])
+            assert kwargs[option] >= 0
+        except (ValueError, AssertionError):
+            msg = _("create arguments must be positive integers")
+            raise exception.InvalidInput(reason=msg)
+
+    # some value are required to be nonzero, not just positive
+    for option in ['memory_mb', 'vcpus']:
+        try:
+            assert kwargs[option] > 0
+        except AssertionError:
+            msg = _("create arguments must be positive integers")
+            raise exception.InvalidInput(reason=msg)
+
+    kwargs['name'] = name
+    kwargs['flavorid'] = flavorid
 
     try:
-        db.instance_type_create(
-                context.get_admin_context(),
-                dict(name=name,
-                    memory_mb=memory,
-                    vcpus=vcpus,
-                    local_gb=local_gb,
-                    flavorid=flavorid,
-                    swap=swap,
-                    rxtx_quota=rxtx_quota,
-                    rxtx_cap=rxtx_cap))
+        return db.instance_type_create(context.get_admin_context(), kwargs)
     except exception.DBError, e:
         LOG.exception(_('DB error: %s') % e)
-        raise exception.ApiError(_("Cannot create instance_type with "
-                                    "name %(name)s and flavorid %(flavorid)s")
-                                    % locals())
+        msg = _("Cannot create instance_type with name %(name)s and "
+                "flavorid %(flavorid)s") % locals()
+        raise exception.ApiError(msg)
 
 
 def destroy(name):
     """Marks instance types as deleted."""
-    if name is None:
-        raise exception.InvalidInstanceType(instance_type=name)
-    else:
-        try:
-            db.instance_type_destroy(context.get_admin_context(), name)
-        except exception.NotFound:
-            LOG.exception(_('Instance type %s not found for deletion') % name)
-            raise exception.ApiError(_("Unknown instance type: %s") % name)
+    try:
+        assert name is not None
+        db.instance_type_destroy(context.get_admin_context(), name)
+    except (AssertionError, exception.NotFound):
+        LOG.exception(_('Instance type %s not found for deletion') % name)
+        raise exception.InstanceTypeNotFoundByName(instance_type_name=name)
 
 
 def purge(name):
     """Removes instance types from database."""
-    if name is None:
-        raise exception.InvalidInstanceType(instance_type=name)
-    else:
-        try:
-            db.instance_type_purge(context.get_admin_context(), name)
-        except exception.NotFound:
-            LOG.exception(_('Instance type %s not found for purge') % name)
-            raise exception.ApiError(_("Unknown instance type: %s") % name)
+    try:
+        assert name is not None
+        db.instance_type_purge(context.get_admin_context(), name)
+    except (AssertionError, exception.NotFound):
+        LOG.exception(_('Instance type %s not found for purge') % name)
+        raise exception.InstanceTypeNotFoundByName(instance_type_name=name)
 
 
-def get_all_types(inactive=0):
+def get_all_types(inactive=0, filters=None):
     """Get all non-deleted instance_types.
 
     Pass true as argument if you want deleted instance types returned also.
 
     """
     ctxt = context.get_admin_context()
-    inst_types = db.instance_type_get_all(ctxt, inactive)
+    inst_types = db.instance_type_get_all(ctxt, inactive, filters)
     inst_type_dict = {}
     for inst_type in inst_types:
         inst_type_dict[inst_type['name']] = inst_type
@@ -110,23 +116,27 @@ def get_default_instance_type():
         raise exception.ApiError(_("Unknown instance type: %s") % name)
 
 
-def get_instance_type(id):
+def get_instance_type(instance_type_id):
     """Retrieves single instance type by id."""
-    if id is None:
+    if instance_type_id is None:
         return get_default_instance_type()
+
+    ctxt = context.get_admin_context()
     try:
-        ctxt = context.get_admin_context()
-        return db.instance_type_get(ctxt, id)
+        return db.instance_type_get(ctxt, instance_type_id)
     except exception.DBError:
-        raise exception.ApiError(_("Unknown instance type: %s") % id)
+        msg = _("Unknown instance type: %s") % instance_type_id
+        raise exception.ApiError(msg)
 
 
 def get_instance_type_by_name(name):
     """Retrieves single instance type by name."""
     if name is None:
         return get_default_instance_type()
+
+    ctxt = context.get_admin_context()
+
     try:
-        ctxt = context.get_admin_context()
         return db.instance_type_get_by_name(ctxt, name)
     except exception.DBError:
         raise exception.ApiError(_("Unknown instance type: %s") % name)
@@ -134,10 +144,10 @@ def get_instance_type_by_name(name):
 
 # TODO(termie): flavor-specific code should probably be in the API that uses
 #               flavors.
-def get_instance_type_by_flavor_id(flavor_id):
-    """Retrieve instance type by flavor_id."""
+def get_instance_type_by_flavor_id(flavorid):
+    """Retrieve instance type by flavorid."""
     ctxt = context.get_admin_context()
     try:
-        return db.instance_type_get_by_flavor_id(ctxt, flavor_id)
-    except ValueError:
-        raise exception.FlavorNotFound(flavor_id=flavor_id)
+        return db.instance_type_get_by_flavor_id(ctxt, flavorid)
+    except exception.DBError:
+        raise exception.ApiError(_("Unknown instance type: %s") % flavorid)
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 2ccf44050..2fc2972bf 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -43,23 +43,24 @@ import time
 
 from eventlet import greenthread
 
-import nova.context
 from nova import block_device
+import nova.context
+from nova.compute import instance_types
+from nova.compute import power_state
+from nova.compute import task_states
+from nova.compute.utils import notify_usage_exists
+from nova.compute import vm_states
 from nova import exception
 from nova import flags
 import nova.image
 from nova import log as logging
 from nova import manager
 from nova import network
+from nova.notifier import api as notifier
 from nova import rpc
 from nova import utils
-from nova import volume
-from nova.compute import power_state
-from nova.compute import task_states
-from nova.compute import vm_states
-from nova.notifier import api as notifier
-from nova.compute.utils import notify_usage_exists
 from nova.virt import driver
+from nova import volume
 
 
 FLAGS = flags.FLAGS
@@ -342,8 +343,7 @@ class ComputeManager(manager.SchedulerDependentManager):
                 return
 
             instance_type_id = instance['instance_type_id']
-            instance_type = self.db.instance_type_get(context,
-                    instance_type_id)
+            instance_type = instance_types.get_instance_type(instance_type_id)
             allowed_size_gb = instance_type['local_gb']
 
             # NOTE(jk0): Since libvirt uses local_gb as a secondary drive, we
@@ -985,8 +985,8 @@ class ComputeManager(manager.SchedulerDependentManager):
         instance_ref = self.db.instance_get_by_uuid(context,
                 migration_ref.instance_uuid)
 
-        instance_type = self.db.instance_type_get(context,
-                migration_ref['old_instance_type_id'])
+        old_instance_type = migration_ref['old_instance_type_id']
+        instance_type = instance_types.get_instance_type(old_instance_type)
 
         # Just roll back the record. There's no need to resize down since
         # the 'old' VM already has the preferred attributes
@@ -1029,10 +1029,10 @@ class ComputeManager(manager.SchedulerDependentManager):
             msg = _('Migration error: destination same as source!')
             raise exception.Error(msg)
 
-        old_instance_type = self.db.instance_type_get(context,
-                instance_ref['instance_type_id'])
-        new_instance_type = self.db.instance_type_get(context,
-                instance_type_id)
+        old_instance_type_id = instance_ref['instance_type_id']
+        old_instance_type = instance_types.get_instance_type(
+                old_instance_type_id)
+        new_instance_type = instance_types.get_instance_type(instance_type_id)
 
         migration_ref = self.db.migration_create(context,
                 {'instance_uuid': instance_ref['uuid'],
@@ -1103,10 +1103,11 @@ class ComputeManager(manager.SchedulerDependentManager):
         resize_instance = False
         instance_ref = self.db.instance_get_by_uuid(context,
                 migration_ref.instance_uuid)
-        if migration_ref['old_instance_type_id'] != \
-           migration_ref['new_instance_type_id']:
-            instance_type = self.db.instance_type_get(context,
-                    migration_ref['new_instance_type_id'])
+        old_instance_type_id = migration_ref['old_instance_type_id']
+        new_instance_type_id = migration_ref['new_instance_type_id']
+        if old_instance_type_id != new_instance_type_id:
+            instance_type = instance_types.get_instance_type(
+                    new_instance_type_id)
             self.db.instance_update(context, instance_ref.uuid,
                    dict(instance_type_id=instance_type['id'],
                         memory_mb=instance_type['memory_mb'],