[Kimchi-devel] [PATCHv3 4/5] Add volume ref_cnt: Add model and mockmodel implementation
Royce Lv
lvroyce at linux.vnet.ibm.com
Thu Feb 27 03:21:39 UTC 2014
On 2014年02月25日 18:29, Sheldon wrote:
> volume ref_cnt is useful for storage pool integrity verification.
>
> But we should be care for ref_cnt.
>
> some questions about this ref_cnt.
> only when all volume ref_cnt of a storage pool(all types of pool) is zero
> 1. we can delete this storage pool, for all types of pool, right?
> 2. we can deactivate this storage pool, for all types of pool, right?
Right for me. I think I can think examples such as nfs pool, the nfs
mount take effect on activate and umount on deactivate.
So if some vm volume is in such pool, deactivate and delete pool will
make the volume inaccessible.
>
> when we decrease ref_cnt?
> 1. VM is deleted? the ref_cnt of attached volume will decrease?
ACK.
> 2. VM is deleted outside kimchi scope, we should decrease the ref_cnt?
Since we count the ref_cnt when kimchi started, I assume we can regard
that when kimchi starts to take control, there is no other "brain"
controls vms.
What do you think? So in this way we do not care about delete outside
kimchi scope, as this will not happen after kimchi started.
> you increase storage volume ref_cn when a vm is created outside kimchi
> scope.
I haven't increased, I just count the ref_cnt when encounter the volume
outside kimchi scope when first lookup.
> 3. A volume detached from a vm inside or outside kimchi scope.
Same as the above
>
> and now, the kimchi can avoid the ref_cnt race right?
> when a request get ref_cnt, and another request is decrease or
> increase ref_cnt at the same time?
I think objstore make sure there will be no race.
>
> from your code:
>
> + vms = VMsModel(**args).get_list()
>
> which is better ?
> VMsModel(**args).get_list() or self.vms_get_list()
This is a good point. I think "self.vms_get_list" is a better way to
refer it, I will have a try.
>
> On 02/25/2014 04:35 PM, lvroyce at linux.vnet.ibm.com wrote:
>> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>
>> Set ref_cnt to 0 when new volume created,
>> if the ref_cnt cannot be found,
>> which means it is created outside of kimchi scope,
>> report it when lookup storage volume,
>> search info about storage volumes used by vms and count the ref_cnt.
>>
>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>> ---
>> src/kimchi/mockmodel.py | 3 +++
>> src/kimchi/model/storagevolumes.py | 31
>> +++++++++++++++++++++++++++++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
>> index b50bf31..7620d38 100644
>> --- a/src/kimchi/mockmodel.py
>> +++ b/src/kimchi/mockmodel.py
>> @@ -423,6 +423,7 @@ class MockModel(object):
>> name = params['name']
>> volume = MockStorageVolume(pool, name, params)
>> volume.info['type'] = params['type']
>> + volume.info['ref_cnt'] = params.get('ref_cnt', 0)
>> volume.info['format'] = params['format']
>> volume.info['path'] = os.path.join(
>> pool.info['path'], name)
>> @@ -884,6 +885,7 @@ class MockVMTemplate(VMTemplate):
>> disk_paths = []
>> for vol_info in volumes:
>> vol_info['capacity'] = vol_info['capacity'] << 10
>> + vol_info['ref_cnt'] = 1
>> self.model.storagevolumes_create(pool.name, vol_info)
>> disk_paths.append({'pool': pool.name, 'volume':
>> vol_info['name']})
>> return disk_paths
>> @@ -1002,6 +1004,7 @@ class MockStorageVolume(object):
>> 'capacity': capacity << 20,
>> 'allocation': params.get('allocation', '512'),
>> 'path': params.get('path'),
>> + 'ref_cnt': params.get('ref_cnt'),
>> 'format': fmt}
>> if fmt == 'iso':
>> self.info['allocation'] = self.info['capacity']
>> diff --git a/src/kimchi/model/storagevolumes.py
>> b/src/kimchi/model/storagevolumes.py
>> index 20c65b9..9906f33 100644
>> --- a/src/kimchi/model/storagevolumes.py
>> +++ b/src/kimchi/model/storagevolumes.py
>> @@ -29,6 +29,8 @@ from kimchi.exception import InvalidOperation,
>> IsoFormatError
>> from kimchi.exception import MissingParameter, NotFoundError,
>> OperationFailed
>> from kimchi.isoinfo import IsoImage
>> from kimchi.model.storagepools import StoragePoolModel
>> +from kimchi.model.vms import VMsModel
>> +from kimchi.model.vmstorages import VMStoragesModel, VMStorageModel
>>
>>
>> VOLUME_TYPE_MAP = {0: 'file',
>> @@ -40,6 +42,7 @@ VOLUME_TYPE_MAP = {0: 'file',
>> class StorageVolumesModel(object):
>> def __init__(self, **kargs):
>> self.conn = kargs['conn']
>> + self.objstore = kargs['objstore']
>>
>> def create(self, pool, params):
>> vol_xml = """
>> @@ -58,6 +61,7 @@ class StorageVolumesModel(object):
>> params.setdefault('format', 'qcow2')
>>
>> name = params['name']
>> + vol_id = '%s:%s' % (pool, name)
>> try:
>> pool = StoragePoolModel.get_storagepool(pool, self.conn)
>> xml = vol_xml % params
>> @@ -71,6 +75,10 @@ class StorageVolumesModel(object):
>> raise OperationFailed("KCHVOL0007E",
>> {'name': name, 'pool': pool,
>> 'err': e.get_error_message()})
>> +
>> + with self.objstore as session:
>> + session.store('storagevolume', vol_id, {'ref_cnt': 0})
>> +
>> return name
>>
>> def get_list(self, pool_name):
>> @@ -89,6 +97,7 @@ class StorageVolumesModel(object):
>> class StorageVolumeModel(object):
>> def __init__(self, **kargs):
>> self.conn = kargs['conn']
>> + self.objstore = kargs['objstore']
>>
>> def _get_storagevolume(self, pool, name):
>> pool = StoragePoolModel.get_storagepool(pool, self.conn)
>> @@ -103,16 +112,38 @@ class StorageVolumeModel(object):
>> else:
>> raise
>>
>> + def _get_ref_cnt(self, pool, name, path):
>> + vol_id = '%s:%s' % (pool, name)
>> + with self.objstore as session:
>> + try:
>> + ref_cnt = session.get('storagevolume',
>> vol_id)['ref_cnt']
>> + except NotFoundError:
>> + # Fix storage volume created outside kimchi scope
>> + ref_cnt = 0
>> + args = {'conn': self.conn, 'objstore': self.objstore}
>> + # try to find this volume in exsisted vm
>> + vms = VMsModel(**args).get_list()
>> + for vm in vms:
>> + storages = VMStoragesModel(**args).get_list(vm)
>> + for disk in storages:
>> + if path == VMStorageModel(**args).lookup(vm,
>> disk)['path']:
>> + ref_cnt = ref_cnt + 1
>> + session.store('storagevolume', vol_id, {'ref_cnt':
>> ref_cnt})
>> +
>> + return ref_cnt
>> +
>> def lookup(self, pool, name):
>> vol = self._get_storagevolume(pool, name)
>> path = vol.path()
>> info = vol.info()
>> xml = vol.XMLDesc(0)
>> fmt = xmlutils.xpath_get_text(xml,
>> "/volume/target/format/@type")[0]
>> + ref_cnt = self._get_ref_cnt(pool, name, path)
>> res = dict(type=VOLUME_TYPE_MAP[info[0]],
>> capacity=info[1],
>> allocation=info[2],
>> path=path,
>> + ref_cnt=ref_cnt,
>> format=fmt)
>> if fmt == 'iso':
>> if os.path.islink(path):
>
>
More information about the Kimchi-devel
mailing list