[PATCH] validate the volume parameter when the pool of template is iscsi or scsi
From: ShaoHe Feng <shaohef@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@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) + 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, -- 1.8.5.3
On 03/25/2014 09:49 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@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?
+ 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,
On 03/26/2014 03:33 AM, Aline Manera wrote:
On 03/25/2014 09:49 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@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? I feel strange use normal importing. $ git diff diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 0e5c2b2..b6d80cf 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -30,6 +30,7 @@ from kimchi.utils import pool_name_from_uri from kimchi.utils import probe_file_permission_as_user from kimchi.vmtemplate import VMTemplate from lxml import objectify +from kimchi.model.storagevolumes import StorageVolumesModel
and error: $ sudo PYTHONPATH=src ./src/kimchid --host "0.0.0.0" --port 8000 $@ Traceback (most recent call last): File "./src/kimchid", line 26, in <module> import kimchi.server File "/home/shhfeng/work/workdir/kimchi/src/kimchi/server.py", line 30, in <module> from kimchi import mockmodel File "/home/shhfeng/work/workdir/kimchi/src/kimchi/mockmodel.py", line 49, in <module> from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES File "/home/shhfeng/work/workdir/kimchi/src/kimchi/model/storagepools.py", line 27, in <module> from kimchi.model.host import DeviceModel File "/home/shhfeng/work/workdir/kimchi/src/kimchi/model/host.py", line 35, in <module> from kimchi.model.vms import DOM_STATE_MAP File "/home/shhfeng/work/workdir/kimchi/src/kimchi/model/vms.py", line 34, in <module> from kimchi.model.templates import TemplateModel File "/home/shhfeng/work/workdir/kimchi/src/kimchi/model/templates.py", line 33, in <module> from kimchi.model.storagevolumes import StorageVolumesModel File "/home/shhfeng/work/workdir/kimchi/src/kimchi/model/storagevolumes.py", line 29, in <module> from kimchi.model.storagepools import StoragePoolModel ImportError: cannot import name StoragePoolModel In [1]: from kimchi.model.storagevolumes import StorageVolumesModel --------------------------------------------------------------------------- ImportError Traceback (most recent call last) <ipython-input-1-c7a006156f1e> in <module>() ----> 1 from kimchi.model.storagevolumes import StorageVolumesModel /home/shhfeng/work/workdir/kimchi/src/kimchi/model/storagevolumes.py in <module>() 27 from kimchi.exception import MissingParameter, NotFoundError, OperationFailed 28 from kimchi.isoinfo import IsoImage ---> 29 from kimchi.model.storagepools import StoragePoolModel 30 from kimchi.utils import kimchi_log 31 from kimchi.model.vms import VMsModel /home/shhfeng/work/workdir/kimchi/src/kimchi/model/storagepools.py in <module>() 25 from kimchi.exception import NotFoundError, OperationFailed 26 from kimchi.model.config import CapabilitiesModel ---> 27 from kimchi.model.host import DeviceModel 28 from kimchi.model.libvirtstoragepool import StoragePoolDef 29 from kimchi.utils import add_task, kimchi_log, pool_name_from_uri, run_command /home/shhfeng/work/workdir/kimchi/src/kimchi/model/host.py in <module>() 33 from kimchi.model.config import CapabilitiesModel 34 from kimchi.model.tasks import TaskModel ---> 35 from kimchi.model.vms import DOM_STATE_MAP 36 from kimchi.repositories import Repositories 37 from kimchi.swupdate import SoftwareUpdate /home/shhfeng/work/workdir/kimchi/src/kimchi/model/vms.py in <module>() 32 from kimchi.exception import MissingParameter, NotFoundError, OperationFailed 33 from kimchi.model.config import CapabilitiesModel ---> 34 from kimchi.model.templates import TemplateModel 35 from kimchi.model.utils import get_vm_name 36 from kimchi.screenshot import VMScreenshot /home/shhfeng/work/workdir/kimchi/src/kimchi/model/templates.py in <module>() 31 from kimchi.vmtemplate import VMTemplate 32 from lxml import objectify ---> 33 from kimchi.model.storagevolumes import StorageVolumesModel 34 35 ImportError: cannot import name StorageVolumesModel I find this is a loop: make is simple: storagevolumes, import xxx xxx, import templates templates, import storagevolumes But I think python interpret should avoid this loop.
+ 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@linux.vnet.ibm.com> IBM Linux Technology Center
On 03/25/2014 09:49 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@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?
On 03/26/2014 03:33 AM, Aline Manera wrote: 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@linux.vnet.ibm.com> IBM Linux Technology Center
on 2014/03/26 12:07, Sheldon wrote:
On 03/25/2014 09:49 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@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?
On 03/26/2014 03:33 AM, Aline Manera wrote: 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)
I think using globals is not a good idea. Maybe a proper refactor to untie the circular import is better.
+ 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! Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397
On 03/26/2014 01:27 PM, Zhou Zheng Sheng wrote:
on 2014/03/26 12:07, Sheldon wrote:
On 03/25/2014 09:49 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@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?
On 03/26/2014 03:33 AM, Aline Manera wrote: 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)
I think using globals is not a good idea. Maybe a proper refactor to untie the circular import is better. sure, This is not the issue of this patch.
we will do a better refactor later.
+ 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@linux.vnet.ibm.com> IBM Linux Technology Center
Applied. Thanks. Regards, Aline Manera
participants (4)
-
Aline Manera -
shaohef＠linux.vnet.ibm.com -
Sheldon
-
Zhou Zheng Sheng