[Kimchi-devel] [PATCH] validate the volume parameter when the pool of template is iscsi or scsi

[Sheldon]

On 03/26/2014 03:33 AM, Aline Manera wrote:
> On 03/25/2014 09:49 AM, shaohef at linux.vnet.ibm.com wrote:
>> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>
>> When the pool of template is iscsi or scsi, the volume parameter can not
>> be NULL.
>> And 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.
>>
>> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>> ---
>> src/kimchi/i18n.py | 2 ++
>> src/kimchi/model/templates.py | 50 
>> +++++++++++++++++++++++++++++++++----------
>> 2 files changed, 41 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
>> index 1f84034..bcc15b7 100644
>> --- a/src/kimchi/i18n.py
>> +++ b/src/kimchi/i18n.py
>> @@ -107,6 +107,8 @@ messages = {
>> "KCHTMPL0015E": _("Invalid storage pool URI %(value)s specified for 
>> template"),
>> "KCHTMPL0016E": _("Specify an ISO image as CDROM 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)"),
>>
>> "KCHPOOL0001E": _("Storage pool %(name)s already exists"),
>> "KCHPOOL0002E": _("Storage pool %(name)s does not exist"),
>> diff --git a/src/kimchi/model/templates.py 
>> b/src/kimchi/model/templates.py
>> index d49632e..0e5c2b2 100644
>> --- a/src/kimchi/model/templates.py
>> +++ b/src/kimchi/model/templates.py
>> @@ -55,15 +55,17 @@ class TemplatesModel(object):
>> params['name'] = name
>>
>> conn = self.conn.get()
>> -
>> pool_uri = params.get(u'storagepool', '')
>> if pool_uri:
>> - pool_name = pool_name_from_uri(pool_uri)
>> try:
>> - conn.storagePoolLookupByName(pool_name.encode("utf-8"))
>> + pool_name = pool_name_from_uri(pool_uri)
>> + pool = conn.storagePoolLookupByName(pool_name.encode("utf-8"))
>> except Exception:
>> raise InvalidParameter("KCHTMPL0004E", {'pool': pool_name,
>> '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:
>> @@ -83,6 +85,28 @@ class TemplatesModel(object):
>> with self.objstore as session:
>> return session.get_list('template')
>>
>> + def template_volume_validate(self, tmp_volumes, pool):
>> + kwargs = {'conn': self.conn, 'objstore': self.objstore}
>> + pool_type = xmlutils.xpath_get_text(pool.XMLDesc(0), "/pool/@type")[0]
>> + pool_name = pool.name()
>> +
>> + # 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:
>> + raise InvalidParameter("KCHTMPL0018E")
>> +
>
>> + storagevolumes = __import__("kimchi.model.storagevolumes",
>> + fromlist=[''])
>> + pool_volumes = storagevolumes.StorageVolumesModel(
>> + **kwargs).get_list(pool_name)
>
> Why are you importing it that way?
this way can also works.
I get the StorageVolumesModel from globals()

$ git diff
diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py
index 0e5c2b2..9f45dc1 100644
--- a/src/kimchi/model/templates.py
+++ b/src/kimchi/model/templates.py
@@ -98,10 +98,8 @@ class TemplatesModel(object):
if not tmp_volumes:
raise InvalidParameter("KCHTMPL0018E")

- storagevolumes = __import__("kimchi.model.storagevolumes",
- fromlist=[''])
- pool_volumes = storagevolumes.StorageVolumesModel(
- **kwargs).get_list(pool_name)
+ StorageVolumesModel = globals()["StorageVolumesModel"]
+ pool_volumes = StorageVolumesModel(**kwargs).get_list(pool_name)
vols = set(tmp_volumes) - set(pool_volumes)

>
>> + vols = set(tmp_volumes) - set(pool_volumes)
>> + if vols:
>> + raise InvalidParameter("KCHTMPL0019E", {'pool': pool_name,
>> + 'volume': vols})
>> +
>>
>> class TemplateModel(object):
>> def __init__(self, **kargs):
>> @@ -128,18 +152,22 @@ class TemplateModel(object):
>> new_t.update(params)
>> ident = name
>>
>> + conn = self.conn.get()
>> pool_uri = new_t.get(u'storagepool', '')
>> - pool_name = pool_name_from_uri(pool_uri)
>> - try:
>> - conn = self.conn.get()
>> - conn.storagePoolLookupByName(pool_name.encode("utf-8"))
>> - except Exception:
>> - raise InvalidParameter("KCHTMPL0004E", {'pool': pool_name,
>> - 'template': name})
>> +
>> + 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_name,
>> + '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)
>> except Exception:
>> raise InvalidParameter("KCHTMPL0003E", {'network': net_name,
>
>
>


-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center