5:19 p.m.
Changes:
v4:
Add support for templates (creation and update)
v3:
Add docs
v2:
Use REST API for tests
General fixes
Ramon Medeiros (4):
Read io and cache option from disks
Allow disks to update cache and io flags
Bug fix #1091: Allow to set disk performance options per guest and
template
Add tests to verify if cache and io of a disk can be changed
API.json | 26 ++++++++++++++++++++++++-
docs/API.md | 4 ++++
i18n.py | 1 -
model/vmstorages.py | 55 ++++++++++++++++++++++++++++++++++------------------
tests/test_rest.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
vmtemplate.py | 19 +++++++++++++++++-
xmlutils/disk.py | 23 ++++++++++++++++++++--
7 files changed, 159 insertions(+), 25 deletions(-)
--
2.9.3
5:19 p.m.
New subject: [PATCH 1/4] Read io and cache option from disks
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
xmlutils/disk.py | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/xmlutils/disk.py b/xmlutils/disk.py
index 02d6811..8edb991 100644
--- a/xmlutils/disk.py
+++ b/xmlutils/disk.py
@@ -147,12 +147,21 @@ def get_vm_disk_info(dom, dev_name):
except:
path = ""
- return {'dev': dev_name,
+ base = {'dev': dev_name,
'path': path,
'type': disk.attrib['device'],
'format': disk.driver.attrib['type'],
'bus': disk.target.attrib['bus']}
+ # optional parameters
+ if disk.driver.attrib.get('io') != None:
+ base.update({'io': disk.driver.attrib['io']})
+
+ if disk.driver.attrib.get('cache') != None:
+ base.update({'cache': disk.driver.attrib['cache']})
+
+ return base
+
def get_vm_disks(dom):
xml = dom.XMLDesc(0)
--
2.9.3
5:19 p.m.
New subject: [PATCH 2/4] Allow disks to update cache and io flags
This is a part of Bug fix #1092 solution
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
API.json | 11 ++++++++++-
docs/API.md | 3 +++
i18n.py | 1 -
model/vmstorages.py | 51 +++++++++++++++++++++++++++++++++------------------
xmlutils/disk.py | 12 +++++++++++-
5 files changed, 57 insertions(+), 21 deletions(-)
diff --git a/API.json b/API.json
index 5729e5d..abe980c 100644
--- a/API.json
+++ b/API.json
@@ -761,8 +761,17 @@
"description": "Path of iso image file or disk mount
point",
"type": "string",
"pattern":
"^(|(/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$",
- "required": true,
"error": "KCHVMSTOR0003E"
+ },
+ "cache": {
+ "description": "Cache options",
+ "type": "string",
+ "pattern":
"^(none|writethrough|writeback|directsync|unsafe|default)$"
+ },
+ "io": {
+ "description": "I/O options",
+ "type": "string",
+ "pattern": "^(native|threads|default)$"
}
},
"additionalProperties": false
diff --git a/docs/API.md b/docs/API.md
index 3ecc7a0..6f00600 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -253,6 +253,9 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* bus: Bus type of disk attached.
* **PUT**: Update storage information
* path: Path of cdrom iso. Can not be blank. Now just support cdrom type.
+ * io: IO settings for disks. Values accepted: native, threads and default.
+ * cache: Cache settings for disks. Values accepted: none, writethrough,
+ writeback, directsync, unsafe and default.
* **DELETE**: Remove the storage.
**Actions (POST):**
diff --git a/i18n.py b/i18n.py
index 4460ce5..4bb3a4f 100644
--- a/i18n.py
+++ b/i18n.py
@@ -327,7 +327,6 @@ messages = {
"KCHVMSTOR0002E": _("Invalid storage type. Types supported:
'cdrom', 'disk'"),
"KCHVMSTOR0003E": _("The path '%(value)s' is not a valid
local/remote path for the device"),
- "KCHVMSTOR0006E": _("Only CDROM path can be update."),
"KCHVMSTOR0007E": _("The storage device %(dev_name)s does not exist in
the virtual machine %(vm_name)s"),
"KCHVMSTOR0008E": _("Error while creating new storage device:
%(error)s"),
"KCHVMSTOR0009E": _("Error while updating storage device:
%(error)s"),
diff --git a/model/vmstorages.py b/model/vmstorages.py
index db68121..007e88c 100644
--- a/model/vmstorages.py
+++ b/model/vmstorages.py
@@ -1,7 +1,7 @@
#
# Project Kimchi
#
-# Copyright IBM Corp, 2015-2016
+# Copyright IBM Corp, 2015-2017
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -35,7 +35,6 @@ from wok.plugins.kimchi.utils import create_disk_image, is_s390x
from wok.plugins.kimchi.xmlutils.disk import get_device_node, get_disk_xml
from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks
-
HOTPLUG_TYPE = ['scsi', 'virtio']
@@ -232,26 +231,42 @@ class VMStorageModel(object):
dom = VMModel.get_vm(vm_name, self.conn)
dev_info = self.lookup(vm_name, dev_name)
+
+ # only change path to cdrom devices
if dev_info['type'] != 'cdrom':
- raise InvalidOperation("KCHVMSTOR0006E")
+ old_dev, old_xml = get_disk_xml(dev_info)
+ dev_info.update(params)
+ dev, xml = get_disk_xml(dev_info)
- params['path'] = params.get('path', '')
- old_disk_path = dev_info['path']
- new_disk_path = params['path']
- if new_disk_path != old_disk_path:
- # An empty path means a CD-ROM was empty or ejected:
- if old_disk_path is not '':
- old_disk_used_by = get_disk_used_by(self.conn, old_disk_path)
- if new_disk_path is not '':
- new_disk_used_by = get_disk_used_by(self.conn, new_disk_path)
+ # remove
+ self.delete(vm_name, dev_name)
- dev_info.update(params)
- dev, xml = get_disk_xml(dev_info)
+ try:
+ dom.attachDeviceFlags(xml)
+ except Exception as e:
+ dom.attachDeviceFlags(old_xml)
+ raise OperationFailed("KCHVMSTOR0009E", {'error':
e.message})
- try:
- dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
- except Exception as e:
- raise OperationFailed("KCHVMSTOR0009E", {'error':
e.message})
+ else:
+ dev_info.update(params)
+ dev, xml = get_disk_xml(dev_info)
+
+ params['path'] = params.get('path', '')
+ old_disk_path = dev_info['path']
+ new_disk_path = params['path']
+ if new_disk_path != old_disk_path:
+ # An empty path means a CD-ROM was empty or ejected:
+ if old_disk_path is not '':
+ old_disk_used_by = get_disk_used_by(self.conn,
+ old_disk_path)
+ if new_disk_path is not '':
+ new_disk_used_by = get_disk_used_by(self.conn,
+ new_disk_path)
+
+ try:
+ dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
+ except Exception as e:
+ raise OperationFailed("KCHVMSTOR0009E", {'error':
e.message})
try:
if old_disk_used_by is not None and \
diff --git a/xmlutils/disk.py b/xmlutils/disk.py
index 8edb991..9a9987a 100644
--- a/xmlutils/disk.py
+++ b/xmlutils/disk.py
@@ -1,7 +1,7 @@
#
# Project Kimchi
#
-# Copyright IBM Corp, 2015-2016
+# Copyright IBM Corp, 2015-2017
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -50,6 +50,8 @@ def get_disk_xml(params):
if disk_type is None:
disk_type = _get_disk_type(path) if len(path) > 0 else 'file'
disk = E.disk(type=disk_type, device=params['type'])
+
+ # <driver />
driver = E.driver(name='qemu', type=params['format'])
if params['type'] != 'cdrom':
driver.set('cache', 'none')
@@ -57,6 +59,14 @@ def get_disk_xml(params):
if params.get('pool_type') == "netfs":
driver.set("io", "native")
+ # set io
+ if params.get("io") is not None:
+ driver.set("io", params.get("io"))
+
+ # set cache
+ if params.get("cache") is not None:
+ driver.set("cache", params.get("cache"))
+
disk.append(driver)
# Get device name according to bus and index values
--
2.9.3
5:19 p.m.
New subject: [PATCH 3/4] Bug fix #1091: Allow to set disk performance options per guest and template
Allow vmstorage creation passing disk bus
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
API.json | 15 +++++++++++++++
docs/API.md | 1 +
model/vmstorages.py | 4 +++-
vmtemplate.py | 19 ++++++++++++++++++-
4 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/API.json b/API.json
index abe980c..bae9c9c 100644
--- a/API.json
+++ b/API.json
@@ -658,6 +658,21 @@
"error": "KCHTMPL0015E"
}
}
+ },
+ "cache": {
+ "description": "Cache options",
+ "type": "string",
+ "pattern":
"^(none|writethrough|writeback|directsync|unsafe|default)$"
+ },
+ "io": {
+ "description": "I/O options",
+ "type": "string",
+ "pattern":
"^(native|threads|default)$"
+ },
+ "bus": {
+ "description": "Bus disk",
+ "type": "string",
+ "pattern": "^(scsi|virtio|ide)$"
}
}
},
diff --git a/docs/API.md b/docs/API.md
index 6f00600..3657db3 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -243,6 +243,7 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* dir_path: s390x specific attribute to attach direct storage without libvirt
* format: s390x specific attribute specify the format of direct storage
* size: s390x specific attribute to specify the size of direct storage
+ * bus: Bus type of disk.
### Sub-resource: storage
**URI:** /plugins/kimchi/vms/*:name*/storages/*:dev*
diff --git a/model/vmstorages.py b/model/vmstorages.py
index 007e88c..bf55300 100644
--- a/model/vmstorages.py
+++ b/model/vmstorages.py
@@ -91,7 +91,9 @@ class VMStoragesModel(object):
raise InvalidParameter("KCHVMSTOR0019E")
dom = VMModel.get_vm(vm_name, self.conn)
- params['bus'] = _get_device_bus(params['type'], dom)
+
+ if params.get('bus') is None:
+ params['bus'] = _get_device_bus(params['type'], dom)
if is_s390x() and params['type'] == 'disk' and 'dir_path'
in params:
if 'format' not in params:
diff --git a/vmtemplate.py b/vmtemplate.py
index 1acd4db..3f37d1a 100644
--- a/vmtemplate.py
+++ b/vmtemplate.py
@@ -1,7 +1,7 @@
#
# Project Kimchi
#
-# Copyright IBM Corp, 2015-2016
+# Copyright IBM Corp, 2015-2017
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -144,6 +144,12 @@ class VMTemplate(object):
keys = sorted(disk_info.keys())
+ # remove optional parameters
+ optional = ["bus", "io", "cache"]
+ for opt in optional:
+ if opt in keys:
+ keys.remove(opt)
+
if ((keys != sorted(basic_disk)) and
(keys != sorted(ro_disk)) and
(keys != sorted(base_disk))):
@@ -253,6 +259,7 @@ class VMTemplate(object):
return xml
def _get_disks_xml(self, vm_uuid):
+ optional = ["io", "cache"]
base_disk_params = {'type': 'disk', 'disk':
'file',
'bus': self.info['disk_bus']}
logical_disk_params = {'format': 'raw'}
@@ -267,6 +274,16 @@ class VMTemplate(object):
params = dict(base_disk_params)
params['format'] = disk['format']
params['index'] = index
+
+ # bus passed: overwrite
+ if disk.get("bus") is not None:
+ params["bus"] = disk.get("bus")
+
+ # add optionals
+ for opt in optional:
+ if disk.get(opt) is not None:
+ params[opt] = disk[opt]
+
if disk.get('pool'):
params.update(locals().get('%s_disk_params' %
disk['pool']['type'], {}))
--
2.9.3
5:19 p.m.
New subject: [PATCH 4/4] Add tests to verify if cache and io of a disk can be changed
Tests run both on update and template creation
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
tests/test_rest.py | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 852e4bd..a704283 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -151,8 +151,17 @@ class RestTests(unittest.TestCase):
self.assertEquals(200, resp.status)
def test_edit_vm(self):
+ disks = [
+ {'index': 0, 'size': 10, 'format': 'qcow2',
+ 'pool': {"name":
"/plugins/kimchi/storagepools/default-pool"}},
+ {'index': 1, 'size': 10, 'format': 'qcow2',
'io': 'threads',
+ 'cache': 'unsafe', 'bus': 'scsi',
+ 'pool': {"name":
"/plugins/kimchi/storagepools/default-pool"}}]
+
req = json.dumps({'name': 'test',
- 'source_media': {'type': 'disk',
'path': fake_iso}})
+ 'source_media': {'type': 'disk',
'path': fake_iso},
+ 'disks': disks})
+
resp = self.request('/plugins/kimchi/templates', req, 'POST')
self.assertEquals(201, resp.status)
@@ -166,6 +175,23 @@ class RestTests(unittest.TestCase):
vm = json.loads(self.request('/plugins/kimchi/vms/vm-1').read())
self.assertEquals('vm-1', vm['name'])
+ # test vm storage
+ storages = json.loads(
+ self.request('/plugins/kimchi/vms/vm-1/storages').read())
+ self.assertEquals(len(storages), 3)
+
+ # test sdb disk
+ resp = self.request('/plugins/kimchi/vms/vm-1/storages/sdb',
'GET')
+ self.assertEquals(resp.status, 200)
+ scsi_disk = json.loads(resp.read())
+ self.assertEquals(scsi_disk['cache'], 'unsafe')
+ self.assertEquals(scsi_disk['io'], 'threads')
+
+ # remove sdb disk
+ resp = self.request('/plugins/kimchi/vms/vm-1/storages/sdb',
+ '', 'DELETE')
+ self.assertEquals(resp.status, 204)
+
req = json.dumps({'cpu_info': {'maxvcpus': 5, 'vcpus':
3}})
resp = self.request('/plugins/kimchi/vms/vm-1', req, 'PUT')
self.assertEquals(200, resp.status)
@@ -330,6 +356,34 @@ class RestTests(unittest.TestCase):
resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd', req,
'PUT')
self.assertEquals(400, resp.status)
+ # add volume as scsi
+ mock_base = '/tmp/mock.img'
+ os.system("qemu-img create -f qcow2 %s 100M" % mock_base)
+ req = json.dumps({'type': 'disk',
+ 'path': mock_base,
+ 'bus': 'scsi'})
+ resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd/storages',
+ req, 'POST')
+ self.assertEquals(201, resp.status)
+ resp = json.loads(resp.read())
+ self.assertEquals("scsi", resp["bus"])
+ dev = resp["dev"]
+
+ # change io/cache
+ req = json.dumps({"io": "threads", "cache":
"unsafe"})
+ resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd/storages/' +
+ str(dev), req, 'PUT')
+ self.assertEquals(200, resp.status)
+ resp = json.loads(resp.read())
+ self.assertEquals('threads', resp['io'])
+ self.assertEquals('unsafe', resp['cache'])
+
+ # remove disk
+ resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd/storages/' +
+ str(dev), req, 'DELETE')
+ os.system("rm -rf " + mock_base)
+ self.assertEquals(204, resp.status)
+
def test_vm_lifecycle(self):
# Create a Template
req = json.dumps({'name': 'test',
--
2.9.3
4:30 p.m.
New subject: [PATCH 1/4] Read io and cache option from disks
On 03/10/2017 08:19 PM, Ramon Medeiros wrote:
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
xmlutils/disk.py | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/xmlutils/disk.py b/xmlutils/disk.py
index 02d6811..8edb991 100644
--- a/xmlutils/disk.py
+++ b/xmlutils/disk.py
@@ -147,12 +147,21 @@ def get_vm_disk_info(dom, dev_name):
except:
path = ""
- return {'dev': dev_name,
+ base = {'dev': dev_name,
'path': path,
'type': disk.attrib['device'],
'format': disk.driver.attrib['type'],
'bus': disk.target.attrib['bus']}
+ # optional parameters
+ if disk.driver.attrib.get('io') != None:
+ base.update({'io': disk.driver.attrib['io']})
+
+ if disk.driver.attrib.get('cache') != None:
+ base.update({'cache': disk.driver.attrib['cache']})
+
You should return always the same group of data. Fallback io and cache
to None or empty string in case it is not set for a given XML.
+ return base
+
def get_vm_disks(dom):
xml = dom.XMLDesc(0)
4:34 p.m.
New subject: [PATCH 2/4] Allow disks to update cache and io flags
Ramon,
The patch looks good, just add more details to the commit message.
Is it only related to guests?
On 03/10/2017 08:19 PM, Ramon Medeiros wrote:
This is a part of Bug fix #1092 solution
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
API.json | 11 ++++++++++-
docs/API.md | 3 +++
i18n.py | 1 -
model/vmstorages.py | 51 +++++++++++++++++++++++++++++++++------------------
xmlutils/disk.py | 12 +++++++++++-
5 files changed, 57 insertions(+), 21 deletions(-)
diff --git a/API.json b/API.json
index 5729e5d..abe980c 100644
--- a/API.json
+++ b/API.json
@@ -761,8 +761,17 @@
"description": "Path of iso image file or disk mount
point",
"type": "string",
"pattern":
"^(|(/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$",
- "required": true,
"error": "KCHVMSTOR0003E"
+ },
+ "cache": {
+ "description": "Cache options",
+ "type": "string",
+ "pattern":
"^(none|writethrough|writeback|directsync|unsafe|default)$"
+ },
+ "io": {
+ "description": "I/O options",
+ "type": "string",
+ "pattern": "^(native|threads|default)$"
}
},
"additionalProperties": false
diff --git a/docs/API.md b/docs/API.md
index 3ecc7a0..6f00600 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -253,6 +253,9 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* bus: Bus type of disk attached.
* **PUT**: Update storage information
* path: Path of cdrom iso. Can not be blank. Now just support cdrom type.
+ * io: IO settings for disks. Values accepted: native, threads and default.
+ * cache: Cache settings for disks. Values accepted: none, writethrough,
+ writeback, directsync, unsafe and default.
* **DELETE**: Remove the storage.
**Actions (POST):**
diff --git a/i18n.py b/i18n.py
index 4460ce5..4bb3a4f 100644
--- a/i18n.py
+++ b/i18n.py
@@ -327,7 +327,6 @@ messages = {
"KCHVMSTOR0002E": _("Invalid storage type. Types supported:
'cdrom', 'disk'"),
"KCHVMSTOR0003E": _("The path '%(value)s' is not a valid
local/remote path for the device"),
- "KCHVMSTOR0006E": _("Only CDROM path can be update."),
"KCHVMSTOR0007E": _("The storage device %(dev_name)s does not exist
in the virtual machine %(vm_name)s"),
"KCHVMSTOR0008E": _("Error while creating new storage device:
%(error)s"),
"KCHVMSTOR0009E": _("Error while updating storage device:
%(error)s"),
diff --git a/model/vmstorages.py b/model/vmstorages.py
index db68121..007e88c 100644
--- a/model/vmstorages.py
+++ b/model/vmstorages.py
@@ -1,7 +1,7 @@
#
# Project Kimchi
#
-# Copyright IBM Corp, 2015-2016
+# Copyright IBM Corp, 2015-2017
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -35,7 +35,6 @@ from wok.plugins.kimchi.utils import create_disk_image, is_s390x
from wok.plugins.kimchi.xmlutils.disk import get_device_node, get_disk_xml
from wok.plugins.kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks
-
HOTPLUG_TYPE = ['scsi', 'virtio']
@@ -232,26 +231,42 @@ class VMStorageModel(object):
dom = VMModel.get_vm(vm_name, self.conn)
dev_info = self.lookup(vm_name, dev_name)
+
+ # only change path to cdrom devices
if dev_info['type'] != 'cdrom':
- raise InvalidOperation("KCHVMSTOR0006E")
+ old_dev, old_xml = get_disk_xml(dev_info)
+ dev_info.update(params)
+ dev, xml = get_disk_xml(dev_info)
- params['path'] = params.get('path', '')
- old_disk_path = dev_info['path']
- new_disk_path = params['path']
- if new_disk_path != old_disk_path:
- # An empty path means a CD-ROM was empty or ejected:
- if old_disk_path is not '':
- old_disk_used_by = get_disk_used_by(self.conn, old_disk_path)
- if new_disk_path is not '':
- new_disk_used_by = get_disk_used_by(self.conn, new_disk_path)
+ # remove
+ self.delete(vm_name, dev_name)
- dev_info.update(params)
- dev, xml = get_disk_xml(dev_info)
+ try:
+ dom.attachDeviceFlags(xml)
+ except Exception as e:
+ dom.attachDeviceFlags(old_xml)
+ raise OperationFailed("KCHVMSTOR0009E", {'error':
e.message})
- try:
- dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
- except Exception as e:
- raise OperationFailed("KCHVMSTOR0009E", {'error':
e.message})
+ else:
+ dev_info.update(params)
+ dev, xml = get_disk_xml(dev_info)
+
+ params['path'] = params.get('path', '')
+ old_disk_path = dev_info['path']
+ new_disk_path = params['path']
+ if new_disk_path != old_disk_path:
+ # An empty path means a CD-ROM was empty or ejected:
+ if old_disk_path is not '':
+ old_disk_used_by = get_disk_used_by(self.conn,
+ old_disk_path)
+ if new_disk_path is not '':
+ new_disk_used_by = get_disk_used_by(self.conn,
+ new_disk_path)
+
+ try:
+ dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
+ except Exception as e:
+ raise OperationFailed("KCHVMSTOR0009E", {'error':
e.message})
try:
if old_disk_used_by is not None and \
diff --git a/xmlutils/disk.py b/xmlutils/disk.py
index 8edb991..9a9987a 100644
--- a/xmlutils/disk.py
+++ b/xmlutils/disk.py
@@ -1,7 +1,7 @@
#
# Project Kimchi
#
-# Copyright IBM Corp, 2015-2016
+# Copyright IBM Corp, 2015-2017
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -50,6 +50,8 @@ def get_disk_xml(params):
if disk_type is None:
disk_type = _get_disk_type(path) if len(path) > 0 else 'file'
disk = E.disk(type=disk_type, device=params['type'])
+
+ # <driver />
driver = E.driver(name='qemu', type=params['format'])
if params['type'] != 'cdrom':
driver.set('cache', 'none')
@@ -57,6 +59,14 @@ def get_disk_xml(params):
if params.get('pool_type') == "netfs":
driver.set("io", "native")
+ # set io
+ if params.get("io") is not None:
+ driver.set("io", params.get("io"))
+
+ # set cache
+ if params.get("cache") is not None:
+ driver.set("cache", params.get("cache"))
+
disk.append(driver)
# Get device name according to bus and index values
4:36 p.m.
New subject: [PATCH 3/4] Bug fix #1091: Allow to set disk performance options per guest and template
HI Ramon,
How is that different from the previous patch? Should they me merged
together?
More comments below:
On 03/10/2017 08:19 PM, Ramon Medeiros wrote:
Allow vmstorage creation passing disk bus
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
API.json | 15 +++++++++++++++
docs/API.md | 1 +
model/vmstorages.py | 4 +++-
vmtemplate.py | 19 ++++++++++++++++++-
4 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/API.json b/API.json
index abe980c..bae9c9c 100644
--- a/API.json
+++ b/API.json
@@ -658,6 +658,21 @@
"error": "KCHTMPL0015E"
}
}
+ },
+ "cache": {
+ "description": "Cache options",
+ "type": "string",
+ "pattern":
"^(none|writethrough|writeback|directsync|unsafe|default)$"
+ },
+ "io": {
+ "description": "I/O options",
+ "type": "string",
+ "pattern":
"^(native|threads|default)$"
+ },
+ "bus": {
+ "description": "Bus disk",
+ "type": "string",
+ "pattern": "^(scsi|virtio|ide)$"
}
}
},
diff --git a/docs/API.md b/docs/API.md
index 6f00600..3657db3 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -243,6 +243,7 @@ Represents a snapshot of the Virtual Machine's primary monitor.
* dir_path: s390x specific attribute to attach direct storage without libvirt
* format: s390x specific attribute specify the format of direct storage
* size: s390x specific attribute to specify the size of direct storage
+ * bus: Bus type of disk.
### Sub-resource: storage
**URI:** /plugins/kimchi/vms/*:name*/storages/*:dev*
diff --git a/model/vmstorages.py b/model/vmstorages.py
index 007e88c..bf55300 100644
--- a/model/vmstorages.py
+++ b/model/vmstorages.py
@@ -91,7 +91,9 @@ class VMStoragesModel(object):
raise InvalidParameter("KCHVMSTOR0019E")
dom = VMModel.get_vm(vm_name, self.conn)
- params['bus'] = _get_device_bus(params['type'], dom)
+
+ if params.get('bus') is None:
+ params['bus'] = _get_device_bus(params['type'], dom)
if is_s390x() and params['type'] == 'disk' and
'dir_path' in params:
if 'format' not in params:
diff --git a/vmtemplate.py b/vmtemplate.py
index 1acd4db..3f37d1a 100644
--- a/vmtemplate.py
+++ b/vmtemplate.py
@@ -1,7 +1,7 @@
#
# Project Kimchi
#
-# Copyright IBM Corp, 2015-2016
+# Copyright IBM Corp, 2015-2017
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -144,6 +144,12 @@ class VMTemplate(object):
keys = sorted(disk_info.keys())
+ # remove optional parameters
+ optional = ["bus", "io", "cache"]
+ for opt in optional:
+ if opt in keys:
+ keys.remove(opt)
+
if ((keys != sorted(basic_disk)) and
(keys != sorted(ro_disk)) and
(keys != sorted(base_disk))):
@@ -253,6 +259,7 @@ class VMTemplate(object):
return xml
def _get_disks_xml(self, vm_uuid):
+ optional = ["io", "cache"]
Shouldn't be 'bus' in the list above?
base_disk_params = {'type': 'disk',
'disk': 'file',
'bus': self.info['disk_bus']}
logical_disk_params = {'format': 'raw'}
@@ -267,6 +274,16 @@ class VMTemplate(object):
params = dict(base_disk_params)
params['format'] = disk['format']
params['index'] = index
+
+ # bus passed: overwrite
+ if disk.get("bus") is not None:
+ params["bus"] = disk.get("bus")
+
+ # add optionals
+ for opt in optional:
+ if disk.get(opt) is not None:
+ params[opt] = disk[opt]
+
if disk.get('pool'):
params.update(locals().get('%s_disk_params' %
disk['pool']['type'], {}))
4:39 p.m.
New subject: [PATCH 4/4] Add tests to verify if cache and io of a disk can be changed
On 03/10/2017 08:19 PM, Ramon Medeiros wrote:
Tests run both on update and template creation
Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
---
tests/test_rest.py | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 852e4bd..a704283 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -151,8 +151,17 @@ class RestTests(unittest.TestCase):
self.assertEquals(200, resp.status)
def test_edit_vm(self):
+ disks = [
+ {'index': 0, 'size': 10, 'format': 'qcow2',
+ 'pool': {"name":
"/plugins/kimchi/storagepools/default-pool"}},
+ {'index': 1, 'size': 10, 'format': 'qcow2',
'io': 'threads',
+ 'cache': 'unsafe', 'bus': 'scsi',
+ 'pool': {"name":
"/plugins/kimchi/storagepools/default-pool"}}]
+
req = json.dumps({'name': 'test',
- 'source_media': {'type': 'disk',
'path': fake_iso}})
+ 'source_media': {'type': 'disk',
'path': fake_iso},
+ 'disks': disks})
+
resp = self.request('/plugins/kimchi/templates', req, 'POST')
self.assertEquals(201, resp.status)
Please, add tests to update an existing Template to change io, cache and
bus disk information.
@@ -166,6 +175,23 @@ class RestTests(unittest.TestCase):
vm = json.loads(self.request('/plugins/kimchi/vms/vm-1').read())
self.assertEquals('vm-1', vm['name'])
+ # test vm storage
+ storages = json.loads(
+ self.request('/plugins/kimchi/vms/vm-1/storages').read())
+ self.assertEquals(len(storages), 3)
+
+ # test sdb disk
+ resp = self.request('/plugins/kimchi/vms/vm-1/storages/sdb',
'GET')
+ self.assertEquals(resp.status, 200)
+ scsi_disk = json.loads(resp.read())
+ self.assertEquals(scsi_disk['cache'], 'unsafe')
+ self.assertEquals(scsi_disk['io'], 'threads')
+
+ # remove sdb disk
+ resp = self.request('/plugins/kimchi/vms/vm-1/storages/sdb',
+ '', 'DELETE')
+ self.assertEquals(resp.status, 204)
+
req = json.dumps({'cpu_info': {'maxvcpus': 5, 'vcpus':
3}})
resp = self.request('/plugins/kimchi/vms/vm-1', req, 'PUT')
self.assertEquals(200, resp.status)
@@ -330,6 +356,34 @@ class RestTests(unittest.TestCase):
resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd', req,
'PUT')
self.assertEquals(400, resp.status)
+ # add volume as scsi
+ mock_base = '/tmp/mock.img'
+ os.system("qemu-img create -f qcow2 %s 100M" % mock_base)
+ req = json.dumps({'type': 'disk',
+ 'path': mock_base,
+ 'bus': 'scsi'})
+ resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd/storages',
+ req, 'POST')
+ self.assertEquals(201, resp.status)
+ resp = json.loads(resp.read())
+ self.assertEquals("scsi", resp["bus"])
+ dev = resp["dev"]
+
+ # change io/cache
+ req = json.dumps({"io": "threads", "cache":
"unsafe"})
+ resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd/storages/' +
+ str(dev), req, 'PUT')
+ self.assertEquals(200, resp.status)
+ resp = json.loads(resp.read())
+ self.assertEquals('threads', resp['io'])
+ self.assertEquals('unsafe', resp['cache'])
+
+ # remove disk
+ resp = self.request('/plugins/kimchi/vms/∨м-црdαtеd/storages/' +
+ str(dev), req, 'DELETE')
+ os.system("rm -rf " + mock_base)
+ self.assertEquals(204, resp.status)
+
def test_vm_lifecycle(self):
# Create a Template
req = json.dumps({'name': 'test',
5:40 p.m.
New subject: [PATCH 3/4] Bug fix #1091: Allow to set disk performance options per guest and template
On 03/13/2017 06:36 PM, Aline Manera wrote:
HI Ramon,
How is that different from the previous patch? Should they me merged
together?
More comments below:
On 03/10/2017 08:19 PM, Ramon Medeiros wrote:
> Allow vmstorage creation passing disk bus
>
> Signed-off-by: Ramon Medeiros <ramonn(a)linux.vnet.ibm.com>
> ---
> API.json | 15 +++++++++++++++
> docs/API.md | 1 +
> model/vmstorages.py | 4 +++-
> vmtemplate.py | 19 ++++++++++++++++++-
> 4 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/API.json b/API.json
> index abe980c..bae9c9c 100644
> --- a/API.json
> +++ b/API.json
> @@ -658,6 +658,21 @@
> "error":
"KCHTMPL0015E"
> }
> }
> + },
> + "cache": {
> + "description": "Cache options",
> + "type": "string",
> + "pattern":
> "^(none|writethrough|writeback|directsync|unsafe|default)$"
> + },
> + "io": {
> + "description": "I/O options",
> + "type": "string",
> + "pattern":
"^(native|threads|default)$"
> + },
> + "bus": {
> + "description": "Bus disk",
> + "type": "string",
> + "pattern":
"^(scsi|virtio|ide)$"
> }
> }
> },
> diff --git a/docs/API.md b/docs/API.md
> index 6f00600..3657db3 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -243,6 +243,7 @@ Represents a snapshot of the Virtual Machine's
> primary monitor.
> * dir_path: s390x specific attribute to attach direct storage
> without libvirt
> * format: s390x specific attribute specify the format of direct
> storage
> * size: s390x specific attribute to specify the size of direct
> storage
> + * bus: Bus type of disk.
>
> ### Sub-resource: storage
> **URI:** /plugins/kimchi/vms/*:name*/storages/*:dev*
> diff --git a/model/vmstorages.py b/model/vmstorages.py
> index 007e88c..bf55300 100644
> --- a/model/vmstorages.py
> +++ b/model/vmstorages.py
> @@ -91,7 +91,9 @@ class VMStoragesModel(object):
> raise InvalidParameter("KCHVMSTOR0019E")
>
> dom = VMModel.get_vm(vm_name, self.conn)
> - params['bus'] = _get_device_bus(params['type'], dom)
> +
> + if params.get('bus') is None:
> + params['bus'] = _get_device_bus(params['type'], dom)
>
> if is_s390x() and params['type'] == 'disk' and
'dir_path'
> in params:
> if 'format' not in params:
> diff --git a/vmtemplate.py b/vmtemplate.py
> index 1acd4db..3f37d1a 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -1,7 +1,7 @@
> #
> # Project Kimchi
> #
> -# Copyright IBM Corp, 2015-2016
> +# Copyright IBM Corp, 2015-2017
> #
> # This library is free software; you can redistribute it and/or
> # modify it under the terms of the GNU Lesser General Public
> @@ -144,6 +144,12 @@ class VMTemplate(object):
>
> keys = sorted(disk_info.keys())
>
> + # remove optional parameters
> + optional = ["bus", "io", "cache"]
> + for opt in optional:
> + if opt in keys:
> + keys.remove(opt)
> +
> if ((keys != sorted(basic_disk)) and
> (keys != sorted(ro_disk)) and
> (keys != sorted(base_disk))):
> @@ -253,6 +259,7 @@ class VMTemplate(object):
> return xml
>
> def _get_disks_xml(self, vm_uuid):
> + optional = ["io", "cache"]
Shouldn't be 'bus' in the list above?
no. 'bus' is already added by osinfo. So, it include in the checking.
> base_disk_params = {'type': 'disk',
'disk': 'file',
> 'bus': self.info['disk_bus']}
> logical_disk_params = {'format': 'raw'}
> @@ -267,6 +274,16 @@ class VMTemplate(object):
> params = dict(base_disk_params)
> params['format'] = disk['format']
> params['index'] = index
> +
> + # bus passed: overwrite
> + if disk.get("bus") is not None:
> + params["bus"] = disk.get("bus")
> +
> + # add optionals
> + for opt in optional:
> + if disk.get(opt) is not None:
> + params[opt] = disk[opt]
> +
> if disk.get('pool'):
> params.update(locals().get('%s_disk_params' %
> disk['pool']['type'], {}))
2603
days inactive
2613
days old
9 comments
2 participants
participants (2)
-
Aline Manera -
Ramon Medeiros