[Kimchi-devel] [PATCH 6/9 - v7] Remove 'stogarepool' referencies from Templates code backend
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Wed Dec 2 18:38:08 UTC 2015
It is not necessary to pass the parameter 'storagepool' when create a
new template or update one. User is going to set a pool/name for each
disk instead.
This patch remove the legacy code related to 'storagepool' in templates.
It also fixes minor issues found and improve some checkings.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
---
API.json | 12 ----
control/templates.py | 1 -
docs/API.md | 3 -
i18n.py | 3 +-
model/storagepools.py | 10 +--
model/templates.py | 103 ++++++++++++++-------------
utils.py | 2 +-
vmtemplate.py | 52 ++++++--------
8 files changed, 83 insertions(+), 103 deletions(-)
diff --git a/API.json b/src/wok/plugins/kimchi/API.json
index 265ae36..2b64d07 100644
--- a/API.json
+++ b/API.json
@@ -512,12 +512,6 @@
"minItems": 1,
"uniqueItems": true
},
- "storagepool": {
- "description": "Location of the storage pool",
- "type": "string",
- "pattern": "^/plugins/kimchi/storagepools/[^/]+/?$",
- "error": "KCHTMPL0015E"
- },
"networks": {
"description": "list of which networks will be assigned to the new VM.",
"type": "array",
@@ -695,12 +689,6 @@
"minItems": 1,
"uniqueItems": true
},
- "storagepool": {
- "description": "Location of the storage pool",
- "type": "string",
- "pattern": "^/plugins/kimchi/storagepools/[^/]+/?$",
- "error": "KCHTMPL0015E"
- },
"networks": {
"description": "list of which networks will be assigned to the new VM.",
"type": "array",
diff --git a/control/templates.py b/src/wok/plugins/kimchi/control/templates.py
index fc58815..4cd70c2 100644
--- a/control/templates.py
+++ b/control/templates.py
@@ -50,7 +50,6 @@ class Template(Resource):
'memory': self.info['memory'],
'cdrom': self.info.get('cdrom', None),
'disks': self.info['disks'],
- 'storagepool': self.info['storagepool'],
'networks': self.info['networks'],
'folder': self.info.get('folder', []),
'graphics': self.info['graphics'],
diff --git a/docs/API.md b/src/wok/plugins/kimchi/docs/API.md
index 8ef48d6..1c4ee50 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -281,8 +281,6 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* memory *(optional)*: The amount of memory assigned to the VM.
Default is 1024M.
* cdrom *(optional)*: A volume name or URI to an ISO image.
- * storagepool *(optional)*: URI of the storagepool.
- Default is '/storagepools/default'
* networks *(optional)*: list of networks will be assigned to the new VM.
Default is '[default]'
* disks *(optional)*: An array of requested disks with the following optional fields
@@ -420,7 +418,6 @@ A interface represents available network interface on VM.
* cpus: The number of CPUs assigned to the VM
* memory: The amount of memory assigned to the VM
* cdrom: A volume name or URI to an ISO image
- * storagepool: URI of the storagepool where template allocates vm storage.
* networks *(optional)*: list of networks will be assigned to the new VM.
* disks: An array of requested disks with the following optional fields
(either *size* or *volume* must be specified):
diff --git a/i18n.py b/src/wok/plugins/kimchi/i18n.py
index 7154a62..cf67085 100644
--- a/i18n.py
+++ b/i18n.py
@@ -128,7 +128,6 @@ messages = {
"KCHVM0069E": _("Password field must be a string."),
"KCHVM0070E": _("Error creating local host ssh rsa key of user 'root'."),
"KCHVM0071E": _("Memory value %(mem)s must be aligned to %(alignment)sMiB."),
- "KCHVM0072E": _("Template given has multiple disks assigned to different types of storage pools, conflicting with storage pool provided."),
"KCHVM0073E": _("Unable to update the following parameters while the VM is offline: %(params)s"),
"KCHVM0074E": _("Unable to update the following parameters while the VM is online: %(params)s"),
"KCHVM0075E": _("Cannot change VCPU value because '%(vm)s' has a topology defined - sockets: %(sockets)s, cores: %(cores)s, threads: %(threads)s."),
@@ -179,6 +178,8 @@ messages = {
"KCHTMPL0025E": _("When specifying CPU topology, VCPUs must be a product of sockets, cores, and threads."),
"KCHTMPL0026E": _("When specifying CPU topology, each element must be an integer greater than zero."),
"KCHTMPL0027E": _("Invalid disk image format. Valid formats: bochs, cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc."),
+ "KCHTMPL0028E": _("When setting template disks, following parameters are required: 'index', 'pool name', 'format', 'size' or 'volume' (for scsi/iscsi pools)"),
+ "KCHTMPL0029E": _("Disk format must be 'raw', for logical, iscsi, and scsi pools."),
"KCHPOOL0001E": _("Storage pool %(name)s already exists"),
"KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
diff --git a/model/storagepools.py b/src/wok/plugins/kimchi/model/storagepools.py
index ec866c5..ddfa7fa 100644
--- a/model/storagepools.py
+++ b/model/storagepools.py
@@ -34,7 +34,6 @@ from wok.plugins.kimchi.model.host import DeviceModel
from wok.plugins.kimchi.model.libvirtstoragepool import StoragePoolDef
from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults
from wok.plugins.kimchi.scan import Scanner
-from wok.plugins.kimchi.utils import pool_name_from_uri
ISO_POOL_NAME = u'kimchi_isos'
@@ -72,7 +71,7 @@ class StoragePoolsModel(object):
def _check_default_pools(self):
pools = {}
- default_pool = tmpl_defaults['storagepool']
+ default_pool = tmpl_defaults['disks'][0]['pool']['name']
default_pool = default_pool.split('/')[-1]
pools[default_pool] = {}
@@ -446,9 +445,10 @@ class StoragePoolModel(object):
templates = session.get_list('template')
for tmpl in templates:
t_info = session.get('template', tmpl)
- t_pool = pool_name_from_uri(t_info['storagepool'])
- if t_pool == pool_name:
- return True
+ for disk in t_info['disks']:
+ t_pool = disk['pool']['name']
+ if t_pool == pool_name:
+ return True
return False
def deactivate(self, name):
diff --git a/model/templates.py b/src/wok/plugins/kimchi/model/templates.py
index eae2714..7b1412b 100644
--- a/model/templates.py
+++ b/model/templates.py
@@ -22,7 +22,7 @@ import libvirt
import os
import stat
-from wok.exception import InvalidOperation, InvalidParameter
+from wok.exception import InvalidOperation, InvalidParameter, MissingParameter
from wok.exception import NotFoundError, OperationFailed
from wok.utils import probe_file_permission_as_user, run_setfacl_set_attr
from wok.xmlutils.utils import xpath_get_text
@@ -34,6 +34,43 @@ from wok.plugins.kimchi.utils import pool_name_from_uri
from wok.plugins.kimchi.vmtemplate import VMTemplate
+def _check_disks_params(params, conn, objstore):
+ if 'disks' not in params:
+ return
+
+ basic_disk = ['index', 'format', 'pool', 'size']
+ ro_disk = ['index', 'format', 'pool', 'volume']
+ base_disk = ['index', 'base', 'pool']
+ for disk in params['disks']:
+ keys = sorted(disk.keys())
+ if ((keys == sorted(basic_disk)) or (keys == sorted(ro_disk)) or
+ (keys == sorted(base_disk))):
+ pass
+ else:
+ raise MissingParameter('KCHTMPL0028E')
+
+ if disk.get('pool', {}).get('name') is None:
+ raise MissingParameter('KCHTMPL0028E')
+
+ pool_uri = disk['pool']['name']
+ try:
+ pool_name = pool_name_from_uri(pool_uri)
+ conn.get().storagePoolLookupByName(pool_name.encode("utf-8"))
+ except Exception:
+ raise InvalidParameter("KCHTMPL0004E",
+ {'pool': pool_uri,
+ 'template': pool_name})
+ if 'volume' in disk:
+ kwargs = {'conn': conn, 'objstore': objstore}
+ storagevolumes = __import__(
+ "wok.plugins.kimchi.model.storagevolumes", fromlist=[''])
+ pool_volumes = storagevolumes.StorageVolumesModel(
+ **kwargs).get_list(pool_name)
+ if disk['volume'] not in pool_volumes:
+ raise InvalidParameter("KCHTMPL0019E", {'pool': pool_name,
+ 'volume': disk['volume']})
+
+
class TemplatesModel(object):
def __init__(self, **kargs):
self.objstore = kargs['objstore']
@@ -54,6 +91,9 @@ class TemplatesModel(object):
{'filename': iso, 'user': user,
'err': excp})
+ # Check disks information
+ _check_disks_params(params, self.conn, self.objstore)
+
cpu_info = params.get('cpu_info')
if cpu_info:
topology = cpu_info.get('topology')
@@ -71,19 +111,6 @@ class TemplatesModel(object):
check_topology(params['cpus'], topology)
conn = self.conn.get()
- pool_uri = params.get(u'storagepool', '')
- if pool_uri:
- try:
- pool_name = pool_name_from_uri(pool_uri)
- pool = conn.storagePoolLookupByName(pool_name.encode("utf-8"))
- except Exception:
- raise InvalidParameter("KCHTMPL0004E", {'pool': pool_uri,
- 'template': name})
-
- tmp_volumes = [disk['volume'] for disk in params.get('disks', [])
- if 'volume' in disk]
- self.template_volume_validate(tmp_volumes, pool)
-
for net_name in params.get(u'networks', []):
try:
conn.networkLookupByName(net_name.encode('utf-8'))
@@ -112,27 +139,22 @@ class TemplatesModel(object):
with self.objstore as session:
return session.get_list('template')
- def template_volume_validate(self, tmp_volumes, pool):
+ def template_volume_validate(self, volume, pool):
kwargs = {'conn': self.conn, 'objstore': self.objstore}
pool_type = xpath_get_text(pool.XMLDesc(0), "/pool/@type")[0]
pool_name = unicode(pool.name(), 'utf-8')
- # as we discussion, we do not mix disks from 2 different types of
- # storage pools, for instance: we do not create a template with 2
- # disks, where one comes from a SCSI pool and other is a .img in
- # a DIR pool.
if pool_type in ['iscsi', 'scsi']:
- if not tmp_volumes:
+ if not volume:
raise InvalidParameter("KCHTMPL0018E")
storagevolumes = __import__(
"wok.plugins.kimchi.model.storagevolumes", fromlist=[''])
pool_volumes = storagevolumes.StorageVolumesModel(
**kwargs).get_list(pool_name)
- vols = set(tmp_volumes) - set(pool_volumes)
- if vols:
+ if volume not in pool_volumes:
raise InvalidParameter("KCHTMPL0019E", {'pool': pool_name,
- 'volume': vols})
+ 'volume': volume})
class TemplateModel(object):
@@ -146,17 +168,14 @@ class TemplateModel(object):
with objstore as session:
params = session.get('template', name)
if overrides:
- params.update(overrides)
- if 'storagepool' in params:
- libvVMT = LibvirtVMTemplate(params, conn=conn)
- poolType = libvVMT._get_storage_type(params['storagepool'])
+ if 'storagepool' in overrides:
for i, disk in enumerate(params['disks']):
- if disk['pool']['type'] != poolType:
- raise InvalidOperation('KCHVM0072E')
- else:
- params['disks'][i]['pool']['name'] = \
- params['storagepool']
+ params['disks'][i]['pool']['name'] = \
+ overrides['storagepool']
+ del overrides['storagepool']
+ params.update(overrides)
+ _check_disks_params(params, conn, objstore)
return LibvirtVMTemplate(params, False, conn)
def lookup(self, name):
@@ -193,27 +212,15 @@ class TemplateModel(object):
new_t = copy.copy(old_t)
new_t.update(params)
+ # Check disks information
+ _check_disks_params(new_t, self.conn, self.objstore)
+
if not self._validate_updated_cpu_params(new_t):
raise InvalidParameter('KCHTMPL0025E')
- ident = name
-
- conn = self.conn.get()
- pool_uri = new_t.get(u'storagepool', '')
-
- if pool_uri:
- try:
- pool_name = pool_name_from_uri(pool_uri)
- pool = conn.storagePoolLookupByName(pool_name.encode("utf-8"))
- except Exception:
- raise InvalidParameter("KCHTMPL0004E", {'pool': pool_uri,
- 'template': name})
- tmp_volumes = [disk['volume'] for disk in new_t.get('disks', [])
- if 'volume' in disk]
- self.templates.template_volume_validate(tmp_volumes, pool)
-
for net_name in params.get(u'networks', []):
try:
+ conn = self.conn.get()
conn.networkLookupByName(net_name.encode('utf-8'))
except Exception:
raise InvalidParameter("KCHTMPL0003E", {'network': net_name,
diff --git a/utils.py b/src/wok/plugins/kimchi/utils.py
index 31def2e..a5f5e97 100644
--- a/utils.py
+++ b/utils.py
@@ -38,7 +38,7 @@ def _uri_to_name(collection, uri):
expr = '/plugins/kimchi/%s/(.*?)$' % collection
m = re.match(expr, uri)
if not m:
- raise InvalidParameter("KCHUTILS0001E", {'uri': uri})
+ raise InvalidParameter("WOKUTILS0001E", {'uri': uri})
return m.group(1)
diff --git a/vmtemplate.py b/src/wok/plugins/kimchi/vmtemplate.py
index 08adf4c..62d0e1a 100644
--- a/vmtemplate.py
+++ b/vmtemplate.py
@@ -76,36 +76,23 @@ class VMTemplate(object):
graphics.update(graph_args)
args['graphics'] = graphics
- # Provide compatibility with old template version and sets data to
- # correct output format if necessary
- def _fix_disk_compatibilities(disks):
- for i, disk in enumerate(disks):
- if 'pool' not in disk:
- disk['pool'] = {'name': self.info['storagepool']}
- if (isinstance(disk['pool'], str) or
- isinstance(disk['pool'], unicode)):
- disk['pool'] = {'name': disk['pool']}
- disk['pool']['type'] = \
- self._get_storage_type(disk['pool']['name'])
-
- if self.info.get('disks') is not None:
- _fix_disk_compatibilities(self.info['disks'])
- if args.get('disks') is not None:
- _fix_disk_compatibilities(args['disks'])
-
- # Merge disks dict with disks provided by user
- default_disk = self.info['disks'][0]
- for i, d in enumerate(args.get('disks', [])):
- disk = dict(default_disk)
- disk['index'] = i
- disk.update(d)
- if disk['pool']['type'] in ['logical', 'iscsi', 'scsi']:
- disk['format'] = 'raw'
- args['disks'][i] = disk
-
# Override template values according to 'args'
self.info.update(args)
+ # Find pool type for each disk
+ disks = self.info.get('disks')
+ for disk in disks:
+ pool_name = disk.get('pool', {}).get('name')
+ if pool_name is None:
+ raise MissingParameter('KCHTMPL0029E')
+
+ pool_type = self._get_storage_type(disk['pool']['name'])
+ disk['pool']['type'] = pool_type
+
+ if pool_type in ['logical', 'iscsi', 'scsi']:
+ if disk['format'] != 'raw':
+ raise InvalidParameter('KCHTMPL0029E')
+
def _get_os_info(self, args, scan):
distro = version = 'unknown'
@@ -197,9 +184,9 @@ class VMTemplate(object):
for index, disk in enumerate(self.info['disks']):
params = dict(base_disk_params)
params['format'] = disk['format']
+ params['index'] = index
params.update(locals().get('%s_disk_params' %
disk['pool']['type'], {}))
- params['index'] = index
volume = disk.get('volume')
if volume is not None:
@@ -459,10 +446,11 @@ class VMTemplate(object):
invalid['networks'] = invalid_networks
# validate storagepools integrity
- pool_uri = self.info['storagepool']
- pool_name = pool_name_from_uri(pool_uri)
- if pool_name not in self._get_all_storagepools_name():
- invalid['storagepools'] = [pool_name]
+ for disk in self.info['disks']:
+ pool_uri = disk['pool']['name']
+ pool_name = pool_name_from_uri(pool_uri)
+ if pool_name not in self._get_all_storagepools_name():
+ invalid['storagepools'] = [pool_name]
# validate iso integrity
# FIXME when we support multiples cdrom devices
--
2.1.0
More information about the Kimchi-devel
mailing list