Merge "Dom0 plugin now returns data in proper format."

4 files changed, 35 insertions, 40 deletions

diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py
index f6689c794..255f26a22 100644
--- a/nova/tests/xenapi/stubs.py
+++ b/nova/tests/xenapi/stubs.py
@@ -41,7 +41,7 @@ def stubout_instance_snapshot(stubs):
                 'kernel': dict(uuid=_make_fake_vdi(), file=None),
                 'ramdisk': dict(uuid=_make_fake_vdi(), file=None)}
 
-    stubs.Set(vm_utils, 'fetch_image', fake_fetch_image)
+    stubs.Set(vm_utils, '_fetch_image', fake_fetch_image)
 
     def fake_wait_for_vhd_coalesce(*args):
         #TODO(sirp): Should we actually fake out the data here
@@ -172,8 +172,8 @@ class FakeSessionForVMTests(fake.SessionBase):
 
     def host_call_plugin(self, _1, _2, plugin, method, _5):
         if (plugin, method) == ('glance', 'download_vhd'):
-            return fake.as_json(dict(vdi_type='root',
-                                     vdi_uuid=_make_fake_vdi()))
+            root_uuid = _make_fake_vdi()
+            return jsonutils.dumps(dict(root=dict(uuid=root_uuid)))
         elif (plugin, method) == ("xenhost", "iptables_config"):
             return fake.as_json(out=self._fake_iptables_save_output,
                                 err='')
@@ -183,10 +183,10 @@ class FakeSessionForVMTests(fake.SessionBase):
 
     def host_call_plugin_swap(self, _1, _2, plugin, method, _5):
         if (plugin, method) == ('glance', 'download_vhd'):
-            return fake.as_json(dict(vdi_type='root',
-                                     vdi_uuid=_make_fake_vdi()),
-                                dict(vdi_type='swap',
-                                     vdi_uuid=_make_fake_vdi()))
+            root_uuid = _make_fake_vdi()
+            swap_uuid = _make_fake_vdi()
+            return jsonutils.dumps(dict(root=dict(uuid=root_uuid),
+                                        swap=dict(uuid=swap_uuid)))
         else:
             return (super(FakeSessionForVMTests, self).
                     host_call_plugin(_1, _2, plugin, method, _5))
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index b8ff2983d..cbc7a70ce 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -604,7 +604,7 @@ def create_kernel_image(context, session, instance, image_id, user_id,
                                        args)
 
     if filename == "":
-        return fetch_image(context, session, instance, image_id, image_type)
+        return _fetch_image(context, session, instance, image_id, image_type)
     else:
         vdi_type = ImageType.to_string(image_type)
         return {vdi_type: dict(uuid=None, file=filename)}
@@ -623,9 +623,9 @@ def _create_cached_image(context, session, instance, image_id, image_type):
 
     root_vdi_ref = find_cached_image(session, image_id, sr_ref)
     if root_vdi_ref is None:
-        fetched_vdis = fetch_image(context, session, instance, image_id,
+        vdis = _fetch_image(context, session, instance, image_id,
                                    image_type)
-        root_vdi = fetched_vdis['root']
+        root_vdi = vdis['root']
         root_vdi_ref = session.call_xenapi('VDI.get_by_uuid',
                                            root_vdi['uuid'])
         set_vdi_name(session, root_vdi['uuid'], 'Glance Image %s' % image_id,
@@ -633,7 +633,7 @@ def _create_cached_image(context, session, instance, image_id, image_type):
         session.call_xenapi('VDI.add_to_other_config',
                             root_vdi_ref, 'image-id', str(image_id))
 
-        for vdi_type, vdi in fetched_vdis.iteritems():
+        for vdi_type, vdi in vdis.iteritems():
             vdi_ref = session.call_xenapi('VDI.get_by_uuid',
                                           vdi['uuid'])
 
@@ -709,7 +709,7 @@ def create_image(context, session, instance, image_id, image_type):
         vdis = _create_cached_image(
                 context, session, instance, image_id, image_type)
     else:
-        vdis = fetch_image(
+        vdis = _fetch_image(
                 context, session, instance, image_id, image_type)
 
     # Set the name label and description to easily identify what
@@ -720,18 +720,26 @@ def create_image(context, session, instance, image_id, image_type):
     return vdis
 
 
-def fetch_image(context, session, instance, image_id, image_type):
+def _fetch_image(context, session, instance, image_id, image_type):
     """Fetch image from glance based on image type.
 
     Returns: A single filename if image_type is KERNEL or RAMDISK
              A list of dictionaries that describe VDIs, otherwise
     """
     if image_type == ImageType.DISK_VHD:
-        return _fetch_vhd_image(context, session, instance, image_id)
+        vdis = _fetch_vhd_image(context, session, instance, image_id)
     else:
-        return _fetch_disk_image(context, session, instance, image_id,
+        vdis = _fetch_disk_image(context, session, instance, image_id,
                                  image_type)
 
+    for vdi_type, vdi in vdis.iteritems():
+        vdi_uuid = vdi['uuid']
+        LOG.debug(_("Fetched VDIs of type '%(vdi_type)s' with UUID"
+                    "  '%(vdi_uuid)s'"),
+                  locals(), instance=instance)
+
+    return vdis
+
 
 def _fetch_using_dom0_plugin_with_retry(context, session, image_id,
                                         plugin_name, params, callback=None):
@@ -785,22 +793,13 @@ def _fetch_vhd_image(context, session, instance, image_id):
         params['glance_port'] = glance_port
 
     plugin_name = 'glance'
-    fetched_vdis = _fetch_using_dom0_plugin_with_retry(
+    vdis = _fetch_using_dom0_plugin_with_retry(
             context, session, image_id, plugin_name, params,
             callback=pick_glance)
 
     sr_ref = safe_find_sr(session)
     scan_sr(session, sr_ref)
 
-    # TODO(sirp): the plugin should return the correct format rather than
-    # munging here
-    vdis = {}
-    for vdi in fetched_vdis:
-        LOG.debug(_("xapi 'download_vhd' returned VDI of "
-                    "type '%(vdi_type)s' with UUID '%(vdi_uuid)s'"),
-                  vdi, instance=instance)
-        vdis[vdi['vdi_type']] = dict(uuid=vdi['vdi_uuid'], file=None)
-
     # Pull out the UUID of the root VDI
     root_vdi_uuid = vdis['root']['uuid']
 
diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance
index b826a8711..58de40a01 100755
--- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance
+++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance
@@ -263,13 +263,13 @@ def download_vhd(session, args):
             auth_token)
 
         # Move the VHDs from the staging area into the storage repository
-        vdi_list = utils.import_vhds(sr_path, staging_path, uuid_stack)
+        imported_vhds = utils.import_vhds(sr_path, staging_path, uuid_stack)
     finally:
         utils.cleanup_staging_area(staging_path)
 
     # Right now, it's easier to return a single string via XenAPI,
     # so we'll json encode the list of VHDs.
-    return json.dumps(vdi_list)
+    return json.dumps(imported_vhds)
 
 
 def upload_vhd(session, args):
diff --git a/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py b/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py
index ff88501f3..77a0cf171 100644
--- a/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py
+++ b/plugins/xenserver/xenapi/etc/xapi.d/plugins/utils.py
@@ -124,14 +124,10 @@ def import_vhds(sr_path, staging_path, uuid_stack):
            however, the chances of an SR.scan occuring between the rename()s
            invocations is so small that we can safely ignore it)
 
-    Returns: A list of VDIs.  Each list element is a dictionary containing
-    information about the VHD.  Dictionary keys are:
-        1. "vdi_type" - The type of VDI. Currently they can be "root" or
-           "swap"
-        2. "vdi_uuid" - The UUID of the VDI
-
-    Example return: [{"vdi_type": "root","vdi_uuid": "ffff-aaa..vhd"},
-                     {"vdi_type": "swap","vdi_uuid": "ffff-bbb..vhd"}]
+    Returns: A dict of the VDIs imported. For example:
+
+        {'root': {'uuid': 'ffff-aaaa'},
+         'swap': {'uuid': 'ffff-bbbb'}}
     """
     def rename_with_uuid(orig_path):
         """Rename VHD using UUID so that it will be recognized by SR on a
@@ -233,7 +229,7 @@ def import_vhds(sr_path, staging_path, uuid_stack):
         while cur_path:
             cur_path = get_parent_path(cur_path)
 
-    vdi_return_list = []
+    imported_vhds = {}
     paths_to_move = []
 
     image_parent = None
@@ -258,23 +254,23 @@ def import_vhds(sr_path, staging_path, uuid_stack):
         # delete the base_copy since it is an unreferenced parent.
         paths_to_move.insert(0, snap_info[0])
         # We return this snap as the VDI instead of image.vhd
-        vdi_return_list.append(dict(vdi_type="root", vdi_uuid=snap_info[1]))
+        imported_vhds['root'] = dict(uuid=snap_info[1])
     else:
         validate_vdi_chain(image_info[0])
         assert_vhd_not_hidden(image_info[0])
         # If there's no snap, we return the image.vhd UUID
-        vdi_return_list.append(dict(vdi_type="root", vdi_uuid=image_info[1]))
+        imported_vhds['root'] = dict(uuid=image_info[1])
 
     swap_info = prepare_if_exists(staging_path, 'swap.vhd')
     if swap_info:
         assert_vhd_not_hidden(swap_info[0])
         paths_to_move.append(swap_info[0])
-        vdi_return_list.append(dict(vdi_type="swap", vdi_uuid=swap_info[1]))
+        imported_vhds['swap'] = dict(uuid=swap_info[1])
 
     for path in paths_to_move:
         move_into_sr(path)
 
-    return vdi_return_list
+    return imported_vhds
 
 
 def prepare_staging_area_for_upload(sr_path, staging_path, vdi_uuids):