From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; For pools which not allowed create and delete volume, if create vm fails, its storage volume should not be destoryed. Put the constant in single file to prevent circular import. For storage volume deletion, fix deleting vol clear part for readonly pool. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/config.py.in | 3 +++ src/kimchi/model/vms.py | 16 +++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index 2747164..d1affea 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -31,6 +31,9 @@ from kimchi.xmlutils import xpath_get_text DEFAULT_LOG_LEVEL = "debug" +# Storage pool constant for read-only pool types +READONLY_POOL_TYPE = ['iscsi', 'scsi', 'mpath'] + def get_object_store(): return os.path.join(paths.state_dir, 'objectstore') diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index a3b6c31..6267d03 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -27,6 +27,7 @@ from cherrypy.process.plugins import BackgroundTask from kimchi import vnc from kimchi import xmlutils +from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -183,7 +184,7 @@ class VMsModel(object): # If storagepool is SCSI, volumes will be LUNs and must be passed by # the user from UI or manually. vol_list = [] - if t._get_storage_type() == 'scsi': + if t._get_storage_type() in READONLY_POOL_TYPE: if not params.get('volumes'): raise MissingParameter('KCHVM0017E') else: @@ -215,9 +216,10 @@ class VMsModel(object): try: conn.defineXML(xml.encode('utf-8')) except libvirt.libvirtError as e: - for v in vol_list: - vol = conn.storageVolLookupByPath(v['path']) - vol.delete(0) + if t._get_storage_type() not in READONLY_POOL_TYPE: + for v in vol_list: + vol = conn.storageVolLookupByPath(v['path']) + vol.delete(0) raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()}) @@ -356,7 +358,11 @@ class VMModel(object): for path in paths: vol = conn.storageVolLookupByPath(path) - vol.delete(0) + pool = vol.storagePoolLookupByVolume() + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] + if pool_type not in READONLY_POOL_TYPE: + vol.delete(0) with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True)

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; As iscsi, scsi, mpath pools do not support create/delete storage volumes, when creating and deleting volumes, these pool types should be eleminated. REF: http://libvirt.org/storage.html Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/control/base.py | 2 ++ src/kimchi/i18n.py | 1 + src/kimchi/model/storagevolumes.py | 18 +++++++++++++++--- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 61b4341..005acec 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -130,6 +130,8 @@ class Resource(object): return self.delete() except NotFoundError, e: raise cherrypy.HTTPError(404, e.message) + except InvalidParameter, e: + raise cherrypy.HTTPError(400, e.message) elif method == 'PUT': try: return self.update() diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index e22bb4b..558490a 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -149,6 +149,7 @@ messages = { "KCHVOL0009E": _("Unable to wipe storage volumes %(name)s. Details: %(err)s"), "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), + "KCHVOL0012E": _("Storage type %(type)s does not support volume create and delete"), "KCHIFACE0001E": _("Interface %(name)s does not exist"), diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 8defdb7..98b6864 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -22,7 +22,8 @@ import os import libvirt from kimchi import xmlutils -from kimchi.exception import InvalidOperation, IsoFormatError +from kimchi.config import READONLY_POOL_TYPE +from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.isoinfo import IsoImage from kimchi.model.storagepools import StoragePoolModel @@ -37,8 +38,9 @@ VOLUME_TYPE_MAP = {0: 'file', class StorageVolumesModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.objstore = kargs['objstore'] - def create(self, pool, params): + def create(self, pool_name, params): vol_xml = """ <volume> <name>%(name)s</name> @@ -56,12 +58,16 @@ class StorageVolumesModel(object): name = params['name'] try: - pool = StoragePoolModel.get_storagepool(pool, self.conn) + pool = StoragePoolModel.get_storagepool(pool_name, self.conn) xml = vol_xml % params except KeyError, item: raise MissingParameter("KCHVOL0004E", {'item': str(item), 'volume': name}) + pool_info = StoragePoolModel(conn=self.conn, + objstore=self.objstore).lookup(pool_name) + if pool_info['type'] in READONLY_POOL_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) try: pool.createXML(xml, 0) except libvirt.libvirtError as e: @@ -86,6 +92,7 @@ class StorageVolumesModel(object): class StorageVolumeModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.objstore = kargs['objstore'] def _get_storagevolume(self, pool, name): pool = StoragePoolModel.get_storagepool(pool, self.conn) @@ -136,6 +143,11 @@ class StorageVolumeModel(object): {'name': name, 'err': e.get_error_message()}) def delete(self, pool, name): + pool_info = StoragePoolModel(conn=self.conn, + objstore=self.objstore).lookup(pool) + if pool_info['type'] in READONLY_POOL_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) + volume = self._get_storagevolume(pool, name) try: volume.delete(0)

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; isoinfo.py, storageservers.py and test_model.py fail in make check because of pep8, fix it. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/isoinfo.py | 4 ++-- src/kimchi/model/storageservers.py | 4 ++-- tests/test_model.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py index 1e9e02d..8d423ad 100644 --- a/src/kimchi/isoinfo.py +++ b/src/kimchi/isoinfo.py @@ -183,7 +183,7 @@ class IsoImage(object): for i in xrange(1, 4): fmt = IsoImage.EL_TORITO_BOOT_RECORD ptr = i * IsoImage.SECTOR_SIZE - tmp_data = data[ptr:ptr+fmt.size] + tmp_data = data[ptr:ptr + fmt.size] if len(tmp_data) < fmt.size: return @@ -212,7 +212,7 @@ class IsoImage(object): {'filename': self.path}) fmt = IsoImage.EL_TORITO_BOOT_ENTRY - tmp_data = data[ptr:ptr+fmt.size] + tmp_data = data[ptr:ptr + fmt.size] (boot, media_type, load_seg, sys_type, pad0, sectors, load_rba) = self._unpack(fmt, tmp_data) if boot == 0x88: diff --git a/src/kimchi/model/storageservers.py b/src/kimchi/model/storageservers.py index d4fba9d..167fedd 100644 --- a/src/kimchi/model/storageservers.py +++ b/src/kimchi/model/storageservers.py @@ -64,8 +64,8 @@ class StorageServerModel(object): for pool in pools: try: pool_info = self.pool.lookup(pool) - if pool_info['source'] and \ - pool_info['source']['addr'] == server: + if (pool_info['source'] and + pool_info['source']['addr'] == server): return dict(host=server) except NotFoundError: # Avoid inconsistent pool result because of lease between list diff --git a/tests/test_model.py b/tests/test_model.py index 2b34619..298b0a2 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -956,7 +956,7 @@ class ModelTests(unittest.TestCase): for repo in test_repos: inst.repositories_create(repo) host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+len(test_repos), len(host_repos)) + self.assertEquals(system_host_repos + len(test_repos), len(host_repos)) for repo in test_repos: repo_info = inst.repository_lookup(repo.get('repo_id')) @@ -994,7 +994,7 @@ class ModelTests(unittest.TestCase): inst.repositories_create(repo) host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+1, len(host_repos)) + self.assertEquals(system_host_repos + 1, len(host_repos)) new_repo = {'repo_id': 'fedora-fake', 'repo_name': 'Fedora 19 Update FAKE', @@ -1024,7 +1024,7 @@ class ModelTests(unittest.TestCase): inst.repositories_create(repo) host_repos = inst.repositories_get_list() - self.assertEquals(system_host_repos+1, len(host_repos)) + self.assertEquals(system_host_repos + 1, len(host_repos)) repo_info = inst.repository_lookup(repo.get('repo_id')) self.assertEquals(True, repo_info.get('enabled'))