Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> On 03/10/2014 05:05 PM, lvroyce@linux.vnet.ibm.com wrote:

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

List vms is a common function which is likely to be refered by multiple module, make it static will avoid unnecessary instantiation of vms model and stats collecting task to start.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 3f3a152..3b8a666 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -226,7 +226,11 @@ class VMsModel(object): return name

def get_list(self): - conn = self.conn.get() + return self.get_vms(self.conn) + + @staticmethod + def get_vms(conn): + conn = conn.get() names = [dom.name().decode('utf-8') for dom in conn.listAllDomains(0)] return sorted(names, key=unicode.lower)

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

Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> a log info is better? On 03/10/2014 05:05 PM, lvroyce@linux.vnet.ibm.com wrote:

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

When vm disk does not have source element, objectify library will error with 'no such element: source' Trap this exception.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 6298a81..c1c90ce 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -163,17 +163,20 @@ class VMStorageModel(object): if disk is None: raise NotFoundError("KCHCDROM0007E", {'dev_name': dev_name, 'vm_name': vm_name}) - source = disk.source path = "" - if source is not None: - src_type = disk.attrib['type'] - if src_type == 'network': - host = source.host - path = (source.attrib['protocol'] + '://' + - host.attrib['name'] + ':' + - host.attrib['port'] + source.attrib['name']) - else: - path = source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]] + try: + source = disk.source + if source is not None: + src_type = disk.attrib['type'] + if src_type == 'network': + host = source.host + path = (source.attrib['protocol'] + '://' + + host.attrib['name'] + ':' + + host.attrib['port'] + source.attrib['name']) + else: + path = source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]] + except: + pass dev_type = disk.attrib['device'] return {'dev': dev_name, 'type': dev_type,

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

Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> On 03/10/2014 05:05 PM, lvroyce@linux.vnet.ibm.com wrote:

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

Update doc to support storage volume reference count.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/docs/API.md b/docs/API.md index 834c861..b3b6c49 100644 --- a/docs/API.md +++ b/docs/API.md @@ -395,6 +395,9 @@ A interface represents available network interface on VM. * os_distro *(optional)*: os distribution of the volume, for iso volume only. * os_version *(optional)*: os version of the volume, for iso volume only. * bootable *(optional)*: True if iso image is bootable and not corrupted. + * ref_cnt: Number of vms which used this volume, + 0 for volumes which are available for attachment. + >1 indicate number of vms used this volume.

* **DELETE**: Remove the Storage Volume * **POST**: *See Storage Volume Actions*

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

Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> On 03/10/2014 05:05 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 | 25 +++++++++++++++++++++++++ src/kimchi/control/storagevolumes.py | 1 + src/kimchi/i18n.py | 4 ++++ 3 files changed, 30 insertions(+)

diff --git a/src/kimchi/API.json b/src/kimchi/API.json index f595bbf..6e932ed 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -143,6 +143,31 @@ } } }, + "storagevolumes_create": { + "type": "object", + "error": "KCHVOL0016E", + "properties": { + "name": { + "description": "The name of the Storage Volume", + "type": "string", + "minLength": 1, + "required": true, + "error": "KCHVOL0013E" + }, + "allocation": { + "description": "The size(MiB) of allocation when create the storage volume", + "type": "number", + "minimum": 1, + "error": "KCHVOL0014E" + }, + "format": { + "description": "The format of the volume", + "type": "string", + "pattern": "^qcow2|raw$", + "error": "KCHVOL0015E" + } + } + }, "vms_create": { "type": "object", "error": "KCHVM0016E", diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index 718c97a..c4d6c41 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -49,6 +49,7 @@ class StorageVolume(Resource): 'capacity': self.info['capacity'], 'allocation': self.info['allocation'], 'path': self.info['path'], + 'ref_cnt': self.info['ref_cnt'], '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 cbfcf5d..3108921 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -152,6 +152,10 @@ messages = { "KCHVOL0010E": _("Unable to delete storage volume %(name)s. Details: %(err)s"), "KCHVOL0011E": _("Unable to resize storage volume %(name)s. Details: %(err)s"), "KCHVOL0012E": _("Storage type %(type)s does not support volume create and delete"), + "KCHVOL0013E": _("Storage volume name must be a string"), + "KCHVOL0014E": _("Storage volume allocation must be an integer number"), + "KCHVOL0015E": _("Storage volume format not supported"), + "KCHVOL0016E": _("Storage volume requires a volume name"),

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

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

Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> On 03/10/2014 05:05 PM, lvroyce@linux.vnet.ibm.com wrote:

From: Royce Lv <lvroyce@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@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 3 +++ src/kimchi/model/storagevolumes.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)

diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 4ead84b..1460d67 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -426,6 +426,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) @@ -890,6 +891,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 @@ -1009,6 +1011,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 6811e15..3b4da82 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -28,6 +28,8 @@ from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.isoinfo import IsoImage from kimchi.model.storagepools import StoragePoolModel from kimchi.utils import kimchi_log +from kimchi.model.vms import VMsModel +from kimchi.model.vmstorages import VMStoragesModel, VMStorageModel

VOLUME_TYPE_MAP = {0: 'file', @@ -58,6 +60,7 @@ class StorageVolumesModel(object): params.setdefault('format', 'qcow2')

name = params['name'] + vol_id = '%s:%s' % (pool_name, name) try: pool = StoragePoolModel.get_storagepool(pool_name, self.conn) xml = vol_xml % params @@ -75,6 +78,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): @@ -108,6 +115,26 @@ 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.get_vms(self.conn) + 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() @@ -121,10 +148,12 @@ class StorageVolumeModel(object): # infomation. When there is no format information, we assume # it's 'raw'. fmt = 'raw' + 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):

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

Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> On 03/10/2014 05:05 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. 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 1fa40fa..d661247 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -353,6 +353,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 @@ -402,6 +403,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'])

# reset template to default storage pool # so we can remove the storage pool created 'test-pool' diff --git a/tests/test_rest.py b/tests/test_rest.py index 17bd253..65d50e0 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -245,6 +245,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') @@ -848,6 +849,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

Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> On 03/10/2014 05:05 PM, lvroyce@linux.vnet.ibm.com wrote:

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

These can be diagnosed by pep8 1.3.3, but may not report error in some version, These errors does not comply: http://www.python.org/dev/peps/pep-0008/ so fix them.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 7 ++++--- src/kimchi/model/debugreports.py | 2 +- src/kimchi/model/storagevolumes.py | 3 ++- src/kimchi/model/templates.py | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 1460d67..1d78ca0 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -220,7 +220,7 @@ class MockModel(object): if not name: iso = params['cdrom'] iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0] - name = iso_name + str(int(time.time()*1000)) + name = iso_name + str(int(time.time() * 1000)) params['name'] = name

if name in self._mock_templates: @@ -918,7 +918,8 @@ class MockVMIface(object):

@classmethod def get_mac(cls): - mac = ":".join(["52", "54"] + ["%02x" % (cls.counter/(256**i) % 256) + mac = ":".join(["52", "54"] + + ["%02x" % (cls.counter / (256 ** i) % 256) for i in range(3, -1, -1)]) return mac

@@ -1046,7 +1047,7 @@ class MockVMScreenshot(VMScreenshot): self.coord = (self.coord[0], self.coord[1], min(MockVMScreenshot.BOX_COORD[2], - self.coord[2]+random.randrange(50)), + self.coord[2] + random.randrange(50)), self.coord[3])

image = Image.new("RGB", (256, 256), self.background) diff --git a/src/kimchi/model/debugreports.py b/src/kimchi/model/debugreports.py index 3e14848..c6e698b 100644 --- a/src/kimchi/model/debugreports.py +++ b/src/kimchi/model/debugreports.py @@ -41,7 +41,7 @@ class DebugReportsModel(object): ident = params.get('name').strip() # Generate a name with time and millisec precision, if necessary if ident is None or ident == "": - ident = 'report-' + str(int(time.time()*1000)) + ident = 'report-' + str(int(time.time() * 1000)) taskid = self._gen_debugreport_file(ident) return self.task.lookup(taskid)

diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 3b4da82..1253823 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -129,7 +129,8 @@ class StorageVolumeModel(object): for vm in vms: storages = VMStoragesModel(**args).get_list(vm) for disk in storages: - if path == VMStorageModel(**args).lookup(vm, disk)['path']: + d_info = VMStorageModel(**args).lookup(vm, disk) + if path == d_info['path']: ref_cnt = ref_cnt + 1 session.store('storagevolume', vol_id, {'ref_cnt': ref_cnt})

diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 52e7eab..2e66a94 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -39,7 +39,7 @@ class TemplatesModel(object): if not name: iso = params['cdrom'] iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0] - name = iso_name + str(int(time.time()*1000)) + name = iso_name + str(int(time.time() * 1000)) params['name'] = name

conn = self.conn.get()

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