addressed review comments, complied with HACKING guidelines

5 files changed, 71 insertions, 223 deletions

diff --git a/nova/virt/xenapi/__init__.py b/nova/virt/xenapi/__init__.py
index ed8c293a3..3d598c463 100644
--- a/nova/virt/xenapi/__init__.py
+++ b/nova/virt/xenapi/__init__.py
@@ -13,11 +13,3 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
-"""
-This is loaded late so that there's no need to install this library
-when not using XenAPI
-"""
-
-XenAPI = __import__('XenAPI')
-global XenAPI
\ No newline at end of file
diff --git a/nova/virt/xenapi/novadeps.py b/nova/virt/xenapi/novadeps.py
deleted file mode 100644
index a68fd8e77..000000000
--- a/nova/virt/xenapi/novadeps.py
+++ /dev/null
@@ -1,170 +0,0 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright (c) 2010 Citrix Systems, Inc.
-#
-#    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.
-
-"""
-It captures all the inner details of Nova classes and avoid their exposure
-to the implementation of the XenAPI module. One benefit of this, is to avoid
-sprawl of code changes
-"""
-
-from nova import db
-from nova import flags
-from nova import context
-
-from nova.compute import power_state
-from nova.auth.manager import AuthManager
-from nova.compute import instance_types
-from nova.virt import images
-
-XENAPI_POWER_STATE = {
-    'Halted': power_state.SHUTDOWN,
-    'Running': power_state.RUNNING,
-    'Paused': power_state.PAUSED,
-    'Suspended': power_state.SHUTDOWN,  # FIXME
-    'Crashed': power_state.CRASHED}
-
-
-flags.DEFINE_string('xenapi_connection_url',
-                    None,
-                    'URL for connection to XenServer/Xen Cloud Platform.'
-                    ' Required if connection_type=xenapi.')
-flags.DEFINE_string('xenapi_connection_username',
-                    'root',
-                    'Username for connection to XenServer/Xen Cloud Platform.'
-                    ' Used only if connection_type=xenapi.')
-flags.DEFINE_string('xenapi_connection_password',
-                    None,
-                    'Password for connection to XenServer/Xen Cloud Platform.'
-                    ' Used only if connection_type=xenapi.')
-flags.DEFINE_float('xenapi_task_poll_interval',
-                   0.5,
-                   'The interval used for polling of remote tasks '
-                   '(Async.VM.start, etc).  Used only if '
-                   'connection_type=xenapi.')
-
-
-class Configuration(object):
-    """ Wraps Configuration details into common class """
-    def __init__(self):
-        self._flags = flags.FLAGS
-
-    @property
-    def xenapi_connection_url(self):
-        """ Return the connection url """
-        return self._flags.xenapi_connection_url
-
-    @property
-    def xenapi_connection_username(self):
-        """ Return the username used for the connection """
-        return self._flags.xenapi_connection_username
-
-    @property
-    def xenapi_connection_password(self):
-        """ Return the password used for the connection """
-        return self._flags.xenapi_connection_password
-
-    @property
-    def xenapi_task_poll_interval(self):
-        """ Return the poll interval for the connection """
-        return self._flags.xenapi_task_poll_interval
-
-
-class Instance(object):
-    """ Wraps up instance specifics """
-
-    @classmethod
-    def get_name(cls, instance):
-        """ The name of the instance """
-        return instance.name
-
-    @classmethod
-    def get_type(cls, instance):
-        """ The type of the instance """
-        return instance_types.INSTANCE_TYPES[instance.instance_type]
-
-    @classmethod
-    def get_project(cls, instance):
-        """ The project the instance belongs """
-        return AuthManager().get_project(instance.project_id)
-
-    @classmethod
-    def get_project_id(cls, instance):
-        """ The id of the project the instance belongs """
-        return instance.project_id
-
-    @classmethod
-    def get_image_id(cls, instance):
-        """ The instance's image id """
-        return instance.image_id
-
-    @classmethod
-    def get_kernel_id(cls, instance):
-        """ The instance's kernel id """
-        return instance.kernel_id
-
-    @classmethod
-    def get_ramdisk_id(cls, instance):
-        """ The instance's ramdisk id """
-        return instance.ramdisk_id
-
-    @classmethod
-    def get_network(cls, instance):
-        """ The network the instance is connected to """
-        # TODO: is ge_admin_context the right context to retrieve?
-        return db.project_get_network(context.get_admin_context(),
-                                      instance.project_id)
-
-    @classmethod
-    def get_mac(cls, instance):
-        """ The instance's MAC address """
-        return instance.mac_address
-
-    @classmethod
-    def get_user(cls, instance):
-        """ The owner of the instance """
-        return AuthManager().get_user(instance.user_id)
-
-
-class Network(object):
-    """ Wraps up network specifics """
-
-    @classmethod
-    def get_bridge(cls, network):
-        """ the bridge for the network """
-        return network.bridge
-
-
-class Image(object):
-    """ Wraps up image specifics """
-
-    @classmethod
-    def get_url(cls, image):
-        """ the url to get the image from """
-        return images.image_url(image)
-
-
-class User(object):
-    """ Wraps up user specifics """
-
-    @classmethod
-    def get_access(cls, user, project):
-        """ access key """
-        return AuthManager().get_access_key(user, project)
-
-    @classmethod
-    def get_secret(cls, user):
-        """ access secret """
-        return user.secret
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index 52ab2901d..407acda6e 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -25,10 +25,17 @@ import XenAPI
 from twisted.internet import defer
 
 from nova import utils
+from nova.auth.manager import AuthManager
+from nova.compute import instance_types
+from nova.virt import images
+from nova.compute import power_state
 
-from novadeps import Instance
-from novadeps import Image
-from novadeps import User
+XENAPI_POWER_STATE = {
+    'Halted': power_state.SHUTDOWN,
+    'Running': power_state.RUNNING,
+    'Paused': power_state.PAUSED,
+    'Suspended': power_state.SHUTDOWN,  # FIXME
+    'Crashed': power_state.CRASHED}
 
 
 class VMHelper():
@@ -44,7 +51,7 @@ class VMHelper():
         """Create a VM record.  Returns a Deferred that gives the new
         VM reference."""
 
-        instance_type = Instance.get_type(instance)
+        instance_type = instance_types.INSTANCE_TYPES[instance.instance_type]
         mem = str(long(instance_type['memory_mb']) * 1024 * 1024)
         vcpus = str(instance_type['vcpus'])
         rec = {
@@ -76,10 +83,9 @@ class VMHelper():
             'user_version': '0',
             'other_config': {},
             }
-        logging.debug('Created VM %s...', Instance.get_name(instance))
+        logging.debug('Created VM %s...', instance.name)
         vm_ref = yield session.call_xenapi('VM.create', rec)
-        logging.debug('Created VM %s as %s.',
-                      Instance.get_name(instance), vm_ref)
+        logging.debug('Created VM %s as %s.', instance.name, vm_ref)
         defer.returnValue(vm_ref)
 
     @classmethod
@@ -137,14 +143,14 @@ class VMHelper():
         its kernel and ramdisk (if external kernels are being used).
         Returns a Deferred that gives the new VDI UUID."""
 
-        url = Image.get_url(image)
-        access = User.get_access(user, project)
+        url = images.image_url(image)
+        access = AuthManager().get_access_key(user, project)
         logging.debug("Asking xapi to fetch %s as %s", url, access)
         fn = use_sr and 'get_vdi' or 'get_kernel'
         args = {}
         args['src_url'] = url
         args['username'] = access
-        args['password'] = User.get_secret(user)
+        args['password'] = user.secret
         if use_sr:
             args['add_partition'] = 'true'
         task = yield session.async_call_plugin('objectstore', fn, args)
@@ -179,7 +185,7 @@ class VMHelper():
     def lookup_vm_vdis_blocking(cls, session, vm):
         """ Synchronous lookup_vm_vdis """
         # Firstly we get the VBDs, then the VDIs.
-        # TODO: do we leave the read-only devices?
+        # TODO(Armando): do we leave the read-only devices?
         vbds = session.get_xenapi().VM.get_VBDs(vm)
         vdis = []
         if vbds:
@@ -197,3 +203,11 @@ class VMHelper():
                 return vdis
             else:
                 return None
+
+    @classmethod
+    def compile_info(cls, record):
+        return {'state': XENAPI_POWER_STATE[record['power_state']],
+                'max_mem': long(record['memory_static_max']) >> 10,
+                'mem': long(record['memory_dynamic_max']) >> 10,
+                'num_cpu': record['VCPUs_max'],
+                'cpu_time': 0}
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 3db86f179..3696782b3 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -23,12 +23,11 @@ import XenAPI
 
 from twisted.internet import defer
 
-from novadeps import XENAPI_POWER_STATE
-from novadeps import Instance
-from novadeps import Network
-
-from vm_utils import VMHelper
-from network_utils import NetworkHelper
+from nova import db
+from nova import context
+from nova.auth.manager import AuthManager
+from nova.virt.xenapi.network_utils import NetworkHelper
+from nova.virt.xenapi.vm_utils import VMHelper
 
 
 class VMOps(object):
@@ -46,39 +45,40 @@ class VMOps(object):
     @defer.inlineCallbacks
     def spawn(self, instance):
         """ Create VM instance """
-        vm = yield VMHelper.lookup(self._session, Instance.get_name(instance))
+        vm = yield VMHelper.lookup(self._session, instance.name)
         if vm is not None:
             raise Exception('Attempted to create non-unique name %s' %
-                            Instance.get_name(instance))
+                            instance.name)
 
-        bridge = Network.get_bridge(Instance.get_network(instance))
+        bridge = db.project_get_network(context.get_admin_context(),
+                                      instance.project_id).bridge
         network_ref = \
             yield NetworkHelper.find_network_with_bridge(self._session, bridge)
 
-        user = Instance.get_user(instance)
-        project = Instance.get_project(instance)
+        user = AuthManager().get_user(instance.user_id)
+        project = AuthManager().get_project(instance.project_id)
         vdi_uuid = yield VMHelper.fetch_image(self._session,
-            Instance.get_image_id(instance), user, project, True)
+            instance.image_id, user, project, True)
         kernel = yield VMHelper.fetch_image(self._session,
-            Instance.get_kernel_id(instance), user, project, False)
+            instance.kernel_id, user, project, False)
         ramdisk = yield VMHelper.fetch_image(self._session,
-            Instance.get_ramdisk_id(instance), user, project, False)
+            instance.ramdisk_id, user, project, False)
         vdi_ref = yield self._session.call_xenapi('VDI.get_by_uuid', vdi_uuid)
         vm_ref = yield VMHelper.create_vm(self._session,
                                           instance, kernel, ramdisk)
         yield VMHelper.create_vbd(self._session, vm_ref, vdi_ref, 0, True)
         if network_ref:
             yield VMHelper.create_vif(self._session, vm_ref,
-                                      network_ref, Instance.get_mac(instance))
+                                      network_ref, instance.mac_address)
         logging.debug('Starting VM %s...', vm_ref)
         yield self._session.call_xenapi('VM.start', vm_ref, False, False)
-        logging.info('Spawning VM %s created %s.', Instance.get_name(instance),
+        logging.info('Spawning VM %s created %s.', instance.name,
                      vm_ref)
 
     @defer.inlineCallbacks
     def reboot(self, instance):
         """ Reboot VM instance """
-        instance_name = Instance.get_name(instance)
+        instance_name = instance.name
         vm = yield VMHelper.lookup(self._session, instance_name)
         if vm is None:
             raise Exception('instance not present %s' % instance_name)
@@ -88,7 +88,7 @@ class VMOps(object):
     @defer.inlineCallbacks
     def destroy(self, instance):
         """ Destroy VM instance """
-        vm = yield VMHelper.lookup(self._session, Instance.get_name(instance))
+        vm = yield VMHelper.lookup(self._session, instance.name)
         if vm is None:
             # Don't complain, just return.  This lets us clean up instances
             # that have already disappeared from the underlying platform.
@@ -122,11 +122,7 @@ class VMOps(object):
         if vm is None:
             raise Exception('instance not present %s' % instance_id)
         rec = self._session.get_xenapi().VM.get_record(vm)
-        return {'state': XENAPI_POWER_STATE[rec['power_state']],
-                'max_mem': long(rec['memory_static_max']) >> 10,
-                'mem': long(rec['memory_dynamic_max']) >> 10,
-                'num_cpu': rec['VCPUs_max'],
-                'cpu_time': 0}
+        return VMHelper.compile_info(rec)
 
     def get_console_output(self, instance):
         """ Return snapshot of console """
diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py
index 2839a753c..a2eac4dc2 100644
--- a/nova/virt/xenapi_conn.py
+++ b/nova/virt/xenapi_conn.py
@@ -49,26 +49,42 @@ reactor thread if the VM.get_by_name_label or VM.get_record calls block.
 
 import logging
 import xmlrpclib
+import XenAPI
 
 from twisted.internet import defer
 from twisted.internet import reactor
 
 from nova import utils
-
-from xenapi.vmops import VMOps
-from xenapi.volumeops import VolumeOps
-from xenapi.novadeps import Configuration
-from xenapi import XenAPI
-
-Config = Configuration()
+from nova import flags
+from nova.virt.xenapi.vmops import VMOps
+from nova.virt.xenapi.volumeops import VolumeOps
+
+FLAGS = flags.FLAGS
+flags.DEFINE_string('xenapi_connection_url',
+                    None,
+                    'URL for connection to XenServer/Xen Cloud Platform.'
+                    ' Required if connection_type=xenapi.')
+flags.DEFINE_string('xenapi_connection_username',
+                    'root',
+                    'Username for connection to XenServer/Xen Cloud Platform.'
+                    ' Used only if connection_type=xenapi.')
+flags.DEFINE_string('xenapi_connection_password',
+                    None,
+                    'Password for connection to XenServer/Xen Cloud Platform.'
+                    ' Used only if connection_type=xenapi.')
+flags.DEFINE_float('xenapi_task_poll_interval',
+                   0.5,
+                   'The interval used for polling of remote tasks '
+                   '(Async.VM.start, etc).  Used only if '
+                   'connection_type=xenapi.')
 
 
 def get_connection(_):
     """Note that XenAPI doesn't have a read-only connection mode, so
     the read_only parameter is ignored."""
-    url = Config.xenapi_connection_url
-    username = Config.xenapi_connection_username
-    password = Config.xenapi_connection_password
+    url = FLAGS.xenapi_connection_url
+    username = FLAGS.xenapi_connection_username
+    password = FLAGS.xenapi_connection_password
     if not url or password is None:
         raise Exception('Must specify xenapi_connection_url, '
                         'xenapi_connection_username (optionally), and '
@@ -165,7 +181,7 @@ class XenAPISession(object):
             #logging.debug('Polling task %s...', task)
             status = self._session.xenapi.task.get_status(task)
             if status == 'pending':
-                reactor.callLater(Config.xenapi_task_poll_interval,
+                reactor.callLater(FLAGS.xenapi_task_poll_interval,
                                   self._poll_task, task, deferred)
             elif status == 'success':
                 result = self._session.xenapi.task.get_result(task)