Re: [Kimchi-devel] [PATCH 6/8] Create storage volume based on an existing volume

In order to clone VMs, we will need to be able to copy storage volumes. Add a new method to create a storage volume. This method requires the parameter 'volume_path' which specifies the base storage volume. The new volume will be created with the same content as the original one. Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com&gt; --- docs/API.md | 1 + src/kimchi/API.json | 5 +++ src/kimchi/i18n.py | 1 + src/kimchi/mockmodel.py | 44 ++++++++++++++++++++++- src/kimchi/model/storagevolumes.py | 73 +++++++++++++++++++++++++++++++++++--- tests/test_model.py | 19 ++++++++++ tests/test_rest.py | 23 ++++++++++++ 7 files changed, 161 insertions(+), 5 deletions(-) diff --git a/docs/API.md b/docs/API.md index 9c06f85..29ae4e1 100644 --- a/docs/API.md +++ b/docs/API.md @@ -445,6 +445,7 @@ A interface represents available network interface on VM. * format: The format of the defined Storage Volume * file: File to be uploaded, passed through form data * url: URL to be downloaded + * volume_path: The path of an existing storage volume to be copied.

### Resource: Storage Volume diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 0ad36ab..81492f5 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -216,6 +216,11 @@ "type": "string", "pattern": "^(http|ftp)[s]?://", "error": "KCHVOL0021E" + }, + "volume_path": { + "description": "The path of an existing storage volume to be copied", + "type": "string", + "error": "KCHVOL0023E" } } }, diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 10408bf..712e5a6 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -207,6 +207,7 @@ messages = { "KCHVOL0020E": _("Storage volume capacity must be an integer number."), "KCHVOL0021E": _("Storage volume URL must be http://, https://, ftp:// or ftps://."), "KCHVOL0022E": _("Unable to access file %(url)s. Please, check it."), + "KCHVOL0023E": _("Storage volume path must be a string"), "KCHIFACE0001E": _("Interface %(name)s does not exist"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 7163f8d..baee0b6 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -485,7 +485,7 @@ class MockModel(object): raise NotFoundError("KCHPOOL0002E", {'name': name}) def storagevolumes_create(self, pool_name, params): - vol_source = ['file', 'url', 'capacity'] + vol_source = ['file', 'url', 'capacity', 'volume_path'] require_name_params = ['capacity'] name = params.get('name') @@ -506,6 +506,10 @@ class MockModel(object): name = os.path.basename(params['file'].filename) elif create_param == 'url': name = os.path.basename(params['url']) + elif create_param == 'volume_path': + basename = os.path.basename(params['volume_path']) + split = os.path.splitext(basename) + name = u'%s-clone%s' % (split[0], split[1]) else: name = 'upload-%s' % int(time.time()) params['name'] = name @@ -588,6 +592,44 @@ class MockModel(object): cb('OK', True) + def _create_volume_with_volume_path(self, cb, params): + try: + new_vol_name = params['name'].decode('utf-8') + new_pool_name = params['pool'].decode('utf-8') + orig_vol_path = params['volume_path'].decode('utf-8') + + orig_pool = orig_vol = None + for pn, p in self._mock_storagepools.items(): + for vn, v in p._volumes.items(): + if v.info['path'] == orig_vol_path: + orig_pool = self._get_storagepool(pn) + orig_vol = self._get_storagevolume(pn, vn) + break + + if orig_vol is not None: + break + + if orig_vol is None: + raise OperationFailed('KCHVOL0007E', + {'name': new_vol_name, + 'pool': new_pool_name, + 'err': 'could not find volume \'%s\'' % + orig_vol_path}) + + new_vol = copy.deepcopy(orig_vol) + new_vol.info['name'] = new_vol_name + new_vol.info['path'] = os.path.join(orig_pool.info['path'], + new_vol_name) + + new_pool = self._get_storagepool(new_pool_name) + new_pool._volumes[new_vol_name] = new_vol + except (KeyError, NotFoundError), e: + raise OperationFailed('KCHVOL0007E', + {'name': new_vol_name, 'pool': new_pool_name, + 'err': e.message}) + + cb('OK', True) + def storagevolume_lookup(self, pool, name): if self._get_storagepool(pool).info['state'] != 'active': raise InvalidOperation("KCHVOL0005E", {'pool': pool, diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 9ff43e6..d7325e4 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -18,9 +18,11 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import contextlib +import lxml.etree as ET import os import time import urllib2 +from lxml.builder import E import libvirt @@ -53,9 +55,10 @@ class StorageVolumesModel(object): self.conn = kargs['conn'] self.objstore = kargs['objstore'] self.task = TaskModel(**kargs) + self.storagevolume = StorageVolumeModel(**kargs) def create(self, pool_name, params): - vol_source = ['file', 'url', 'capacity'] + vol_source = ['file', 'url', 'capacity', 'volume_path'] name = params.get('name') @@ -81,13 +84,17 @@ class StorageVolumesModel(object): if create_param in REQUIRE_NAME_PARAMS: raise InvalidParameter('KCHVOL0016E') - # if 'name' is omitted - except for the methods listed in - # 'REQUIRE_NAME_PARAMS' - the default volume name will be the - # file/URL basename. + # if 'name' is omitted,the default volume name for 'file' and 'url' + # will be the file/URL basename, and for 'volume' it will be + # '<volume-name>-clone.<volume-extension>'. if create_param == 'file': name = os.path.basename(params['file'].filename) elif create_param == 'url': name = os.path.basename(params['url']) + elif create_param == 'volume_path': + basename = os.path.basename(params['volume_path']) + split = os.path.splitext(basename) + name = u'%s-clone%s' % (split[0], split[1]) else: name = 'upload-%s' % int(time.time()) params['name'] = name @@ -221,6 +228,64 @@ class StorageVolumesModel(object): StoragePoolModel.get_storagepool(pool_name, self.conn).refresh(0) cb('OK', True) + def _create_volume_with_volume_path(self, cb, params): + """Create a storage volume based on an existing volume. + + The existing volume is referenced by its path. This function copies all + the data inside the original volume into the new one. + + Arguments: + cb -- A callback function to signal the Task's progress. + params -- A dict with the following values: + "pool": the name of the new pool. + "name": the name of the new volume. + "volume_path": the file path of the original storage volume. + """ + try: + new_vol_name = params['name'].decode('utf-8') + new_pool_name = params['pool'].decode('utf-8') + orig_vol_path = params['volume_path'].decode('utf-8') + except KeyError, e: + raise MissingParameter('KCHVOL0004E', + {'item': str(e), 'volume': new_vol_name}) + + try: + cb('setting up volume creation') + vir_conn = self.conn.get() + orig_vir_vol = vir_conn.storageVolLookupByPath(orig_vol_path) + orig_vol_name = orig_vir_vol.name().decode('utf-8') + orig_vir_pool = orig_vir_vol.storagePoolLookupByVolume() + orig_pool_name = orig_vir_pool.name().decode('utf-8') + orig_vol = self.storagevolume.lookup(orig_pool_name, orig_vol_name) + + new_vir_pool = StoragePoolModel.get_storagepool(new_pool_name, + self.conn) + + cb('building volume XML') + root_elem = E.volume() + root_elem.append(E.name(new_vol_name)) + root_elem.append(E.capacity(unicode(orig_vol['capacity']), + unit='bytes')) + target_elem = E.target() + target_elem.append(E.format(type=orig_vol['format'])) + root_elem.append(target_elem) + new_vol_xml = ET.tostring(root_elem, encoding='utf-8', + pretty_print=True) + + cb('creating volume') + new_vir_pool.createXMLFrom(new_vol_xml, orig_vir_vol, 0) + except (InvalidOperation, NotFoundError, libvirt.libvirtError), e: + raise OperationFailed("KCHVOL0007E", + {'name': new_vol_name, 'pool': new_pool_name, + 'err': e.get_error_message()}) + + cb('adding volume to the object store') + new_vol_id = '%s:%s' % (new_pool_name, new_vol_name) + with self.objstore as session: + session.store('storagevolume', new_vol_id, {'ref_cnt': 0}) + + cb('OK', True) + def get_list(self, pool_name): pool = StoragePoolModel.get_storagepool(pool_name, self.conn) if not pool.isActive(): diff --git a/tests/test_model.py b/tests/test_model.py index 21e1b6b..b165731 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -623,6 +623,25 @@ class ModelTests(unittest.TestCase): cp_content = cp_file.read() self.assertEquals(vol_content, cp_content) + # clone the volume created above + params = {'volume_path': vol_path} + task = inst.storagevolumes_create(pool, params) + taskid = task['id'] + cloned_vol_name = task['target_uri'].split('/')[-1] + inst.task_wait(taskid) + self.assertEquals('finished', inst.task_lookup(taskid)['status']) + rollback.prependDefer(inst.storagevolume_delete, pool, + cloned_vol_name) + + orig_vol = inst.storagevolume_lookup(pool, vol_name) + cloned_vol = inst.storagevolume_lookup(pool, cloned_vol_name) + + self.assertNotEquals(orig_vol['path'], cloned_vol['path']) + del orig_vol['path'] + del cloned_vol['path'] + + self.assertEquals(orig_vol, cloned_vol) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_storage_customise(self): inst = model.Model(objstore_loc=self.tmp_store) diff --git a/tests/test_rest.py b/tests/test_rest.py index 9bc930f..66072bc 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1047,6 +1047,29 @@ class RestTests(unittest.TestCase): resp = self.request('/storagepools/pool-1/storagevolumes/%s' % vol_name, '{}', 'GET') self.assertEquals(200, resp.status) + vol = json.loads(resp.read()) + + # clone the volume created above + req = json.dumps({'volume_path': vol['path']}) + resp = self.request('/storagepools/pool-1/storagevolumes', req, 'POST') + self.assertEquals(202, resp.status) + task = json.loads(resp.read()) + cloned_vol_name = task['target_uri'].split('/')[-1] + wait_task(self._task_lookup, task['id']) + task = json.loads(self.request('/tasks/%s' % task['id']).read()) + self.assertEquals('finished', task['status']) + resp = self.request('/storagepools/pool-1/storagevolumes/%s' % + cloned_vol_name, '{}', 'GET') + self.assertEquals(200, resp.status) + cloned_vol = json.loads(resp.read()) + + self.assertNotEquals(vol['name'], cloned_vol['name']) + del vol['name'] + del cloned_vol['name'] + self.assertNotEquals(vol['path'], cloned_vol['path']) + del vol['path'] + del cloned_vol['path'] + self.assertEquals(vol, cloned_vol) # Now remove the StoragePool from mock model self._delete_pool('pool-1')