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

On 03/26/2014 03:33 AM, Aline Manera wrote: > On 03/25/2014 09:49 AM, shaohef(a)linux.vnet.ibm.com wrote: >> From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; >> >> 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(a)linux.vnet.ibm.com&gt; >> --- >> 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, > > >