4:35 p.m.
From: Daniel Henrique Barboza <dhbarboza82(a)gmail.com>
Kimchi does not obey the value set by the template, always
creating a disk of format 'qcow2' even if the user set another
format.
This patchset fixes this issue by:
- always set the disk format with the value the user set, unless there
is a storagepool restrition
- getting the default disk format type in template.conf. This value
is optional and will be used in the case the user does not provide
any other value.
- if there is no user choice and no default set in template.conf, the
default disk format will be 'qcow2'.
See the code and unit tests for more info.
Daniel Henrique Barboza (2):
Getting default disk format from template.conf
New unit tests to check disk creation behavior
src/kimchi/vmtemplate.py | 19 +++++++--
tests/test_model.py | 106 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 120 insertions(+), 5 deletions(-)
--
2.4.3
4:35 p.m.
New subject: [PATCH 1/2] Getting default disk format from template.conf
From: Daniel Henrique Barboza <dhbarboza82(a)gmail.com>
If the user change disk format in template.conf, templates are going to
be created correctly and vm xml as well. But, when the disk is created,
the format is always qcow2.
This patch considers the settings in the template.conf to define
the default volume format when creating the disks. User defined settings
will override any default (unless the storage type is logical,
iscsi or scsi. In those cases the format will always be 'raw').
If the user does not provide any format and template.conf does not
declare any default as well, the default value to be used as volume
type will be 'qcow2' (considering the storage type restriction
mentioned above).
Signed-off-by: Daniel Henrique Barboza <dhbarboza82(a)gmail.com>
---
src/kimchi/vmtemplate.py | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
index 3e42971..1ad9131 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -24,11 +24,13 @@ import urlparse
import uuid
import platform
+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
@@ -158,6 +160,13 @@ class VMTemplate(object):
dev, xml = get_disk_xml(params)
return xml
+ def _get_default_disk0_format(self):
+ 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)
@@ -165,7 +174,9 @@ class VMTemplate(object):
storage_path = self._get_storage_path()
base_disk_params = {'type': 'disk', 'disk':
'file',
- 'bus': self.info['disk_bus'],
'format': 'qcow2'}
+ 'bus': self.info['disk_bus'],
+ 'format': self._get_default_disk0_format()}
+
logical_disk_params = {'format': 'raw'}
iscsi_disk_params = {'disk': 'block', 'format':
'raw'}
@@ -193,8 +204,8 @@ 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()
- fmt = 'raw' if self._get_storage_type() in ['logical'] else
'qcow2'
ret = []
for i, d in enumerate(self.info['disks']):
index = d.get('index', i)
@@ -202,11 +213,11 @@ class VMTemplate(object):
info = {'name': volume,
'capacity': d['size'],
- 'format': fmt,
+ 'format': d.get('format', default_vol_format),
'path': '%s/%s' % (storage_path, volume)}
if 'logical' == self._get_storage_type() or \
- fmt not in ['qcow2', 'raw']:
+ info['format'] not in ['qcow2', 'raw']:
info['allocation'] = info['capacity']
else:
info['allocation'] = 0
--
2.4.3
4:35 p.m.
New subject: [PATCH 2/2] New unit tests to check disk creation behavior
From: Daniel Henrique Barboza <dhbarboza82(a)gmail.com>
Three new unit tests were added:
- check that the vm template creates always the disk format
set by the user, doesn't matter the default value from
template.conf
- check that the vm template creates a disk with format set by
the default valuein the disk.0 entry of template.conf, if
the user didn't provide any other.
- check that the vm template creates a disk with format 'qcow2'
if the user didn't provide any other and there is no default
value in template.conf
Signed-off-by: Daniel Henrique Barboza <dhbarboza82(a)gmail.com>
---
tests/test_model.py | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 105 insertions(+), 1 deletion(-)
diff --git a/tests/test_model.py b/tests/test_model.py
index 896e29b..ef8bb25 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -33,14 +33,16 @@ import kimchi.objectstore
import utils
from kimchi import netinfo
from kimchi.basemodel import Singleton
-from kimchi.config import config
+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
from kimchi.rollbackcontext import RollbackContext
from kimchi.utils import add_task
+from kimchi.xmlutils.utils import xpath_get_text
invalid_repository_urls = ['www.fedora.org', # missing protocol
@@ -600,6 +602,108 @@ class ModelTests(unittest.TestCase):
self.assertTrue(os.access(disk_path, os.F_OK))
self.assertFalse(os.access(disk_path, os.F_OK))
+ def _create_template_conf_with_disk_format(self, vol_format):
+ if vol_format is None:
+ conf_file_data = "[main]\n\n[storage]\n\n[[disk.0]]\n" \
+ "#format = \n\n[graphics]\n\n[processor]\n"
+ else:
+ conf_file_data = "[main]\n\n[storage]\n\n[[disk.0]]\n" \
+ "format = %s\n\n[graphics]\n\n[processor]\n"\
+ % vol_format
+
+ 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_file, config_bkp_file)
+
+ with open(config_file, 'w') as f:
+ f.write(conf_file_data)
+
+ 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)
+
+ def _get_disk_format_from_vm(self, vm, conn):
+ dom = VMModel.get_vm(vm, conn)
+ xml = dom.XMLDesc(0)
+ xpath = "/domain/devices/disk[@device='disk']/driver/@type"
+ return xpath_get_text(xml, xpath)[0]
+
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
+ def test_template_get_default_vol_format_from_conf(self):
+ inst = model.Model(objstore_loc=self.tmp_store)
+
+ with RollbackContext() as rollback:
+ self._create_template_conf_with_disk_format('vmdk')
+ rollback.prependDefer(self._restore_template_conf_file)
+
+ params = {'name': 'test', 'disks': [{'size':
1}],
+ 'cdrom': UBUNTU_ISO}
+ inst.templates_create(params)
+ rollback.prependDefer(inst.template_delete, 'test')
+
+ params = {'name': 'test-vm-1', 'template':
'/templates/test'}
+ task = inst.vms_create(params)
+ inst.task_wait(task['id'])
+ rollback.prependDefer(inst.vm_delete, 'test-vm-1')
+
+ created_disk_format = self._get_disk_format_from_vm(
+ 'test-vm-1', inst.conn
+ )
+ self.assertEqual(created_disk_format, 'vmdk')
+
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
+ def test_template_creates_user_defined_vol_format_instead_default(self):
+ inst = model.Model(objstore_loc=self.tmp_store)
+
+ default_vol = 'vmdk'
+ user_vol = 'raw'
+ with RollbackContext() as rollback:
+ self._create_template_conf_with_disk_format(default_vol)
+ rollback.prependDefer(self._restore_template_conf_file)
+
+ params = {'name': 'test',
+ 'disks': [{'size': 1, 'format':
user_vol}],
+ 'cdrom': UBUNTU_ISO}
+ inst.templates_create(params)
+ rollback.prependDefer(inst.template_delete, 'test')
+
+ params = {'name': 'test-vm-1', 'template':
'/templates/test'}
+ task = inst.vms_create(params)
+ inst.task_wait(task['id'])
+ rollback.prependDefer(inst.vm_delete, 'test-vm-1')
+
+ created_disk_format = self._get_disk_format_from_vm(
+ 'test-vm-1', inst.conn
+ )
+ self.assertEqual(created_disk_format, user_vol)
+
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
+ def test_template_uses_qcow2_format_if_no_user_or_default_defined(self):
+ inst = model.Model(objstore_loc=self.tmp_store)
+
+ with RollbackContext() as rollback:
+ self._create_template_conf_with_disk_format(None)
+ rollback.prependDefer(self._restore_template_conf_file)
+
+ params = {'name': 'test',
+ 'disks': [{'size': 1}], 'cdrom':
UBUNTU_ISO}
+ inst.templates_create(params)
+ rollback.prependDefer(inst.template_delete, 'test')
+
+ params = {'name': 'test-vm-1', 'template':
'/templates/test'}
+ task = inst.vms_create(params)
+ inst.task_wait(task['id'])
+ rollback.prependDefer(inst.vm_delete, 'test-vm-1')
+
+ created_disk_format = self._get_disk_format_from_vm(
+ 'test-vm-1', inst.conn
+ )
+ self.assertEqual(created_disk_format, 'qcow2')
+
def test_vm_memory_hotplug(self):
config.set("authentication", "method", "pam")
inst = model.Model(None, objstore_loc=self.tmp_store)
--
2.4.3
7:04 a.m.
Reviewed-by: Jose Ricardo Ziviani <joserz(a)linux.vnet.ibm.com>
On 15-07-2015 18:35, dhbarboza82(a)gmail.com wrote:
From: Daniel Henrique Barboza <dhbarboza82(a)gmail.com>
Kimchi does not obey the value set by the template, always
creating a disk of format 'qcow2' even if the user set another
format.
This patchset fixes this issue by:
- always set the disk format with the value the user set, unless there
is a storagepool restrition
- getting the default disk format type in template.conf. This value
is optional and will be used in the case the user does not provide
any other value.
- if there is no user choice and no default set in template.conf, the
default disk format will be 'qcow2'.
See the code and unit tests for more info.
Daniel Henrique Barboza (2):
Getting default disk format from template.conf
New unit tests to check disk creation behavior
src/kimchi/vmtemplate.py | 19 +++++++--
tests/test_model.py | 106 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 120 insertions(+), 5 deletions(-)
--
Jose Ricardo Ziviani
-----------------------------
Software Engineer
Linux Technology Center - IBM
7:09 a.m.
Patch set applied to master branch of https://github.com/danielhb/kimchi
Thanks for the review!
On 07/16/2015 09:04 AM, Jose Ricardo Ziviani wrote:
Reviewed-by: Jose Ricardo Ziviani <joserz(a)linux.vnet.ibm.com>
On 15-07-2015 18:35, dhbarboza82(a)gmail.com wrote:
> From: Daniel Henrique Barboza <dhbarboza82(a)gmail.com>
>
> Kimchi does not obey the value set by the template, always
> creating a disk of format 'qcow2' even if the user set another
> format.
>
> This patchset fixes this issue by:
>
> - always set the disk format with the value the user set, unless there
> is a storagepool restrition
>
> - getting the default disk format type in template.conf. This value
> is optional and will be used in the case the user does not provide
> any other value.
>
> - if there is no user choice and no default set in template.conf, the
> default disk format will be 'qcow2'.
>
>
> See the code and unit tests for more info.
>
> Daniel Henrique Barboza (2):
> Getting default disk format from template.conf
> New unit tests to check disk creation behavior
>
> src/kimchi/vmtemplate.py | 19 +++++++--
> tests/test_model.py | 106
> ++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 120 insertions(+), 5 deletions(-)
>
3187
days inactive
3207
days old
5 comments
4 participants
participants (4)
-
Aline Manera -
Daniel Henrique Barboza -
dhbarboza82＠gmail.com
-
Jose Ricardo Ziviani