comments below On 02/17/2014 04:38 PM, lvroyce@linux.vnet.ibm.com wrote:

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

Add ref_cnt to controller to report reference count information. Update json schema validation.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 24 ++++++++++++++++++++++++ src/kimchi/control/storagevolumes.py | 1 + src/kimchi/i18n.py | 3 +++ 3 files changed, 28 insertions(+)

diff --git a/src/kimchi/API.json b/src/kimchi/API.json index a61e778..77c9f5c 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -131,6 +131,30 @@ } } }, + "storagevolumes_create": { + "type": "object", + "properties": { + "name": { + "description": "The name of the Storage Volume", + "type": "string", + "minLength": 1, + "required": true, + "error": "KCHVOL0012E" + }, + "allocation": { + "description": "The size(MiB) of allocation when create the storage volume", + "type": "number", + "minimum": 1, + "error": "KCHVOL0013E" + }, + "format": { + "description": "The format of the volume", + "type": "string", + "pattern": "^qcow2|raw$", + "error": "KCHVOL0014E" + } + } + }, "vms_create": { "type": "object", "error": "KCHVM0016E", diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index cd15bcc..b05a5a8 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -55,6 +55,7 @@ class StorageVolume(Resource): 'capacity': self.info['capacity'], 'allocation': self.info['allocation'], 'path': self.info['path'], + 'ref_cnt': self.info['ref_cnt'], you API.md says:

+ * ref_count: Number of vms which used this volume,

'format': self.info['format']}

for key in ('os_version', 'os_distro', 'bootable'): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index b5ed850..b7f4ff5 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -144,6 +144,9 @@ messages = { "KCHVOL0009E": _("Unable to wipe storage volumes %(name)s. Details: %(err)s"), "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), + "KCHVOL0012E": _("Storage volume name must be a string"), + "KCHVOL0013E": _("Storage volume allocation must be an integer number"), + "KCHVOL0014E": _("Storage volume format not supported"),

"KCHIFACE0001E": _("Interface %(name)s does not exist"),

-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

On 2014年02月17日 20:13, Sheldon wrote:

On 02/17/2014 04:38 PM, lvroyce@linux.vnet.ibm.com wrote:

...

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

Set ref_cnt to 0 when new volume created, report it when lookup storage volume, for volume outside kimchi scope, For me, I care when ref_cnt decrease and increase? It is useful for disable network/storagpool used by guest/template. I want to discuss this with guys, ref_cnt -1 means this volume is internal of vm and it cannot be used by others. 0 means this volume is not shared and can be used. 1 means vms share this volume.

I think for your case, is that possible to check if ref_cnt !=0: reject inactivate?

...

set ref_cnt to be -1.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/storagevolumes.py | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+)

diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index bde6a5c..1faf2a5 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -386,6 +386,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) @@ -747,6 +748,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 @@ -850,6 +852,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 e3f00ca..3dc89a9 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -40,6 +40,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 +59,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 +73,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 +95,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 +110,30 @@ class StorageVolumeModel(object): else: raise

+ def _get_ref_cnt(self, pool, name): + vol_id = '%s:%s' % (pool, name) + with self.objstore as session: + try: + extra_info = session.get('storagevolume', vol_id) + except NotFoundError: + extra_info = {} + + # -1 for storage volume created beyond kimchi scope + ref_cnt = extra_info.get('ref_cnt', -1) + 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] + res = dict(type=VOLUME_TYPE_MAP[info[0]], capacity=info[1], allocation=info[2], path=path, + ref_cnt=self._get_ref_cnt(pool, name), format=fmt) if fmt == 'iso': if os.path.islink(path):

comments below On 02/17/2014 04:38 PM, lvroyce@linux.vnet.ibm.com wrote:

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

Add mockmodel test and model test to validate: 1. ref_cnt of storage volume forked internal vm creation is -1. should this be 1? for it is used by a VM. 2. ref_cnt of storage volume created from REST API is 0.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_model.py | 4 ++++ tests/test_rest.py | 2 ++ 2 files changed, 6 insertions(+)

diff --git a/tests/test_model.py b/tests/test_model.py index b374d2d..7ab002f 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -288,6 +288,7 @@ class ModelTests(unittest.TestCase): inst.storagevolume_wipe(pool, vol) volinfo = inst.storagevolume_lookup(pool, vol) self.assertEquals(0, volinfo['allocation']) + self.assertEquals(0, volinfo['ref_cnt'])

volinfo = inst.storagevolume_lookup(pool, vol) # Define the size = capacity + 16M @@ -337,6 +338,9 @@ class ModelTests(unittest.TestCase): vm_info = inst.vm_lookup(params['name']) disk_path = '/tmp/kimchi-images/%s-0.img' % vm_info['uuid'] self.assertTrue(os.access(disk_path, os.F_OK)) + vol = '%s-0.img' % vm_info['uuid'] + volinfo = inst.storagevolume_lookup(pool, vol) + self.assertEquals(-1, volinfo['ref_cnt'])

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_create(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index deb6fe8..e052cd0 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -244,6 +244,7 @@ class RestTests(unittest.TestCase): resp = self.request(vol_uri) vol = json.loads(resp.read()) self.assertEquals(1 << 30, vol['capacity']) + self.assertEquals(-1, vol['ref_cnt'])

# Start the VM resp = self.request('/vms/test-vm/start', '{}', 'POST') @@ -769,6 +770,7 @@ class RestTests(unittest.TestCase): storagevolume = json.loads(resp.read()) self.assertEquals('volume-1', storagevolume['name']) self.assertEquals('raw', storagevolume['format']) + self.assertEquals(0, storagevolume['ref_cnt']) self.assertEquals('/var/lib/libvirt/images/volume-1', storagevolume['path'])

-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center