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..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

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; CDROM can be shared by multiple vms, so we shall not check its ref_cnt. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/model/vmstorages.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index cf9748f..cff3108 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -160,10 +160,11 @@ class VMStoragesModel(object): raise InvalidParameter("KCHVMSTOR0012E") except Exception as e: raise InvalidParameter("KCHVMSTOR0015E", {'error': e}) - if vol_info['ref_cnt'] != 0: + if vol_info['ref_cnt'] != 0 and params['type'] == 'disk': raise InvalidParameter("KCHVMSTOR0016E") params['format'] = vol_info['format'] params['path'] = vol_info['path'] + params['src_type'] = _check_path(params['path']) params.setdefault( 'bus', _get_device_bus(params['type'], dom)) @@ -178,12 +179,15 @@ class VMStoragesModel(object): conn = self.conn.get() dom = conn.lookupByName(vm_name) dom.attachDeviceFlags(dev_xml, libvirt.VIR_DOMAIN_AFFECT_CURRENT) + + except Exception as e: + raise OperationFailed("KCHVMSTOR0008E", {'error': e.message}) + + if params.get('vol'): 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'] def _get_storage_device_name(self, vm_name): -- 1.8.3.2

On 2014年05月15日 22:21, Sheldon wrote: For cdrom attach and detach, we used path as parameter. When dealing with disk attach detach, we used pool/vol as parameters

On 2014年05月15日 22:32, Sheldon wrote: The other types will be rejected by json schema