Addressing Rick Clark's comments.

5 files changed, 82 insertions, 74 deletions

diff --git a/nova/virt/disk.py b/nova/virt/disk.py
index eae667c01..ee6d3e36a 100644
--- a/nova/virt/disk.py
+++ b/nova/virt/disk.py
@@ -166,12 +166,13 @@ def _free_device(device):
 
 
 def get_injectables(inst, template=None, template_data=None):
-    #load cheetah.template if necessary
+    # Note(salvatore-orlando):
+    # it the caller does not provide template object and data
+    # we will import the Cheetah template module and load the
+    # data from the file specified by injected_network_template flag
     if not template:
-        t = __import__('Cheetah.Template', globals(), locals(), ['Template'],
-                       -1)
+        from Cheetah import Template as t
         template = t.Template
-    #load template file if necessary
     if not template_data:
         template_data = open(FLAGS.injected_network_template).read()
 
diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py
index 60db86ecd..18d558058 100644
--- a/nova/virt/xenapi/fake.py
+++ b/nova/virt/xenapi/fake.py
@@ -427,7 +427,6 @@ class SessionBase(object):
                 if (field in _db_content[cls][ref]):
                     return _db_content[cls][ref][field]
             else:
-                LOG.debug(_('Raising Failure'))
                 raise Failure(['HANDLE_INVALID', cls, ref])
 
         LOG.debug(_('Raising NotImplemented'))
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index 7b6f2b5da..d429dae3c 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -683,65 +683,12 @@ class VMHelper(HelperBase):
         # everything
         mount_required = False
         key, net = disk.get_injectables(instance)
-        if key is not None or net is not None:
-            mount_required = True
-
-        if mount_required:
-
-            def _mounted_processing(device):
-                """Callback which runs with the image VDI attached"""
+        mount_required = key or net
+        if not mount_required:
+            return
 
-                dev_path = '/dev/' + device + '1'  # NB: Partition 1 hardcoded
-                tmpdir = tempfile.mkdtemp()
-                try:
-                    # Mount only Linux filesystems, to avoid disturbing
-                    # NTFS images
-                    try:
-                        out, err = utils.execute('sudo', 'mount',
-                                                 '-t', 'ext2,ext3',
-                                                 dev_path, tmpdir)
-                    except exception.ProcessExecutionError as e:
-                        err = str(e)
-                    if err:
-                        LOG.info(_('Failed to mount filesystem (expected for '
-                            'non-linux instances): %s') % err)
-                    else:
-                        try:
-                            # This try block ensures that the umount occurs
-                            xe_guest_agent_filename = os.path.join(
-                                tmpdir, FLAGS.xenapi_agent_path)
-                            if os.path.isfile(xe_guest_agent_filename):
-                                # The presence of the guest agent
-                                # file indicates that this instance can
-                                # reconfigure the network from xenstore data,
-                                # so manipulation of files in /etc is not
-                                # required
-                                LOG.info(_('XenServer tools installed in this '
-                                    'image are capable of network injection.  '
-                                    'Networking files will not be'
-                                    'manipulated'))
-                            else:
-                                xe_daemon_filename = os.path.join(tmpdir,
-                                    'usr', 'sbin', 'xe-daemon')
-                                if os.path.isfile(xe_daemon_filename):
-                                    LOG.info(_('XenServer tools are present '
-                                        'in this image but are not capable '
-                                        'of network injection'))
-                                else:
-                                    LOG.info(_('XenServer tools are not '
-                                        'installed in this image'))
-                                LOG.info(_('Manipulating interface files '
-                                         'directly'))
-                                disk.inject_data_into_fs(tmpdir, key, net,
-                                    utils.execute)
-                        finally:
-                            utils.execute('sudo', 'umount', dev_path)
-                finally:
-                    # remove temporary directory
-                    os.rmdir(tmpdir)
-
-            with_vdi_attached_here(session, vdi_ref, False,
-                _mounted_processing)
+        with_vdi_attached_here(session, vdi_ref, False,
+                               lambda dev: _mounted_processing(dev, key, net))
 
     @classmethod
     def lookup_kernel_ramdisk(cls, session, vm):
@@ -1077,3 +1024,65 @@ def _write_partition(virtual_size, dev):
 def get_name_label_for_image(image):
     # TODO(sirp): This should eventually be the URI for the Glance image
     return _('Glance image %s') % image
+
+
+def _mount_filesystem(dev_path, dir):
+    """mounts the device specified by dev_path in dir"""
+    try:
+        out, err = utils.execute('sudo', 'mount',
+                                 '-t', 'ext2,ext3',
+                                 dev_path, dir)
+    except exception.ProcessExecutionError as e:
+        err = str(e)
+    return err
+
+
+def _find_guest_agent(base_dir, agent_rel_path):
+    agent_path = os.path.join(base_dir, agent_rel_path)
+    if os.path.isfile(agent_path):
+        # The presence of the guest agent
+        # file indicates that this instance can
+        # reconfigure the network from xenstore data,
+        # so manipulation of files in /etc is not
+        # required
+        LOG.info(_('XenServer tools installed in this '
+                'image are capable of network injection.  '
+                'Networking files will not be'
+                'manipulated'))
+        return True
+    xe_daemon_filename = os.path.join(base_dir,
+        'usr', 'sbin', 'xe-daemon')
+    if os.path.isfile(xe_daemon_filename):
+        LOG.info(_('XenServer tools are present '
+                'in this image but are not capable '
+                'of network injection'))
+    else:
+        LOG.info(_('XenServer tools are not '
+                'installed in this image'))
+    return False
+
+
+def _mounted_processing(device, key, net):
+    """Callback which runs with the image VDI attached"""
+
+    dev_path = '/dev/' + device + '1'  # NB: Partition 1 hardcoded
+    tmpdir = tempfile.mkdtemp()
+    try:
+        # Mount only Linux filesystems, to avoid disturbing NTFS images
+        err = _mount_filesystem(dev_path, tmpdir)
+        if not err:
+            try:
+                # This try block ensures that the umount occurs
+                if not _find_guest_agent(tmpdir, FLAGS.xenapi_agent_path):
+                    LOG.info(_('Manipulating interface files '
+                            'directly'))
+                    disk.inject_data_into_fs(tmpdir, key, net,
+                        utils.execute)
+            finally:
+                utils.execute('sudo', 'umount', dev_path)
+        else:
+            LOG.info(_('Failed to mount filesystem (expected for '
+                'non-linux instances): %s') % err)
+    finally:
+        # remove temporary directory
+        os.rmdir(tmpdir)
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 59c5f3c13..e3810e218 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -169,7 +169,6 @@ class VMOps(object):
 
         def _wait_for_boot():
             try:
-                LOG.debug("ENTERING WAIT FOR BOOT!")
                 state = self.get_info(instance_name)['state']
                 db.instance_set_state(context.get_admin_context(),
                                       instance['id'], state)
diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py
index 4f31f1071..7c3d3544f 100644
--- a/nova/virt/xenapi_conn.py
+++ b/nova/virt/xenapi_conn.py
@@ -107,17 +107,17 @@ flags.DEFINE_integer('xenapi_vhd_coalesce_max_attempts',
                      'Max number of times to poll for VHD to coalesce.'
                      '  Used only if connection_type=xenapi.')
 flags.DEFINE_bool('xenapi_inject_image',
-                     True,
-                     'Specifies whether an attempt to inject network/key'
-                     '  data into the disk image should be made.'
-                     '  Used only if connection_type=xenapi.')
+                  True,
+                  'Specifies whether an attempt to inject network/key'
+                  '  data into the disk image should be made.'
+                  '  Used only if connection_type=xenapi.')
 flags.DEFINE_string('xenapi_agent_path',
-                     'usr/sbin/xe-update-networking',
-                     'Specifies the path in which the xenapi guest agent'
-                     '  should be located. If the agent is present,'
-                     '  network configuration if not injected into the image'
-                     '  Used only if connection_type=xenapi.'
-                     '  and xenapi_inject_image=True')
+                    'usr/sbin/xe-update-networking',
+                    'Specifies the path in which the xenapi guest agent'
+                    '  should be located. If the agent is present,'
+                    '  network configuration if not injected into the image'
+                    '  Used only if connection_type=xenapi.'
+                    '  and xenapi_inject_image=True')
 
 flags.DEFINE_string('xenapi_sr_base_path', '/var/run/sr-mount',
                     'Base path to the storage repository')