
When generating the disk XML, the most common information to generate the device name are: 1) the bus - to identify the right prefix, and 2) the disk index. Add this logic to get_disk_xml() so when no 'dev' name is specified the device name will be automatically generated according to bus and index value. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/storagevolumes.py | 6 ++-- src/kimchi/model/vmstorages.py | 67 ++++++++++++++++++-------------------- src/kimchi/xmlutils/disk.py | 25 +++++++++----- 3 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index db596c3..9ff43e6 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -32,7 +32,7 @@ from kimchi.model.storagepools import StoragePoolModel from kimchi.model.tasks import TaskModel from kimchi.model.vms import VMsModel, VMModel from kimchi.utils import add_task, kimchi_log -from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disk_list +from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks from kimchi.xmlutils.utils import xpath_get_text @@ -265,8 +265,8 @@ class StorageVolumeModel(object): vms = VMsModel.get_vms(self.conn) for vm in vms: dom = VMModel.get_vm(vm, self.conn) - storages = get_vm_disk_list(dom) - for disk in storages: + storages = get_vm_disks(dom) + for disk in storages.keys(): d_info = get_vm_disk_info(dom, disk) if path == d_info['path']: ref_cnt = ref_cnt + 1 diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 3a8b6b3..b3b1514 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -31,10 +31,9 @@ from kimchi.model.utils import get_vm_config_flag from kimchi.utils import check_url_path from kimchi.osinfo import lookup from kimchi.xmlutils.disk import get_device_node, get_disk_xml -from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disk_list +from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks HOTPLUG_TYPE = ['scsi', 'virtio'] -PREFIX_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'} def _get_device_bus(dev_type, dom): @@ -95,14 +94,28 @@ class VMStoragesModel(object): return dict(address=address) def create(self, vm_name, params): - dom = VMModel.get_vm(vm_name, self.conn) - params['bus'] = _get_device_bus(params['type'], dom) - self._get_storage_device_name(vm_name, params) # Path will never be blank due to API.json verification. # There is no need to cover this case here. - params['format'] = 'raw' if not ('vol' in params) ^ ('path' in params): raise InvalidParameter("KCHVMSTOR0017E") + + dom = VMModel.get_vm(vm_name, self.conn) + params['bus'] = _get_device_bus(params['type'], dom) + params['format'] = 'raw' + + dev_list = [dev for dev, bus in get_vm_disks(dom).iteritems() + if bus == params['bus']] + dev_list.sort() + if len(dev_list) == 0: + params['index'] = 0 + else: + char = dev_list.pop()[2] + params['index'] = string.ascii_lowercase.index(char) + 1 + + if (params['bus'] not in HOTPLUG_TYPE + and DOM_STATE_MAP[dom.info()[0]] != 'shutoff'): + raise InvalidOperation('KCHVMSTOR0011E') + if params.get('vol'): try: pool = params['pool'] @@ -119,48 +132,30 @@ class VMStoragesModel(object): supported_format = { "disk": ["raw", "bochs", "qcow", "qcow2", "qed", "vmdk"], "cdrom": "iso"} - if vol_info['format'] in supported_format[params['type']]: - if params['type'] == 'disk': - params['format'] = vol_info['format'] - else: + if vol_info['format'] not in supported_format[params['type']]: raise InvalidParameter("KCHVMSTOR0018E", {"format": vol_info['format'], "type": params['type']}) - params['path'] = vol_info['path'] - if (params['bus'] not in HOTPLUG_TYPE - and DOM_STATE_MAP[dom.info()[0]] != 'shutoff'): - raise InvalidOperation('KCHVMSTOR0011E') + if params['type'] == 'disk': + params['format'] = vol_info['format'] + + params['path'] = vol_info['path'] params.update(self._get_available_bus_address(params['bus'], vm_name)) # Add device to VM - dev_xml = get_disk_xml(_check_path(params['path']), params) + dev, xml = get_disk_xml(_check_path(params['path']), params) try: conn = self.conn.get() dom = conn.lookupByName(vm_name) - dom.attachDeviceFlags(dev_xml, get_vm_config_flag(dom, 'all')) + dom.attachDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0008E", {'error': e.message}) - return params['dev'] - - def _get_storage_device_name(self, vm_name, params): - bus_prefix = PREFIX_MAP[params['bus']] - dev_list = [dev for dev in self.get_list(vm_name) - if dev.startswith(bus_prefix)] - if len(dev_list) == 0: - params['dev'] = bus_prefix + 'a' - else: - dev_list.sort() - last_dev = dev_list.pop() - # TODO: Improve to device names "greater then" hdz - next_dev_letter_pos =\ - string.ascii_lowercase.index(last_dev[2]) + 1 - params['dev'] =\ - bus_prefix + string.ascii_lowercase[next_dev_letter_pos] + return dev def get_list(self, vm_name): dom = VMModel.get_vm(vm_name, self.conn) - return get_vm_disk_list(dom) + return get_vm_disks(dom).keys() class VMStorageModel(object): @@ -207,11 +202,11 @@ class VMStorageModel(object): dev_info = self.lookup(vm_name, dev_name) if dev_info['type'] != 'cdrom': raise InvalidOperation("KCHVMSTOR0006E") - dev_info.update(params) - xml = get_disk_xml(src_type, dev_info, ignore_source) + dev_info.update(params) + dev, xml = get_disk_xml(src_type, dev_info, ignore_source) try: dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message}) - return dev_name + return dev diff --git a/src/kimchi/xmlutils/disk.py b/src/kimchi/xmlutils/disk.py index 800a64c..aadbfb8 100644 --- a/src/kimchi/xmlutils/disk.py +++ b/src/kimchi/xmlutils/disk.py @@ -19,6 +19,7 @@ import lxml.etree as ET import socket +import string import urlparse from lxml import objectify @@ -26,6 +27,7 @@ from lxml.builder import E from kimchi.exception import NotFoundError +BUS_TO_DEV_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'} DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', 'block': 'dev'} @@ -42,7 +44,11 @@ def get_disk_xml(src_type, params, ignore_src=False): """ disk = E.disk(type=src_type, device=params['type']) disk.append(E.driver(name='qemu', type=params['format'])) - disk.append(E.target(dev=params['dev'], bus=params['bus'])) + + # Get device name according to bus and index values + dev = params.get('dev', (BUS_TO_DEV_MAP[params['bus']] + + string.lowercase[params.get('index', 0)])) + disk.append(E.target(dev=dev, bus=params['bus'])) if params.get('address'): # ide disk target id is always '0' @@ -52,7 +58,7 @@ def get_disk_xml(src_type, params, ignore_src=False): unit=params['address']['unit'])) if ignore_src: - return ET.tostring(disk, encoding='utf-8', pretty_print=True) + return (dev, ET.tostring(disk, encoding='utf-8', pretty_print=True)) if src_type == 'network': """ @@ -73,7 +79,7 @@ def get_disk_xml(src_type, params, ignore_src=False): source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], params['path']) disk.append(source) - return ET.tostring(disk, encoding='utf-8', pretty_print=True) + return (dev, ET.tostring(disk, encoding='utf-8', pretty_print=True)) def get_device_node(dom, dev_name): @@ -116,11 +122,14 @@ def get_vm_disk_info(dom, dev_name): 'bus': disk.target.attrib['bus']} -def get_vm_disk_list(dom): +def get_vm_disks(dom): xml = dom.XMLDesc(0) devices = objectify.fromstring(xml).devices - storages = [disk.target.attrib['dev'] - for disk in devices.xpath("./disk[@device='disk']")] - storages += [disk.target.attrib['dev'] - for disk in devices.xpath("./disk[@device='cdrom']")] + + storages = {} + all_disks = devices.xpath("./disk[@device='disk']") + all_disks.extend(devices.xpath("./disk[@device='cdrom']")) + for disk in all_disks: + storages[disk.target.attrib['dev']] = disk.target.attrib['bus'] + return storages -- 1.9.3