[Kimchi-devel] [PATCHv4 3/7] Change 'cdrom' to a optional param

lvroyce at linux.vnet.ibm.com lvroyce at linux.vnet.ibm.com
Thu Jul 24 09:02:20 UTC 2014


From: Royce Lv <lvroyce at linux.vnet.ibm.com>

Multiple files modification for change 'cdrom' to optional param.

Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
---
 src/kimchi/control/templates.py |  2 +-
 src/kimchi/i18n.py              |  2 +-
 src/kimchi/imageinfo.py         | 19 ++++++++++-
 src/kimchi/mockmodel.py         | 17 ++++------
 src/kimchi/model/templates.py   | 12 ++-----
 src/kimchi/vmtemplate.py        | 73 +++++++++++++++++++++++++++++++----------
 tests/test_mockmodel.py         | 12 ++++---
 tests/test_rest.py              |  5 +--
 tests/test_vmtemplate.py        | 25 +++++++++-----
 9 files changed, 114 insertions(+), 53 deletions(-)

diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py
index 907929f..65b6805 100644
--- a/src/kimchi/control/templates.py
+++ b/src/kimchi/control/templates.py
@@ -47,7 +47,7 @@ class Template(Resource):
                 'os_version': self.info['os_version'],
                 'cpus': self.info['cpus'],
                 'memory': self.info['memory'],
-                'cdrom': self.info['cdrom'],
+                'cdrom': self.info.get('cdrom', None),
                 'disks': self.info['disks'],
                 'storagepool': self.info['storagepool'],
                 'networks': self.info['networks'],
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 0c3aae5..f872bee 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -118,7 +118,7 @@ messages = {
     "KCHTMPL0013E": _("Amount of memory (MB) must be an integer greater than 512"),
     "KCHTMPL0014E": _("Template CDROM must be a local or remote ISO file"),
     "KCHTMPL0015E": _("Invalid storage pool URI %(value)s specified for template"),
-    "KCHTMPL0016E": _("Specify an ISO image as CDROM to create a template"),
+    "KCHTMPL0016E": _("Specify an ISO image as CDROM or a base image to create a template"),
     "KCHTMPL0017E": _("All networks for the template must be specified in a list."),
     "KCHTMPL0018E": _("Must specify a volume to a template, when storage pool is iscsi or scsi"),
     "KCHTMPL0019E": _("The volume: %(volume)s in not in storage pool %(pool)s"),
diff --git a/src/kimchi/imageinfo.py b/src/kimchi/imageinfo.py
index f874ece..f4c6356 100644
--- a/src/kimchi/imageinfo.py
+++ b/src/kimchi/imageinfo.py
@@ -17,11 +17,28 @@
 # License along with this library; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
 
+import json
 import os
 import sys
 import guestfs
 
-from kimchi.exception import ImageFormatError
+from kimchi.exception import ImageFormatError, TimeoutExpired
+from kimchi.utils import run_command, kimchi_log
+
+
+def probe_img_info(path):
+    cmd = ["qemu-img", "info", "--output=json", path]
+    info = dict()
+    try:
+        out = run_command(cmd, 10)[0]
+    except TimeoutExpired:
+        kimchi_log.warning("Cannot decide format of base img %s", path)
+        return None
+
+    info = json.loads(out)
+    info['virtual-size'] = info['virtual-size'] >> 30
+    info['actual-size'] = info['actual-size'] >> 30
+    return info
 
 
 def probe_image(image_path):
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index 0e45d1e..db4d465 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -201,8 +201,10 @@ class MockModel(object):
             index += 1
 
         cdrom = "hd" + string.ascii_lowercase[index + 1]
-        cdrom_params = {'dev': cdrom, 'path': t_info['cdrom'], 'type': 'cdrom'}
-        vm.storagedevices[cdrom] = MockVMStorageDevice(cdrom_params)
+        if t_info.get('cdrom'):
+            cdrom_params = {
+                'dev': cdrom, 'path': t_info['cdrom'], 'type': 'cdrom'}
+            vm.storagedevices[cdrom] = MockVMStorageDevice(cdrom_params)
 
         self._mock_vms[name] = vm
         return name
@@ -238,14 +240,6 @@ class MockModel(object):
 
     def templates_create(self, params):
         name = params.get('name', '').strip()
-        if not name:
-            iso = params['cdrom']
-            iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0]
-            name = iso_name + str(int(time.time() * 1000))
-            params['name'] = name
-
-        if name in self._mock_templates:
-            raise InvalidOperation("KCHTMPL0001E", {'name': name})
 
         for net_name in params.get(u'networks', []):
             try:
@@ -255,6 +249,9 @@ class MockModel(object):
                 raise InvalidParameter("KCHTMPL0003E", msg_args)
 
         t = MockVMTemplate(params, self)
+        if t.name in self._mock_templates:
+            raise InvalidOperation("KCHTMPL0001E", {'name': name})
+
         self._mock_templates[name] = t
         return name
 
diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py
index 60f4de5..f3a5d20 100644
--- a/src/kimchi/model/templates.py
+++ b/src/kimchi/model/templates.py
@@ -19,7 +19,6 @@
 
 import copy
 import os
-import time
 
 import libvirt
 
@@ -40,9 +39,9 @@ class TemplatesModel(object):
 
     def create(self, params):
         name = params.get('name', '').strip()
-        iso = params['cdrom']
+        iso = params.get('cdrom')
         # check search permission
-        if iso.startswith('/') and os.path.isfile(iso):
+        if iso and iso.startswith('/') and os.path.isfile(iso):
             user = UserTests().probe_user()
             ret, excp = probe_file_permission_as_user(iso, user)
             if ret is False:
@@ -50,11 +49,6 @@ class TemplatesModel(object):
                                        {'filename': iso, 'user': user,
                                         'err': excp})
 
-        if not name:
-            iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0]
-            name = iso_name + str(int(time.time() * 1000))
-            params['name'] = name
-
         conn = self.conn.get()
         pool_uri = params.get(u'storagepool', '')
         if pool_uri:
@@ -78,7 +72,7 @@ class TemplatesModel(object):
         # Checkings will be done while creating this class, so any exception
         # will be raised here
         t = LibvirtVMTemplate(params, scan=True)
-
+        name = params['name']
         try:
             with self.objstore as session:
                 if name in session.get_list('template'):
diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
index 8d5217a..b564d2c 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -20,11 +20,15 @@
 import os
 import string
 import socket
+import time
 import urlparse
+import uuid
 
 
 from kimchi import osinfo
-from kimchi.exception import InvalidParameter, IsoFormatError
+from kimchi.exception import InvalidParameter, IsoFormatError, ImageFormatError
+from kimchi.exception import MissingParameter
+from kimchi.imageinfo import probe_image, probe_img_info
 from kimchi.isoinfo import IsoImage
 from kimchi.utils import check_url_path, pool_name_from_uri
 from lxml import etree
@@ -46,22 +50,17 @@ class VMTemplate(object):
         defaults.  If scan is True and a cdrom is present, the operating system
         will be detected by probing the installation media.
         """
-        self.name = args['name']
         self.info = {}
         self.fc_host_support = args.get('fc_host_support')
 
-        # Identify the cdrom if present
-        iso_distro = iso_version = 'unknown'
-        iso = args.get('cdrom', '')
-
-        if scan and len(iso) > 0:
-            iso_distro, iso_version = self.get_iso_info(iso)
-            if not iso.startswith('/'):
-                self.info.update({'iso_stream': True})
-
+        distro, version = self._get_os_info(args, scan)
         # Fetch defaults based on the os distro and version
-        os_distro = args.get('os_distro', iso_distro)
-        os_version = args.get('os_version', iso_version)
+        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)
 
@@ -73,6 +72,43 @@ class VMTemplate(object):
             args['graphics'] = graphics
         self.info.update(args)
 
+    def _get_os_info(self, args, scan):
+        # Identify the cdrom if present
+        distro = version = 'unknown'
+        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
+                    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 not valid:
+            raise MissingParameter("KCHTMPL0016E")
+
+        return distro, version
+
+    def _gen_name(self, distro, version):
+        if distro == 'unknown':
+            name = str(uuid.uuid4())
+        else:
+            name = distro + version + '.' + str(int(time.time() * 1000))
+        return name
+
     def get_iso_info(self, iso):
         iso_prefixes = ['/', 'http', 'https', 'ftp', 'ftps', 'tftp']
         if len(filter(iso.startswith, iso_prefixes)) == 0:
@@ -84,6 +120,8 @@ class VMTemplate(object):
             raise InvalidParameter("KCHISO0001E", {'filename': iso})
 
     def _get_cdrom_xml(self, libvirt_stream_protocols, qemu_stream_dns):
+        if 'cdrom' not in self.info:
+            return ''
         bus = self.info['cdrom_bus']
         dev = "%s%s" % (self._bus_to_dev[bus],
                         string.lowercase[self.info['cdrom_index']])
@@ -326,8 +364,9 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
         cdrom_xml = self._get_cdrom_xml(libvirt_stream_protocols,
                                         qemu_stream_dns)
 
-        if not urlparse.urlparse(self.info['cdrom']).scheme in \
-           libvirt_stream_protocols and params.get('iso_stream', False):
+        if not urlparse.urlparse(self.info.get('cdrom', "")).scheme in \
+                libvirt_stream_protocols and \
+                params.get('iso_stream', False):
             params['qemu-namespace'] = QEMU_NAMESPACE
             params['qemu-stream-cmdline'] = cdrom_xml
         else:
@@ -414,8 +453,8 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
 
         # validate iso integrity
         # FIXME when we support multiples cdrom devices
-        iso = self.info['cdrom']
-        if not (os.path.isfile(iso) or check_url_path(iso)):
+        iso = self.info.get('cdrom')
+        if iso and not (os.path.isfile(iso) or check_url_path(iso)):
             invalid['cdrom'] = [iso]
 
         self.info['invalid'] = invalid
diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py
index e04b740..916a6c9 100644
--- a/tests/test_mockmodel.py
+++ b/tests/test_mockmodel.py
@@ -34,11 +34,12 @@ model = None
 host = None
 port = None
 ssl_port = None
+fake_iso = None
 
 
 class MockModelTests(unittest.TestCase):
     def setUp(self):
-        global host, port, ssl_port, model, test_server
+        global host, port, ssl_port, model, test_server, fake_iso
         cherrypy.request.headers = {'Accept': 'application/json'}
         model = kimchi.mockmodel.MockModel('/tmp/obj-store-test')
         patch_auth()
@@ -47,10 +48,13 @@ class MockModelTests(unittest.TestCase):
         host = '127.0.0.1'
         test_server = run_server(host, port, ssl_port, test_mode=True,
                                  model=model)
+        fake_iso = '/tmp/fake.iso'
+        open(fake_iso, 'w').close()
 
     def tearDown(self):
         test_server.stop()
         os.unlink('/tmp/obj-store-test')
+        os.unlink(fake_iso)
 
     def test_collection(self):
         c = Collection(model)
@@ -88,7 +92,7 @@ class MockModelTests(unittest.TestCase):
 
     def test_screenshot_refresh(self):
         # Create a VM
-        req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'})
+        req = json.dumps({'name': 'test', 'cdrom': fake_iso})
         request(host, ssl_port, '/templates', req, 'POST')
         req = json.dumps({'name': 'test-vm', 'template': '/templates/test'})
         request(host, ssl_port, '/vms', req, 'POST')
@@ -113,7 +117,7 @@ class MockModelTests(unittest.TestCase):
                           resp.getheader('last-modified'))
 
     def test_vm_list_sorted(self):
-        req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'})
+        req = json.dumps({'name': 'test', 'cdrom': fake_iso})
         request(host, ssl_port, '/templates', req, 'POST')
 
         def add_vm(name):
@@ -131,7 +135,7 @@ class MockModelTests(unittest.TestCase):
 
     def test_vm_info(self):
         model.templates_create({'name': u'test',
-                                'cdrom': '/nonexistent.iso'})
+                                'cdrom': fake_iso})
         model.vms_create({'name': u'test', 'template': '/templates/test'})
         vms = model.vms_get_list()
         self.assertEquals(1, len(vms))
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 54209ef..2dde32d 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -1068,7 +1068,7 @@ class RestTests(unittest.TestCase):
         self.assertEquals(200, resp.status)
         self.assertEquals(0, len(json.loads(resp.read())))
 
-        # Create a template without cdrom fails with 400
+        # Create a template without cdrom and disk specified fails with 400
         t = {'name': 'test', 'os_distro': 'ImagineOS',
              'os_version': '1.0', 'memory': 1024, 'cpus': 1,
              'storagepool': '/storagepools/alt'}
@@ -1077,10 +1077,11 @@ class RestTests(unittest.TestCase):
         self.assertEquals(400, resp.status)
 
         # Create a template
+        open('/tmp/mock.iso', 'w').close()
         graphics = {'type': 'spice', 'listen': '127.0.0.1'}
         t = {'name': 'test', 'os_distro': 'ImagineOS',
              'os_version': '1.0', 'memory': 1024, 'cpus': 1,
-             'storagepool': '/storagepools/alt', 'cdrom': '/nonexistent.iso',
+             'storagepool': '/storagepools/alt', 'cdrom': '/tmp/mock.iso',
              'graphics': graphics}
         req = json.dumps(t)
         resp = self.request('/templates', req, 'POST')
diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py
index 821ca24..7efcce2 100644
--- a/tests/test_vmtemplate.py
+++ b/tests/test_vmtemplate.py
@@ -17,6 +17,7 @@
 # License along with this library; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 
+import os
 import unittest
 import uuid
 
@@ -26,14 +27,22 @@ from kimchi.xmlutils import xpath_get_text
 
 
 class VMTemplateTests(unittest.TestCase):
+    def setUp(self):
+        self.iso = '/tmp/mock.iso'
+        open(self.iso, 'w').close()
+
+    def tearDown(self):
+        os.unlink(self.iso)
+
     def test_minimal_construct(self):
         fields = (('name', 'test'), ('os_distro', 'unknown'),
                   ('os_version', 'unknown'), ('cpus', 1),
-                  ('memory', 1024), ('cdrom', ''), ('networks', ['default']),
+                  ('memory', 1024), ('networks', ['default']),
                   ('disk_bus', 'ide'), ('nic_model', 'e1000'),
-                  ('graphics', {'type': 'vnc', 'listen': '0.0.0.0'}))
+                  ('graphics', {'type': 'vnc', 'listen': '0.0.0.0'}),
+                  ('cdrom', self.iso))
 
-        args = {'name': 'test'}
+        args = {'name': 'test', 'cdrom': self.iso}
         t = VMTemplate(args)
         for name, val in fields:
             self.assertEquals(val, t.info.get(name))
@@ -41,7 +50,7 @@ class VMTemplateTests(unittest.TestCase):
     def test_construct_overrides(self):
         graphics = {'type': 'spice', 'listen': '127.0.0.1'}
         args = {'name': 'test', 'disks': [{'size': 10}, {'size': 20}],
-                'graphics': graphics}
+                'graphics': graphics, "cdrom": self.iso}
         t = VMTemplate(args)
         self.assertEquals(2, len(t.info['disks']))
         self.assertEquals(graphics, t.info['graphics'])
@@ -50,7 +59,7 @@ class VMTemplateTests(unittest.TestCase):
         # Test specified listen
         graphics = {'type': 'vnc', 'listen': '127.0.0.1'}
         args = {'name': 'test', 'disks': [{'size': 10}, {'size': 20}],
-                'graphics': graphics}
+                'graphics': graphics, 'cdrom': self.iso}
         t = VMTemplate(args)
         self.assertEquals(graphics, t.info['graphics'])
 
@@ -70,7 +79,7 @@ class VMTemplateTests(unittest.TestCase):
     def test_to_xml(self):
         graphics = {'type': 'spice', 'listen': '0.0.0.0'}
         vm_uuid = str(uuid.uuid4()).replace('-', '')
-        t = VMTemplate({'name': 'test-template'})
+        t = VMTemplate({'name': 'test-template', 'cdrom': self.iso})
         xml = t.to_vm_xml('test-vm', vm_uuid, graphics=graphics)
         self.assertEquals(vm_uuid, xpath_get_text(xml, "/domain/uuid")[0])
         self.assertEquals('test-vm', xpath_get_text(xml, "/domain/name")[0])
@@ -87,10 +96,10 @@ class VMTemplateTests(unittest.TestCase):
         graphics = {'type': 'vnc', 'listen': '127.0.0.1'}
         args = {'name': 'test', 'os_distro': 'opensuse', 'os_version': '12.3',
                 'cpus': 2, 'memory': 2048, 'networks': ['foo'],
-                'cdrom': '/cd.iso', 'graphics': graphics}
+                'cdrom': self.iso, 'graphics': graphics}
         t = VMTemplate(args)
         self.assertEquals(2, t.info.get('cpus'))
         self.assertEquals(2048, t.info.get('memory'))
         self.assertEquals(['foo'], t.info.get('networks'))
-        self.assertEquals('/cd.iso', t.info.get('cdrom'))
+        self.assertEquals(self.iso, t.info.get('cdrom'))
         self.assertEquals(graphics, t.info.get('graphics'))
-- 
1.8.3.2




More information about the Kimchi-devel mailing list