[Kimchi-devel] [PATCH] Improve code to get default disk format for VMTemplate
Daniel Henrique Barboza
dhbarboza82 at gmail.com
Wed Aug 19 13:36:23 UTC 2015
Reviewed-by: Daniel Barboza <dhbarboza82 at gmail.com>
Tested-by: Daniel Barboza <dhbarboza82 at gmail.com>
On 08/18/2015 02:38 PM, Aline Manera wrote:
> The default template configuration is defined in the template.conf file.
> This file is parsed on server start up and the result is always returned
> in the osinfo.lookup() method. That means when creating a new
> VMTemplate instance it will already contain all the default values to
> use for the Template creation.
> The disks are in a dict format like below:
>
> [{u'index': 0, u'size': 10, u'format': u'qcow2'}]
>
> Each disk has its own size and format and we just need to use them.
> We don't need to parse the template.conf file again in order to get the right
> values.
>
> Also adjust the tests according to those changes.
>
> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
> ---
> src/kimchi/vmtemplate.py | 40 ++++++++++++++++++----------------------
> tests/test_model.py | 9 ++++++---
> tests/test_template.py | 8 ++++----
> 3 files changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
> index a165a5d..56a0de8 100644
> --- a/src/kimchi/vmtemplate.py
> +++ b/src/kimchi/vmtemplate.py
> @@ -23,13 +23,11 @@ import time
> import urlparse
> import uuid
>
> -from configobj import ConfigObj
> from lxml import etree
> from lxml.builder import E
>
> from kimchi import imageinfo
> from kimchi import osinfo
> -from kimchi.config import paths
> from kimchi.exception import InvalidParameter, IsoFormatError, MissingParameter
> from kimchi.exception import ImageFormatError, OperationFailed
> from kimchi.isoinfo import IsoImage
> @@ -64,23 +62,31 @@ class VMTemplate(object):
> entry = osinfo.lookup(os_distro, os_version)
> self.info.update(entry)
>
> - # Auto-generate a template name and no one is passed
> + # Auto-generate a template name if 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
> + # Merge graphics settings
> graph_args = args.get('graphics')
> if graph_args:
> graphics = dict(self.info['graphics'])
> graphics.update(graph_args)
> args['graphics'] = graphics
> - self.info.update(args)
>
> - # Assign right disk format to logical and [i]scsi storagepools
> - if self._get_storage_type() in ['logical', 'iscsi', 'scsi']:
> - for i, disk in enumerate(self.info['disks']):
> - self.info['disks'][i]['format'] = 'raw'
> + # Merge disks dict
> + default_disk = self.info['disks'][0]
> + for i, d in enumerate(args.get('disks', [])):
> + disk = dict(default_disk)
> + disk.update(d)
> +
> + # Assign right disk format to logical and [i]scsi storagepools
> + if self._get_storage_type() in ['logical', 'iscsi', 'scsi']:
> + disk['format'] = 'raw'
> + args['disks'][i] = disk
> +
> + # Override template values according to 'args'
> + self.info.update(args)
>
> def _get_os_info(self, args, scan):
> distro = version = 'unknown'
> @@ -159,14 +165,6 @@ class VMTemplate(object):
> dev, xml = get_disk_xml(params)
> return xml
>
> - @staticmethod
> - def get_default_disk0_format():
> - config_file = os.path.join(paths.conf_dir, 'template.conf')
> - config = ConfigObj(config_file)
> -
> - default_vol_format = config['storage']['disk.0'].get('format', 'qcow2')
> - return default_vol_format
> -
> def _get_disks_xml(self, vm_uuid):
> # Current implementation just allows to create disk in one single
> # storage pool, so we cannot mix the types (scsi volumes vs img file)
> @@ -174,8 +172,7 @@ class VMTemplate(object):
> storage_path = self._get_storage_path()
>
> base_disk_params = {'type': 'disk', 'disk': 'file',
> - 'bus': self.info['disk_bus'],
> - 'format': self.get_default_disk0_format()}
> + 'bus': self.info['disk_bus']}
> logical_disk_params = {'format': 'raw'}
> iscsi_disk_params = {'disk': 'block', 'format': 'raw'}
>
> @@ -187,7 +184,7 @@ class VMTemplate(object):
> pool_name = pool_name_from_uri(self.info['storagepool'])
> for index, disk in enumerate(self.info['disks']):
> params = dict(base_disk_params)
> - params['format'] = disk.get('format', params['format'])
> + params['format'] = disk['format']
> params.update(locals().get('%s_disk_params' % storage_type, {}))
> params['index'] = index
>
> @@ -203,7 +200,6 @@ class VMTemplate(object):
> return unicode(disks_xml, 'utf-8')
>
> def to_volume_list(self, vm_uuid):
> - default_vol_format = self.get_default_disk0_format()
> storage_path = self._get_storage_path()
> ret = []
> for i, d in enumerate(self.info['disks']):
> @@ -212,7 +208,7 @@ class VMTemplate(object):
>
> info = {'name': volume,
> 'capacity': d['size'],
> - 'format': d.get('format', default_vol_format),
> + 'format': d['format'],
> 'path': '%s/%s' % (storage_path, volume)}
>
> if 'logical' == self._get_storage_type() or \
> diff --git a/tests/test_model.py b/tests/test_model.py
> index 717b9bc..7ca823a 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -32,11 +32,11 @@ import iso_gen
> import kimchi.objectstore
> import utils
> from kimchi import netinfo
> +from kimchi import osinfo
> from kimchi.basemodel import Singleton
> from kimchi.config import config, paths
> from kimchi.exception import InvalidOperation
> from kimchi.exception import InvalidParameter, NotFoundError, OperationFailed
> -from kimchi.osinfo import get_template_default
> from kimchi.model import model
> from kimchi.model.libvirtconnection import LibvirtConnection
> from kimchi.model.vms import VMModel
> @@ -386,7 +386,7 @@ class ModelTests(unittest.TestCase):
> def test_vm_disk(self):
> disk_path = os.path.join(TMP_DIR, 'existent2.iso')
> open(disk_path, 'w').close()
> - modern_disk_bus = get_template_default('modern', 'disk_bus')
> + modern_disk_bus = osinfo.get_template_default('modern', 'disk_bus')
>
> def _attach_disk(expect_bus=modern_disk_bus):
> disk_args = {"type": "disk",
> @@ -485,7 +485,7 @@ class ModelTests(unittest.TestCase):
> rollback.prependDefer(inst.vm_delete, vm_name)
>
> # Need to check the right disk_bus for old distro
> - disk = _attach_disk(get_template_default('old', 'disk_bus'))
> + disk = _attach_disk(osinfo.get_template_default('old', 'disk_bus'))
> inst.vmstorage_delete('kimchi-ide-bus-vm', disk)
>
> # Hot plug IDE bus disk does not work
> @@ -620,11 +620,14 @@ class ModelTests(unittest.TestCase):
> with open(config_file, 'w') as f:
> f.write(conf_file_data)
>
> + osinfo.defaults = osinfo._get_tmpl_defaults()
> +
> def _restore_template_conf_file(self):
> config_file = os.path.join(paths.conf_dir, 'template.conf')
> config_bkp_file = \
> os.path.join(paths.conf_dir, 'template.conf-unit_test_bkp')
> os.rename(config_bkp_file, config_file)
> + osinfo.defaults = osinfo._get_tmpl_defaults()
>
> def _get_disk_format_from_vm(self, vm, conn):
> dom = VMModel.get_vm(vm, conn)
> diff --git a/tests/test_template.py b/tests/test_template.py
> index 83561c1..48ef229 100644
> --- a/tests/test_template.py
> +++ b/tests/test_template.py
> @@ -24,7 +24,7 @@ import unittest
>
> from functools import partial
>
> -from kimchi.vmtemplate import VMTemplate
> +from kimchi import osinfo
> from kimchi.config import READONLY_POOL_TYPE
> from kimchi.mockmodel import MockModel
> from utils import get_free_port, patch_auth, request, run_server
> @@ -88,7 +88,7 @@ class TemplateTests(unittest.TestCase):
> self.assertEquals(sorted(tmpl.keys()), sorted(keys))
>
> # Verify if default disk format was configured
> - default_disk_format = VMTemplate.get_default_disk0_format()
> + default_disk_format = osinfo.defaults['disks'][0]['format']
> self.assertEquals(tmpl['disks'][0]['format'], default_disk_format)
>
> # Clone a template
> @@ -199,8 +199,8 @@ class TemplateTests(unittest.TestCase):
> self.assertEquals(update_tmpl['cdrom'], cdrom_data['cdrom'])
>
> # Update disks
> - disk_data = {'disks': [{'index': 0, 'size': 10},
> - {'index': 1, 'size': 20}]}
> + disk_data = {'disks': [{'index': 0, 'size': 10, 'format': 'raw'},
> + {'index': 1, 'size': 20, 'format': 'raw'}]}
> resp = self.request(new_tmpl_uri, json.dumps(disk_data), 'PUT')
> self.assertEquals(200, resp.status)
> resp = self.request(new_tmpl_uri)
More information about the Kimchi-devel
mailing list