Adding more documentation, code-cleanup

2 files changed, 70 insertions, 42 deletions

diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index a6312d00c..5eab2a38f 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -24,6 +24,7 @@ import pickle
 import re
 import time
 import urllib
+import uuid
 from xml.dom import minidom
 
 from eventlet import event
@@ -318,9 +319,15 @@ class VMHelper(HelperBase):
         if sr_ref is None:
             raise exception.NotFound('Cannot find SR to write VDI to')
 
+        # NOTE(sirp): The Glance plugin runs under Python 2.4 which does not
+        # have the `uuid` module. To work around this, we generate the uuids
+        # here (under Python 2.6+) and pass them as arguments
+        uuid_stack = [str(uuid.uuid4()) for i in xrange(2)]
+
         params = {'image_id': image,
                   'glance_host': FLAGS.glance_host,
-                  'glance_port': FLAGS.glance_port}
+                  'glance_port': FLAGS.glance_port,
+                  'uuid_stack': uuid_stack}
 
         kwargs = {'params': pickle.dumps(params)}
         task = session.async_call_plugin('glance', 'get_vdi', kwargs)
@@ -372,14 +379,35 @@ class VMHelper(HelperBase):
 
     @classmethod
     def determine_disk_image_type(cls, instance):
-        instance_id = instance.id
+        """Disk Image Types are used to determine where the kernel will reside
+        within an image. To figure out which type we're dealing with, we use
+        the following rules:
+
+        1. If the instance is specifying a kernel explicitly, we must be using
+           a 'disk' image (kernel outside of the image)
+
+        2. If the kernel isn't specified, then we have two different
+           scenarios:
+
+            a) If the image is in Glance, then we can use the 'disk_format'
+               property to determine if the image is really a VHD-style image
+               or if it's a RAW image
+
+            b) If the image is not in Glance, then it must be a RAW image
+               (since we don't have a way of identifying VHD images...yet)
+        """
+        def log_disk_format(disk_format):
+            disk_format = disk_format.upper()
+            image_id = instance.image_id
+            instance_id = instance.id
+            LOG.debug(_("Detected %(disk_format)s format for image "
+                        "%(image_id)s, instance %(instance_id)s") % locals())
+
         if instance.kernel_id:
-            #if kernel is not present we must download a raw disk
-            LOG.debug(_("Instance %(instance_id)s will use DISK format") %
-                      locals())
+            # 1. DISK
+            log_disk_format('disk')
             return ImageType.DISK
-
-        if FLAGS.xenapi_image_service == 'glance':
+        elif FLAGS.xenapi_image_service == 'glance':
             # if using glance, then we could be VHD format
             client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port)
             meta = client.get_image_meta(instance.image_id)
@@ -388,12 +416,12 @@ class VMHelper(HelperBase):
             # TODO(sirp): When Glance treats disk_format as a first class
             # attribute, we should start using that rather than an image-property
             if disk_format == 'vhd':
-                LOG.debug(_("Instance %(instance_id)s will use DISK_VHD format") %
-                          locals())
+                # 2a. DISK_VHD
+                log_disk_format('disk_vhd')
                 return ImageType.DISK_VHD
-        
-        LOG.debug(_("Instance %(instance_id)s will use DISK_RAW format") %
-                  locals())
+      
+        # 2b. DISK_RAW
+        log_disk_format('disk_raw')
         return ImageType.DISK_RAW
 
     @classmethod
diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance
index 0cb3b52db..2018dca5f 100644
--- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance
+++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance
@@ -21,20 +21,14 @@
 # XenAPI plugin for managing glance images
 #
 
-import base64
-import errno
-import hmac
 import httplib
 import os
 import os.path
 import pickle
-import sha
 import shlex
 import shutil
 import subprocess
 import tempfile
-import time
-import urlparse
 
 import XenAPIPlugin
 
@@ -74,11 +68,14 @@ def _copy_kernel_vdi(dest,copy_args):
     return filename
 
 
-def execute(cmd):
-    args = shlex.split(cmd)
-    proc = subprocess.Popen(
-        args, stdout=subprocess.PIPE, stdin=subprocess.PIPE)
-    return proc
+def assert_process_success(proc, cmd):
+    """Ensure that the process returned a zero exit code indicating success
+    """
+    out, err = proc.communicate()
+    ret = proc.returncode
+    if ret != 0:
+        msg = "%(cmd)s returned non-zero exit code (%i): '%s'" % (ret, err)
+        raise Exception(msg)
 
 
 def get_vdi(session, args):
@@ -88,6 +85,7 @@ def get_vdi(session, args):
     image_id = params["image_id"]
     glance_host = params["glance_host"]
     glance_port = params["glance_port"]
+    uuid_stack = params["uuid_stack"]
 
     def unbundle_xfer(sr_path, staging_path):
         """
@@ -101,17 +99,17 @@ def get_vdi(session, args):
         elif resp.status != httplib.OK:
             raise Exception("Unexpected response from Glance %i" % res.status)
 
-        tar_proc = execute("tar -zx --directory=%(staging_path)s" % locals())
+        tar_args = shlex.split(
+            "tar -zx --directory=%(staging_path)s" % locals())
+        tar_proc = subprocess.Popen(
+            tar_args, stderr=subprocess.PIPE, stdin=subprocess.PIPE)
+
         chunk = resp.read(CHUNK_SIZE)
         while chunk:
             tar_proc.stdin.write(chunk)
             chunk = resp.read(CHUNK_SIZE)
-        out, err = tar_proc.communicate()
-        # TODO(sirp): write assert_process_success
-        ret = tar_proc.returncode
-        if ret != 0:
-            raise Exception(
-                "tar returned non-zero exit code (%i): '%s'" % (ret, err))
+
+        assert_process_success(tar_proc, "tar")
         conn.close()
 
     def fixup_vhds(sr_path, staging_path):
@@ -120,7 +118,7 @@ def get_vdi(session, args):
         def rename_with_uuid(orig_path):
             """Generate a uuid and rename the file with the uuid"""
             orig_dirname = os.path.dirname(orig_path)
-            uuid = generate_uuid()
+            uuid = uuid_stack.pop()
             new_path = os.path.join(orig_dirname, "%s.vhd" % uuid)
             os.rename(orig_path, new_path)
             return new_path, uuid
@@ -133,11 +131,13 @@ def get_vdi(session, args):
             return new_path
 
         def link_vhds(child_path, parent_path):
-            proc = execute("vhd-util modify -n %(child_path)s -p %(parent_path)s"
-                           % locals())
-            out, err = proc.communicate()
-            if proc.returncode != 0:
-                raise Exception("Failed to link vhds: '%s'" % err)
+            modify_args = shlex.split(
+                "vhd-util modify -n %(child_path)s -p %(parent_path)s"
+                % locals())
+            modify_proc = subprocess.Popen(
+                modify_args, stderr=subprocess.PIPE)
+            assert_process_success(modify_proc, "vhd-util")
+
 
         image_path = os.path.join(staging_path, 'image')
 
@@ -210,7 +210,10 @@ def put_vdis(session, args):
             conn.putheader(header, value)
         conn.endheaders()
 
-        tar_proc = execute("tar -zc --directory=%(staging_path)s ." % locals())
+        tar_args = shlex.split(
+            "tar -zc --directory=%(staging_path)s ." % locals())
+        tar_proc = subprocess.Popen(
+            tar_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
         chunk = tar_proc.stdout.read(CHUNK_SIZE)
         while chunk:
@@ -218,6 +221,8 @@ def put_vdis(session, args):
             chunk = tar_proc.stdout.read(CHUNK_SIZE)
         conn.send("0\r\n\r\n")
 
+        assert_process_success(tar_proc, "tar")
+
         resp = conn.getresponse()
         #FIXME(sirp): should this be 201 Created?
         if resp.status != httplib.OK:
@@ -277,11 +282,6 @@ def find_sr(session):
                 return sr
     return None
 
-def generate_uuid():
-    # NOTE(sirp): Python2.4 does not include the uuid module
-    proc = execute("uuidgen")
-    uuid = proc.stdout.read().strip()
-    return uuid
 
 if __name__ == '__main__':
     XenAPIPlugin.dispatch({'put_vdis': put_vdis,