[Kimchi-devel] [PATCH 1/2] Fix vm creation storage rollback clean

[Aline Manera]

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 at linux.vnet.ibm.com wrote:
>>> From: Royce Lv <lvroyce at 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 at 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 at 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)
>>>
>>
>