Allow disk bus to be specified on creation based on commits: Allow disks to update cache and io flags Add tests to verify if cache and io of a disk can be changed Ramon Medeiros (2): Bug fix #1091: Allow to set disk performance options per guest and template Add tests for disk creation with specified bus driver model/vmstorages.py | 4 +++- tests/test_model.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) -- 2.9.3 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel Allow vmstorage creation passing disk bus Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- model/vmstorages.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/model/vmstorages.py b/model/vmstorages.py index XXXXXXX..XXXXXXX 100644 --- a/model/vmstorages.py +++ b/model/vmstorages.py @@ -XXX,XX +XXX,XX @@ 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') == 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: -- 2.9.3 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- tests/test_model.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index XXXXXXX..XXXXXXX 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -XXX,XX +XXX,XX @@ class ModelTests(unittest.TestCase): self.assertRaises(InvalidParameter, inst.vmstorages_create, vm_name, cdrom_args) + # try add disk with specified bus + disk_args = {"type": "disk", + "pool": pool, + "vol": vol, + "bus": "scsi"} + disk = inst.vmstorages_create(vm_name, disk_args) + disk_info = inst.vmstorage_lookup(vm_name, disk) + self.assertEquals("scsi", disk_info['bus']) + self.assertTrue(disk_info["dev"].startswith("sd")) + inst.vmstorage_delete(vm_name, disk) + # Cold plug and unplug a disk disk = _attach_disk() inst.vmstorage_delete(vm_name, disk) -- 2.9.3 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel Changes: v5: Increase commit message verbosity Always return cache and io Improve documentation at API.md 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 | 12 +++++++++ i18n.py | 1 - model/vmstorages.py | 55 ++++++++++++++++++++++++++-------------- tests/test_rest.py | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++- vmtemplate.py | 19 +++++++++++++- xmlutils/disk.py | 14 ++++++++++- 7 files changed, 175 insertions(+), 24 deletions(-) -- 2.9.3 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- xmlutils/disk.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xmlutils/disk.py b/xmlutils/disk.py index XXXXXXX..XXXXXXX 100644 --- a/xmlutils/disk.py +++ b/xmlutils/disk.py @@ -XXX,XX +XXX,XX @@ def get_vm_disk_info(dom, dev_name): 'path': path, 'type': disk.attrib['device'], 'format': disk.driver.attrib['type'], - 'bus': disk.target.attrib['bus']} + 'bus': disk.target.attrib['bus'], + 'io': disk.driver.attrib.get('io', 'none'), + 'cache': disk.driver.attrib.get('cache', 'none')} def get_vm_disks(dom): -- 2.9.3 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel This allows VMs to update disks flags (io and cache). Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- API.json | 11 ++++++++++- docs/API.md | 9 +++++++++ i18n.py | 1 - model/vmstorages.py | 51 +++++++++++++++++++++++++++++++++------------------ xmlutils/disk.py | 10 ++++++++++ 5 files changed, 62 insertions(+), 20 deletions(-) diff --git a/API.json b/API.json index XXXXXXX..XXXXXXX 100644 --- a/API.json +++ b/API.json @@ -XXX,XX +XXX,XX @@ "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 XXXXXXX..XXXXXXX 100644 --- a/docs/API.md +++ b/docs/API.md @@ -XXX,XX +XXX,XX @@ 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 + * 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. + * bus: Bus type of disk. ### Sub-resource: storage **URI:** /plugins/kimchi/vms/*:name*/storages/*:dev* @@ -XXX,XX +XXX,XX @@ Represents a snapshot of the Virtual Machine's primary monitor. * dev: The name of the storage in the vm. * type: The type of the storage (currently support 'cdrom' and 'disk'). * path: Path of cdrom iso or disk image file. + * io: IO settings for disks. + * cache: Cache settings for disks. * 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 XXXXXXX..XXXXXXX 100644 --- a/i18n.py +++ b/i18n.py @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/model/vmstorages.py +++ b/model/vmstorages.py @@ -XXX,XX +XXX,XX @@ # # 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 @@ -XXX,XX +XXX,XX @@ 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'] @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/xmlutils/disk.py +++ b/xmlutils/disk.py @@ -XXX,XX +XXX,XX @@ 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']) try: fd = os.open(path, os.O_RDONLY | os.O_DIRECT) @@ -XXX,XX +XXX,XX @@ 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 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel Allow templates to set and update io, cache and bus of the disks. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- API.json | 15 +++++++++++++++ docs/API.md | 3 +++ model/vmstorages.py | 4 +++- vmtemplate.py | 19 ++++++++++++++++++- 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/API.json b/API.json index XXXXXXX..XXXXXXX 100644 --- a/API.json +++ b/API.json @@ -XXX,XX +XXX,XX @@ "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 XXXXXXX..XXXXXXX 100644 --- a/docs/API.md +++ b/docs/API.md @@ -XXX,XX +XXX,XX @@ Represents a snapshot of the Virtual Machine's primary monitor. * format: Format of the image. Valid formats: qcow, qcow2, qed, raw, vmdk, vpc * pool: Storage pool information * name: URI of the storagepool where disk will be created + * io: Set io flag + * cache: Set cache flag + * bus: Set bus flag * graphics *(optional)*: The graphics paramenters of this template * type: The type of graphics. It can be VNC or spice or None. * vnc: Graphical display using the Virtual Network diff --git a/model/vmstorages.py b/model/vmstorages.py index XXXXXXX..XXXXXXX 100644 --- a/model/vmstorages.py +++ b/model/vmstorages.py @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/vmtemplate.py +++ b/vmtemplate.py @@ -XXX,XX +XXX,XX @@ # # 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 @@ -XXX,XX +XXX,XX @@ 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))): @@ -XXX,XX +XXX,XX @@ 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'} @@ -XXX,XX +XXX,XX @@ 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 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel Tests run both on update and template creation Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- tests/test_rest.py | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/tests/test_rest.py b/tests/test_rest.py index XXXXXXX..XXXXXXX 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -XXX,XX +XXX,XX @@ 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) + resp = json.loads(resp.read()) + + # update template storage + req = json.dumps({'disks': [ + {'index': 0, 'size': 10, 'format': 'qcow2', + 'pool': {"name": "/plugins/kimchi/storagepools/default-pool"}, + 'io': 'threads', 'cache': 'unsafe', 'bus': 'virtio'}, + {'index': 1, 'size': 10, 'format': 'qcow2', 'io': 'threads', + 'cache': 'unsafe', 'bus': 'scsi', + 'pool': {"name": "/plugins/kimchi/storagepools/default-pool"}}]}) + resp = self.request('/plugins/kimchi/templates/' + str(resp["name"]), + req, 'PUT') + resp = json.loads(resp.read()) + self.assertEquals(resp['disks'][0]['bus'], 'virtio') + self.assertEquals(resp['disks'][0]['cache'], 'unsafe') + self.assertEquals(resp['disks'][0]['io'], 'threads') req = json.dumps({'name': 'vm-1', 'template': '/plugins/kimchi/templates/test'}) @@ -XXX,XX +XXX,XX @@ 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) @@ -XXX,XX +XXX,XX @@ 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 _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel