[Kimchi-devel] [PATCH] Fix hardcoded storage bus assignment in vmstorage

[Aline Manera]

On 04/03/2014 04:19 PM, Rodrigo Trujillo wrote:
> On 04/03/2014 04:04 PM, Aline Manera wrote:
>> On 04/03/2014 03:36 PM, Rodrigo Trujillo wrote:
>>> There is an error when a user tries to change the midia path in "Manage
>>> Midia" at Guest tab. If the midia was created with SCSI bus, Kimchi is
>>> going to try replace it by IDE bus, causing the following error:
>>>
>>> No device with bus 'ide' and target 'sdc'
>>>
>>> Bus IDE was hardcoded. This patch retrieves the original midia bus and
>>> pass the value as parameter in order to create the right XML.
>>>
>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>> ---
>>>   src/kimchi/model/vmstorages.py | 12 ++++++++++--
>>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/kimchi/model/vmstorages.py 
>>> b/src/kimchi/model/vmstorages.py
>>> index c1c90ce..ee83155 100644
>>> --- a/src/kimchi/model/vmstorages.py
>>> +++ b/src/kimchi/model/vmstorages.py
>>> @@ -55,7 +55,11 @@ def _get_storage_xml(params):
>>>           source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], 
>>> params.get('path'))
>>>           disk.append(source)
>>>
>>> -    disk.append(E.target(dev=params.get('dev'), bus='ide'))
>>> +    dev_bus = params.get('bus')
>>> +    if dev_bus is None:
>>> +        dev_bus = 'ide'
>>> +
>>
>> You can set "ide" as default value to get()
>>
>> dev_bus = params.get('bus', 'ide')
>
> Yes, sure! It's possible. But I did not implement this way because bus 
> might be 'None' if some error happens
> in the try/except (right below, lines 164 - 180). In this case, the 
> None will be passed to dev_bus and then  to
>  XML (bus=None).

You should never put bus=None in the XML. Otherwise, the vm will not 
start up.
The default value should be ide in error case

> The code avoid these two problems: when 'bus' does not exist in params 
> and when params['bus'] = None
>
> Does make sense ? Or did you mean something like?:
>
> dev_bus = params.get('bus', 'ide')
>     if dev_bus is None:
>         dev_bus = 'ide'
>
>>
>>> + disk.append(E.target(dev=params.get('dev'), bus=dev_bus))
>>>       return ET.tostring(disk)
>>>
>>>
>>> @@ -164,6 +168,7 @@ class VMStorageModel(object):
>>>               raise NotFoundError("KCHCDROM0007E", {'dev_name': 
>>> dev_name,
>>>                                                     'vm_name': 
>>> vm_name})
>>>           path = ""
>>> +        dev_bus = None
>>>           try:
>>>               source = disk.source
>>>               if source is not None:
>>> @@ -175,12 +180,15 @@ class VMStorageModel(object):
>>>                               host.attrib['port'] + 
>>> source.attrib['name'])
>>>                   else:
>>>                       path = 
>>> source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]]
>>> +            # Retrieve storage bus type
>>> +            dev_bus = disk.target.attrib['bus']
>>>           except:
>>>               pass
>>>           dev_type = disk.attrib['device']
>>>           return {'dev': dev_name,
>>>                   'type': dev_type,
>>> -                'path': path}
>>> +                'path': path,
>>> +                'bus': dev_bus}
>>>
>>>       def delete(self, vm_name, dev_name):
>>>           # Get storage device xml
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>