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

[Aline Manera]

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

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