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

From: Bianca Carvalho <bianca(a)linux.vnet.ibm.com&gt; Edited function get_disk_used_by in diskutils.py file to remove used_by verification in objstore and edited all function calls. Also removed set_disk_used_by function from diskutils.py. Signed-off-by: Bianca Carvalho <bianca(a)linux.vnet.ibm.com&gt; --- model/diskutils.py | 61 ++++++++++--------------------------------------- model/storagevolumes.py | 14 ++---------- model/vms.py | 11 --------- model/vmstorages.py | 15 +++--------- 4 files changed, 17 insertions(+), 84 deletions(-) diff --git a/model/diskutils.py b/model/diskutils.py index 6faab7c..a3162d7 100644 --- a/model/diskutils.py +++ b/model/diskutils.py @@ -17,10 +17,6 @@ # 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.utils import wok_log - -from wok.plugins.kimchi.config import get_kimchi_version from wok.plugins.kimchi.model.vms import VMModel, VMsModel from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks @@ -31,49 +27,16 @@ 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: - 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}) - 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}) - return used_by - +def get_disk_used_by(conn, 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) -def set_disk_used_by(objstore, path, new_used_by): - try: - with objstore as session: - session.store('storagevolume', path, {'used_by': new_used_by}, - get_kimchi_version()) - except Exception as e: - raise OperationFailed('KCHVOL0017E', {'err': e.message}) + return used_by diff --git a/model/storagevolumes.py b/model/storagevolumes.py index 4708674..a6ce97b 100644 --- a/model/storagevolumes.py +++ b/model/storagevolumes.py @@ -40,7 +40,6 @@ from wok.plugins.kimchi.config import READONLY_POOL_TYPE from wok.plugins.kimchi.isoinfo import IsoImage from wok.plugins.kimchi.kvmusertests import UserTests from wok.plugins.kimchi.model.diskutils import get_disk_used_by -from wok.plugins.kimchi.model.diskutils import set_disk_used_by from wok.plugins.kimchi.model.storagepools import StoragePoolModel from wok.plugins.kimchi.utils import get_next_clone_name @@ -163,9 +162,7 @@ class StorageVolumesModel(object): vol_info = StorageVolumeModel(conn=self.conn, objstore=self.objstore).lookup(pool_name, name) - vol_path = vol_info['path'] - set_disk_used_by(self.objstore, vol_info['path'], []) if params.get('upload', False): upload_volumes[vol_path] = {'lock': threading.Lock(), @@ -251,11 +248,6 @@ class StorageVolumesModel(object): finally: os.remove(file_path) - vol_info = StorageVolumeModel(conn=self.conn, - objstore=self.objstore).lookup(pool_name, - name) - set_disk_used_by(self.objstore, vol_info['path'], []) - cb('OK', True) def get_list(self, pool_name): @@ -335,7 +327,7 @@ class StorageVolumeModel(object): except UnicodeDecodeError: isvalid = False - used_by = get_disk_used_by(self.objstore, self.conn, path) + used_by = get_disk_used_by(self.conn, path) if (self.libvirt_user is None): self.libvirt_user = UserTests().probe_user() ret, _ = probe_file_permission_as_user(path, self.libvirt_user) @@ -493,9 +485,7 @@ class StorageVolumeModel(object): 'pool': orig_pool_name, 'err': e.get_error_message()}) - new_vol = self.lookup(new_pool_name, new_vol_name) - cb('adding volume to the object store') - set_disk_used_by(self.objstore, new_vol['path'], []) + self.lookup(new_pool_name, new_vol_name) cb('OK', True) diff --git a/model/vms.py b/model/vms.py index 7f607f5..b2ee122 100644 --- a/model/vms.py +++ b/model/vms.py @@ -1398,17 +1398,6 @@ class VMModel(object): except Exception as e: raise OperationFailed('KCHVOL0017E', {'err': e.message}) - try: - with self.objstore as session: - if path in session.get_list('storagevolume'): - used_by = session.get('storagevolume', path)['used_by'] - used_by.remove(name) - session.store('storagevolume', path, - {'used_by': used_by}, - get_kimchi_version()) - except Exception as e: - raise OperationFailed('KCHVOL0017E', {'err': e.message}) - try: with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True) diff --git a/model/vmstorages.py b/model/vmstorages.py index b55b156..2e9f783 100644 --- a/model/vmstorages.py +++ b/model/vmstorages.py @@ -26,7 +26,6 @@ from wok.utils import wok_log from wok.plugins.kimchi.model.config import CapabilitiesModel from wok.plugins.kimchi.model.diskutils import get_disk_used_by -from wok.plugins.kimchi.model.diskutils import set_disk_used_by from wok.plugins.kimchi.model.storagevolumes import StorageVolumeModel from wok.plugins.kimchi.model.utils import get_vm_config_flag from wok.plugins.kimchi.model.vms import DOM_STATE_MAP, VMModel @@ -151,7 +150,6 @@ class VMStoragesModel(object): if params.get('vol'): used_by = vol_info['used_by'] used_by.append(vm_name) - set_disk_used_by(self.objstore, params['path'], used_by) return dev @@ -191,7 +189,7 @@ class VMStorageModel(object): # in the obj store, its ref count would have been updated # by get_disk_used_by() if path is not None: - used_by = get_disk_used_by(self.objstore, self.conn, path) + used_by = get_disk_used_by(self.conn, path) else: wok_log.error("Unable to decrement volume used_by on" " delete because no path could be found.") @@ -202,7 +200,6 @@ class VMStorageModel(object): if used_by is not None and vm_name in used_by: used_by.remove(vm_name) - set_disk_used_by(self.objstore, path, used_by) else: wok_log.error("Unable to update %s:%s used_by on delete." % (vm_name, dev_name)) @@ -223,11 +220,9 @@ class VMStorageModel(object): if new_disk_path != old_disk_path: # An empty path means a CD-ROM was empty or ejected: if old_disk_path is not '': - old_disk_used_by = get_disk_used_by( - self.objstore, self.conn, old_disk_path) + old_disk_used_by = get_disk_used_by(self.conn, old_disk_path) if new_disk_path is not '': - new_disk_used_by = get_disk_used_by( - self.objstore, self.conn, new_disk_path) + new_disk_used_by = get_disk_used_by(self.conn, new_disk_path) dev_info.update(params) dev, xml = get_disk_xml(dev_info) @@ -241,12 +236,8 @@ class VMStorageModel(object): if old_disk_used_by is not None and \ vm_name in old_disk_used_by: old_disk_used_by.remove(vm_name) - set_disk_used_by(self.objstore, old_disk_path, - old_disk_used_by) if new_disk_used_by is not None: new_disk_used_by.append(vm_name) - set_disk_used_by(self.objstore, new_disk_path, - new_disk_used_by) except Exception as e: wok_log.error("Unable to update dev used_by on update due to" " %s:" % e.message) -- 2.7.4