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

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Multiple files modification for change 'cdrom' to optional param. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- 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