On 2014?09?02? 00:17, Aline Manera wrote:

On 09/01/2014 08:50 AM, lvroyce0210@gmail.com wrote:

...

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

As we are starting to support upload and download to create volume, they need to be distinguished from previous creating through libvirt api. Adding a dispatcher to support this.

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

diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 2eae7e8..bbec591 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -185,6 +185,8 @@ messages = { "KCHVOL0015E": _("Storage volume format not supported"), "KCHVOL0016E": _("Storage volume requires a volume name"), "KCHVOL0017E": _("Unable to update database with storage volume information due error: %(err)s"), + "KCHVOL0018E": _("Only one of %(param)s can be specified"), + "KCHVOL0019E": _("Creating volume from %(param)s is not supported"),

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

diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index b60884c..fc63a16 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -44,6 +44,20 @@ class StorageVolumesModel(object): self.objstore = kargs['objstore']

def create(self, pool_name, params): + vol_source = ['file', 'url', 'capacity'] + + if sum(1 for p in vol_source if p in params) != 1: + raise InvalidParameter("KCHVOL0018E", {'param': str(vol_source)})

str(vol_source) will generate a string like "[' file', 'url', 'capacity']" I suggest to use ", ".join(a) to generate a string like "file, url, capacity"

ACK

...

+ + for p in vol_source: + if p in params:

As just one option is allowed you don't need a "for" statement You should be able to do: + try: + create_func = getattr(self, "_create_volume_with_" +*vol_source[0]*) + except AttributeError: + raise InvalidParameter("KCHVOL0019E", {'param':*vol_source[0]*}) + return create_func(pool_name, params)

ACK

...

+ try: + create_func = getattr(self, "_create_volume_with_" + p) + except AttributeError: + raise InvalidParameter("KCHVOL0019E", {'param': p}) + return create_func(pool_name, params) + + def _create_volume_with_capacity(self, pool_name, params): vol_xml = """ <volume> <name>%(name)s</name>

_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 09/01/2014 08:50 AM, lvroyce0210@gmail.com wrote:

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

Because storage volume create will include upload and download, storagevolumes will not present before async task's finished. so change storage volumes to async collection.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/storagevolumes.py | 4 ++-- src/kimchi/model/storagevolumes.py | 13 +++++++++---- tests/test_model.py | 8 ++++++-- tests/test_rest.py | 9 ++++++--- 4 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/kimchi/control/storagevolumes.py b/src/kimchi/control/storagevolumes.py index 327bf75..436e8ef 100644 --- a/src/kimchi/control/storagevolumes.py +++ b/src/kimchi/control/storagevolumes.py @@ -18,11 +18,11 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

import kimchi.template -from kimchi.control.base import Collection, Resource +from kimchi.control.base import Collection, Resource, AsyncCollection from kimchi.control.utils import get_class_name, model_fn

-class StorageVolumes(Collection): +class StorageVolumes(AsyncCollection): def __init__(self, model, pool): super(StorageVolumes, self).__init__(model) self.resource = StorageVolume diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index fc63a16..203d5e1 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -27,7 +27,8 @@ from kimchi.exception import InvalidOperation, InvalidParameter, IsoFormatError 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.utils import kimchi_log, add_task +from kimchi.model.tasks import TaskModel from kimchi.model.vms import VMsModel, VMModel from kimchi.vmdisks import get_vm_disk, get_vm_disk_list

@@ -42,6 +43,7 @@ class StorageVolumesModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] self.objstore = kargs['objstore'] + self.task = TaskModel(**kargs)

def create(self, pool_name, params): vol_source = ['file', 'url', 'capacity'] @@ -55,9 +57,12 @@ class StorageVolumesModel(object): create_func = getattr(self, "_create_volume_with_" + p) except AttributeError: raise InvalidParameter("KCHVOL0019E", {'param': p}) - return create_func(pool_name, params) + params['pool'] = pool_name + taskid = add_task('', create_func, self.objstore, params) + return self.task.lookup(taskid)

- def _create_volume_with_capacity(self, pool_name, params): + def _create_volume_with_capacity(self, cb, params): + pool_name = params.pop('pool') vol_xml = """ <volume> <name>%(name)s</name> @@ -102,7 +107,7 @@ class StorageVolumesModel(object): kimchi_log.warning('Unable to store storage volume id in ' 'objectstore due error: %s', e.message)

- return name + cb('', True)

def get_list(self, pool_name): pool = StoragePoolModel.get_storagepool(pool_name, self.conn) diff --git a/tests/test_model.py b/tests/test_model.py index 59f416c..dc3a4e4 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -118,7 +118,9 @@ class ModelTests(unittest.TestCase): 'capacity': 1024, 'allocation': 1, 'format': 'qcow2'} - inst.storagevolumes_create('default', params) + task_id = inst.storagevolumes_create('default', params)['id'] + self._wait_task(inst, task_id) + self.assertEquals('finished', inst.task_lookup(task_id)['status']) vol_path = inst.storagevolume_lookup('default', vol)['path'] rollback.prependDefer(inst.storagevolume_delete, 'default', vol)

@@ -527,7 +529,9 @@ class ModelTests(unittest.TestCase): 'capacity': 1024, 'allocation': 512, 'format': 'raw'} - inst.storagevolumes_create(pool, params) + task_id = inst.storagevolumes_create(pool, params)['id'] + self._wait_task(inst, task_id) + self.assertEquals('finished', inst.task_lookup(task_id)['status']) rollback.prependDefer(inst.storagevolume_delete, pool, vol)

fd, path = tempfile.mkstemp(dir=path) diff --git a/tests/test_rest.py b/tests/test_rest.py index 2117399..9263e8d 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -494,7 +494,8 @@ class RestTests(unittest.TestCase): 'format': 'raw'}) resp = self.request('/storagepools/tmp/storagevolumes', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + time.sleep(1)

# Attach cdrom with both path and volume specified open('/tmp/existent.iso', 'w').close() @@ -1006,8 +1007,9 @@ class RestTests(unittest.TestCase): 'format': 'raw'}) resp = self.request('/storagepools/pool-1/storagevolumes', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status)

+ time.sleep(5) nr_vols = json.loads( self.request('/storagepools/pool-1').read())['nr_volumes'] self.assertEquals(5, nr_vols) @@ -1042,7 +1044,8 @@ class RestTests(unittest.TestCase): self.assertEquals(200, resp.status) resp = self.request('/storagepools/pool-2/storagevolumes/', req, 'POST') - self.assertEquals(201, resp.status) + self.assertEquals(202, resp.status) + time.sleep(1)

# Verify the storage volume resp = self.request('/storagepools/pool-2/storagevolumes/test-volume')

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 09/01/2014 08:50 AM, lvroyce0210@gmail.com wrote:

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

When pass form to cherrypy, it is stored in cherrypy.request.params, so adjust parse_request for this change. Also avoid body parse so that file content will not be loaded into mem.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/control/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index fbd5177..64e0ac5 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -72,14 +72,16 @@ def mime_in_header(header, mime): def parse_request(): if 'Content-Length' not in cherrypy.request.headers: return {} - rawbody = cherrypy.request.body.read()

if mime_in_header('Content-Type', 'application/json'): + rawbody = cherrypy.request.body.read() try: return json.loads(rawbody) except ValueError: e = OperationFailed('KCHAPI0006E') raise cherrypy.HTTPError(400, e.message) + elif mime_in_header('Content-Type', 'multipart/form-data'): + return cherrypy.request.params else: e = OperationFailed('KCHAPI0007E') raise cherrypy.HTTPError(415, e.message)

On 09/01/2014 08:50 AM, lvroyce0210@gmail.com wrote:

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

Start a task to upload storage volume, limit the upload size to 4G.

The patch is OK for me. I just would like to know what is needed to allow files larger than 4G. Otherwise: Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com>

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/storagevolumes.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+)

diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index bbec591..eded027 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -187,6 +187,7 @@ messages = { "KCHVOL0017E": _("Unable to update database with storage volume information due error: %(err)s"), "KCHVOL0018E": _("Only one of %(param)s can be specified"), "KCHVOL0019E": _("Creating volume from %(param)s is not supported"), + "KCHVOL0020E": _("Upload volume can not be larger than 4G"),

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

diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 203d5e1..44279a2 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -19,6 +19,7 @@

import os

+import cherrypy import libvirt

from kimchi import xmlutils @@ -61,6 +62,42 @@ class StorageVolumesModel(object): taskid = add_task('', create_func, self.objstore, params) return self.task.lookup(taskid)

+ def _create_volume_with_file(self, cb, params): + lcHDRS = {} + for key, val in cherrypy.request.headers.iteritems(): + lcHDRS[key.lower()] = val + + incomingBytes = int(lcHDRS['content-length']) + if incomingBytes > (2 << 31): + raise InvalidParameter("KCHVOL0020E") + + 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'] + try: + f = open(file_path, 'wb') + while True: + data = upload_file.file.read(8192) + if not data: + break + f.write(data) + f.close() + 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 + pool = StoragePoolModel.get_storagepool(pool_name, self.conn) + pool.refresh() + cb('', True) + def _create_volume_with_capacity(self, cb, params): pool_name = params.pop('pool') vol_xml = """

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 09/01/2014 08:50 AM, lvroyce0210@gmail.com wrote:

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

Periodically update current written bytes to task message so that UI can query the latest progress of volume uploading.

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

diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 44279a2..e5f0b24 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -79,13 +79,18 @@ class StorageVolumesModel(object): raise InvalidParameter('KCHVOL0001E', {'name': params['name']})

upload_file = params['file'] + pool = StoragePoolModel.get_storagepool(pool_name, self.conn) try: + size = 0 f = open(file_path, 'wb') + pool.refresh() while True: data = upload_file.file.read(8192) if not data: break + size += len(data) f.write(data) + cb(size, True) f.close() except Exception as e: raise OperationFailed('KCHVOL0007E', @@ -94,9 +99,7 @@ class StorageVolumesModel(object): 'err': e.message})

# Refresh to make sure volume can be found in following lookup - pool = StoragePoolModel.get_storagepool(pool_name, self.conn) pool.refresh() - cb('', True)

def _create_volume_with_capacity(self, cb, params): pool_name = params.pop('pool')