On 10/30/2014 05:13 PM, Aline Manera wrote:

On 10/30/2014 10:05 AM, Daniel Henrique Barboza wrote:

...

These changes implements the backend support for setting the disk image type in an existing VM template, by using a new parameter called "type" in the 'disk' object of the template_update schema.

Supported types: 'bochs', 'cloop', 'cow', 'dmg', 'qcow', 'qcow2', 'qed', 'raw', 'vmdk', 'vpc'.

Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API.md | 1 + src/kimchi/API.json | 8 +++++++- src/kimchi/i18n.py | 1 + src/kimchi/vmtemplate.py | 5 ++++- 4 files changed, 13 insertions(+), 2 deletions(-)

Mockmodel is missing

No changes were made in any /src/kimchi/model class, thus I do not believe any change in mockmodel.py is needed.

...

diff --git a/docs/API.md b/docs/API.md index 6984649..8e2962d 100644 --- a/docs/API.md +++ b/docs/API.md @@ -209,6 +209,7 @@ Represents a snapshot of the Virtual Machine's primary monitor. * index: The device index * size: The device size in GB * base: Base image of this disk + * type: Type of the image. Valid types: bochs, cloop, cow, dmg, qcow, qcow2, qed, raw, vmdk, vpc.

"format" is more meaningful than "type"

I'll change it

You need to update the GET and PUT methods too.

GET /templates/<name> should return the disks dict with the "format" information as well

The methods already returned the dict with the 'type' information. In the case it doesn't after the rename to 'format', I'll update them.

...

* graphics *(optional)*: The graphics paramenters of this template * type: The type of graphics. It can be VNC or spice or None. diff --git a/src/kimchi/API.json b/src/kimchi/API.json index a1156d5..21491d1 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -208,7 +208,7 @@ "format": { "description": "The format of the volume", "type": "string", - "pattern": "^qcow2|raw$", + "pattern": "^(bochs|cloop|cow|dmg|qcow|qcow2|qed|raw|vmdk|vpc)$", "error": "KCHVOL0015E" }, "url": { @@ -619,6 +619,12 @@ "type": "integer", "minimum": 1, "error": "KCHTMPL0022E" + }, + "type": { + "description": "Type of the image of the disk", + "type": "string", + "pattern": "^(bochs|cloop|cow|dmg|qcow|qcow2|qed|raw|vmdk|vpc)$", + "error": "KCHTMPL0027E" } } }, diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 74ea98e..8ca5cc6 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -145,6 +145,7 @@ messages = { "KCHTMPL0024E": _("Cannot identify base image %(path)s format"), "KCHTMPL0025E": _("When specifying CPU topology, VCPUs must be a product of sockets, cores, and threads."), "KCHTMPL0026E": _("When specifying CPU topology, each element must be an integer greater than zero."), + "KCHTMPL0027E": _("Invalid disk image type."),

I think it is good to point to user what are the valid types.

Fair enough.

...

"KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index b48cdbd..9635d67 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -195,7 +195,10 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> src = os.path.join(storage_path, volume) dev = "%s%s" % (self._bus_to_dev[self.info['disk_bus']], string.lowercase[index]) - fmt = 'raw' if self._get_storage_type() in ['logical'] else 'qcow2' + if self._get_storage_type() in ['logical']: + fmt = 'raw' + else: + fmt = disk.get('type', 'qcow2') params = {'src': src, 'dev': dev, 'bus': self.info['disk_bus'], 'type': fmt} ret += """

On 10/30/2014 10:05 AM, Daniel Henrique Barboza wrote:

The tests covered the case where the template is created and the type of the disk is changed.

Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_mockmodel.py | 62 ++++++++++++++++++++++++++++++++++++++++--------- tests/test_model.py | 14 +++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py index 7319531..5d9fdab 100644 --- a/tests/test_mockmodel.py +++ b/tests/test_mockmodel.py @@ -90,12 +90,15 @@ class MockModelTests(unittest.TestCase): else: self.fail("Expected exception not raised")

- def test_template_cpu_info(self): + def _create_default_template(self): # Create default template req = json.dumps({'name': 'test', 'cdrom': fake_iso}) resp = request(host, ssl_port, '/templates', req, 'POST') rsp_body = resp.read() - template_data = json.loads(rsp_body) + return json.loads(rsp_body) + + def test_template_cpu_info(self): + template_data = self._get_default_template() # GET of cpu_info will be {} cpu_info = template_data['cpu_info'] self.assertEquals(cpu_info, {}) @@ -103,16 +106,53 @@ class MockModelTests(unittest.TestCase):

# Update topology # GET of cpu_info will contain 'topology' - req = json.dumps({'cpu_info': {'topology': {'sockets': 1, - 'cores': 1, - 'threads': 1}}}) - resp = request(host, ssl_port, '/templates/test', req, 'PUT') + cpu_info_data = {'cpu_info': {'topology': {'sockets': 1, + 'cores': 1, + 'threads': 1}}} + updated_template, _ = self._get_PUT_response('/templates/test', + cpu_info_data) + self.assertEquals(updated_template['cpu_info'], + cpu_info_data['cpu_info']) + + def test_template_update_disk_type(self): + def _get_default_disk_data(disk_type): + return {'disks': [{'index': 0, 'type': disk_type, 'size': 10}]} + + template = self._create_default_template() + # Default template is created with 1 disk without any declared + # type. + disk_data = template['disks'] + self.assertEquals(disk_data, [{'index': 0, 'size': 10}]) + + # For all supported types, edit the template and check if + # the change was made. + disk_types = ['bochs', 'cloop', 'cow', 'dmg', 'qcow', 'qcow2', + 'qed', 'raw', 'vmdk', 'vpc'] + for disk_type in disk_types: + disk_data = _get_default_disk_data(disk_type) + updated_template, resp_code = \ + self._get_PUT_response('/templates/test', + disk_data) + self.assertEquals(200, resp_code)

Use GET request to confirm the changes were really done.

+ self.assertEquals(updated_template['disks'], disk_data['disks']) + + # Check Bad Request when type is invalid + bad_disk_data = _get_default_disk_data('invalid_disk_type') + _, resp_code = self._get_PUT_response('/templates/test', bad_disk_data) + self.assertEquals(400, resp_code) + + def _get_default_template(self): + # Create default template + req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + resp = request(host, ssl_port, '/templates', req, 'POST') rsp_body = resp.read() - template_data = json.loads(rsp_body) - cpu_info = template_data['cpu_info'] - self.assertEquals(cpu_info, {'topology': {'sockets': 1, - 'cores': 1, - 'threads': 1}}) + return json.loads(rsp_body) + + def _get_PUT_response(self, url, data): + req = json.dumps(data) + resp = request(host, ssl_port, url, req, 'PUT') + rsp_body = resp.read() + return json.loads(rsp_body), resp.status

def test_screenshot_refresh(self): # Create a VM diff --git a/tests/test_model.py b/tests/test_model.py index d9bbe9e..0ff1ea3 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -628,6 +628,20 @@ class ModelTests(unittest.TestCase): self.assertRaises(InvalidParameter, inst.template_update, 'test', params)

+ # For all supported types, edit the template and check if + # the change was made. + disk_types = ['bochs', 'cloop', 'cow', 'dmg', 'qcow', 'qcow2', + 'qed', 'raw', 'vmdk', 'vpc'] + for disk_type in disk_types: + disk_data = {'disks': [{'index': 0, 'type': disk_type, + 'size': 1}]} + inst.template_update('test', disk_data) + updated_template = inst.template_lookup('test') + self.assertEquals(updated_template['disks'], + disk_data['disks']) + # Restore disk data to default value + inst.template_update('test', {'disks': [{'index': 0, 'size': 1}]}) + args = {'name': pool, 'path': path, 'type': 'dir'}