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

Sheldon shaohef at linux.vnet.ibm.com
Wed Mar 26 04:03:27 UTC 2014


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?
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 at linux.vnet.ibm.com>
IBM Linux Technology Center

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140326/ad507b51/attachment.html>


More information about the Kimchi-devel mailing list