[Kimchi-devel] [PATCH] Improve code to get default disk format for VMTemplate
Aline Manera
alinefm at linux.vnet.ibm.com
Tue Aug 18 17:38:12 UTC 2015
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)
--
2.1.0
More information about the Kimchi-devel
mailing list