[Kimchi-devel] [PATCH 3/4] Bug fix #1091: Allow to set disk performance options per guest and template

Ramon Medeiros posted 4 patches 7 years, 7 months ago
[Kimchi-devel] [PATCH 3/4] Bug fix #1091: Allow to set disk performance options per guest and template
Posted by Ramon Medeiros 7 years, 7 months ago
Allow vmstorage creation passing disk bus

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

_______________________________________________
Kimchi-devel mailing list
Kimchi-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Re: [Kimchi-devel] [PATCH 3/4] Bug fix #1091: Allow to set disk performance options per guest and template
Posted by Aline Manera 7 years, 7 months ago
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@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'], {}))

_______________________________________________
Kimchi-devel mailing list
Kimchi-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Re: [Kimchi-devel] [PATCH 3/4] Bug fix #1091: Allow to set disk performance options per guest and template
Posted by Ramon Medeiros 7 years, 6 months ago

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@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'], {}))
>

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