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

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; --- 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..4d90265 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: + 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: + 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