[Kimchi-devel] [PATCH] [Kimchi 1/2] Allow disks to update cache and io flags
Aline Manera
alinefm at linux.vnet.ibm.com
Tue Mar 7 13:35:02 UTC 2017
On 02/28/2017 04:06 PM, Ramon Medeiros wrote:
> This is a part of Bug fix #1092 solution
>
> Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com>
> ---
> API.json | 11 ++++++++++-
> docs/API.md | 3 +++
> i18n.py | 1 -
> model/vmstorages.py | 57 +++++++++++++++++++++++++++++++++--------------------
> xmlutils/disk.py | 12 ++++++++++-
> 5 files changed, 60 insertions(+), 24 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..0414aed 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.
> + * threads: Threads settings for disks. Values accepted: none, writethrough,
> + writeback, directsync, unsafe and default.
Shouldn't it be 'cache' instead of 'threads'?
At least, the API.md should reflect the settings on API.json file.
> * **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..143225c 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)
> - if dev_info['type'] != 'cdrom':
> - raise InvalidOperation("KCHVMSTOR0006E")
> -
> - 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)
> -
> - dev_info.update(params)
> - dev, xml = get_disk_xml(dev_info)
>
> - try:
> - dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all'))
> - except Exception as e:
> - raise OperationFailed("KCHVMSTOR0009E", {'error': e.message})
> + # only change path to cdrom devices
> + if dev_info['type'] == 'cdrom':
> +
> + 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})
The above block is exact the same code of the 'else' statement we have
today in the code.
So I'd say to keep the logic as it is today (if type != cdrom) and just
add an 'else' to the code above. It will simplify the changes and make
easier to see the changes added by this patch.
> + else:
> + old_dev, old_xml = get_disk_xml(dev_info)
> + dev_info.update(params)
> + dev, xml = get_disk_xml(dev_info)
> +
> + # remove
> + self.delete(vm_name, dev_name)
> +
> + try:
> + dom.attachDeviceFlags(xml)
> + except Exception as e:
> + dom.attachDeviceFlags(old_xml)
> + 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
More information about the Kimchi-devel
mailing list