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

[Royce Lv]

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
| |
|______________________|


>
>> 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)
>>
>