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

bianca at linux.vnet.ibm.com bianca at linux.vnet.ibm.com
Wed Aug 31 18:54:23 UTC 2016


From: Bianca Carvalho <bianca at linux.vnet.ibm.com>

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 at linux.vnet.ibm.com>
---
 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




More information about the Kimchi-devel mailing list