[Kimchi-devel] [PATCH] vmstorages changes for Power: allow SCSI devs, block IDE devs

[Daniel H Barboza]

Please ignore this patch and have a look at

[PATCH] guest-storage-add: removing "Storage Name" field


Thanks!

On 09/17/2014 04:26 AM, Royce Lv wrote:
> On 2014年09月17日 09:44, Daniel H Barboza wrote:
>>
>> On 09/16/2014 09:22 PM, Aline Manera wrote:
>>>
>>> On 09/16/2014 09:20 PM, Aline Manera wrote:
>>>>
>>>> And a bigger concern, why are we asking for the device name? It 
>>>> should not be prompted to user.
>>>> If you are option, I suggest removing this option.
>>>
>>> If you are open... =)
>>
>> I thought about the possibilty of removing the device name entirely. 
>> It would simplify a lot of things.
>>
>> If no one objects I can remove it from the UI
> Good for me.
>>>
>>>>
>>>> On 09/16/2014 09:16 PM, Aline Manera wrote:
>>>>>
>>>>> On 09/16/2014 12:02 PM, Daniel Henrique Barboza wrote:
>>>>>> Kimchi does not allow scsi devices to be attached in a VM (devices
>>>>>> with names sdb,sdc ...). A simple change in API.json regex of
>>>>>> vmstorages_create fixes it.
>>>>>>
>>>>>> At the same time, Power systems does not support IDE devices and
>>>>>> adding devices with 'hd[b-z]' name have unpredictable results.
>>>>>> This patch also adds a backend verification to avoid this scenario.
>>>>>>
>>>>>> Signed-off-by: Daniel Henrique Barboza <danielhb at linux.vnet.ibm.com>
>>>>>> ---
>>>>>> src/kimchi/API.json | 2 +-
>>>>>> src/kimchi/i18n.py | 1 +
>>>>>> src/kimchi/model/vmstorages.py | 7 +++++++
>>>>>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
>>>>>> index 1319531..4a3cd03 100644
>>>>>> --- a/src/kimchi/API.json
>>>>>> +++ b/src/kimchi/API.json
>>>>>> @@ -489,7 +489,7 @@
>>>>>> "dev": {
>>>>>> "description": "The storage device name",
>>>>>> "type": "string",
>>>>>> - "pattern": "^h|vd[b-z]$",
>>>>>> + "pattern": "^h|s|vd[b-z]$",
>>>>>> "error": "KCHVMSTOR0001E"
>>>>>> },
>>>>>> "type": {
>>>>>> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
>>>>>> index 9e66c68..0839007 100644
>>>>>> --- a/src/kimchi/i18n.py
>>>>>> +++ b/src/kimchi/i18n.py
>>>>>> @@ -259,6 +259,7 @@ messages = {
>>>>>> "KCHVMSTOR0015E": _("Cannot retrieve disk path information for 
>>>>>> given pool/volume: %(error)s"),
>>>>>> "KCHVMSTOR0016E": _("Volume already in use by other virtual 
>>>>>> machine."),
>>>>>> "KCHVMSTOR0017E": _("Only one of path or pool/volume can be 
>>>>>> specified to add a new virtual machine disk"),
>>>>>> + "KCHVMSTOR0018E": _("Device name %(dev_name)s is not allowed 
>>>>>> because Power systems does not support IDE devices"),
>>>>>>
>>>>>> "KCHREPOS0001E": _("YUM Repository ID must be one word only 
>>>>>> string."),
>>>>>> "KCHREPOS0002E": _("Repository URL must be an http://, ftp:// or 
>>>>>> file:// URL."),
>>>>>> diff --git a/src/kimchi/model/vmstorages.py 
>>>>>> b/src/kimchi/model/vmstorages.py
>>>>>> index 40856d3..c302709 100644
>>>>>> --- a/src/kimchi/model/vmstorages.py
>>>>>> +++ b/src/kimchi/model/vmstorages.py
>>>>>> @@ -18,6 +18,8 @@
>>>>>> # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
>>>>>> 02110-1301 USA
>>>>>>
>>>>>> import os
>>>>>> +import platform
>>>>>> +import re
>>>>>> import socket
>>>>>> import stat
>>>>>> import string
>>>>>> @@ -188,6 +190,11 @@ class VMStoragesModel(object):
>>>>>> string.ascii_lowercase.index(last_dev[2]) + 1
>>>>>> params['dev'] =\
>>>>>> bus_prefix + string.ascii_lowercase[next_dev_letter_pos]
>>>>>
>>>>>
>>>>>> + # Power systems does not support IDE devices
>>>>>> + elif platform.machine().startswith('ppc') and \
>>>>>> + re.match('^hd[b-z]', params.get('dev')):
>>>>>> + raise OperationFailed('KCHVMSTOR0018E',
>>>>>> + {'dev_name': params['dev']})
>>>>>
>>>>> From previous Royce's comments, I understood she asked you to add 
>>>>> a verification based on bus type, independent of platforms.
>>>>>
>>>>> if bus == ide:
>>>>> # name must start with hdX
>>>>> if bus == scsi:
>>>>> # name must start with sdX
>>>>> if bus == virtio:
>>>>> # name must start with vdX
>>>>>
>>>>>> devices = self.get_list(vm_name)
>>>>>> if params['dev'] in devices:
>>>>>
>>>>> _______________________________________________
>>>>> Kimchi-devel mailing list
>>>>> Kimchi-devel at ovirt.org
>>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> Kimchi-devel mailing list
>>>> Kimchi-devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>
>>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>