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

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?

> + 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,