[Kimchi-devel] [PATCHv2 1/2] disk attachment: fix ref_cnt to prevent double attach

[lvroyce at linux.vnet.ibm.com]

From: Royce Lv <lvroyce at linux.vnet.ibm.com>

Ref count is not updated when attach and detach,
this will introduce double attachment.
Fix and update the test case.

Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
---
 src/kimchi/model/vmstorages.py | 25 +++++++++++++++++++++++++
 tests/test_model.py            |  1 +
 2 files changed, 26 insertions(+)

diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py
index ecc20cf..cf9748f 100644
--- a/src/kimchi/model/vmstorages.py
+++ b/src/kimchi/model/vmstorages.py
@@ -155,6 +155,7 @@ class VMStoragesModel(object):
                 vol_info = StorageVolumeModel(
                     conn=self.conn,
                     objstore=self.objstore).lookup(pool, params['vol'])
+                vol_id = '%s:%s' % (pool, params['vol'])
             except KeyError:
                 raise InvalidParameter("KCHVMSTOR0012E")
             except Exception as e:
@@ -177,6 +178,10 @@ class VMStoragesModel(object):
             conn = self.conn.get()
             dom = conn.lookupByName(vm_name)
             dom.attachDeviceFlags(dev_xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT)
+            with self.objstore as session:
+                ref_cnt = session.store('storagevolume', vol_id,
+                                        {'ref_cnt': vol_info['ref_cnt'] + 1})
+
         except Exception as e:
             raise OperationFailed("KCHVMSTOR0008E", {'error': e.message})
         return params['dev']
@@ -200,6 +205,7 @@ class VMStoragesModel(object):
 class VMStorageModel(object):
     def __init__(self, **kargs):
         self.conn = kargs['conn']
+        self.objstore = kargs['objstore']
 
     def lookup(self, vm_name, dev_name):
         # Retrieve disk xml and format return dict
@@ -208,6 +214,7 @@ class VMStorageModel(object):
 
     def delete(self, vm_name, dev_name):
         # Get storage device xml
+        vol = None
         dom = VMModel.get_vm(vm_name, self.conn)
         try:
             bus_type = self.lookup(vm_name, dev_name)['bus']
@@ -218,6 +225,17 @@ class VMStorageModel(object):
         if (bus_type not in HOTPLUG_TYPE and
                 DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
             raise InvalidOperation('KCHVMSTOR0011E')
+        disk_info = get_vm_disk(dom, dev_name)
+        try:
+            conn = self.conn.get()
+            vol = conn.storageVolLookupByPath(disk_info['path'])
+            pool = vol.storagePoolLookupByVolume().name()
+            vol_info = StorageVolumeModel(
+                conn=self.conn,
+                objstore=self.objstore).lookup(pool, vol.name())
+        except libvirt.libvirtError:
+            # disk does not exist in any stoage pool
+            pass
 
         try:
             conn = self.conn.get()
@@ -228,6 +246,13 @@ class VMStorageModel(object):
         except Exception as e:
             raise OperationFailed("KCHVMSTOR0010E", {'error': e.message})
 
+        if vol:
+            # Handle ref_cnt after detach to avoid failure rollback
+            vol_id = "%s:%s" % (pool, vol.name())
+            with self.objstore as session:
+                ref_cnt = session.store('storagevolume', vol_id,
+                                        {'ref_cnt': vol_info['ref_cnt'] - 1})
+
     def update(self, vm_name, dev_name, params):
         params['src_type'] = _check_path(params['path'])
         dom = VMModel.get_vm(vm_name, self.conn)
diff --git a/tests/test_model.py b/tests/test_model.py
index e3dff95..9f43164 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -242,6 +242,7 @@ class ModelTests(unittest.TestCase):
 
             # Cold plug and unplug a disk
             disk = _attach_disk()
+            self.assertRaises(InvalidParameter, _attach_disk)
             inst.vmstorage_delete(vm_name, disk)
 
             # Hot plug a disk
-- 
1.8.3.2