From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Delete self-defined update param validation, and use additionalProperties in json schema validator instead. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/API.json | 12 ++++++++---- src/kimchi/control/base.py | 11 +---------- src/kimchi/control/debugreports.py | 1 - src/kimchi/control/host.py | 1 - src/kimchi/control/storagepools.py | 1 - src/kimchi/control/templates.py | 4 ---- src/kimchi/control/vm/ifaces.py | 1 - src/kimchi/control/vm/storages.py | 1 - src/kimchi/control/vms.py | 2 -- tests/test_rest.py | 6 +++++- 10 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index c5d7bdb..fd4c901 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -80,7 +80,8 @@ "pattern": "^[_A-Za-z0-9-]*$", "error": "KCHDR0007E" } - } + }, + "additionalProperties": false }, "storagepools_create": { "type": "object", @@ -182,7 +183,8 @@ "minItems": 1, "uniqueItems": true } - } + }, + "additionalProperties": false }, "storagevolumes_create": { "type": "object", @@ -303,7 +305,8 @@ "minimum": 512, "error": "KCHTMPL0013E" } - } + }, + "additionalProperties": false }, "networks_create": { "type": "object", @@ -554,7 +557,8 @@ "required": true, "error": "KCHVMSTOR0003E" } - } + }, + "additionalProperties": false }, "template_update": { "type": "object", diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 60db1df..f1449ea 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -54,7 +54,6 @@ class Resource(object): self.model = model self.ident = ident self.model_args = (ident,) - self.update_params = [] self.role_key = None self.admin_methods = [] @@ -163,6 +162,7 @@ class Resource(object): except UnauthorizedError, e: raise cherrypy.HTTPError(403, e.message) except NotFoundError, e: + raise raise cherrypy.HTTPError(404, e.message) except OperationFailed, e: raise cherrypy.HTTPError(500, e.message) @@ -193,15 +193,6 @@ class Resource(object): params = parse_request() validate_params(params, self, 'update') - if self.update_params is not None: - invalids = [v for v in params.keys() if - v not in self.update_params] - if invalids: - msg_args = {'params': ", ".join(invalids), - 'resource': get_class_name(self)} - e = InvalidOperation('KCHAPI0004E', msg_args) - raise cherrypy.HTTPError(405, e.message) - args = list(self.model_args) + [params] ident = update(*args) self._redirect(ident) diff --git a/src/kimchi/control/debugreports.py b/src/kimchi/control/debugreports.py index 1d9ce59..359fe5e 100644 --- a/src/kimchi/control/debugreports.py +++ b/src/kimchi/control/debugreports.py @@ -40,7 +40,6 @@ class DebugReport(Resource): super(DebugReport, self).__init__(model, ident) self.role_key = 'host' self.admin_methods = ['GET', 'PUT', 'POST'] - self.update_params = ["name"] self.uri_fmt = '/debugreports/%s' self.content = DebugReportContent(model, ident) diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py index c690945..2c037ac 100644 --- a/src/kimchi/control/host.py +++ b/src/kimchi/control/host.py @@ -147,7 +147,6 @@ class Repository(Resource): super(Repository, self).__init__(model, id) self.role_key = 'host' self.admin_methods = ['GET', 'PUT', 'POST', 'DELETE'] - self.update_params = ["config", "baseurl"] self.uri_fmt = "/host/repositories/%s" self.enable = self.generate_action_handler('enable') self.disable = self.generate_action_handler('disable') diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 460beb1..4f455b0 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -77,7 +77,6 @@ class StoragePool(Resource): super(StoragePool, self).__init__(model, ident) self.role_key = 'storage' self.admin_methods = ['PUT', 'POST', 'DELETE'] - self.update_params = ["autostart", "disks"] self.uri_fmt = "/storagepools/%s" self.activate = self.generate_action_handler('activate') self.deactivate = self.generate_action_handler('deactivate') diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py index 70c9457..98b949f 100644 --- a/src/kimchi/control/templates.py +++ b/src/kimchi/control/templates.py @@ -35,10 +35,6 @@ class Template(Resource): super(Template, self).__init__(model, ident) self.role_key = 'templates' self.admin_methods = ['PUT', 'POST', 'DELETE'] - self.update_params = ["name", "folder", "icon", "os_distro", - "storagepool", "os_version", "cpus", - "memory", "cdrom", "disks", "networks", - "graphics", "cpu_info"] self.uri_fmt = "/templates/%s" self.clone = self.generate_action_handler('clone') diff --git a/src/kimchi/control/vm/ifaces.py b/src/kimchi/control/vm/ifaces.py index ebcb044..f761660 100644 --- a/src/kimchi/control/vm/ifaces.py +++ b/src/kimchi/control/vm/ifaces.py @@ -34,7 +34,6 @@ class VMIfaces(Collection): class VMIface(Resource): def __init__(self, model, vm, ident): super(VMIface, self).__init__(model, ident) - self.update_params = ["model", "network"] self.vm = vm self.ident = ident self.info = {} diff --git a/src/kimchi/control/vm/storages.py b/src/kimchi/control/vm/storages.py index 984c4d2..58b0771 100644 --- a/src/kimchi/control/vm/storages.py +++ b/src/kimchi/control/vm/storages.py @@ -39,7 +39,6 @@ class VMStorage(Resource): self.info = {} self.model_args = [self.vm, self.ident] self.uri_fmt = '/vms/%s/storages/%s' - self.update_params = ['path'] @property def data(self): diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index a1589ef..0d41e14 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -35,8 +35,6 @@ class VM(Resource): def __init__(self, model, ident): super(VM, self).__init__(model, ident) self.role_key = 'guests' - self.update_params = ["name", "users", "groups", "cpus", "memory", - "graphics"] self.screenshot = VMScreenShot(model, ident) self.uri_fmt = '/vms/%s' for ident, node in sub_nodes.items(): diff --git a/tests/test_rest.py b/tests/test_rest.py index fa15ef6..1ddbd86 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -217,6 +217,10 @@ class RestTests(unittest.TestCase): resp = self.request('/vms/vm-1/start', '{}', 'POST') self.assertEquals(200, resp.status) + req = json.dumps({'unsupported-attr': 'attr'}) + resp = self.request('/vms/vm-1', req, 'PUT') + self.assertEquals(400, resp.status) + req = json.dumps({'name': 'new-vm'}) resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(400, resp.status) @@ -271,7 +275,7 @@ class RestTests(unittest.TestCase): req = json.dumps({'name': 'new-name', 'cpus': 5, 'UUID': 'notallowed'}) resp = self.request('/vms/vm-1', req, 'PUT') - self.assertEquals(405, resp.status) + self.assertEquals(400, resp.status) params = {'name': u'∨м-црdαtеd', 'cpus': 5, 'memory': 4096} req = json.dumps(params) -- 1.9.3

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Wrap posix file lock with contextlib so that file sections can be protected by lock. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/model/utils.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/kimchi/model/utils.py b/src/kimchi/model/utils.py index 9896289..1188b19 100644 --- a/src/kimchi/model/utils.py +++ b/src/kimchi/model/utils.py @@ -18,8 +18,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import libvirt +import posixfile import socket import urlparse +from contextlib import contextmanager from lxml import etree, objectify from lxml.builder import E, ElementMaker @@ -161,3 +163,17 @@ def get_metadata_node(dom, tag, metadata_support, mode="current"): if node is not None: return etree.tostring(node) return "" + + +@contextmanager +def flock(fp, pos, len): + fp.lock('w|', len, pos) + yield + fp.lock('u', len, pos) + + +@contextmanager +def f_open(path, mod): + file = posixfile.open(path, mod) + yield file + file.close() -- 1.9.3

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; Storage update abandoned async task because mem copy is not long lasting task and does not need to use task to track its status. So update just do the data upload, and following storage volume lookup will take care of current upload progress. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/model/storagevolumes.py | 58 ++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 48f715e..b6a17da 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -33,6 +33,7 @@ from kimchi.isoinfo import IsoImage from kimchi.model.diskutils import get_disk_ref_cnt from kimchi.model.storagepools import StoragePoolModel from kimchi.model.tasks import TaskModel +from kimchi.model.utils import f_open, flock from kimchi.utils import add_task, get_next_clone_name, get_unique_file_name from kimchi.utils import kimchi_log from kimchi.xmlutils.utils import xpath_get_text @@ -57,7 +58,7 @@ class StorageVolumesModel(object): self.task = TaskModel(**kargs) def create(self, pool_name, params): - vol_source = ['file', 'url', 'capacity'] + vol_source = ['url', 'capacity'] name = params.get('name') @@ -88,9 +89,7 @@ class StorageVolumesModel(object): # if 'name' is omitted - except for the methods listed in # 'REQUIRE_NAME_PARAMS' - the default volume name will be the # file/URL basename. - if create_param == 'file': - name = os.path.basename(params['file'].filename) - elif create_param == 'url': + if create_param == 'url': name = os.path.basename(params['url']) else: name = 'upload-%s' % int(time.time()) @@ -119,36 +118,6 @@ class StorageVolumesModel(object): taskid = add_task(targeturi, create_func, self.objstore, params) return self.task.lookup(taskid) - def _create_volume_with_file(self, cb, params): - pool_name = params.pop('pool') - dir_path = StoragePoolModel( - conn=self.conn, objstore=self.objstore).lookup(pool_name)['path'] - file_path = os.path.join(dir_path, params['name']) - if os.path.exists(file_path): - raise InvalidParameter('KCHVOL0001E', {'name': params['name']}) - - upload_file = params['file'] - f_len = upload_file.fp.length - try: - size = 0 - with open(file_path, 'wb') as f: - while True: - data = upload_file.file.read(READ_CHUNK_SIZE) - if not data: - break - size += len(data) - f.write(data) - cb('%s/%s' % (size, f_len)) - except Exception as e: - raise OperationFailed('KCHVOL0007E', - {'name': params['name'], - 'pool': pool_name, - 'err': e.message}) - - # Refresh to make sure volume can be found in following lookup - StoragePoolModel.get_storagepool(pool_name, self.conn).refresh(0) - cb('OK', True) - def _create_volume_with_capacity(self, cb, params): pool_name = params.pop('pool') vol_xml = """ @@ -417,6 +386,27 @@ class StorageVolumeModel(object): cb('OK', True) + def update(self, pool, name, params): + vol = self._get_storagevolume(pool, name) + path = vol.path() + + size = int(params['chunk_size']) + index = int(params['index']) + pos = size * index + try: + with f_open(path, 'a+') as fp: + with flock(fp, pos, size): + fp.seek(pos) + fp.write(params['chunk'].fullvalue()) + except Exception as e: + raise OperationFailed('KCHVOL0007E', + {'name': name, + 'pool': pool, + 'err': e.message}) + + # Refresh to make sure volume can be found in following lookup + StoragePoolModel.get_storagepool(pool, self.conn).refresh(0) + class IsoVolumesModel(object): def __init__(self, **kargs): -- 1.9.3