From 198af0ef9e65bc4c2efe74b9d93cf40210eb77bc Mon Sep 17 00:00:00 2001
From: "jaypipes@gmail.com" <>
Date: Thu, 21 Oct 2010 14:29:34 -0400
Subject: Moves db writes into compute manager class. Cleans up sqlalchemy
 model/api to remove redundant calls for updating what is really a dict.

---
 nova/api/ec2/cloud.py                    | 43 ++++++++++++---------
 nova/api/openstack/servers.py            | 25 ++++++------
 nova/compute/manager.py                  | 35 +++++++++++++++++
 nova/db/sqlalchemy/api.py                | 66 +++++++++++---------------------
 nova/db/sqlalchemy/models.py             | 10 +++++
 nova/tests/api/openstack/fakes.py        |  1 +
 nova/tests/api/openstack/test_servers.py |  4 +-
 7 files changed, 104 insertions(+), 80 deletions(-)

(limited to 'nova')

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 6d4f58499..096ddf668 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -97,6 +97,7 @@ class CloudController(object):
 """
     def __init__(self):
         self.network_manager = utils.import_object(FLAGS.network_manager)
+        self.compute_manager = utils.import_object(FLAGS.compute_manager)
         self.setup()
 
     def __str__(self):
@@ -846,27 +847,29 @@ class CloudController(object):
         elevated = context.elevated()
 
         for num in range(num_instances):
-            instance_ref = db.instance_create(context, base_options)
-            inst_id = instance_ref['id']
+            
+            instance_data = base_options
+            instance_data['mac_address'] = utils.generate_mac()
+            instance_data['launch_index'] = num
 
-            for security_group_id in security_groups:
-                db.instance_add_security_group(elevated,
-                                               inst_id,
-                                               security_group_id)
+            instance_ref = self.compute_manager.create_instance(context,
+                                                           instance_data,
+                                                           security_groups)
 
-            inst = {}
-            inst['mac_address'] = utils.generate_mac()
-            inst['launch_index'] = num
             internal_id = instance_ref['internal_id']
             ec2_id = internal_id_to_ec2_id(internal_id)
-            inst['hostname'] = ec2_id
-            db.instance_update(context, inst_id, inst)
+            instance_ref['hostname'] = ec2_id
+
+            self.compute_manager.update_instance(context,
+                                                 instance_ref['id'],
+                                                 instance_ref)
+
             # TODO(vish): This probably should be done in the scheduler
             #             or in compute as a call.  The network should be
             #             allocated after the host is assigned and setup
             #             can happen at the same time.
             address = self.network_manager.allocate_fixed_ip(context,
-                                                             inst_id,
+                                                             instance_ref['id'],
                                                              vpn)
             network_topic = self._get_network_topic(context)
             rpc.cast(elevated,
@@ -878,9 +881,9 @@ class CloudController(object):
                      FLAGS.scheduler_topic,
                      {"method": "run_instance",
                       "args": {"topic": FLAGS.compute_topic,
-                               "instance_id": inst_id}})
+                               "instance_id": instance_ref['id']}})
             logging.debug("Casting to scheduler for %s/%s's instance %s" %
-                      (context.project.name, context.user.name, inst_id))
+                      (context.project.name, context.user.name, instance_ref['id']))
         return self._format_run_instances(context, reservation_id)
 
 
@@ -907,11 +910,13 @@ class CloudController(object):
                               id_str)
                 continue
             now = datetime.datetime.utcnow()
-            db.instance_update(context,
-                               instance_ref['id'],
-                               {'state_description': 'terminating',
-                                'state': 0,
-                                'terminated_at': now})
+            updated_data = {'state_description': 'terminating',
+                            'state': 0,
+                            'terminated_at': now}
+            self.compute_manager.update_instance(context,
+                                                 instance_ref['id'],
+                                                 updated_data)
+
             # FIXME(ja): where should network deallocate occur?
             address = db.instance_get_floating_address(context,
                                                        instance_ref['id'])
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index a73591ccc..6ce364eb7 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -94,6 +94,7 @@ class Controller(wsgi.Controller):
             db_driver = FLAGS.db_driver
         self.db_driver = utils.import_object(db_driver)
         self.network_manager = utils.import_object(FLAGS.network_manager)
+        self.compute_manager = utils.import_object(FLAGS.compute_manager)
         super(Controller, self).__init__()
 
     def index(self, req):
@@ -241,34 +242,30 @@ class Controller(wsgi.Controller):
         inst['memory_mb'] = flavor['memory_mb']
         inst['vcpus'] = flavor['vcpus']
         inst['local_gb'] = flavor['local_gb']
-
-        ref = self.db_driver.instance_create(ctxt, inst)
-        inst['id'] = ref.internal_id
-
         inst['mac_address'] = utils.generate_mac()
-
-        #TODO(dietz) is this necessary?
         inst['launch_index'] = 0
 
-        inst['hostname'] = str(ref.internal_id)
-        self.db_driver.instance_update(ctxt, inst['id'], inst)
+        ref = self.compute_manager.create_instance(ctxt, inst)
+        inst['id'] = ref['internal_id']
+
+        inst['hostname'] = str(ref['internal_id'])
+        self.compute_manager.update_instance(ctxt, inst['id'], inst)
 
-        network_manager = utils.import_object(FLAGS.network_manager)
-        address = network_manager.allocate_fixed_ip(ctxt,
-            inst['id'])
+        address = self.network_manager.allocate_fixed_ip(ctxt,
+                                                         inst['id'])
 
         # TODO(vish): This probably should be done in the scheduler
         #             network is setup when host is assigned
-        network_topic = self._get_network_topic(ctxt, network_manager)
+        network_topic = self._get_network_topic(ctxt)
         rpc.call(ctxt,
                  network_topic,
                  {"method": "setup_fixed_ip",
                   "args": {"address": address}})
         return inst
 
-    def _get_network_topic(self, context, network_manager):
+    def _get_network_topic(self, context):
         """Retrieves the network host for a project"""
-        network_ref = network_manager.get_network(context)
+        network_ref = self.network_manager.get_network(context)
         host = network_ref['host']
         if not host:
             host = rpc.call(context,
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 523bb8893..c752d954b 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -67,6 +67,41 @@ class ComputeManager(manager.Manager):
     def refresh_security_group(self, context, security_group_id, **_kwargs):
         yield self.driver.refresh_security_group(security_group_id)
 
+
+    def create_instance(self, context, instance_data, security_groups=[]):
+        """Creates the instance in the datastore and returns the
+        new instance as a mapping
+
+        :param context: The security context
+        :param instance_data: mapping of instance options
+        :param security_groups: list of security group ids to
+                                attach to the instance
+
+        :retval Returns a mapping of the instance information
+                that has just been created
+
+        """
+        instance_ref = self.db.instance_create(context, instance_data)
+        inst_id = instance_ref['id']
+
+        elevated = context.elevated()
+        for security_group_id in security_groups:
+            self.db.instance_add_security_group(elevated,
+                                                inst_id,
+                                                security_group_id)
+        return instance_ref
+
+    def update_instance(self, context, instance_id, instance_data):
+        """Updates the instance in the datastore
+
+        :param context: The security context
+        :param instance_data: mapping of instance options
+
+        :retval None
+
+        """
+        self.db.instance_update(context, instance_id, instance_data)
+
     @defer.inlineCallbacks
     @exception.wrap_exception
     def run_instance(self, context, instance_id, **_kwargs):
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 209d6e51f..74fd0fdc8 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -235,8 +235,7 @@ def service_get_by_args(context, host, binary):
 @require_admin_context
 def service_create(context, values):
     service_ref = models.Service()
-    for (key, value) in values.iteritems():
-        service_ref[key] = value
+    service_ref.update(values)
     service_ref.save()
     return service_ref
 
@@ -246,8 +245,7 @@ def service_update(context, service_id, values):
     session = get_session()
     with session.begin():
         service_ref = service_get(context, service_id, session=session)
-        for (key, value) in values.iteritems():
-            service_ref[key] = value
+        service_ref.update(values)
         service_ref.save(session=session)
 
 
@@ -278,8 +276,7 @@ def floating_ip_allocate_address(context, host, project_id):
 @require_context
 def floating_ip_create(context, values):
     floating_ip_ref = models.FloatingIp()
-    for (key, value) in values.iteritems():
-        floating_ip_ref[key] = value
+    floating_ip_ref.update(values)
     floating_ip_ref.save()
     return floating_ip_ref['address']
 
@@ -450,8 +447,7 @@ def fixed_ip_associate_pool(context, network_id, instance_id):
 @require_context
 def fixed_ip_create(_context, values):
     fixed_ip_ref = models.FixedIp()
-    for (key, value) in values.iteritems():
-        fixed_ip_ref[key] = value
+    fixed_ip_ref.update(values)
     fixed_ip_ref.save()
     return fixed_ip_ref['address']
 
@@ -520,8 +516,7 @@ def fixed_ip_update(context, address, values):
         fixed_ip_ref = fixed_ip_get_by_address(context,
                                                address,
                                                session=session)
-        for (key, value) in values.iteritems():
-            fixed_ip_ref[key] = value
+        fixed_ip_ref.update(values)
         fixed_ip_ref.save(session=session)
 
 
@@ -534,8 +529,7 @@ def fixed_ip_update(context, address, values):
 @require_context
 def instance_create(context, values):
     instance_ref = models.Instance()
-    for (key, value) in values.iteritems():
-        instance_ref[key] = value
+    instance_ref.update(values)
 
     session = get_session()
     with session.begin():
@@ -727,8 +721,7 @@ def instance_update(context, instance_id, values):
     session = get_session()
     with session.begin():
         instance_ref = instance_get(context, instance_id, session=session)
-        for (key, value) in values.iteritems():
-            instance_ref[key] = value
+        instance_ref.update(values)
         instance_ref.save(session=session)
 
 
@@ -750,8 +743,7 @@ def instance_add_security_group(context, instance_id, security_group_id):
 @require_context
 def key_pair_create(context, values):
     key_pair_ref = models.KeyPair()
-    for (key, value) in values.iteritems():
-        key_pair_ref[key] = value
+    key_pair_ref.update(values)
     key_pair_ref.save()
     return key_pair_ref
 
@@ -866,8 +858,7 @@ def network_count_reserved_ips(context, network_id):
 @require_admin_context
 def network_create_safe(context, values):
     network_ref = models.Network()
-    for (key, value) in values.iteritems():
-        network_ref[key] = value
+    network_ref.update(values)
     try:
         network_ref.save()
         return network_ref
@@ -976,8 +967,7 @@ def network_update(context, network_id, values):
     session = get_session()
     with session.begin():
         network_ref = network_get(context, network_id, session=session)
-        for (key, value) in values.iteritems():
-            network_ref[key] = value
+        network_ref.update(values)
         network_ref.save(session=session)
 
 
@@ -1027,8 +1017,7 @@ def export_device_count(context):
 @require_admin_context
 def export_device_create_safe(context, values):
     export_device_ref = models.ExportDevice()
-    for (key, value) in values.iteritems():
-        export_device_ref[key] = value
+    export_device_ref.update(values)
     try:
         export_device_ref.save()
         return export_device_ref
@@ -1054,8 +1043,7 @@ def auth_get_token(_context, token_hash):
 
 def auth_create_token(_context, token):
     tk = models.AuthToken()
-    for k,v in token.iteritems():
-        tk[k] = v
+    tk.update(token)
     tk.save()
     return tk
 
@@ -1081,8 +1069,7 @@ def quota_get(context, project_id, session=None):
 @require_admin_context
 def quota_create(context, values):
     quota_ref = models.Quota()
-    for (key, value) in values.iteritems():
-        quota_ref[key] = value
+    quota_ref.update(values)
     quota_ref.save()
     return quota_ref
 
@@ -1092,8 +1079,7 @@ def quota_update(context, project_id, values):
     session = get_session()
     with session.begin():
         quota_ref = quota_get(context, project_id, session=session)
-        for (key, value) in values.iteritems():
-            quota_ref[key] = value
+        quota_ref.update(values)
         quota_ref.save(session=session)
 
 
@@ -1141,8 +1127,7 @@ def volume_attached(context, volume_id, instance_id, mountpoint):
 @require_context
 def volume_create(context, values):
     volume_ref = models.Volume()
-    for (key, value) in values.iteritems():
-        volume_ref[key] = value
+    volume_ref.update(values)
 
     session = get_session()
     with session.begin():
@@ -1298,8 +1283,7 @@ def volume_update(context, volume_id, values):
     session = get_session()
     with session.begin():
         volume_ref = volume_get(context, volume_id, session=session)
-        for (key, value) in values.iteritems():
-            volume_ref[key] = value
+        volume_ref.update(values)
         volume_ref.save(session=session)
 
 
@@ -1392,8 +1376,7 @@ def security_group_create(context, values):
     # FIXME(devcamcar): Unless I do this, rules fails with lazy load exception
     # once save() is called.  This will get cleaned up in next orm pass.
     security_group_ref.rules
-    for (key, value) in values.iteritems():
-        security_group_ref[key] = value
+    security_group_ref.update(values)
     security_group_ref.save()
     return security_group_ref
 
@@ -1446,8 +1429,7 @@ def security_group_rule_get(context, security_group_rule_id, session=None):
 @require_context
 def security_group_rule_create(context, values):
     security_group_rule_ref = models.SecurityGroupIngressRule()
-    for (key, value) in values.iteritems():
-        security_group_rule_ref[key] = value
+    security_group_rule_ref.update(values)
     security_group_rule_ref.save()
     return security_group_rule_ref
 
@@ -1498,8 +1480,7 @@ def user_get_by_access_key(context, access_key, session=None):
 @require_admin_context
 def user_create(_context, values):
     user_ref = models.User()
-    for (key, value) in values.iteritems():
-        user_ref[key] = value
+    user_ref.update(values)
     user_ref.save()
     return user_ref
 
@@ -1527,8 +1508,7 @@ def user_get_all(context):
 
 def project_create(_context, values):
     project_ref = models.Project()
-    for (key, value) in values.iteritems():
-        project_ref[key] = value
+    project_ref.update(values)
     project_ref.save()
     return project_ref
 
@@ -1590,8 +1570,7 @@ def user_update(context, user_id, values):
     session = get_session()
     with session.begin():
         user_ref = user_get(context, user_id, session=session)
-        for (key, value) in values.iteritems():
-            user_ref[key] = value
+        user_ref.update(values)
         user_ref.save(session=session)
 
 
@@ -1599,8 +1578,7 @@ def project_update(context, project_id, values):
     session = get_session()
     with session.begin():
         project_ref = project_get(context, project_id, session=session)
-        for (key, value) in values.iteritems():
-            project_ref[key] = value
+        project_ref.update(values)
         project_ref.save(session=session)
 
 
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index a63bca2b0..853c320e4 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -90,6 +90,16 @@ class NovaBase(object):
         n = self._i.next().name
         return n, getattr(self, n)
 
+    def update(self, values):
+        """Make the model object behave like a dict"""
+        for k, v in values.iteritems():
+            setattr(self, k, v)
+
+    def iteritems(self):
+        """Make the model object behave like a dict"""
+        return iter(self)
+
+
 # TODO(vish): Store images in the database instead of file system
 #class Image(BASE, NovaBase):
 #    """Represents an image in the datastore"""
diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py
index 14170fbb2..f12c7b610 100644
--- a/nova/tests/api/openstack/fakes.py
+++ b/nova/tests/api/openstack/fakes.py
@@ -30,6 +30,7 @@ from nova import exception as exc
 import nova.api.openstack.auth
 from nova.image import service
 from nova.image.services import glance
+from nova.tests import fake_flags
 from nova.wsgi import Router
 
 
diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py
index d1ee533b6..f4a09fd97 100644
--- a/nova/tests/api/openstack/test_servers.py
+++ b/nova/tests/api/openstack/test_servers.py
@@ -92,9 +92,7 @@ class ServersTest(unittest.TestCase):
             pass 
 
         def instance_create(context, inst):
-            class Foo(object):
-                internal_id = 1
-            return Foo()
+            return {'id': 1, 'internal_id': 1}
 
         def fake_method(*args, **kwargs):
             pass
-- 
cgit 


From d8d12549a5e47c7c44f449f12d6b556e2c56483d Mon Sep 17 00:00:00 2001
From: Eric Day <eday@oddments.org>
Date: Tue, 26 Oct 2010 15:37:32 -0700
Subject: More PEP8 fixes that were introduced in the last couple commits.

---
 nova/tests/api/openstack/test_api.py | 6 ++++++
 nova/utils.py                        | 5 ++---
 2 files changed, 8 insertions(+), 3 deletions(-)

(limited to 'nova')

diff --git a/nova/tests/api/openstack/test_api.py b/nova/tests/api/openstack/test_api.py
index a8c0ff9f8..dd83991b9 100644
--- a/nova/tests/api/openstack/test_api.py
+++ b/nova/tests/api/openstack/test_api.py
@@ -24,22 +24,28 @@ from nova.api.openstack import API
 from nova.api.openstack import faults
 from webob import Request
 
+
 class APITest(unittest.TestCase):
 
     def test_exceptions_are_converted_to_faults(self):
+
         @webob.dec.wsgify
         def succeed(req):
             return 'Succeeded'
+
         @webob.dec.wsgify
         def raise_webob_exc(req):
             raise webob.exc.HTTPNotFound(explanation='Raised a webob.exc')
+
         @webob.dec.wsgify
         def fail(req):
             raise Exception("Threw an exception")
+
         @webob.dec.wsgify
         def raise_api_fault(req):
             exc = webob.exc.HTTPNotFound(explanation='Raised a webob.exc')
             return faults.Fault(exc)
+
         api = API()
 
         api.application = succeed
diff --git a/nova/utils.py b/nova/utils.py
index 2c53b027e..bc495a691 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -213,10 +213,10 @@ def deferredToThread(f):
 
 def xhtml_escape(value):
     """Escapes a string so it is valid within XML or XHTML.
-    
+
     Code is directly from the utf8 function in
     http://github.com/facebook/tornado/blob/master/tornado/escape.py
-    
+
     """
     return saxutils.escape(value, {'"': "&quot;"})
 
@@ -232,4 +232,3 @@ def utf8(value):
         return value.encode("utf-8")
     assert isinstance(value, str)
     return value
-    
-- 
cgit 


From 79acdcca7d37e81d626be7a3369394ef9dface1b Mon Sep 17 00:00:00 2001
From: "jaypipes@gmail.com" <>
Date: Wed, 27 Oct 2010 11:10:50 -0400
Subject: Style cleanups and review from Eric.

---
 nova/api/ec2/cloud.py   | 23 ++++++++++++-----------
 nova/compute/manager.py | 15 ++++++++++++---
 2 files changed, 24 insertions(+), 14 deletions(-)

(limited to 'nova')

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 51e972aa7..9084958a1 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -836,29 +836,30 @@ class CloudController(object):
         elevated = context.elevated()
 
         for num in range(num_instances):
-            
+
             instance_data = base_options
-            instance_data['mac_address'] = utils.generate_mac()
-            instance_data['launch_index'] = num
 
             instance_ref = self.compute_manager.create_instance(context,
-                                                           instance_data,
-                                                           security_groups)
+                                           instance_data,
+                                           security_groups,
+                                           mac_address=utils.generate_mac(),
+                                           launch_index=num)
+            inst_id = instance_ref['id']
 
             internal_id = instance_ref['internal_id']
             ec2_id = internal_id_to_ec2_id(internal_id)
-            instance_ref['hostname'] = ec2_id
 
             self.compute_manager.update_instance(context,
-                                                 instance_ref['id'],
-                                                 instance_ref)
+                                                 inst_id,
+                                                 instance_ref,
+                                                 hostname=ec2_id)
 
             # TODO(vish): This probably should be done in the scheduler
             #             or in compute as a call.  The network should be
             #             allocated after the host is assigned and setup
             #             can happen at the same time.
             address = self.network_manager.allocate_fixed_ip(context,
-                                                             instance_ref['id'],
+                                                             inst_id,
                                                              vpn)
             network_topic = self._get_network_topic(context)
             rpc.cast(elevated,
@@ -870,9 +871,9 @@ class CloudController(object):
                      FLAGS.scheduler_topic,
                      {"method": "run_instance",
                       "args": {"topic": FLAGS.compute_topic,
-                               "instance_id": instance_ref['id']}})
+                               "instance_id": inst_id}})
             logging.debug("Casting to scheduler for %s/%s's instance %s" %
-                      (context.project.name, context.user.name, instance_ref['id']))
+                      (context.project.name, context.user.name, inst_id))
         return self._format_run_instances(context, reservation_id)
 
     def terminate_instances(self, context, instance_id, **kwargs):
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index d99d938af..c04dd213a 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -69,8 +69,8 @@ class ComputeManager(manager.Manager):
     def refresh_security_group(self, context, security_group_id, **_kwargs):
         yield self.driver.refresh_security_group(security_group_id)
 
-
-    def create_instance(self, context, instance_data, security_groups=[]):
+    def create_instance(self, context, instance_data, security_groups=[],
+                        **kwargs):
         """Creates the instance in the datastore and returns the
         new instance as a mapping
 
@@ -78,11 +78,15 @@ class ComputeManager(manager.Manager):
         :param instance_data: mapping of instance options
         :param security_groups: list of security group ids to
                                 attach to the instance
+        :param **kwargs: All additional keyword args are treated
+                         as data fields of the instance to be
+                         created
 
         :retval Returns a mapping of the instance information
                 that has just been created
 
         """
+        instance_data.update(kwargs)
         instance_ref = self.db.instance_create(context, instance_data)
         inst_id = instance_ref['id']
 
@@ -93,15 +97,20 @@ class ComputeManager(manager.Manager):
                                                 security_group_id)
         return instance_ref
 
-    def update_instance(self, context, instance_id, instance_data):
+    def update_instance(self, context, instance_id, instance_data,
+                        **kwargs):
         """Updates the instance in the datastore
 
         :param context: The security context
         :param instance_data: mapping of instance options
+        :param **kwargs: All additional keyword args are treated
+                         as data fields of the instance to be
+                         updated
 
         :retval None
 
         """
+        instance_data.update(kwargs)
         self.db.instance_update(context, instance_id, instance_data)
 
     @defer.inlineCallbacks
-- 
cgit 


From 213b9987365c4b336b63e08e1ca187a43d00fa3d Mon Sep 17 00:00:00 2001
From: "jaypipes@gmail.com" <>
Date: Wed, 27 Oct 2010 14:55:01 -0400
Subject: OK, let's try this one more time.

---
 nova/api/ec2/cloud.py         | 13 ++++++-------
 nova/api/openstack/servers.py |  4 ++--
 nova/compute/manager.py       | 16 ++++++----------
 3 files changed, 14 insertions(+), 19 deletions(-)

(limited to 'nova')

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 9084958a1..7b6144ba5 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -837,13 +837,11 @@ class CloudController(object):
 
         for num in range(num_instances):
 
-            instance_data = base_options
-
             instance_ref = self.compute_manager.create_instance(context,
-                                           instance_data,
                                            security_groups,
                                            mac_address=utils.generate_mac(),
-                                           launch_index=num)
+                                           launch_index=num,
+                                           **base_options)
             inst_id = instance_ref['id']
 
             internal_id = instance_ref['internal_id']
@@ -851,7 +849,6 @@ class CloudController(object):
 
             self.compute_manager.update_instance(context,
                                                  inst_id,
-                                                 instance_ref,
                                                  hostname=ec2_id)
 
             # TODO(vish): This probably should be done in the scheduler
@@ -903,8 +900,10 @@ class CloudController(object):
                             'state': 0,
                             'terminated_at': now}
             self.compute_manager.update_instance(context,
-                                                 instance_ref['id'],
-                                                 updated_data)
+                                         instance_ref['id'],
+                                         state_description='terminating',
+                                         state=0,
+                                         terminated_at=now)
 
             # FIXME(ja): where should network deallocate occur?
             address = db.instance_get_floating_address(context,
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index e1a254d4e..1d8aa2fa4 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -246,11 +246,11 @@ class Controller(wsgi.Controller):
         inst['mac_address'] = utils.generate_mac()
         inst['launch_index'] = 0
 
-        ref = self.compute_manager.create_instance(ctxt, inst)
+        ref = self.compute_manager.create_instance(ctxt, **inst)
         inst['id'] = ref['internal_id']
 
         inst['hostname'] = str(ref['internal_id'])
-        self.compute_manager.update_instance(ctxt, inst['id'], inst)
+        self.compute_manager.update_instance(ctxt, inst['id'], **inst)
 
         address = self.network_manager.allocate_fixed_ip(ctxt,
                                                          inst['id'])
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index c04dd213a..7cdd6b110 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -69,13 +69,11 @@ class ComputeManager(manager.Manager):
     def refresh_security_group(self, context, security_group_id, **_kwargs):
         yield self.driver.refresh_security_group(security_group_id)
 
-    def create_instance(self, context, instance_data, security_groups=[],
-                        **kwargs):
+    def create_instance(self, context, security_groups=[], **kwargs):
         """Creates the instance in the datastore and returns the
         new instance as a mapping
 
         :param context: The security context
-        :param instance_data: mapping of instance options
         :param security_groups: list of security group ids to
                                 attach to the instance
         :param **kwargs: All additional keyword args are treated
@@ -86,23 +84,22 @@ class ComputeManager(manager.Manager):
                 that has just been created
 
         """
-        instance_data.update(kwargs)
-        instance_ref = self.db.instance_create(context, instance_data)
+        instance_ref = self.db.instance_create(context, kwargs)
         inst_id = instance_ref['id']
 
         elevated = context.elevated()
+        security_groups = kwargs.get('security_groups', [])
         for security_group_id in security_groups:
             self.db.instance_add_security_group(elevated,
                                                 inst_id,
                                                 security_group_id)
         return instance_ref
 
-    def update_instance(self, context, instance_id, instance_data,
-                        **kwargs):
+    def update_instance(self, context, instance_id, **kwargs):
         """Updates the instance in the datastore
 
         :param context: The security context
-        :param instance_data: mapping of instance options
+        :param instance_id: ID of the instance to update
         :param **kwargs: All additional keyword args are treated
                          as data fields of the instance to be
                          updated
@@ -110,8 +107,7 @@ class ComputeManager(manager.Manager):
         :retval None
 
         """
-        instance_data.update(kwargs)
-        self.db.instance_update(context, instance_id, instance_data)
+        self.db.instance_update(context, instance_id, kwargs)
 
     @defer.inlineCallbacks
     @exception.wrap_exception
-- 
cgit 


From 4012860b57593632d1f0061099e0d211dba58a59 Mon Sep 17 00:00:00 2001
From: "jaypipes@gmail.com" <>
Date: Thu, 28 Oct 2010 11:43:08 -0400
Subject: Remove unused updated_data variable

---
 nova/api/ec2/cloud.py | 3 ---
 1 file changed, 3 deletions(-)

(limited to 'nova')

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 7b6144ba5..f2a6dc3b0 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -896,9 +896,6 @@ class CloudController(object):
                               id_str)
                 continue
             now = datetime.datetime.utcnow()
-            updated_data = {'state_description': 'terminating',
-                            'state': 0,
-                            'terminated_at': now}
             self.compute_manager.update_instance(context,
                                          instance_ref['id'],
                                          state_description='terminating',
-- 
cgit 


From 7cc4bcd344221d517054641171f759b88112a459 Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Thu, 28 Oct 2010 20:13:40 -0400
Subject: Pep-257 cleanups.

---
 nova/db/api.py | 128 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 70 insertions(+), 58 deletions(-)

(limited to 'nova')

diff --git a/nova/db/api.py b/nova/db/api.py
index 0731e2e05..7cce591ad 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -16,7 +16,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 """
-Defines interface for DB access
+Defines interface for DB access.
 """
 
 from nova import exception
@@ -34,17 +34,17 @@ IMPL = utils.LazyPluggable(FLAGS['db_backend'],
 
 
 class NoMoreAddresses(exception.Error):
-    """No more available addresses"""
+    """No more available addresses."""
     pass
 
 
 class NoMoreBlades(exception.Error):
-    """No more available blades"""
+    """No more available blades."""
     pass
 
 
 class NoMoreNetworks(exception.Error):
-    """No more available networks"""
+    """No more available networks."""
     pass
 
 
@@ -62,30 +62,33 @@ def service_get(context, service_id):
 
 
 def service_get_all_by_topic(context, topic):
-    """Get all compute services for a given topic """
+    """Get all compute services for a given topic."""
     return IMPL.service_get_all_by_topic(context, topic)
 
 
 def service_get_all_compute_sorted(context):
-    """Get all compute services sorted by instance count
+    """Get all compute services sorted by instance count.
+
+    Returns a list of (Service, instance_count) tuples.
 
-    Returns a list of (Service, instance_count) tuples
     """
     return IMPL.service_get_all_compute_sorted(context)
 
 
 def service_get_all_network_sorted(context):
-    """Get all network services sorted by network count
+    """Get all network services sorted by network count.
+
+    Returns a list of (Service, network_count) tuples.
 
-    Returns a list of (Service, network_count) tuples
     """
     return IMPL.service_get_all_network_sorted(context)
 
 
 def service_get_all_volume_sorted(context):
-    """Get all volume services sorted by volume count
+    """Get all volume services sorted by volume count.
+
+    Returns a list of (Service, volume_count) tuples.
 
-    Returns a list of (Service, volume_count) tuples
     """
     return IMPL.service_get_all_volume_sorted(context)
 
@@ -116,6 +119,7 @@ def floating_ip_allocate_address(context, host, project_id):
     """Allocate free floating ip and return the address.
 
     Raises if one is not available.
+
     """
     return IMPL.floating_ip_allocate_address(context, host, project_id)
 
@@ -144,6 +148,7 @@ def floating_ip_disassociate(context, address):
     """Disassociate an floating ip from a fixed ip by address.
 
     Returns the address of the existing fixed ip.
+
     """
     return IMPL.floating_ip_disassociate(context, address)
 
@@ -182,6 +187,7 @@ def fixed_ip_associate(context, address, instance_id):
     """Associate fixed ip to instance.
 
     Raises if fixed ip is not available.
+
     """
     return IMPL.fixed_ip_associate(context, address, instance_id)
 
@@ -190,6 +196,7 @@ def fixed_ip_associate_pool(context, network_id, instance_id):
     """Find free ip in network and associate it to instance.
 
     Raises if one is not available.
+
     """
     return IMPL.fixed_ip_associate_pool(context, network_id, instance_id)
 
@@ -205,7 +212,7 @@ def fixed_ip_disassociate(context, address):
 
 
 def fixed_ip_disassociate_all_by_timeout(context, host, time):
-    """Disassociate old fixed ips from host"""
+    """Disassociate old fixed ips from host."""
     return IMPL.fixed_ip_disassociate_all_by_timeout(context, host, time)
 
 
@@ -283,7 +290,7 @@ def instance_get_floating_address(context, instance_id):
 
 
 def instance_get_by_internal_id(context, internal_id):
-    """Get an instance by ec2 id."""
+    """Get an instance by internal id."""
     return IMPL.instance_get_by_internal_id(context, internal_id)
 
 
@@ -307,7 +314,7 @@ def instance_update(context, instance_id, values):
 
 
 def instance_add_security_group(context, instance_id, security_group_id):
-    """Associate the given security group with the given instance"""
+    """Associate the given security group with the given instance."""
     return IMPL.instance_add_security_group(context, instance_id,
                                             security_group_id)
 
@@ -369,10 +376,12 @@ def network_count_reserved_ips(context, network_id):
 
 
 def network_create_safe(context, values):
-    """Create a network from the values dict
+    """Create a network from the values dict.
 
     The network is only returned if the create succeeds. If the create violates
-    constraints because the network already exists, no exception is raised."""
+    constraints because the network already exists, no exception is raised.
+    
+    """
     return IMPL.network_create_safe(context, values)
 
 
@@ -413,22 +422,22 @@ def network_get_by_instance(context, instance_id):
 
 
 def network_get_index(context, network_id):
-    """Get non-conflicting index for network"""
+    """Get non-conflicting index for network."""
     return IMPL.network_get_index(context, network_id)
 
 
 def network_get_vpn_ip(context, network_id):
-    """Get non-conflicting index for network"""
+    """Get non-conflicting index for network."""
     return IMPL.network_get_vpn_ip(context, network_id)
 
 
 def network_set_cidr(context, network_id, cidr):
-    """Set the Classless Inner Domain Routing for the network"""
+    """Set the Classless Inner Domain Routing for the network."""
     return IMPL.network_set_cidr(context, network_id, cidr)
 
 
 def network_set_host(context, network_id, host_id):
-    """Safely set the host for network"""
+    """Safely set the host for network."""
     return IMPL.network_set_host(context, network_id, host_id)
 
 
@@ -474,7 +483,9 @@ def export_device_create_safe(context, values):
 
     The device is not returned. If the create violates the unique
     constraints because the shelf_id and blade_id already exist,
-    no exception is raised."""
+    no exception is raised.
+    
+    """
     return IMPL.export_device_create_safe(context, values)
 
 
@@ -482,17 +493,17 @@ def export_device_create_safe(context, values):
 
 
 def auth_destroy_token(context, token):
-    """Destroy an auth token"""
+    """Destroy an auth token."""
     return IMPL.auth_destroy_token(context, token)
 
 
 def auth_get_token(context, token_hash):
-    """Retrieves a token given the hash representing it"""
+    """Retrieves a token given the hash representing it."""
     return IMPL.auth_get_token(context, token_hash)
 
 
 def auth_create_token(context, token):
-    """Creates a new token"""
+    """Creates a new token."""
     return IMPL.auth_create_token(context, token)
 
 
@@ -595,47 +606,47 @@ def volume_update(context, volume_id, values):
 
 
 def security_group_get_all(context):
-    """Get all security groups"""
+    """Get all security groups."""
     return IMPL.security_group_get_all(context)
 
 
 def security_group_get(context, security_group_id):
-    """Get security group by its internal id"""
+    """Get security group by its internal id."""
     return IMPL.security_group_get(context, security_group_id)
 
 
 def security_group_get_by_name(context, project_id, group_name):
-    """Returns a security group with the specified name from a project"""
+    """Returns a security group with the specified name from a project."""
     return IMPL.security_group_get_by_name(context, project_id, group_name)
 
 
 def security_group_get_by_project(context, project_id):
-    """Get all security groups belonging to a project"""
+    """Get all security groups belonging to a project."""
     return IMPL.security_group_get_by_project(context, project_id)
 
 
 def security_group_get_by_instance(context, instance_id):
-    """Get security groups to which the instance is assigned"""
+    """Get security groups to which the instance is assigned."""
     return IMPL.security_group_get_by_instance(context, instance_id)
 
 
 def security_group_exists(context, project_id, group_name):
-    """Indicates if a group name exists in a project"""
+    """Indicates if a group name exists in a project."""
     return IMPL.security_group_exists(context, project_id, group_name)
 
 
 def security_group_create(context, values):
-    """Create a new security group"""
+    """Create a new security group."""
     return IMPL.security_group_create(context, values)
 
 
 def security_group_destroy(context, security_group_id):
-    """Deletes a security group"""
+    """Deletes a security group."""
     return IMPL.security_group_destroy(context, security_group_id)
 
 
 def security_group_destroy_all(context):
-    """Deletes a security group"""
+    """Deletes a security group."""
     return IMPL.security_group_destroy_all(context)
 
 
@@ -643,18 +654,18 @@ def security_group_destroy_all(context):
 
 
 def security_group_rule_create(context, values):
-    """Create a new security group"""
+    """Create a new security group."""
     return IMPL.security_group_rule_create(context, values)
 
 
 def security_group_rule_get_by_security_group(context, security_group_id):
-    """Get all rules for a a given security group"""
+    """Get all rules for a a given security group."""
     return IMPL.security_group_rule_get_by_security_group(context,
                                                           security_group_id)
 
 
 def security_group_rule_destroy(context, security_group_rule_id):
-    """Deletes a security group rule"""
+    """Deletes a security group rule."""
     return IMPL.security_group_rule_destroy(context, security_group_rule_id)
 
 
@@ -662,107 +673,107 @@ def security_group_rule_destroy(context, security_group_rule_id):
 
 
 def user_get(context, id):
-    """Get user by id"""
+    """Get user by id."""
     return IMPL.user_get(context, id)
 
 
 def user_get_by_uid(context, uid):
-    """Get user by uid"""
+    """Get user by uid."""
     return IMPL.user_get_by_uid(context, uid)
 
 
 def user_get_by_access_key(context, access_key):
-    """Get user by access key"""
+    """Get user by access key."""
     return IMPL.user_get_by_access_key(context, access_key)
 
 
 def user_create(context, values):
-    """Create a new user"""
+    """Create a new user."""
     return IMPL.user_create(context, values)
 
 
 def user_delete(context, id):
-    """Delete a user"""
+    """Delete a user."""
     return IMPL.user_delete(context, id)
 
 
 def user_get_all(context):
-    """Create a new user"""
+    """Create a new user."""
     return IMPL.user_get_all(context)
 
 
 def user_add_role(context, user_id, role):
-    """Add another global role for user"""
+    """Add another global role for user."""
     return IMPL.user_add_role(context, user_id, role)
 
 
 def user_remove_role(context, user_id, role):
-    """Remove global role from user"""
+    """Remove global role from user."""
     return IMPL.user_remove_role(context, user_id, role)
 
 
 def user_get_roles(context, user_id):
-    """Get global roles for user"""
+    """Get global roles for user."""
     return IMPL.user_get_roles(context, user_id)
 
 
 def user_add_project_role(context, user_id, project_id, role):
-    """Add project role for user"""
+    """Add project role for user."""
     return IMPL.user_add_project_role(context, user_id, project_id, role)
 
 
 def user_remove_project_role(context, user_id, project_id, role):
-    """Remove project role from user"""
+    """Remove project role from user."""
     return IMPL.user_remove_project_role(context, user_id, project_id, role)
 
 
 def user_get_roles_for_project(context, user_id, project_id):
-    """Return list of roles a user holds on project"""
+    """Return list of roles a user holds on project."""
     return IMPL.user_get_roles_for_project(context, user_id, project_id)
 
 
 def user_update(context, user_id, values):
-    """Update user"""
+    """Update user."""
     return IMPL.user_update(context, user_id, values)
 
 
 def project_get(context, id):
-    """Get project by id"""
+    """Get project by id."""
     return IMPL.project_get(context, id)
 
 
 def project_create(context, values):
-    """Create a new project"""
+    """Create a new project."""
     return IMPL.project_create(context, values)
 
 
 def project_add_member(context, project_id, user_id):
-    """Add user to project"""
+    """Add user to project."""
     return IMPL.project_add_member(context, project_id, user_id)
 
 
 def project_get_all(context):
-    """Get all projects"""
+    """Get all projects."""
     return IMPL.project_get_all(context)
 
 
 def project_get_by_user(context, user_id):
-    """Get all projects of which the given user is a member"""
+    """Get all projects of which the given user is a member."""
     return IMPL.project_get_by_user(context, user_id)
 
 
 def project_remove_member(context, project_id, user_id):
-    """Remove the given user from the given project"""
+    """Remove the given user from the given project."""
     return IMPL.project_remove_member(context, project_id, user_id)
 
 
 def project_update(context, project_id, values):
-    """Update Remove the given user from the given project"""
+    """Update Remove the given user from the given project."""
     return IMPL.project_update(context, project_id, values)
 
 
 def project_delete(context, project_id):
-    """Delete project"""
+    """Delete project."""
     return IMPL.project_delete(context, project_id)
 
 
@@ -771,6 +782,7 @@ def project_delete(context, project_id):
 
 def host_get_networks(context, host):
     """Return all networks for which the given host is the designated
-    network host
+    network host.
+
     """
     return IMPL.host_get_networks(context, host)
-- 
cgit 


From 2132c0de46fd3f1b938e4b3b01b73fb2efaf6a38 Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Thu, 28 Oct 2010 20:28:13 -0400
Subject: Pep-257

---
 nova/db/sqlalchemy/models.py | 47 +++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

(limited to 'nova')

diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 2a3cfa94c..894ebcddd 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -15,7 +15,6 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
 """
 SQLAlchemy models for nova data
 """
@@ -35,13 +34,13 @@ from nova import auth
 from nova import exception
 from nova import flags
 
-FLAGS = flags.FLAGS
 
+FLAGS = flags.FLAGS
 BASE = declarative_base()
 
 
 class NovaBase(object):
-    """Base class for Nova Models"""
+    """Base class for Nova Models."""
     __table_args__ = {'mysql_engine': 'InnoDB'}
     __table_initialized__ = False
     created_at = Column(DateTime, default=datetime.datetime.utcnow)
@@ -50,7 +49,7 @@ class NovaBase(object):
     deleted = Column(Boolean, default=False)
 
     def save(self, session=None):
-        """Save this object"""
+        """Save this object."""
         if not session:
             session = get_session()
         session.add(self)
@@ -63,7 +62,7 @@ class NovaBase(object):
                 raise
 
     def delete(self, session=None):
-        """Delete this object"""
+        """Delete this object."""
         self.deleted = True
         self.deleted_at = datetime.datetime.utcnow()
         self.save(session=session)
@@ -128,7 +127,8 @@ class NovaBase(object):
 
 
 class Service(BASE, NovaBase):
-    """Represents a running service on a host"""
+    """Represents a running service on a host."""
+
     __tablename__ = 'services'
     id = Column(Integer, primary_key=True)
     host = Column(String(255))  # , ForeignKey('hosts.id'))
@@ -139,7 +139,7 @@ class Service(BASE, NovaBase):
 
 
 class Instance(BASE, NovaBase):
-    """Represents a guest vm"""
+    """Represents a guest vm."""
     __tablename__ = 'instances'
     id = Column(Integer, primary_key=True)
     internal_id = Column(Integer, unique=True)
@@ -215,7 +215,7 @@ class Instance(BASE, NovaBase):
 
 
 class Volume(BASE, NovaBase):
-    """Represents a block storage device that can be attached to a vm"""
+    """Represents a block storage device that can be attached to a vm."""
     __tablename__ = 'volumes'
     id = Column(Integer, primary_key=True)
     ec2_id = Column(String(12), unique=True)
@@ -246,7 +246,7 @@ class Volume(BASE, NovaBase):
 
 
 class Quota(BASE, NovaBase):
-    """Represents quota overrides for a project"""
+    """Represents quota overrides for a project."""
     __tablename__ = 'quotas'
     id = Column(Integer, primary_key=True)
 
@@ -260,7 +260,7 @@ class Quota(BASE, NovaBase):
 
 
 class ExportDevice(BASE, NovaBase):
-    """Represates a shelf and blade that a volume can be exported on"""
+    """Represates a shelf and blade that a volume can be exported on."""
     __tablename__ = 'export_devices'
     __table_args__ = (schema.UniqueConstraint("shelf_id", "blade_id"),
                       {'mysql_engine': 'InnoDB'})
@@ -283,7 +283,7 @@ class SecurityGroupInstanceAssociation(BASE, NovaBase):
 
 
 class SecurityGroup(BASE, NovaBase):
-    """Represents a security group"""
+    """Represents a security group."""
     __tablename__ = 'security_groups'
     id = Column(Integer, primary_key=True)
 
@@ -313,7 +313,7 @@ class SecurityGroup(BASE, NovaBase):
 
 
 class SecurityGroupIngressRule(BASE, NovaBase):
-    """Represents a rule in a security group"""
+    """Represents a rule in a security group."""
     __tablename__ = 'security_group_rules'
     id = Column(Integer, primary_key=True)
 
@@ -335,7 +335,7 @@ class SecurityGroupIngressRule(BASE, NovaBase):
 
 
 class KeyPair(BASE, NovaBase):
-    """Represents a public key pair for ssh"""
+    """Represents a public key pair for ssh."""
     __tablename__ = 'key_pairs'
     id = Column(Integer, primary_key=True)
 
@@ -348,7 +348,7 @@ class KeyPair(BASE, NovaBase):
 
 
 class Network(BASE, NovaBase):
-    """Represents a network"""
+    """Represents a network."""
     __tablename__ = 'networks'
     __table_args__ = (schema.UniqueConstraint("vpn_public_address",
                                               "vpn_public_port"),
@@ -377,9 +377,12 @@ class Network(BASE, NovaBase):
 
 
 class AuthToken(BASE, NovaBase):
-    """Represents an authorization token for all API transactions. Fields
-    are a string representing the actual token and a user id for mapping
-    to the actual user"""
+    """Represents an authorization token for all API transactions.
+    
+    Fields are a string representing the actual token and a user id for
+    mapping to the actual user
+    
+    """
     __tablename__ = 'auth_tokens'
     token_hash = Column(String(255), primary_key=True)
     user_id = Column(Integer)
@@ -390,7 +393,7 @@ class AuthToken(BASE, NovaBase):
 
 # TODO(vish): can these both come from the same baseclass?
 class FixedIp(BASE, NovaBase):
-    """Represents a fixed ip for an instance"""
+    """Represents a fixed ip for an instance."""
     __tablename__ = 'fixed_ips'
     id = Column(Integer, primary_key=True)
     address = Column(String(255))
@@ -409,7 +412,7 @@ class FixedIp(BASE, NovaBase):
 
 
 class User(BASE, NovaBase):
-    """Represents a user"""
+    """Represents a user."""
     __tablename__ = 'users'
     id = Column(String(255), primary_key=True)
 
@@ -421,7 +424,7 @@ class User(BASE, NovaBase):
 
 
 class Project(BASE, NovaBase):
-    """Represents a project"""
+    """Represents a project."""
     __tablename__ = 'projects'
     id = Column(String(255), primary_key=True)
     name = Column(String(255))
@@ -469,7 +472,7 @@ class UserProjectAssociation(BASE, NovaBase):
 
 
 class FloatingIp(BASE, NovaBase):
-    """Represents a floating ip that dynamically forwards to a fixed ip"""
+    """Represents a floating ip that dynamically forwards to a fixed ip."""
     __tablename__ = 'floating_ips'
     id = Column(Integer, primary_key=True)
     address = Column(String(255))
@@ -485,7 +488,7 @@ class FloatingIp(BASE, NovaBase):
 
 
 def register_models():
-    """Register Models and create metadata"""
+    """Register Models and create metadata."""
     from sqlalchemy import create_engine
     models = (Service, Instance, Volume, ExportDevice, FixedIp,
               FloatingIp, Network, SecurityGroup,
-- 
cgit 


From 133cd9973e17458bea3594490e70ccd3c524cf12 Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Fri, 29 Oct 2010 11:58:57 -0400
Subject: Document Fakes

---
 nova/auth/fakeldap.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

(limited to 'nova')

diff --git a/nova/auth/fakeldap.py b/nova/auth/fakeldap.py
index 176a00f06..46e0135b4 100644
--- a/nova/auth/fakeldap.py
+++ b/nova/auth/fakeldap.py
@@ -15,9 +15,7 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
-"""
-Fake LDAP server for test harness, backs to ReDIS.
+"""Fake LDAP server for test harness, backs to ReDIS.
 
 This class does very little error checking, and knows nothing about ldap
 class definitions.  It implements the minimum emulation of the python ldap
-- 
cgit 


From 3ec095bed60490c844067c8d58ed43dbedee5f0a Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Fri, 29 Oct 2010 12:35:46 -0400
Subject: Update database page a bit.

---
 nova/db/sqlalchemy/api.py    | 2 +-
 nova/db/sqlalchemy/models.py | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

(limited to 'nova')

diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index a3d8dde2f..d9b98655e 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -16,7 +16,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 """
-Implementation of SQLAlchemy backend
+Implementation of SQLAlchemy backend.
 """
 
 import random
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 894ebcddd..29c3b74da 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -16,7 +16,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 """
-SQLAlchemy models for nova data
+SQLAlchemy models for nova data.
 """
 
 import datetime
@@ -488,7 +488,11 @@ class FloatingIp(BASE, NovaBase):
 
 
 def register_models():
-    """Register Models and create metadata."""
+    """Register Models and create metadata.
+
+    Called from nova.db.sqlalchemy.__init__ as part of loading the driver,
+    it will never need to be called explicitly elsewhere.
+    """
     from sqlalchemy import create_engine
     models = (Service, Instance, Volume, ExportDevice, FixedIp,
               FloatingIp, Network, SecurityGroup,
-- 
cgit 


From 489ddea1668c742f62acd6fd3e9af78f2f782912 Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Fri, 29 Oct 2010 15:30:39 -0400
Subject: Update database docs.

---
 nova/db/api.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

(limited to 'nova')

diff --git a/nova/db/api.py b/nova/db/api.py
index 7cce591ad..659bfd6b8 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -17,6 +17,16 @@
 #    under the License.
 """
 Defines interface for DB access.
+
+The underlying driver is loaded as a :class:`LazyPluggable`.
+
+**Related Flags**
+
+:db_backend:  string to lookup in the list of LazyPluggable backends.
+              `sqlalchemy` is the only supported backend right now.
+
+:sql_connection:  string specifying the sqlalchemy connection to use, like:
+                  `sqlite:///var/lib/nova/nova.sqlite`.
 """
 
 from nova import exception
-- 
cgit 


From 7ca505a729ee3caae968cf25059721ab51d1327c Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Fri, 29 Oct 2010 16:18:00 -0400
Subject: Volume documentation.

---
 nova/tests/volume_unittest.py | 21 ++++++++++++---------
 nova/volume/driver.py         | 24 +++++++++++++-----------
 nova/volume/manager.py        | 39 +++++++++++++++++++++++++++++++--------
 3 files changed, 56 insertions(+), 28 deletions(-)

(limited to 'nova')

diff --git a/nova/tests/volume_unittest.py b/nova/tests/volume_unittest.py
index fdee30b48..896800cea 100644
--- a/nova/tests/volume_unittest.py
+++ b/nova/tests/volume_unittest.py
@@ -16,7 +16,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 """
-Tests for Volume Code
+Tests for Volume Code.
+
 """
 import logging
 
@@ -33,7 +34,8 @@ FLAGS = flags.FLAGS
 
 
 class VolumeTestCase(test.TrialTestCase):
-    """Test Case for volumes"""
+    """Test Case for volumes."""
+
     def setUp(self):
         logging.getLogger().setLevel(logging.DEBUG)
         super(VolumeTestCase, self).setUp()
@@ -44,7 +46,7 @@ class VolumeTestCase(test.TrialTestCase):
 
     @staticmethod
     def _create_volume(size='0'):
-        """Create a volume object"""
+        """Create a volume object."""
         vol = {}
         vol['size'] = size
         vol['user_id'] = 'fake'
@@ -56,7 +58,7 @@ class VolumeTestCase(test.TrialTestCase):
 
     @defer.inlineCallbacks
     def test_create_delete_volume(self):
-        """Test volume can be created and deleted"""
+        """Test volume can be created and deleted."""
         volume_id = self._create_volume()
         yield self.volume.create_volume(self.context, volume_id)
         self.assertEqual(volume_id, db.volume_get(context.get_admin_context(),
@@ -70,7 +72,7 @@ class VolumeTestCase(test.TrialTestCase):
 
     @defer.inlineCallbacks
     def test_too_big_volume(self):
-        """Ensure failure if a too large of a volume is requested"""
+        """Ensure failure if a too large of a volume is requested."""
         # FIXME(vish): validation needs to move into the data layer in
         #              volume_create
         defer.returnValue(True)
@@ -83,7 +85,7 @@ class VolumeTestCase(test.TrialTestCase):
 
     @defer.inlineCallbacks
     def test_too_many_volumes(self):
-        """Ensure that NoMoreBlades is raised when we run out of volumes"""
+        """Ensure that NoMoreBlades is raised when we run out of volumes."""
         vols = []
         total_slots = FLAGS.num_shelves * FLAGS.blades_per_shelf
         for _index in xrange(total_slots):
@@ -100,7 +102,7 @@ class VolumeTestCase(test.TrialTestCase):
 
     @defer.inlineCallbacks
     def test_run_attach_detach_volume(self):
-        """Make sure volume can be attached and detached from instance"""
+        """Make sure volume can be attached and detached from instance."""
         inst = {}
         inst['image_id'] = 'ami-test'
         inst['reservation_id'] = 'r-fakeres'
@@ -149,12 +151,13 @@ class VolumeTestCase(test.TrialTestCase):
 
     @defer.inlineCallbacks
     def test_concurrent_volumes_get_different_blades(self):
-        """Ensure multiple concurrent volumes get different blades"""
+        """Ensure multiple concurrent volumes get different blades."""
+
         volume_ids = []
         shelf_blades = []
 
         def _check(volume_id):
-            """Make sure blades aren't duplicated"""
+            """Make sure blades aren't duplicated."""
             volume_ids.append(volume_id)
             admin_context = context.get_admin_context()
             (shelf_id, blade_id) = db.volume_get_shelf_and_blade(admin_context,
diff --git a/nova/volume/driver.py b/nova/volume/driver.py
index 3fa29ba37..b99089374 100644
--- a/nova/volume/driver.py
+++ b/nova/volume/driver.py
@@ -15,9 +15,9 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
 """
-Drivers for volumes
+Drivers for volumes.
+
 """
 
 import logging
@@ -39,7 +39,8 @@ flags.DEFINE_string('num_shell_tries', 3,
 
 
 class AOEDriver(object):
-    """Executes commands relating to AOE volumes"""
+    """Executes commands relating to AOE volumes."""
+
     def __init__(self, execute=process.simple_execute, *args, **kwargs):
         self._execute = execute
 
@@ -63,7 +64,7 @@ class AOEDriver(object):
 
     @defer.inlineCallbacks
     def create_volume(self, volume_name, size):
-        """Creates a logical volume"""
+        """Creates a logical volume."""
         # NOTE(vish): makes sure that the volume group exists
         yield self._execute("vgs %s" % FLAGS.volume_group)
         if int(size) == 0:
@@ -77,14 +78,14 @@ class AOEDriver(object):
 
     @defer.inlineCallbacks
     def delete_volume(self, volume_name):
-        """Deletes a logical volume"""
+        """Deletes a logical volume."""
         yield self._try_execute("sudo lvremove -f %s/%s" %
                                 (FLAGS.volume_group,
                                  volume_name))
 
     @defer.inlineCallbacks
     def create_export(self, volume_name, shelf_id, blade_id):
-        """Creates an export for a logical volume"""
+        """Creates an export for a logical volume."""
         yield self._try_execute(
                 "sudo vblade-persist setup %s %s %s /dev/%s/%s" %
                 (shelf_id,
@@ -95,13 +96,13 @@ class AOEDriver(object):
 
     @defer.inlineCallbacks
     def discover_volume(self, _volume_name):
-        """Discover volume on a remote host"""
+        """Discover volume on a remote host."""
         yield self._execute("sudo aoe-discover")
         yield self._execute("sudo aoe-stat")
 
     @defer.inlineCallbacks
     def remove_export(self, _volume_name, shelf_id, blade_id):
-        """Removes an export for a logical volume"""
+        """Removes an export for a logical volume."""
         yield self._try_execute("sudo vblade-persist stop %s %s" %
                                 (shelf_id, blade_id))
         yield self._try_execute("sudo vblade-persist destroy %s %s" %
@@ -109,7 +110,7 @@ class AOEDriver(object):
 
     @defer.inlineCallbacks
     def ensure_exports(self):
-        """Runs all existing exports"""
+        """Runs all existing exports."""
         # NOTE(vish): The standard _try_execute does not work here
         #             because these methods throw errors if other
         #             volumes on this host are in the process of
@@ -125,11 +126,12 @@ class AOEDriver(object):
 
 
 class FakeAOEDriver(AOEDriver):
-    """Logs calls instead of executing"""
+    """Logs calls instead of executing."""
+
     def __init__(self, *args, **kwargs):
         super(FakeAOEDriver, self).__init__(self.fake_execute)
 
     @staticmethod
     def fake_execute(cmd, *_args, **_kwargs):
-        """Execute that simply logs the command"""
+        """Execute that simply logs the command."""
         logging.debug("FAKE AOE: %s", cmd)
diff --git a/nova/volume/manager.py b/nova/volume/manager.py
index 2874459f9..f6e220c5f 100644
--- a/nova/volume/manager.py
+++ b/nova/volume/manager.py
@@ -15,10 +15,31 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
 """
-Volume manager manages creating, attaching, detaching, and
-destroying persistent storage volumes, ala EBS.
+Volume manager manages creating, attaching, detaching, and persistent storage.
+
+Persistant storage volumes keep their state independent of instances.  You can
+attach to an instance, terminate the instance, spawn a new instance (even
+one from a different image) and re-attach the volume with the same data
+intact.
+
+**Related Flags**
+
+:volume_topic:  What :mod:`rpc` topic to listen to (default: `volume`).
+:volume_manager:  The module name of a class derived from
+                  :class:`manager.Manager` (default:
+                  :class:`nova.volume.manager.AOEManager`).
+:storage_availability_zone:  Defaults to `nova`.
+:volume_driver:  Used by :class:`AOEManager`.  Defaults to
+                 :class:`nova.volume.driver.AOEDriver`.
+:num_shelves:  Number of shelves for AoE (default: 100).
+:num_blades:  Number of vblades per shelf to allocate AoE storage from
+              (default: 16).
+:volume_group:  Name of the group that will contain exported volumes (default:
+                `nova-volumes`)
+:aoe_eth_dev:  Device name the volumes will be exported on (default: `eth0`).
+:num_shell_tries:  Number of times to attempt to run AoE commands (default: 3)
+
 """
 
 import logging
@@ -47,15 +68,17 @@ flags.DEFINE_integer('blades_per_shelf',
 
 
 class AOEManager(manager.Manager):
-    """Manages Ata-Over_Ethernet volumes"""
+    """Manages Ata-Over_Ethernet volumes."""
+
     def __init__(self, volume_driver=None, *args, **kwargs):
+        """Load the driver from the one specified in args, or from flags."""
         if not volume_driver:
             volume_driver = FLAGS.volume_driver
         self.driver = utils.import_object(volume_driver)
         super(AOEManager, self).__init__(*args, **kwargs)
 
     def _ensure_blades(self, context):
-        """Ensure that blades have been created in datastore"""
+        """Ensure that blades have been created in datastore."""
         total_blades = FLAGS.num_shelves * FLAGS.blades_per_shelf
         if self.db.export_device_count(context) >= total_blades:
             return
@@ -66,7 +89,7 @@ class AOEManager(manager.Manager):
 
     @defer.inlineCallbacks
     def create_volume(self, context, volume_id):
-        """Creates and exports the volume"""
+        """Creates and exports the volume."""
         context = context.elevated()
         logging.info("volume %s: creating", volume_id)
 
@@ -104,7 +127,7 @@ class AOEManager(manager.Manager):
 
     @defer.inlineCallbacks
     def delete_volume(self, context, volume_id):
-        """Deletes and unexports volume"""
+        """Deletes and unexports volume."""
         context = context.elevated()
         volume_ref = self.db.volume_get(context, volume_id)
         if volume_ref['attach_status'] == "attached":
@@ -123,7 +146,7 @@ class AOEManager(manager.Manager):
 
     @defer.inlineCallbacks
     def setup_compute_volume(self, context, volume_id):
-        """Setup remote volume on compute host
+        """Setup remote volume on compute host.
 
         Returns path to device.
         """
-- 
cgit 


From bf15a6eb3de8c688dc1364959dd3e00d3e26a563 Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Sat, 30 Oct 2010 20:05:31 -0400
Subject: Update compute/disk.py docs.

---
 nova/compute/disk.py | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

(limited to 'nova')

diff --git a/nova/compute/disk.py b/nova/compute/disk.py
index e362b4507..0b8568d33 100644
--- a/nova/compute/disk.py
+++ b/nova/compute/disk.py
@@ -15,10 +15,11 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
 """
 Utility methods to resize, repartition, and modify disk images.
+
 Includes injection of SSH PGP keys into authorized_keys file.
+
 """
 
 import logging
@@ -41,20 +42,23 @@ flags.DEFINE_integer('block_size', 1024 * 1024 * 256,
 @defer.inlineCallbacks
 def partition(infile, outfile, local_bytes=0, resize=True,
               local_type='ext2', execute=None):
-    """Takes a single partition represented by infile and writes a bootable
-    drive image into outfile.
+    """
+    Turns a partition (infile) into a bootable drive image (outfile).
 
     The first 63 sectors (0-62) of the resulting image is a master boot record.
     Infile becomes the first primary partition.
     If local bytes is specified, a second primary partition is created and
     formatted as ext2.
 
-    In the diagram below, dashes represent drive sectors.
-    +-----+------. . .-------+------. . .------+
-    | 0  a| b               c|d               e|
-    +-----+------. . .-------+------. . .------+
-    | mbr | primary partiton | local partition |
-    +-----+------. . .-------+------. . .------+
+    ::
+
+        In the diagram below, dashes represent drive sectors.
+        +-----+------. . .-------+------. . .------+
+        | 0  a| b               c|d               e|
+        +-----+------. . .-------+------. . .------+
+        | mbr | primary partiton | local partition |
+        +-----+------. . .-------+------. . .------+
+
     """
     sector_size = 512
     file_size = os.path.getsize(infile)
-- 
cgit 


From 878eb4d25075f8d78f24ad9f78eb5d43702192ca Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Mon, 1 Nov 2010 16:13:18 -0400
Subject: Virt documentation.

---
 nova/virt/connection.py   | 20 ++++++++++++--------
 nova/virt/fake.py         |  7 +++++--
 nova/virt/libvirt_conn.py | 22 +++++++++++++++++++++-
 nova/virt/xenapi.py       | 12 ++++++++++++
 4 files changed, 50 insertions(+), 11 deletions(-)

(limited to 'nova')

diff --git a/nova/virt/connection.py b/nova/virt/connection.py
index ceb7f1e4b..11f0fa8ce 100644
--- a/nova/virt/connection.py
+++ b/nova/virt/connection.py
@@ -32,19 +32,23 @@ FLAGS = flags.FLAGS
 
 
 def get_connection(read_only=False):
-    """Returns an object representing the connection to a virtualization
-    platform.  This could be nova.virt.fake.FakeConnection in test mode,
-    a connection to KVM or QEMU via libvirt, or a connection to XenServer
-    or Xen Cloud Platform via XenAPI.
+    """
+    Returns an object representing the connection to a virtualization
+    platform.
+
+    This could be :mod:`nova.virt.fake.FakeConnection` in test mode,
+    a connection to KVM, QEMU, or UML via :mod:`libvirt_conn`, or a connection
+    to XenServer or Xen Cloud Platform via :mod:`xenapi`.
 
     Any object returned here must conform to the interface documented by
-    FakeConnection.
+    :mod:`FakeConnection`.
+
+    **Related flags**
 
-    Related flags
-    -------------
     :connection_type:  A string literal that falls through a if/elif structure
                        to determine what virtualization mechanism to use.
-                       Values may be:
+                       Values may be
+
                             * fake
                             * libvirt
                             * xenapi
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index 66eff4c66..f855523d3 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -18,8 +18,11 @@
 #    under the License.
 
 """
-A fake (in-memory) hypervisor+api. Allows nova testing w/o a hypervisor.
-This module also documents the semantics of real hypervisor connections.
+A fake (in-memory) hypervisor+api.
+
+Allows nova testing w/o a hypervisor.  This module also documents the
+semantics of real hypervisor connections.
+
 """
 
 from twisted.internet import defer
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index e32945fa5..0170fc6d1 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -18,7 +18,27 @@
 #    under the License.
 
 """
-A connection to a hypervisor (e.g. KVM) through libvirt.
+A connection to a hypervisor through libvirt.
+
+Supports KVM, QEMU, UML, and XEN.
+
+**Related Flags**
+
+:libvirt_type:  Libvirt domain type.  Can be kvm, qemu, uml, xen
+                (default: kvm).
+:libvirt_uri:  Override for the default libvirt URI (depends on libvirt_type).
+:libvirt_xml_template:  Libvirt XML Template (QEmu/KVM).
+:libvirt_xen_xml_template:  Libvirt XML Template (Xen).
+:libvirt_uml_xml_template:  Libvirt XML Template (User Mode Linux).
+:libvirt_rescue_xml_template:  XML template for rescue mode (KVM & QEMU).
+:libvirt_rescue_xen_xml_template:  XML templage for rescue mode (XEN).
+:libvirt_rescue_uml_xml_template:  XML template for rescue mode (UML).
+:rescue_image_id:  Rescue ami image (default: ami-rescue).
+:rescue_kernel_id:  Rescue aki image (default: aki-rescue).
+:rescue_ramdisk_id:  Rescue ari image (default: ari-rescue).
+:injected_network_template:  Template file for injected network
+:allow_project_net_traffic:  Whether to allow in project network traffic
+
 """
 
 import logging
diff --git a/nova/virt/xenapi.py b/nova/virt/xenapi.py
index a17e405ab..0f563aa41 100644
--- a/nova/virt/xenapi.py
+++ b/nova/virt/xenapi.py
@@ -33,6 +33,18 @@ long-running operations.
 
 FIXME: get_info currently doesn't conform to these rules, and will block the
 reactor thread if the VM.get_by_name_label or VM.get_record calls block.
+
+**Related Flags**
+
+:xenapi_connection_url:  URL for connection to XenServer/Xen Cloud Platform.
+:xenapi_connection_username:  Username for connection to XenServer/Xen Cloud
+                              Platform (default: root).
+:xenapi_connection_password:  Password for connection to XenServer/Xen Cloud
+                              Platform.
+:xenapi_task_poll_interval:  The interval (seconds) used for polling of
+                             remote tasks (Async.VM.start, etc)
+                             (default: 0.5).
+
 """
 
 import logging
-- 
cgit 


From c8e2341c98ffacfafffbadb7d204f10ff87cf89c Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Mon, 1 Nov 2010 20:33:03 -0400
Subject: API endpoint documentation.

---
 nova/api/__init__.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

(limited to 'nova')

diff --git a/nova/api/__init__.py b/nova/api/__init__.py
index 8a1d9fe32..27b8199db 100644
--- a/nova/api/__init__.py
+++ b/nova/api/__init__.py
@@ -15,9 +15,15 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
 """
 Root WSGI middleware for all API controllers.
+
+**Related Flags**
+
+:osapi_subdomain:  subdomain running the OpenStack API (default: api)
+:ec2api_subdomain:  subdomain running the EC2 API (default: ec2)
+:FAKE_subdomain:  set to 'api' or 'ec2', requests default to that endpoint
+
 """
 
 import routes
-- 
cgit 


From e0f889443f5c0732db28871f350c45e7c8e8d031 Mon Sep 17 00:00:00 2001
From: Todd Willey <todd@ansolabs.com>
Date: Mon, 1 Nov 2010 21:47:16 -0400
Subject: Add ec2 api docs.

---
 nova/api/ec2/__init__.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'nova')

diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py
index 0df4d3710..816314901 100644
--- a/nova/api/ec2/__init__.py
+++ b/nova/api/ec2/__init__.py
@@ -15,8 +15,10 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+"""
+Starting point for routing EC2 requests.
 
-"""Starting point for routing EC2 requests"""
+"""
 
 import logging
 import routes
-- 
cgit