[Kimchi-devel] [PATCH 2/4] Allow disks to update cache and io flags

Ramon Medeiros posted 4 patches 7 years, 7 months ago
[Kimchi-devel] [PATCH 2/4] Allow disks to update cache and io flags
Posted by Ramon Medeiros 7 years, 7 months ago
This is a part of Bug fix #1092 solution

Signed-off-by: Ramon Medeiros <ramonn@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

_______________________________________________
Kimchi-devel mailing list
Kimchi-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Re: [Kimchi-devel] [PATCH 2/4] Allow disks to update cache and io flags
Posted by Aline Manera 7 years, 7 months ago
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@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

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