[PATCHv6 0/8] Add support of storage server and target

From: Royce Lv <lvroyce@linux.vnet.ibm.com> v5>v6, change GET param support to cover more scenario of filter collected results. v4>v5, remove storage server list reload function, merge storage server and targets v3>v4, fix inconsistency between doc and json schema v1>v3, fix racing problem, fix style. Add parameters to GET request so that we will query storage server as: /storageservers?target_type=netfs Query all servers of any type supported(now only netfs): /storageservers Query all storage targets on a server: /storageservers/host-name/storagetargets Query a given type of storage targets: /storageservers/host-name/storagetargets?target_type=netfs Note: Due to Ubuntu 13.04 bug: ubuntu host report empty list for targets query, as my solution already accepted and is easy to fix, we will wait it upstream before 2weeks before release, I'll provide a work around by then if not available yet. REF: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1264955 Royce Lv (8): Support params for GET method Add testcase for GET param Storage server: Update API.md storage server: update controller.py storage server: Update model and mockmodel storage target: Update API.md storage target: Update controller and json schema storage target: Add model support docs/API.md | 22 +++++++++++ src/kimchi/API.json | 20 ++++++++++ src/kimchi/control/base.py | 26 +++++++++---- src/kimchi/control/storagepools.py | 4 +- src/kimchi/control/storageserver.py | 63 +++++++++++++++++++++++++++++++ src/kimchi/control/utils.py | 2 + src/kimchi/mockmodel.py | 28 ++++++++++++++ src/kimchi/model.py | 74 +++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 + tests/test_rest.py | 34 +++++++++++++++++ 10 files changed, 266 insertions(+), 9 deletions(-) create mode 100644 src/kimchi/control/storageserver.py -- 1.8.1.2

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add support for passing params for GET method, we will call it like: GET /vms?state=running Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 26 +++++++++++++++++++------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/utils.py | 2 ++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..73b70fb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2 import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,11 @@ class Collection(object): return res.get() - def _get_resources(self): + def _get_resources(self, filter_params): try: + validate_params(filter_params, self, 'get_list') get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params) res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,15 +235,26 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args) - def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if not ( key in res.data and res.data[key] == val): + valid = False + break + if valid: + data.append(res.data) + return data + + def get(self): + filter_params = parse_param() + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) return kimchi.template.render(get_class_name(self), data) @cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 466b4b6..0df14c5 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -61,9 +61,9 @@ class StoragePools(Collection): return resp - def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'") +def parse_param(): + return cherrypy.request.params def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8")) -- 1.8.1.2

*Not sure 'GET' method always mean "get_list" I have tried just add validate_params at the entry of check 'GET', it can works well.* @@ -242,11 +245,13 @@ class Collection(object): return kimchi.template.render(get_class_name(self), data) @cherrypy.expose - def index(self, *args): + def index(self, *args, **argv): method = validate_method(('GET', 'POST')) if method == 'GET': try: + params = cherrypy.request.params + validate_params(params, self, 'get_list') return self.get() except InvalidOperation, param: error = "Invalid operation: '%s'" % param raise cherrypy.HTTPError(400, error) *add a test for ** **/storagepools?pool_type=dir** **and** **storagepools/default/storagevolumes?volume_type=file* diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 19b1c51..fe3a85a 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -73,6 +73,26 @@ } } }, + "storagepools_get_list": { + "type": "object", + "properties": { + "pool_type": { + "description": "type of storagepools", + "type": "string", + "pattern": "^dir$" + } + } + }, + "storagevolumes_get_list": { + "type": "object", + "properties": { + "volume_type": { + "description": "type of storagevolumes", + "type": "string", + "pattern": "^file$" + } + } + }, "vms_create": { "type": "object", "properties": { *$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools?pool_type=dir* /[// // {// // "available":27511504896,// // "source":{},// // "state":"active",// // "capacity":195949113344,// // "name":"default",// // "nr_volumes":2,// // "path":"/home/shhfeng/var/lib/libvirt/images",// // "allocated":168437608448,// // "autostart":true,// // "type":"dir"// // },// // {// // "state":"active",// // "type":"kimchi-iso",// // "name":"kimchi_isos"// // }// //]/ *$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools?pool_type=dirxxx* /{// // "reason":"The server encountered an unexpected condition which prevented it from fulfilling the request.",// // "code":"500 Internal Server Error",// // "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/base.py\", line 254, in index\n validate_params(cherrypy.request.params, self, 'get_list')\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/utils.py\", line 105, in validate_params\n e.message for e in validator.iter_errors(request)))\nInvalidParameter: u'dirxxx' does not match u'^dir$'\n"/ * **$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools/default/storagevolumes?volume_type=file* /[// // {// // "capacity":21474836480,// // "name":"RHEL6.5.img",// // "format":"qcow2",// // "allocation":5200818176,// //"path":"/home/shhfeng/var/lib/libvirt/images/RHEL6.5.img",// // "type":"file"// // },// // {// // "capacity":21474836480,// // "name":"RHEL6.5.img.back",// // "format":"qcow2",// // "allocation":3681751040,// //"path":"/home/shhfeng/var/lib/libvirt/images/RHEL6.5.img.back",// // "type":"file"// // }// //]/ *$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools/default/storagevolumes?volume_type=filexxx* /{// // "reason":"The server encountered an unexpected condition which prevented it from fulfilling the request.",// // "code":"500 Internal Server Error",// // "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/base.py\", line 254, in index\n validate_params(cherrypy.request.params, self, 'get_list')\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/utils.py\", line 105, in validate_params\n e.message for e in validator.iter_errors(request)))\nInvalidParameter: u'filexxx' does not match u'^file$'\n"// / On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add support for passing params for GET method, we will call it like: GET /vms?state=running
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 26 +++++++++++++++++++------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/utils.py | 2 ++ 3 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..73b70fb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2
import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,11 @@ class Collection(object):
return res.get()
- def _get_resources(self): + def _get_resources(self, filter_params): try: + validate_params(filter_params, self, 'get_list') get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params) res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,15 +235,26 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args)
- def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if not ( key in res.data and res.data[key] == val): + valid = False + break + if valid: + data.append(res.data) + return data + + def get(self): + filter_params = parse_param() + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) return kimchi.template.render(get_class_name(self), data)
@cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 466b4b6..0df14c5 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -61,9 +61,9 @@ class StoragePools(Collection):
return resp
- def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'")
+def parse_param(): + return cherrypy.request.params
def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8"))
-- Thanks and best regards! Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

Sheldon, As we discussed, pls refer to demo decorator of PATCH 2/8. If you want to support GET_LIST of some function without change model.py, pls wrap it with my decorator and it will work. Also, if guys agree, I will put all these wrap in a single patchset. On 2014?01?11? 08:48, Sheldon wrote:
*Not sure 'GET' method always mean "get_list"
I have tried just add validate_params at the entry of check 'GET', it can works well.*
@@ -242,11 +245,13 @@ class Collection(object): return kimchi.template.render(get_class_name(self), data)
@cherrypy.expose - def index(self, *args): + def index(self, *args, **argv): method = validate_method(('GET', 'POST')) if method == 'GET': try: + params = cherrypy.request.params + validate_params(params, self, 'get_list') return self.get() except InvalidOperation, param: error = "Invalid operation: '%s'" % param raise cherrypy.HTTPError(400, error)
*add a test for ** **/storagepools?pool_type=dir** **and** **storagepools/default/storagevolumes?volume_type=file*
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 19b1c51..fe3a85a 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -73,6 +73,26 @@ } } }, + "storagepools_get_list": { + "type": "object", + "properties": { + "pool_type": { + "description": "type of storagepools", + "type": "string", + "pattern": "^dir$" + } + } + }, + "storagevolumes_get_list": { + "type": "object", + "properties": { + "volume_type": { + "description": "type of storagevolumes", + "type": "string", + "pattern": "^file$" + } + } + }, "vms_create": { "type": "object", "properties": {
*$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools?pool_type=dir* /[// // {// // "available":27511504896,// // "source":{},// // "state":"active",// // "capacity":195949113344,// // "name":"default",// // "nr_volumes":2,// // "path":"/home/shhfeng/var/lib/libvirt/images",// // "allocated":168437608448,// // "autostart":true,// // "type":"dir"// // },// // {// // "state":"active",// // "type":"kimchi-iso",// // "name":"kimchi_isos"// // }// //]/ *$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools?pool_type=dirxxx* /{// // "reason":"The server encountered an unexpected condition which prevented it from fulfilling the request.",// // "code":"500 Internal Server Error",// // "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/base.py\", line 254, in index\n validate_params(cherrypy.request.params, self, 'get_list')\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/utils.py\", line 105, in validate_params\n e.message for e in validator.iter_errors(request)))\nInvalidParameter: u'dirxxx' does not match u'^dir$'\n"/
* **$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools/default/storagevolumes?volume_type=file* /[// // {// // "capacity":21474836480,// // "name":"RHEL6.5.img",// // "format":"qcow2",// // "allocation":5200818176,// //"path":"/home/shhfeng/var/lib/libvirt/images/RHEL6.5.img",// // "type":"file"// // },// // {// // "capacity":21474836480,// // "name":"RHEL6.5.img.back",// // "format":"qcow2",// // "allocation":3681751040,// //"path":"/home/shhfeng/var/lib/libvirt/images/RHEL6.5.img.back",// // "type":"file"// // }// //]/ *$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools/default/storagevolumes?volume_type=filexxx* /{// // "reason":"The server encountered an unexpected condition which prevented it from fulfilling the request.",// // "code":"500 Internal Server Error",// // "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/base.py\", line 254, in index\n validate_params(cherrypy.request.params, self, 'get_list')\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/utils.py\", line 105, in validate_params\n e.message for e in validator.iter_errors(request)))\nInvalidParameter: u'filexxx' does not match u'^file$'\n"// /
On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv<lvroyce@linux.vnet.ibm.com>
Add support for passing params for GET method, we will call it like: GET /vms?state=running
Signed-off-by: Royce Lv<lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 26 +++++++++++++++++++------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/utils.py | 2 ++ 3 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..73b70fb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2
import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,11 @@ class Collection(object):
return res.get()
- def _get_resources(self): + def _get_resources(self, filter_params): try: + validate_params(filter_params, self, 'get_list') get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params) res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,15 +235,26 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args)
- def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if not ( key in res.data and res.data[key] == val): + valid = False + break + if valid: + data.append(res.data) + return data + + def get(self): + filter_params = parse_param() + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) return kimchi.template.render(get_class_name(self), data)
@cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 466b4b6..0df14c5 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -61,9 +61,9 @@ class StoragePools(Collection):
return resp
- def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'")
+def parse_param(): + return cherrypy.request.params
def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8"))
-- Thanks and best regards!
Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 01/13/2014 01:50 PM, Royce Lv wrote:
Sheldon, As we discussed, pls refer to demo decorator of PATCH 2/8. If you want to support GET_LIST of some function without change model.py, pls wrap it with my decorator and it will work. Also, if guys agree, I will put all these wrap in a single patchset. Just a suggestion.
Though I prefer the entry of GET "method" to get and check query parameters, this is more simple. Then we can trust cherrypy without any test case for getting query parameters, And other developer do not need to get and check query parameters again. You can use your way.
On 2014?01?11? 08:48, Sheldon wrote:
*Not sure 'GET' method always mean "get_list"
I have tried just add validate_params at the entry of check 'GET', it can works well.*
@@ -242,11 +245,13 @@ class Collection(object): return kimchi.template.render(get_class_name(self), data)
@cherrypy.expose - def index(self, *args): + def index(self, *args, **argv): method = validate_method(('GET', 'POST')) if method == 'GET': try: + params = cherrypy.request.params + validate_params(params, self, 'get_list') return self.get() except InvalidOperation, param: error = "Invalid operation: '%s'" % param raise cherrypy.HTTPError(400, error)
*add a test for ** **/storagepools?pool_type=dir** **and** **storagepools/default/storagevolumes?volume_type=file*
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 19b1c51..fe3a85a 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -73,6 +73,26 @@ } } }, + "storagepools_get_list": { + "type": "object", + "properties": { + "pool_type": { + "description": "type of storagepools", + "type": "string", + "pattern": "^dir$" + } + } + }, + "storagevolumes_get_list": { + "type": "object", + "properties": { + "volume_type": { + "description": "type of storagevolumes", + "type": "string", + "pattern": "^file$" + } + } + }, "vms_create": { "type": "object", "properties": {
*$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools?pool_type=dir* /[// // {// // "available":27511504896,// // "source":{},// // "state":"active",// // "capacity":195949113344,// // "name":"default",// // "nr_volumes":2,// // "path":"/home/shhfeng/var/lib/libvirt/images",// // "allocated":168437608448,// // "autostart":true,// // "type":"dir"// // },// // {// // "state":"active",// // "type":"kimchi-iso",// // "name":"kimchi_isos"// // }// //]/ *$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools?pool_type=dirxxx* /{// // "reason":"The server encountered an unexpected condition which prevented it from fulfilling the request.",// // "code":"500 Internal Server Error",// // "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/base.py\", line 254, in index\n validate_params(cherrypy.request.params, self, 'get_list')\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/utils.py\", line 105, in validate_params\n e.message for e in validator.iter_errors(request)))\nInvalidParameter: u'dirxxx' does not match u'^dir$'\n"/
* **$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools/default/storagevolumes?volume_type=file* /[// // {// // "capacity":21474836480,// // "name":"RHEL6.5.img",// // "format":"qcow2",// // "allocation":5200818176,// //"path":"/home/shhfeng/var/lib/libvirt/images/RHEL6.5.img",// // "type":"file"// // },// // {// // "capacity":21474836480,// // "name":"RHEL6.5.img.back",// // "format":"qcow2",// // "allocation":3681751040,// //"path":"/home/shhfeng/var/lib/libvirt/images/RHEL6.5.img.back",// // "type":"file"// // }// //]/ *$ curl -u <user>:<password> -H 'Content-type: application/json' -H 'Accept: application/json' -X GET http//localhost:8000/storagepools/default/storagevolumes?volume_type=filexxx* /{// // "reason":"The server encountered an unexpected condition which prevented it from fulfilling the request.",// // "code":"500 Internal Server Error",// // "call_stack":"Traceback (most recent call last):\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n response.body = self.handler()\n File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n return self.callable(*self.args, **self.kwargs)\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/base.py\", line 254, in index\n validate_params(cherrypy.request.params, self, 'get_list')\n File \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/utils.py\", line 105, in validate_params\n e.message for e in validator.iter_errors(request)))\nInvalidParameter: u'filexxx' does not match u'^file$'\n"// /
On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv<lvroyce@linux.vnet.ibm.com>
Add support for passing params for GET method, we will call it like: GET /vms?state=running
Signed-off-by: Royce Lv<lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 26 +++++++++++++++++++------- src/kimchi/control/storagepools.py | 4 ++-- src/kimchi/control/utils.py | 2 ++ 3 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 185c8d8..73b70fb 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -28,7 +28,7 @@ import urllib2
import kimchi.template from kimchi.control.utils import get_class_name, internal_redirect, model_fn -from kimchi.control.utils import parse_request, validate_method +from kimchi.control.utils import parse_request, parse_param, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -212,10 +212,11 @@ class Collection(object):
return res.get()
- def _get_resources(self): + def _get_resources(self, filter_params): try: + validate_params(filter_params, self, 'get_list') get_list = getattr(self.model, model_fn(self, 'get_list')) - idents = get_list(*self.model_args) + idents = get_list(*self.model_args, **filter_params) res_list = [] for ident in idents: # internal text, get_list changes ident to unicode for sorted @@ -234,15 +235,26 @@ class Collection(object): args = self.resource_args + [ident.decode("utf-8")] return self.resource(self.model, *args)
- def get(self): - resources = self._get_resources() + def filter_data(self, resources, filter_params): data = [] for res in resources: - data.append(res.data) + valid = True + for key, val in filter_params.items(): + if not ( key in res.data and res.data[key] == val): + valid = False + break + if valid: + data.append(res.data) + return data + + def get(self): + filter_params = parse_param() + resources = self._get_resources(filter_params) + data = self.filter_data(resources, filter_params) return kimchi.template.render(get_class_name(self), data)
@cherrypy.expose - def index(self, *args): + def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) if method == 'GET': try: diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 466b4b6..0df14c5 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -61,9 +61,9 @@ class StoragePools(Collection):
return resp
- def _get_resources(self): + def _get_resources(self, filter_params): try: - res_list = super(StoragePools, self)._get_resources() + res_list = super(StoragePools, self)._get_resources(filter_params) # Append reserved pools isos = getattr(self, ISO_POOL_NAME) isos.lookup() diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..4b3c4b0 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -81,6 +81,8 @@ def parse_request(): raise cherrypy.HTTPError(415, "This API only supports" " 'application/json'")
+def parse_param(): + return cherrypy.request.params
def internal_redirect(url): raise cherrypy.InternalRedirect(url.encode("utf-8"))
-- Thanks and best regards!
Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Sheldon Feng(???)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add a testcase to test GET param passing. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_rest.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_rest.py b/tests/test_rest.py index a960868..5b29d2c 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1129,6 +1129,40 @@ class RestTests(unittest.TestCase): self.assertIn('net_recv_rate', stats) self.assertIn('net_sent_rate', stats) + def test_get_param(self): + def hack_model(func): + def _get_param_func(*args, **kwargs): + res = func() + return res + return _get_param_func + + global model + old_handler = model.vms_get_list + model.vms_get_list = hack_model(model.vms_get_list) + + req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'}) + self.request('/templates', req, 'POST') + + # Create a VM + req = json.dumps({'name': 'test-vm1', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + req = json.dumps({'name': 'test-vm2', 'template': '/templates/test'}) + resp = self.request('/vms', req, 'POST') + self.assertEquals(201, resp.status) + + resp = request(host, port, '/vms') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(2, len(res)) + + resp = request(host, port, '/vms?name=test-vm1') + self.assertEquals(200, resp.status) + res = json.loads(resp.read()) + self.assertEquals(1, len(res)) + self.assertEquals('test-vm1', res[0]['name']) + + model.vms_get_list = old_handler class HttpsRestTests(RestTests): """ -- 1.8.1.2

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Update API.md to specify storage server api. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/API.md b/docs/API.md index 0013e86..7a25eda 100644 --- a/docs/API.md +++ b/docs/API.md @@ -456,6 +456,19 @@ creation. not tested yet * **POST**: *See Configuration Actions* +**Actions (POST):** + +*No actions defined* + +### Collection: Storage Servers + +**URI:** /storageservers + +**Methods:** + +* **GET**: Retrieve a summarized list of used storage servers. + * target_type: Filter server list with given type, currently support 'netfs'. + ### Collection: Distros **URI:** /config/distros -- 1.8.1.2

On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Update API.md to specify storage server api.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 0013e86..7a25eda 100644 --- a/docs/API.md +++ b/docs/API.md @@ -456,6 +456,19 @@ creation. not tested yet * **POST**: *See Configuration Actions*
+**Actions (POST):** + +*No actions defined* + +### Collection: Storage Servers + +**URI:** /storageservers + +**Methods:** + +* **GET**: Retrieve a summarized list of used storage servers. + * target_type: Filter server list with given type, currently support 'netfs'. + not sure API.md reader can know well this is a query params? GET /storageservers ?target_type=netfs
you can check with the UI guys.
### Collection: Distros
**URI:** /config/distros
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

On 2014年01月14日 12:05, Sheldon wrote:
On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Update API.md to specify storage server api.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 0013e86..7a25eda 100644 --- a/docs/API.md +++ b/docs/API.md @@ -456,6 +456,19 @@ creation. not tested yet * **POST**: *See Configuration Actions*
+**Actions (POST):** + +*No actions defined* + +### Collection: Storage Servers + +**URI:** /storageservers + +**Methods:** + +* **GET**: Retrieve a summarized list of used storage servers. + * target_type: Filter server list with given type, currently support 'netfs'. + not sure API.md reader can know well this is a query params? GET /storageservers ?target_type=netfs
According to http://www.w3schools.com/tags/ref_httpmethods.asp I think people use GET parameter knows it, I added to the commit msg about the usage. Do you think we need to doc in the API.md about GET parameter usage? Personally I suppose not.
you can check with the UI guys.
### Collection: Distros
**URI:** /config/distros

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add storage server collection and resource to report used storage server. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 10 ++++++++++ src/kimchi/control/storageserver.py | 38 +++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 ++ 3 files changed, 50 insertions(+) create mode 100644 src/kimchi/control/storageserver.py diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 3a3c48f..e135ebb 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -208,6 +208,16 @@ }, "additionalProperties": false }, + "storageservers_get_list": { + "type": "object", + "properties": { + "target_type": { + "description": "List storage servers of given type", + "type": "string", + "pattern": "^netfs$" + } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/storageserver.py b/src/kimchi/control/storageserver.py new file mode 100644 index 0000000..0d4cb05 --- /dev/null +++ b/src/kimchi/control/storageserver.py @@ -0,0 +1,38 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +from kimchi.control.base import Collection, Resource + + +class StorageServers(Collection): + def __init__(self, model): + super(StorageServers, self).__init__(model) + self.resource = StorageServer + + +class StorageServer(Resource): + def __init__(self, model, ident): + super(StorageServer, self).__init__(model, ident) + + @property + def data(self): + return self.info diff --git a/src/kimchi/root.py b/src/kimchi/root.py index 3cc6321..ec531c0 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.py @@ -36,6 +36,7 @@ from kimchi.control.interfaces import Interfaces from kimchi.control.networks import Networks from kimchi.control.plugins import Plugins from kimchi.control.storagepools import StoragePools +from kimchi.control.storageserver import StorageServers from kimchi.control.tasks import Tasks from kimchi.control.templates import Templates from kimchi.control.utils import parse_request @@ -60,6 +61,7 @@ class Root(Resource): self.vms = VMs(model) self.templates = Templates(model) self.storagepools = StoragePools(model) + self.storageservers = StorageServers(model) self.interfaces = Interfaces(model) self.networks = Networks(model) self.tasks = Tasks(model) -- 1.8.1.2

Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add storage server collection and resource to report used storage server.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 10 ++++++++++ src/kimchi/control/storageserver.py | 38 +++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 ++ 3 files changed, 50 insertions(+) create mode 100644 src/kimchi/control/storageserver.py
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 3a3c48f..e135ebb 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -208,6 +208,16 @@ }, "additionalProperties": false }, + "storageservers_get_list": { + "type": "object", + "properties": { + "target_type": { + "description": "List storage servers of given type", + "type": "string", + "pattern": "^netfs$" + } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/storageserver.py b/src/kimchi/control/storageserver.py new file mode 100644 index 0000000..0d4cb05 --- /dev/null +++ b/src/kimchi/control/storageserver.py @@ -0,0 +1,38 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2014 +# +# Authors: +# Royce Lv <lvroyce@linux.vnet.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +from kimchi.control.base import Collection, Resource + + +class StorageServers(Collection): + def __init__(self, model): + super(StorageServers, self).__init__(model) + self.resource = StorageServer + + +class StorageServer(Resource): + def __init__(self, model, ident): + super(StorageServer, self).__init__(model, ident) + + @property + def data(self): + return self.info diff --git a/src/kimchi/root.py b/src/kimchi/root.py index 3cc6321..ec531c0 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.py @@ -36,6 +36,7 @@ from kimchi.control.interfaces import Interfaces from kimchi.control.networks import Networks from kimchi.control.plugins import Plugins from kimchi.control.storagepools import StoragePools +from kimchi.control.storageserver import StorageServers from kimchi.control.tasks import Tasks from kimchi.control.templates import Templates from kimchi.control.utils import parse_request @@ -60,6 +61,7 @@ class Root(Resource): self.vms = VMs(model) self.templates = Templates(model) self.storagepools = StoragePools(model) + self.storageservers = StorageServers(model) self.interfaces = Interfaces(model) self.networks = Networks(model) self.tasks = Tasks(model)
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Query all storage pool to retrieve storage server we used. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 28 ++++++++++++++++++++++++++++ src/kimchi/model.py | 26 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 9488078..0116c1b 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -419,6 +419,34 @@ class MockModel(object): iso_volumes.append(res) return iso_volumes + def storageservers_get_list(self, target_type=None): + # FIXME: When added new storage server support, this needs to be updated + target_type = kimchi.model.STORAGE_SOURCES.keys() \ + if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['type'] in target_type: + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(target_type=pool_info['type'], addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + raise NotFoundError + def dummy_interfaces(self): interfaces = {} ifaces = {"eth1": "nic", "bond0": "bonding", diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 7115583..f005121 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1238,6 +1238,32 @@ class Model(object): else: raise + def storageservers_get_list(self, target_type=None): + target_type = STORAGE_SOURCES.keys() if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['type'] in target_type: + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(target_type=pool_info['type'], addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + raise NotFoundError + def _get_screenshot(self, vm_uuid): with self.objstore as session: try: -- 1.8.1.2

Just a minor comment below On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Query all storage pool to retrieve storage server we used.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 28 ++++++++++++++++++++++++++++ src/kimchi/model.py | 26 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 9488078..0116c1b 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -419,6 +419,34 @@ class MockModel(object): iso_volumes.append(res) return iso_volumes
+ def storageservers_get_list(self, target_type=None): + # FIXME: When added new storage server support, this needs to be updated + target_type = kimchi.model.STORAGE_SOURCES.keys() \ + if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['type'] in target_type: + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(target_type=pool_info['type'], addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + raise NotFoundError + def dummy_interfaces(self): interfaces = {} ifaces = {"eth1": "nic", "bond0": "bonding", diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 7115583..f005121 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1238,6 +1238,32 @@ class Model(object): else: raise
+ def storageservers_get_list(self, target_type=None): + target_type = STORAGE_SOURCES.keys() if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['type'] in target_type: + server_list.append(pool_info['source']['addr']) if we want to to get "netfs",
All servers such as: [ {"server": "192.168.1.2", "target_type ": "SAN"}, {"server": "192.168.1.2", "target_type ": "netfs"}, ] storageserver_lookup may return a SAN server. so we will not add it in server_list.
+ except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: if we want to to get "netfs", Here, we may return: {"server": "192.168.1.2", "target_type ": "SAN"} for it is the first element of pool list.
The SAN server and netfs server are same server. so when will find {"server": "192.168.1.2", "target_type ": "netfs"} ? so you will report NotFoundError ?
+ return dict(target_type=pool_info['type'], addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup Does the inconsistent means what I say above?
+ pass + raise NotFoundError + def _get_screenshot(self, vm_uuid): with self.objstore as session: try:
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

Just a minor comment below
On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Query all storage pool to retrieve storage server we used.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 28 ++++++++++++++++++++++++++++ src/kimchi/model.py | 26 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 9488078..0116c1b 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -419,6 +419,34 @@ class MockModel(object): iso_volumes.append(res) return iso_volumes
+ def storageservers_get_list(self, target_type=None): + # FIXME: When added new storage server support, this needs to be updated + target_type = kimchi.model.STORAGE_SOURCES.keys() \ + if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['type'] in target_type: + server_list.append(pool_info['source']['addr']) + except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: + return dict(target_type=pool_info['type'], addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup + pass + raise NotFoundError + def dummy_interfaces(self): interfaces = {} ifaces = {"eth1": "nic", "bond0": "bonding", diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 7115583..f005121 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1238,6 +1238,32 @@ class Model(object): else: raise
+ def storageservers_get_list(self, target_type=None): + target_type = STORAGE_SOURCES.keys() if not target_type else [target_type] + pools = self.storagepools_get_list() + server_list = [] + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['type'] in target_type: + server_list.append(pool_info['source']['addr']) if we want to to get "netfs",
All servers such as: [ {"server": "192.168.1.2", "target_type ": "SAN"}, {"server": "192.168.1.2", "target_type ": "netfs"}, ]
storageserver_lookup may return a SAN server. so we will not add it in server_list. True, we need to filter it out. Thanks for pointing out.
+ except NotFoundError: + pass + + return server_list + + def storageserver_lookup(self, server): + pools = self.storagepools_get_list() + for pool in pools: + try: + pool_info = self.storagepool_lookup(pool) + if pool_info['source'] and pool_info['source']['addr'] == server: if we want to to get "netfs", Here, we may return: {"server": "192.168.1.2", "target_type ": "SAN"} for it is the first element of pool list.
The SAN server and netfs server are same server.
so when will find {"server": "192.168.1.2", "target_type ": "netfs"} ?
so you will report NotFoundError ? Good catch! I haven't considered this scenario, thanks, Sheldon!
+ return dict(target_type=pool_info['type'], addr=server) + except NotFoundError: + # Avoid inconsistent pool result because of lease between list and lookup Does the inconsistent means what I say above? No, this is a suggest from Ming that when we get_list and then lookup
On 2014年01月14日 12:30, Sheldon wrote: there is a lease that a pool appears in the get_list result, but deleted after. so lookup may not find it.
+ pass + raise NotFoundError + def _get_screenshot(self, vm_uuid): with self.objstore as session: try:

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add colleciton of storage targets to API.md. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/API.md b/docs/API.md index 7a25eda..0244416 100644 --- a/docs/API.md +++ b/docs/API.md @@ -469,6 +469,15 @@ creation. * **GET**: Retrieve a summarized list of used storage servers. * target_type: Filter server list with given type, currently support 'netfs'. +### Collection: Storage Targets + +**URI:** /storageservers/*:name*/storagetargets + +**Methods:** + +* **GET**: Retrieve a list of available storage targets. + * target_type: Filter target list with given type, currently support 'netfs'. + ### Collection: Distros **URI:** /config/distros -- 1.8.1.2

On 01/08/2014 11:50 PM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Add colleciton of storage targets to API.md.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- docs/API.md | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 7a25eda..0244416 100644 --- a/docs/API.md +++ b/docs/API.md @@ -469,6 +469,15 @@ creation. * **GET**: Retrieve a summarized list of used storage servers. * target_type: Filter server list with given type, currently support 'netfs'.
+### Collection: Storage Targets + +**URI:** /storageservers/*:name*/storagetargets + +**Methods:** + +* **GET**: Retrieve a list of available storage targets. + * target_type: Filter target list with given type, currently support 'netfs'. + not sure API.md reader can know well this is a query params? ### Collection: Distros
**URI:** /config/distros
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add json schema to validate mandatory param of target_type, also update controller.py. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 10 ++++++++++ src/kimchi/control/storageserver.py | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index e135ebb..9c5bb23 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -218,6 +218,16 @@ } } }, + "storagetargets_get_list": { + "type": "object", + "properties": { + "target_type": { + "description": "List storage servers of given type", + "type": "string", + "pattern": "^netfs$" + } + } + }, "template_update": { "type": "object", "properties": { diff --git a/src/kimchi/control/storageserver.py b/src/kimchi/control/storageserver.py index 0d4cb05..fbaf898 100644 --- a/src/kimchi/control/storageserver.py +++ b/src/kimchi/control/storageserver.py @@ -21,6 +21,9 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA from kimchi.control.base import Collection, Resource +from kimchi.control.utils import parse_param, validate_params +from kimchi.control.utils import get_class_name, model_fn +import kimchi.template class StorageServers(Collection): @@ -36,3 +39,25 @@ class StorageServer(Resource): @property def data(self): return self.info + + def _cp_dispatch(self, vpath): + if vpath: + subcollection = vpath.pop(0) + if subcollection == 'storagetargets': + # incoming text, from URL, is not unicode, need decode + return StorageTargets(self.model, self.ident.decode("utf-8")) + +class StorageTargets(Collection): + def __init__(self, model, server): + super(StorageTargets, self).__init__(model) + self.server = server + self.resource_args = [self.server, ] + self.model_args = [self.server, ] + + def get(self): + res_list = [] + params = parse_param() + validate_params(params, self, 'get_list') + get_list = getattr(self.model, model_fn(self, 'get_list')) + res_list = get_list(*self.model_args, **params) + return kimchi.template.render(get_class_name(self), res_list) -- 1.8.1.2

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Construct xml to query storage targets information from storage server. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index f005121..fe841da 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1264,6 +1264,24 @@ class Model(object): pass raise NotFoundError + def storagetargets_get_list(self, storage_server, target_type=None): + target_types = STORAGE_SOURCES.keys() if not target_type else [target_type] + target_list = list() + + for target_type in target_types: + xml = _get_storage_server_spec(server=storage_server, target_type=target_type) + conn = self.conn.get() + + try: + ret = conn.findStoragePoolSources(target_type, xml, 0) + except libvirt.libvirtError as e: + kimchi_log.warning("Query storage pool source fails because of %s", + e.get_error_message()) + continue + + target_list.extend(_parse_target_source_result(target_type, ret)) + return target_list + def _get_screenshot(self, vm_uuid): with self.objstore as session: try: @@ -1474,6 +1492,36 @@ class LibvirtVMScreenshot(VMScreenshot): os.close(fd) +def _parse_target_source_result(target_type, xml_str): + root = ElementTree.fromstring(xml_str) + ret = [] + for source in root.getchildren(): + if target_type == 'netfs': + host_name = source.find('host').attrib.values()[0] + target_path = source.find('dir').attrib.values()[0] + type = source.find('format').attrib.values()[0] + ret.append(dict(host=host_name, target_type=type, target=target_path)) + + return ret + + +def _get_storage_server_spec(**kwargs): + # Required parameters: + # server: + # target_type: + if kwargs['target_type'] == 'netfs': + kwargs['format_type'] = """<format type='nfs'/>""" + else: + kwargs['format_type'] = "" + xml = """ + <source> + <host name='%(server)s'/> + %(format_type)s + </source> + """ % kwargs + return xml + + class StoragePoolDef(object): @classmethod def create(cls, poolArgs): -- 1.8.1.2

Ping? On 2014年01月08日 23:50, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
v5>v6, change GET param support to cover more scenario of filter collected results. v4>v5, remove storage server list reload function, merge storage server and targets v3>v4, fix inconsistency between doc and json schema v1>v3, fix racing problem, fix style.
Add parameters to GET request so that we will query storage server as: /storageservers?target_type=netfs Query all servers of any type supported(now only netfs): /storageservers Query all storage targets on a server: /storageservers/host-name/storagetargets Query a given type of storage targets: /storageservers/host-name/storagetargets?target_type=netfs
Note: Due to Ubuntu 13.04 bug: ubuntu host report empty list for targets query, as my solution already accepted and is easy to fix, we will wait it upstream before 2weeks before release, I'll provide a work around by then if not available yet. REF: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1264955
Royce Lv (8): Support params for GET method Add testcase for GET param Storage server: Update API.md storage server: update controller.py storage server: Update model and mockmodel storage target: Update API.md storage target: Update controller and json schema storage target: Add model support
docs/API.md | 22 +++++++++++ src/kimchi/API.json | 20 ++++++++++ src/kimchi/control/base.py | 26 +++++++++---- src/kimchi/control/storagepools.py | 4 +- src/kimchi/control/storageserver.py | 63 +++++++++++++++++++++++++++++++ src/kimchi/control/utils.py | 2 + src/kimchi/mockmodel.py | 28 ++++++++++++++ src/kimchi/model.py | 74 +++++++++++++++++++++++++++++++++++++ src/kimchi/root.py | 2 + tests/test_rest.py | 34 +++++++++++++++++ 10 files changed, 266 insertions(+), 9 deletions(-) create mode 100644 src/kimchi/control/storageserver.py
participants (3)
-
lvroyce0210@gmail.com
-
Royce Lv
-
Sheldon