[Kimchi-devel] [PATCH 4/7] Guest disks: Choose proper bus for device

[Aline Manera]

On 04/13/2014 05:16 AM, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> When bus type is not passed by user,
> kimchi choose bus type based on stored os distro and version.
> And make right hot plug behavior based on bus type.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   src/kimchi/model/vmstorages.py | 43 ++++++++++++++++++++++++++++++------------
>   1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py
> index c31af24..6fdf580 100644
> --- a/src/kimchi/model/vmstorages.py
> +++ b/src/kimchi/model/vmstorages.py
> @@ -32,9 +32,11 @@ from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError
>   from kimchi.exception import OperationFailed
>   from kimchi.model.vms import DOM_STATE_MAP, VMModel
>   from kimchi.utils import check_url_path
> +from kimchi.osinfo import lookup
>
>   DEV_TYPE_SRC_ATTR_MAP = {'file': 'file',
>                            'block': 'dev'}
> +HOTPLUG_TYPE = ['scsi', 'virtio']
>
>
>   def _get_device_xml(dom, dev_name):
> @@ -47,6 +49,16 @@ def _get_device_xml(dom, dev_name):
>       return disk[0]
>
>
> +def _get_device_bus(dev_type, dom, objstore):
> +    try:
> +        with objstore as session:
> +            vm_info = session.get('vm', dom.UUIDString())
> +        distro, version = (vm_info['distro'], vm_info['version'])
> +    except:
> +        distro, version = ('unknown', 'unknown')
> +    return lookup(distro, version)[dev_type+'_bus']
> +
> +

The same I commented in previous patch about store bus type in VM metadata

>   def _get_storage_xml(params):
>       src_type = params.get('src_type')
>       disk = E.disk(type=src_type, device=params.get('type'))
> @@ -65,7 +77,7 @@ 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=params.get('bus', 'ide')))
> +    disk.append(E.target(dev=params.get('dev'), bus=params['bus']))
>       if params.get('address'):
>           # ide disk target id is always '0'
>           disk.append(E.address(
> @@ -104,8 +116,11 @@ def _check_cdrom_path(path):
>   class VMStoragesModel(object):
>       def __init__(self, **kargs):
>           self.conn = kargs['conn']
> +        self.objstore = kargs['objstore']
>
> -    def _get_available_ide_address(self, vm_name):
> +    def _get_available_bus_address(self, bus_type, vm_name):
> +        if bus_type not in ['ide']:
> +            return dict()
>           # libvirt limitation of just 1 ide controller
>           # each controller have at most 2 buses and each bus 2 units.
>           dom = VMModel.get_vm(vm_name, self.conn)
> @@ -130,9 +145,6 @@ class VMStoragesModel(object):
>
>       def create(self, vm_name, params):
>           dom = VMModel.get_vm(vm_name, self.conn)
> -        if DOM_STATE_MAP[dom.info()[0]] != 'shutoff':
> -            raise InvalidOperation('KCHVMSTOR0011E')
> -
>           # Use device name passed or pick next
>           dev_name = params.get('dev', None)
>           if dev_name is None:
> @@ -148,7 +160,13 @@ class VMStoragesModel(object):
>           path = params['path']
>           params['src_type'] = _check_cdrom_path(path)
>
> -        params.update(self._get_available_ide_address(vm_name))
> +        params.setdefault(
> +            'bus', _get_device_bus(params['type'], dom, self.objstore))
> +        if (params['bus'] not in HOTPLUG_TYPE
> +                and DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
> +            raise InvalidOperation('KCHVMSTOR0011E')
> +
> +        params.update(self._get_available_bus_address(params['bus'], vm_name))
>           # Add device to VM
>           dev_xml = _get_storage_xml(params)
>           try:
> @@ -218,19 +236,20 @@ class VMStorageModel(object):
>       def delete(self, vm_name, dev_name):
>           # Get storage device xml
>           dom = VMModel.get_vm(vm_name, self.conn)
> -        disk = _get_device_xml(dom, dev_name)
> -        if disk is None:
> -            raise NotFoundError("KCHVMSTOR0007E",
> -                                {'dev_name': dev_name,
> -                                 'vm_name': vm_name})
> +        try:
> +            bus_type = self.lookup(vm_name, dev_name)['bus']
> +        except NotFoundError:
> +            raise

Please, raise the proper error or fallback to a default bus_type

>
>           dom = VMModel.get_vm(vm_name, self.conn)
> -        if DOM_STATE_MAP[dom.info()[0]] != 'shutoff':
> +        if (bus_type not in HOTPLUG_TYPE and
> +                DOM_STATE_MAP[dom.info()[0]] != 'shutoff'):
>               raise InvalidOperation('KCHVMSTOR0011E')
>
>           try:
>               conn = self.conn.get()
>               dom = conn.lookupByName(vm_name)
> +            disk = _get_device_xml(dom, dev_name)
>               dom.detachDeviceFlags(etree.tostring(disk),
>                                     libvirt.VIR_DOMAIN_AFFECT_CURRENT)
>           except Exception as e: