[PATCH 0/2] Fix vm creation on readonly pool type
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Some pool types (scsi, iscsi, mpath) do not allow create/delete storage volume, so it is wrong to delete these volumes when vm delete. Fix it by just delete non read only pool types. Royce Lv (2): Fix vm creation storage rollback clean Prevent volume create and delete for certain pool types src/kimchi/control/base.py | 2 ++ src/kimchi/i18n.py | 1 + src/kimchi/model/constant.py | 24 ++++++++++++++++++++++++ src/kimchi/model/storagevolumes.py | 18 +++++++++++++++--- src/kimchi/model/vms.py | 14 ++++++-------- 5 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 src/kimchi/model/constant.py -- 1.8.1.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com> 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, work around it by delete vol clear part, in the future, this will be fixed by volume reference information tracking. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/constant.py | 24 ++++++++++++++++++++++++ src/kimchi/model/vms.py | 14 ++++++-------- 2 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 src/kimchi/model/constant.py diff --git a/src/kimchi/model/constant.py b/src/kimchi/model/constant.py new file mode 100644 index 0000000..579cc66 --- /dev/null +++ b/src/kimchi/model/constant.py @@ -0,0 +1,24 @@ +# +# Kimchi +# +# Copyright IBM Corp, 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +# storage pool constant +READONLY_TYPE = ['iscsi', 'scsi', 'mpath'] diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b482e80..f4bf371 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -33,6 +33,7 @@ from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel +from kimchi.model.constant import READONLY_TYPE from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot @@ -186,7 +187,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_TYPE: if not params.get('volumes'): raise MissingParameter('KCHVM0017E') else: @@ -218,9 +219,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_TYPE: + for v in vol_list: + vol = conn.storageVolLookupByPath(v['path']) + vol.delete(0) raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()}) @@ -357,10 +359,6 @@ class VMModel(object): dom.undefine() - for path in paths: - vol = conn.storageVolLookupByPath(path) - vol.delete(0) - with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True) -- 1.8.1.2
On 02/24/2014 07:24 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
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, work around it by delete vol clear part, in the future, this will be fixed by volume reference information tracking.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/constant.py | 24 ++++++++++++++++++++++++ src/kimchi/model/vms.py | 14 ++++++-------- 2 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 src/kimchi/model/constant.py
diff --git a/src/kimchi/model/constant.py b/src/kimchi/model/constant.py new file mode 100644 index 0000000..579cc66 --- /dev/null +++ b/src/kimchi/model/constant.py @@ -0,0 +1,24 @@ +# +# Kimchi +# +# Copyright IBM Corp, 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +# storage pool constant +READONLY_TYPE = ['iscsi', 'scsi', 'mpath']
It refers to storage pool, so we should put it in model/storagepools.py I don't think putting it there will cause some circular import error
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b482e80..f4bf371 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -33,6 +33,7 @@ from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel +from kimchi.model.constant import READONLY_TYPE from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot @@ -186,7 +187,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_TYPE: if not params.get('volumes'): raise MissingParameter('KCHVM0017E') else: @@ -218,9 +219,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_TYPE: + for v in vol_list: + vol = conn.storageVolLookupByPath(v['path']) + vol.delete(0) raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()})
@@ -357,10 +359,6 @@ class VMModel(object):
dom.undefine()
- for path in paths: - vol = conn.storageVolLookupByPath(path) - vol.delete(0) - with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True)
On 2014年02月24日 21:34, Aline Manera wrote:
On 02/24/2014 07:24 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
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, work around it by delete vol clear part, in the future, this will be fixed by volume reference information tracking.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/constant.py | 24 ++++++++++++++++++++++++ src/kimchi/model/vms.py | 14 ++++++-------- 2 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 src/kimchi/model/constant.py
diff --git a/src/kimchi/model/constant.py b/src/kimchi/model/constant.py new file mode 100644 index 0000000..579cc66 --- /dev/null +++ b/src/kimchi/model/constant.py @@ -0,0 +1,24 @@ +# +# Kimchi +# +# Copyright IBM Corp, 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +# storage pool constant +READONLY_TYPE = ['iscsi', 'scsi', 'mpath']
It refers to storage pool, so we should put it in model/storagepools.py I don't think putting it there will cause some circular import error
Well, I will never do this without investigation or examination. Here is error when I got when it in storage pool: Traceback (most recent call last): File "src/kimchid", line 31, in <module> import kimchi.server File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/server.py", line 34, in <module> from kimchi import mockmodel File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/mockmodel.py", line 51, in <module> from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/model/storagepools.py", line 30, in <module> from kimchi.model.host import DeviceModel File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/model/host.py", line 38, in <module> from kimchi.model.vms import DOM_STATE_MAP File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/model/vms.py", line 36, in <module> from kimchi.model.storagepools import READONLY_TYPE ImportError: cannot import name READONLY_TYPE The circular reference can be described by following picture: server.py -->mockmodel-->storagepools--->host-->vms | | |______________________|
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b482e80..f4bf371 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -33,6 +33,7 @@ from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel +from kimchi.model.constant import READONLY_TYPE from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot @@ -186,7 +187,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_TYPE: if not params.get('volumes'): raise MissingParameter('KCHVM0017E') else: @@ -218,9 +219,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_TYPE: + for v in vol_list: + vol = conn.storageVolLookupByPath(v['path']) + vol.delete(0) raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()})
@@ -357,10 +359,6 @@ class VMModel(object):
dom.undefine()
- for path in paths: - vol = conn.storageVolLookupByPath(path) - vol.delete(0) - with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True)
On 02/24/2014 11:02 PM, Royce Lv wrote:
On 2014年02月24日 21:34, Aline Manera wrote:
On 02/24/2014 07:24 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
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, work around it by delete vol clear part, in the future, this will be fixed by volume reference information tracking.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/constant.py | 24 ++++++++++++++++++++++++ src/kimchi/model/vms.py | 14 ++++++-------- 2 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 src/kimchi/model/constant.py
diff --git a/src/kimchi/model/constant.py b/src/kimchi/model/constant.py new file mode 100644 index 0000000..579cc66 --- /dev/null +++ b/src/kimchi/model/constant.py @@ -0,0 +1,24 @@ +# +# Kimchi +# +# Copyright IBM Corp, 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +# storage pool constant +READONLY_TYPE = ['iscsi', 'scsi', 'mpath']
It refers to storage pool, so we should put it in model/storagepools.py I don't think putting it there will cause some circular import error
Well, I will never do this without investigation or examination.
Here is error when I got when it in storage pool: Traceback (most recent call last): File "src/kimchid", line 31, in <module> import kimchi.server File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/server.py", line 34, in <module> from kimchi import mockmodel File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/mockmodel.py", line 51, in <module> from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/model/storagepools.py", line 30, in <module> from kimchi.model.host import DeviceModel File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/model/host.py", line 38, in <module> from kimchi.model.vms import DOM_STATE_MAP File "/home/royce/lvroyce/kimchi/kimchi/src/kimchi/model/vms.py", line 36, in <module> from kimchi.model.storagepools import READONLY_TYPE ImportError: cannot import name READONLY_TYPE
The circular reference can be described by following picture:
server.py -->mockmodel-->storagepools--->host-->vms | | |______________________|
Ops... I haven't thought on mockmodel What about add the constant to config.py?
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b482e80..f4bf371 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -33,6 +33,7 @@ from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel +from kimchi.model.constant import READONLY_TYPE from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot @@ -186,7 +187,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_TYPE: if not params.get('volumes'): raise MissingParameter('KCHVM0017E') else: @@ -218,9 +219,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_TYPE: + for v in vol_list: + vol = conn.storageVolLookupByPath(v['path']) + vol.delete(0) raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()})
@@ -357,10 +359,6 @@ class VMModel(object):
dom.undefine()
- for path in paths: - vol = conn.storageVolLookupByPath(path) - vol.delete(0) - with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True)
From: Royce Lv <lvroyce@linux.vnet.ibm.com> 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@linux.vnet.ibm.com> --- 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 e8f03ae..300981c 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 7d08254..03ca3cb 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -145,6 +145,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 20c65b9..d202eaa 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -25,9 +25,10 @@ import os import libvirt from kimchi import xmlutils -from kimchi.exception import InvalidOperation, IsoFormatError +from kimchi.exception import InvalidOperation, IsoFormatError, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.isoinfo import IsoImage +from kimchi.model.constant import READONLY_TYPE from kimchi.model.storagepools import StoragePoolModel @@ -40,8 +41,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> @@ -59,12 +61,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': item, 'volume': name}) + pool_info = StoragePoolModel(conn=self.conn, + objstore=self.objstore).lookup(pool_name) + if pool_info['type'] in READONLY_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) try: pool.createXML(xml, 0) except libvirt.libvirtError as e: @@ -89,6 +95,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) @@ -139,6 +146,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_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) + volume = self._get_storagevolume(pool, name) try: volume.delete(0) -- 1.8.1.2
Am 24-02-2014 07:24, schrieb lvroyce@linux.vnet.ibm.com:
@@ -25,9 +25,10 @@ import os import libvirt
from kimchi import xmlutils -from kimchi.exception import InvalidOperation, IsoFormatError +from kimchi.exception import InvalidOperation, IsoFormatError, InvalidParameter Alphabetical order.
On 2014年02月24日 20:41, Crístian Viana wrote:
Am 24-02-2014 07:24, schrieb lvroyce@linux.vnet.ibm.com:
@@ -25,9 +25,10 @@ import os import libvirt
from kimchi import xmlutils -from kimchi.exception import InvalidOperation, IsoFormatError +from kimchi.exception import InvalidOperation, IsoFormatError, InvalidParameter Alphabetical order. ACK
participants (4)
-
Aline Manera -
Crístian Viana -
lvroyce＠linux.vnet.ibm.com -
Royce Lv