[PATCH] [Kimchi] Issue #626: Snapshot revert does not release storage volume

From: Bianca Carvalho <bianca@linux.vnet.ibm.com> Edited diskutils.py file to remove used_by verification in objstore, now this field will be only verified in the vm when get_disk_used_by is used by StorageVolumeModel lookup. Signed-off-by: Bianca Carvalho <bianca@linux.vnet.ibm.com> --- model/diskutils.py | 50 ++++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/model/diskutils.py b/model/diskutils.py index 6faab7c..3b8f441 100644 --- a/model/diskutils.py +++ b/model/diskutils.py @@ -17,7 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -from wok.exception import OperationFailed, NotFoundError +from wok.exception import OperationFailed from wok.utils import wok_log from wok.plugins.kimchi.config import get_kimchi_version @@ -34,34 +34,28 @@ from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks def get_disk_used_by(objstore, conn, path): try: with objstore as session: + used_by = [] + # try to find this volume in existing vm + vms_list = VMsModel.get_vms(conn) + for vm in vms_list: + dom = VMModel.get_vm(vm, conn) + storages = get_vm_disks(dom) + for disk in storages.keys(): + d_info = get_vm_disk_info(dom, disk) + if path == d_info['path']: + used_by.append(vm) try: - used_by = session.get('storagevolume', path)['used_by'] - except (KeyError, NotFoundError): - wok_log.info('Volume %s not found in obj store.' % path) - used_by = [] - # try to find this volume in existing vm - vms_list = VMsModel.get_vms(conn) - for vm in vms_list: - dom = VMModel.get_vm(vm, conn) - storages = get_vm_disks(dom) - for disk in storages.keys(): - d_info = get_vm_disk_info(dom, disk) - if path == d_info['path']: - used_by.append(vm) - try: - session.store('storagevolume', path, - {'used_by': used_by}, - get_kimchi_version()) - except Exception as e: - # Let the exception be raised. If we allow disks' - # used_by to be out of sync, data corruption could - # occour if a disk is added to two guests - # unknowingly. - wok_log.error('Unable to store storage volume id in' - ' objectstore due error: %s', - e.message) - raise OperationFailed('KCHVOL0017E', - {'err': e.message}) + session.store('storagevolume', path, {'used_by': used_by}, + get_kimchi_version()) + except Exception as e: + # Let the exception be raised. If we allow disks' + # used_by to be out of sync, data corruption could + # occour if a disk is added to two guests + # unknowingly. + wok_log.error('Unable to store storage volume id in' + ' objectstore due error: %s', + e.message) + raise OperationFailed('KCHVOL0017E', {'err': e.message}) except Exception as e: # This exception is going to catch errors returned by 'with', # specially ones generated by 'session.store'. It is outside -- 2.7.4

Reviewed-By: Ramon Medeiros <ramonn@br.ibm.com> On 08/29/2016 04:44 PM, bianca@linux.vnet.ibm.com wrote:
From: Bianca Carvalho <bianca@linux.vnet.ibm.com>
Edited diskutils.py file to remove used_by verification in objstore, now this field will be only verified in the vm when get_disk_used_by is used by StorageVolumeModel lookup.
Signed-off-by: Bianca Carvalho <bianca@linux.vnet.ibm.com> --- model/diskutils.py | 50 ++++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 28 deletions(-)
diff --git a/model/diskutils.py b/model/diskutils.py index 6faab7c..3b8f441 100644 --- a/model/diskutils.py +++ b/model/diskutils.py @@ -17,7 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-from wok.exception import OperationFailed, NotFoundError +from wok.exception import OperationFailed from wok.utils import wok_log
from wok.plugins.kimchi.config import get_kimchi_version @@ -34,34 +34,28 @@ from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks def get_disk_used_by(objstore, conn, path): try: with objstore as session: + used_by = [] + # try to find this volume in existing vm + vms_list = VMsModel.get_vms(conn) + for vm in vms_list: + dom = VMModel.get_vm(vm, conn) + storages = get_vm_disks(dom) + for disk in storages.keys(): + d_info = get_vm_disk_info(dom, disk) + if path == d_info['path']: + used_by.append(vm) try: - used_by = session.get('storagevolume', path)['used_by'] - except (KeyError, NotFoundError): - wok_log.info('Volume %s not found in obj store.' % path) - used_by = [] - # try to find this volume in existing vm - vms_list = VMsModel.get_vms(conn) - for vm in vms_list: - dom = VMModel.get_vm(vm, conn) - storages = get_vm_disks(dom) - for disk in storages.keys(): - d_info = get_vm_disk_info(dom, disk) - if path == d_info['path']: - used_by.append(vm) - try: - session.store('storagevolume', path, - {'used_by': used_by}, - get_kimchi_version()) - except Exception as e: - # Let the exception be raised. If we allow disks' - # used_by to be out of sync, data corruption could - # occour if a disk is added to two guests - # unknowingly. - wok_log.error('Unable to store storage volume id in' - ' objectstore due error: %s', - e.message) - raise OperationFailed('KCHVOL0017E', - {'err': e.message}) + session.store('storagevolume', path, {'used_by': used_by}, + get_kimchi_version()) + except Exception as e: + # Let the exception be raised. If we allow disks' + # used_by to be out of sync, data corruption could + # occour if a disk is added to two guests + # unknowingly. + wok_log.error('Unable to store storage volume id in' + ' objectstore due error: %s', + e.message) + raise OperationFailed('KCHVOL0017E', {'err': e.message}) except Exception as e: # This exception is going to catch errors returned by 'with', # specially ones generated by 'session.store'. It is outside
-- Ramon Nunes Medeiros Kimchi Developer Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn@br.ibm.com

On 08/29/2016 04:44 PM, bianca@linux.vnet.ibm.com wrote:
From: Bianca Carvalho <bianca@linux.vnet.ibm.com>
Edited diskutils.py file to remove used_by verification in objstore, now this field will be only verified in the vm when get_disk_used_by is used by StorageVolumeModel lookup.
Signed-off-by: Bianca Carvalho <bianca@linux.vnet.ibm.com> --- model/diskutils.py | 50 ++++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 28 deletions(-)
diff --git a/model/diskutils.py b/model/diskutils.py index 6faab7c..3b8f441 100644 --- a/model/diskutils.py +++ b/model/diskutils.py @@ -17,7 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-from wok.exception import OperationFailed, NotFoundError +from wok.exception import OperationFailed from wok.utils import wok_log
from wok.plugins.kimchi.config import get_kimchi_version @@ -34,34 +34,28 @@ from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks def get_disk_used_by(objstore, conn, path):
try: with objstore as session:
The objectstore is not needed anymore. You can remove this 'with' statement. Also, the above 'try' statement is not needed anymore. The correspondent except statement has the following comment: except Exception as e: # This exception is going to catch errors returned by 'with', # specially ones generated by 'session.store'. It is outside # to avoid conflict with the __exit__ function of 'with' raise OperationFailed('KCHVOL0017E', {'err': e.message}) So as we are not checking the objectstore anymore, we don't need the 'with' statement and also that 'try/expect' statement.
+ used_by = [] + # try to find this volume in existing vm + vms_list = VMsModel.get_vms(conn) + for vm in vms_list: + dom = VMModel.get_vm(vm, conn) + storages = get_vm_disks(dom) + for disk in storages.keys(): + d_info = get_vm_disk_info(dom, disk) + if path == d_info['path']: + used_by.append(vm) try: - used_by = session.get('storagevolume', path)['used_by'] - except (KeyError, NotFoundError): - wok_log.info('Volume %s not found in obj store.' % path) - used_by = [] - # try to find this volume in existing vm - vms_list = VMsModel.get_vms(conn) - for vm in vms_list: - dom = VMModel.get_vm(vm, conn) - storages = get_vm_disks(dom) - for disk in storages.keys(): - d_info = get_vm_disk_info(dom, disk) - if path == d_info['path']: - used_by.append(vm) - try: - session.store('storagevolume', path, - {'used_by': used_by}, - get_kimchi_version()) - except Exception as e: - # Let the exception be raised. If we allow disks' - # used_by to be out of sync, data corruption could - # occour if a disk is added to two guests - # unknowingly. - wok_log.error('Unable to store storage volume id in' - ' objectstore due error: %s', - e.message) - raise OperationFailed('KCHVOL0017E', - {'err': e.message}) + session.store('storagevolume', path, {'used_by': used_by}, + get_kimchi_version()) + except Exception as e: + # Let the exception be raised. If we allow disks' + # used_by to be out of sync, data corruption could + # occour if a disk is added to two guests + # unknowingly. + wok_log.error('Unable to store storage volume id in' + ' objectstore due error: %s', + e.message) + raise OperationFailed('KCHVOL0017E', {'err': e.message}) except Exception as e: # This exception is going to catch errors returned by 'with', # specially ones generated by 'session.store'. It is outside

Applied. Thanks. Regards, Aline Manera
participants (3)
-
Aline Manera
-
bianca@linux.vnet.ibm.com
-
Ramon Medeiros