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

[Rodrigo Trujillo]

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).
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
>