[Kimchi-devel] [PATCH] bug fix: Probe image file only during the template creation

[Aline Manera]

We use guestfs to probe the OS name and version from an image file.
But it should be done only during the template creation. While lookup
for a template it is supposed to already has all the information in it.

To know when probing or not the image file or cdrom, VMTemplate has
the "scan" parameter: when set to true the OS name and version will be get
from the file; otherwise, use the already set values.

The current code was ignoring the "scan" value when probing the image
file. And as guestfs takes some time to do the scanning, the GET response also
delayed.

Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
---
 src/kimchi/vmtemplate.py | 52 ++++++++++++++++++++++++++----------------------
 tests/test_rest.py       |  2 +-
 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
index c5bb7b3..4994a5f 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -50,23 +50,24 @@ class VMTemplate(object):
         The only required parameter is a name for the VMTemplate.  If present,
         the os_distro and os_version fields are used to lookup recommended
         settings.  Any parameters provided by the caller will override the
-        defaults.  If scan is True and a cdrom is present, the operating system
-        will be detected by probing the installation media.
+        defaults.  If scan is True and a cdrom or a base img is present, the
+        operating system will be detected by probing the installation media.
         """
         self.info = {}
         self.fc_host_support = args.get('fc_host_support')
 
-        distro, version = self._get_os_info(args, scan)
         # Fetch defaults based on the os distro and version
+        distro, version = self._get_os_info(args, scan)
         os_distro = args.get('os_distro', distro)
         os_version = args.get('os_version', version)
-        if 'name' not in args or args['name'] == '':
-            args['name'] = self._gen_name(os_distro, os_version)
-        self.name = args['name']
-
         entry = osinfo.lookup(os_distro, os_version)
         self.info.update(entry)
 
+        # Auto-generate a template name and no one is passed
+        if 'name' not in args or args['name'] == '':
+            args['name'] = self._gen_name(distro, version)
+        self.name = args['name']
+
         # Override with the passed in parameters
         graph_args = args.get('graphics')
         if graph_args:
@@ -76,31 +77,34 @@ class VMTemplate(object):
         self.info.update(args)
 
     def _get_os_info(self, args, scan):
-        # Identify the cdrom if present
         distro = version = 'unknown'
+
+        # Identify the cdrom if present
         iso = args.get('cdrom', '')
-        valid = False
-        # if ISO not specified and base disk image specified,
-        # prevent cdrom from filling automatically
-        if len(iso) == 0 and 'disks' in args:
-            for d in args['disks']:
-                if 'base' in d:
-                    valid = True
+        if len(iso) > 0:
+            if not iso.startswith('/'):
+                self.info.update({'iso_stream': True})
+
+            if scan:
+                distro, version = self.get_iso_info(iso)
+
+            return distro, version
+
+        # CDROM is not presented: check for base image
+        base_imgs = []
+        for d in args.get('disks', []):
+            if 'base' in d.keys():
+                base_imgs.append(d)
+                if scan:
                     try:
                         distro, version = probe_image(d['base'])
                     except ImageFormatError:
                         pass
-                    if 'size' not in d:
-                        d['size'] = probe_img_info(d['base'])['virtual-size']
 
-        if len(iso) > 0:
-            valid = True
-            if scan:
-                distro, version = self.get_iso_info(iso)
-            if not iso.startswith('/'):
-                self.info.update({'iso_stream': True})
+                if 'size' not in d.keys():
+                    d['size'] = probe_img_info(d['base'])['virtual-size']
 
-        if not valid:
+        if len(base_imgs) == 0:
             raise MissingParameter("KCHTMPL0016E")
 
         return distro, version
diff --git a/tests/test_rest.py b/tests/test_rest.py
index c34b4d5..7778019 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -1406,7 +1406,7 @@ class RestTests(unittest.TestCase):
     def test_screenshot_refresh(self):
         # Create a VM
         req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'})
-        self.request('/templates', req, 'POST')
+        resp = self.request('/templates', req, 'POST')
         req = json.dumps({'name': 'test-vm', 'template': '/templates/test'})
         self.request('/vms', req, 'POST')
 
-- 
1.9.3