[PATCH 0/8] Create a common function to generate guest disk XML

This patch set moves common functions related to guest disk XML to xmlutils/disk.py. That way model/vmstorages.py and vmtemplate.py can make use of it and we make sure the logic is the same in both cases. It is the first step and only changes the way vmtemplate.py generated the guest CDROM XML. But in the next patches all the guest disk generation on vmtemplate.py will do the same (I prefered to do in parts to avoid huge patch sets) Aline Manera (8): Move _get_storage_xml() to xmlutils/disk.py Move vmdisks.py functions to xmlutils/disk.py Remove 'bus' paramater from /vms/<name>/storages documentation Update get_disk_xml() to get the device same according to bus and index values Remove ignore_src parameter from get_disk_xml() Get disk type according to file path on get_disk_xml() Check QEMU stream DNS capability while generating guest disk XML Update vmtemplate.py to use get_disk_xml() while generating CDROM XML docs/API.md | 1 - src/kimchi/model/storagevolumes.py | 8 +- src/kimchi/model/vms.py | 1 - src/kimchi/model/vmstorages.py | 140 ++++++++----------------------- src/kimchi/vmdisks.py | 75 ----------------- src/kimchi/vmtemplate.py | 78 +++++------------- src/kimchi/xmlutils/disk.py | 163 +++++++++++++++++++++++++++++++++++++ tests/test_model.py | 11 ++- 8 files changed, 233 insertions(+), 244 deletions(-) delete mode 100644 src/kimchi/vmdisks.py create mode 100644 src/kimchi/xmlutils/disk.py -- 1.9.3

It is the first step to have xmlutils/disk.py holding all the disk XML manipulation. So we can reuse the same function on vmtemplate.py to build the guest XML. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 49 ++++------------------------ src/kimchi/xmlutils/disk.py | 73 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 43 deletions(-) create mode 100644 src/kimchi/xmlutils/disk.py diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 055aa50..808b3d7 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -18,14 +18,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import os -import socket import stat import string -import urlparse -import lxml.etree as ET from lxml import etree -from lxml.builder import E from kimchi.exception import InvalidOperation, InvalidParameter, NotFoundError from kimchi.exception import OperationFailed @@ -35,7 +31,7 @@ from kimchi.model.utils import get_vm_config_flag from kimchi.utils import check_url_path from kimchi.osinfo import lookup from kimchi.vmdisks import get_device_xml, get_vm_disk, get_vm_disk_list -from kimchi.vmdisks import DEV_TYPE_SRC_ATTR_MAP +from kimchi.xmlutils.disk import get_disk_xml HOTPLUG_TYPE = ['scsi', 'virtio'] PREFIX_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'} @@ -49,39 +45,6 @@ def _get_device_bus(dev_type, dom): return lookup(distro, version)[dev_type+'_bus'] -def _get_storage_xml(params, ignore_source=False): - src_type = params.get('src_type') - disk = E.disk(type=src_type, device=params.get('type')) - disk.append(E.driver(name='qemu', type=params['format'])) - - 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( - type='drive', controller=params['address']['controller'], - bus=params['address']['bus'], target='0', - unit=params['address']['unit'])) - - if ignore_source: - return ET.tostring(disk) - - # Working with url paths - if src_type == 'network': - output = urlparse.urlparse(params.get('path')) - port = str(output.port or socket.getservbyname(output.scheme)) - host = E.host(name=output.hostname, port=port) - source = E.source(protocol=output.scheme, name=output.path) - source.append(host) - disk.append(source) - else: - # Fixing source attribute - source = E.source() - source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], params.get('path')) - disk.append(source) - - return ET.tostring(disk) - - def _check_path(path): if check_url_path(path): src_type = 'network' @@ -164,14 +127,14 @@ class VMStoragesModel(object): {"format": vol_info['format'], "type": params['type']}) params['path'] = vol_info['path'] - params['src_type'] = _check_path(params['path']) + 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) + dev_xml = get_disk_xml(_check_path(params['path']), params) try: conn = self.conn.get() dom = conn.lookupByName(vm_name) @@ -234,10 +197,10 @@ class VMStorageModel(object): def update(self, vm_name, dev_name, params): path = params.get('path') if path and len(path) != 0: - params['src_type'] = _check_path(path) + src_type = _check_path(path) ignore_source = False else: - params['src_type'] = 'file' + src_type = 'file' ignore_source = True dom = VMModel.get_vm(vm_name, self.conn) @@ -245,8 +208,8 @@ class VMStorageModel(object): if dev_info['type'] != 'cdrom': raise InvalidOperation("KCHVMSTOR0006E") dev_info.update(params) - xml = _get_storage_xml(dev_info, ignore_source) + xml = get_disk_xml(src_type, dev_info, ignore_source) try: dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: diff --git a/src/kimchi/xmlutils/disk.py b/src/kimchi/xmlutils/disk.py new file mode 100644 index 0000000..8c64ff4 --- /dev/null +++ b/src/kimchi/xmlutils/disk.py @@ -0,0 +1,73 @@ +# +# Project Kimchi +# +# Copyright IBM, Corp. 2014 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +import lxml.etree as ET +import socket +import urlparse + +from lxml.builder import E + +DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', 'block': 'dev'} + + +def get_disk_xml(src_type, params, ignore_src=False): + """ + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + + [source XML according to src_type] + + <target dev='%(dev)s' bus='%(bus)s'/> + <readonly/> + </disk> + """ + 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'])) + + if params.get('address'): + # ide disk target id is always '0' + disk.append(E.address( + type='drive', controller=params['address']['controller'], + bus=params['address']['bus'], target='0', + unit=params['address']['unit'])) + + if ignore_src: + return ET.tostring(disk, encoding='utf-8', pretty_print=True) + + if src_type == 'network': + """ + <source protocol='%(protocol)s' name='%(url_path)s'> + <host name='%(hostname)s' port='%(port)s'/> + </source> + """ + output = urlparse.urlparse(params['path']) + port = str(output.port or socket.getservbyname(output.scheme)) + + source = E.source(protocol=output.scheme, name=output.path) + source.append(E.host(name=output.hostname, port=port)) + else: + """ + <source file='%(src)s' /> + """ + source = E.source() + source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], params['path']) + + disk.append(source) + return ET.tostring(disk, encoding='utf-8', pretty_print=True) -- 1.9.3

vmdisks.py also had some functions to manipulate the guest disk XML. So move them to xmlutils/disk.py Also update the references to it on Kimchi code. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/storagevolumes.py | 4 +- src/kimchi/model/vmstorages.py | 10 ++--- src/kimchi/vmdisks.py | 75 -------------------------------------- src/kimchi/xmlutils/disk.py | 53 +++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 82 deletions(-) delete mode 100644 src/kimchi/vmdisks.py diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 1ee8d0a..db596c3 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.vmdisks import get_vm_disk, get_vm_disk_list +from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disk_list from kimchi.xmlutils.utils import xpath_get_text @@ -267,7 +267,7 @@ class StorageVolumeModel(object): dom = VMModel.get_vm(vm, self.conn) storages = get_vm_disk_list(dom) for disk in storages: - d_info = get_vm_disk(dom, disk) + d_info = get_vm_disk_info(dom, disk) if path == d_info['path']: ref_cnt = ref_cnt + 1 session.store('storagevolume', vol_id, diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 808b3d7..3a8b6b3 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -30,8 +30,8 @@ from kimchi.model.storagevolumes import StorageVolumeModel from kimchi.model.utils import get_vm_config_flag from kimchi.utils import check_url_path from kimchi.osinfo import lookup -from kimchi.vmdisks import get_device_xml, get_vm_disk, get_vm_disk_list -from kimchi.xmlutils.disk import get_disk_xml +from kimchi.xmlutils.disk import get_device_node, get_disk_xml +from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disk_list HOTPLUG_TYPE = ['scsi', 'virtio'] PREFIX_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'} @@ -78,7 +78,7 @@ class VMStoragesModel(object): valid_id = [('0', '0'), ('0', '1'), ('1', '0'), ('1', '1')] controller_id = '0' for dev_name in disks: - disk = get_device_xml(dom, dev_name) + disk = get_device_node(dom, dev_name) if disk.target.attrib['bus'] == 'ide': controller_id = disk.address.attrib['controller'] bus_id = disk.address.attrib['bus'] @@ -170,7 +170,7 @@ class VMStorageModel(object): def lookup(self, vm_name, dev_name): # Retrieve disk xml and format return dict dom = VMModel.get_vm(vm_name, self.conn) - return get_vm_disk(dom, dev_name) + return get_vm_disk_info(dom, dev_name) def delete(self, vm_name, dev_name): # Get storage device xml @@ -188,7 +188,7 @@ class VMStorageModel(object): try: conn = self.conn.get() dom = conn.lookupByName(vm_name) - disk = get_device_xml(dom, dev_name) + disk = get_device_node(dom, dev_name) dom.detachDeviceFlags(etree.tostring(disk), get_vm_config_flag(dom, 'all')) except Exception as e: diff --git a/src/kimchi/vmdisks.py b/src/kimchi/vmdisks.py deleted file mode 100644 index f1c3f02..0000000 --- a/src/kimchi/vmdisks.py +++ /dev/null @@ -1,75 +0,0 @@ -# -# Project Kimchi -# -# Copyright IBM, Corp. 2014 -# -# This library is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License as published by the Free Software Foundation; either -# version 2.1 of the License, or (at your option) any later version. -# -# This library is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public -# License along with this library; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - -from lxml import objectify - -from kimchi.exception import NotFoundError - -DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', - 'block': 'dev'} - - -def get_device_xml(dom, dev_name): - # Get VM xml and then devices xml - xml = dom.XMLDesc(0) - devices = objectify.fromstring(xml).devices - disk = devices.xpath("./disk/target[@dev='%s']/.." % dev_name) - if not disk: - return None - return disk[0] - - -def get_vm_disk(dom, dev_name): - # Retrieve disk xml and format return dict - disk = get_device_xml(dom, dev_name) - if disk is None: - raise NotFoundError("KCHVMSTOR0007E", - {'dev_name': dev_name, - 'vm_name': dom.name()}) - path = "" - dev_bus = disk.target.attrib['bus'] - try: - source = disk.source - if source is not None: - src_type = disk.attrib['type'] - if src_type == 'network': - host = source.host - path = (source.attrib['protocol'] + '://' + - host.attrib['name'] + ':' + - host.attrib['port'] + source.attrib['name']) - else: - path = source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]] - except: - pass - dev_type = disk.attrib['device'] - return {'dev': dev_name, - 'type': dev_type, - 'path': path, - 'format': disk.driver.attrib['type'], - 'bus': dev_bus} - - -def get_vm_disk_list(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']")] - return storages diff --git a/src/kimchi/xmlutils/disk.py b/src/kimchi/xmlutils/disk.py index 8c64ff4..800a64c 100644 --- a/src/kimchi/xmlutils/disk.py +++ b/src/kimchi/xmlutils/disk.py @@ -21,8 +21,11 @@ import lxml.etree as ET import socket import urlparse +from lxml import objectify from lxml.builder import E +from kimchi.exception import NotFoundError + DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', 'block': 'dev'} @@ -71,3 +74,53 @@ def get_disk_xml(src_type, params, ignore_src=False): disk.append(source) return ET.tostring(disk, encoding='utf-8', pretty_print=True) + + +def get_device_node(dom, dev_name): + xml = dom.XMLDesc(0) + devices = objectify.fromstring(xml).devices + disk = devices.xpath("./disk/target[@dev='%s']/.." % dev_name) + + if not disk: + raise NotFoundError("KCHVMSTOR0007E", + {'dev_name': dev_name, + 'vm_name': dom.name()}) + + return disk[0] + + +def get_vm_disk_info(dom, dev_name): + # Retrieve disk xml and format return dict + disk = get_device_node(dom, dev_name) + if disk is None: + return None + + try: + source = disk.source + if source is not None: + src_type = disk.attrib['type'] + if src_type == 'network': + host = source.host + path = (source.attrib['protocol'] + '://' + + host.attrib['name'] + ':' + + host.attrib['port'] + source.attrib['name']) + else: + path = source.attrib[DEV_TYPE_SRC_ATTR_MAP[src_type]] + except: + path = "" + + return {'dev': dev_name, + 'path': path, + 'type': disk.attrib['device'], + 'format': disk.driver.attrib['type'], + 'bus': disk.target.attrib['bus']} + + +def get_vm_disk_list(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']")] + return storages -- 1.9.3

While attaching a new disk to a guest, Kimchi will automatically identify the best bus for it. So update the documentation to reflect it. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- docs/API.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/API.md b/docs/API.md index 92fbbd5..37c979c 100644 --- a/docs/API.md +++ b/docs/API.md @@ -152,7 +152,6 @@ Represents a snapshot of the Virtual Machine's primary monitor. * path: Path of cdrom iso. * pool: Storage pool which disk image file locate in. * vol: Storage volume name of disk image. - * bus: Target bus type for disk to attach. ### Sub-resource: storage **URI:** /vms/*:name*/storages/*:dev* -- 1.9.3

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

The ignore_src parameter can be automatically identified by the disk path value, ie, when it is an empty string the source can be ignored. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 7 +++---- src/kimchi/xmlutils/disk.py | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index b3b1514..4c4682c 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -190,13 +190,12 @@ class VMStorageModel(object): raise OperationFailed("KCHVMSTOR0010E", {'error': e.message}) def update(self, vm_name, dev_name, params): - path = params.get('path') - if path and len(path) != 0: + path = params.get('path', '') + params['path'] = path + if len(path) != 0: src_type = _check_path(path) - ignore_source = False else: src_type = 'file' - ignore_source = True dom = VMModel.get_vm(vm_name, self.conn) dev_info = self.lookup(vm_name, dev_name) diff --git a/src/kimchi/xmlutils/disk.py b/src/kimchi/xmlutils/disk.py index aadbfb8..f40f34f 100644 --- a/src/kimchi/xmlutils/disk.py +++ b/src/kimchi/xmlutils/disk.py @@ -31,7 +31,7 @@ BUS_TO_DEV_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'} DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', 'block': 'dev'} -def get_disk_xml(src_type, params, ignore_src=False): +def get_disk_xml(src_type, params): """ <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> @@ -57,7 +57,7 @@ def get_disk_xml(src_type, params, ignore_src=False): bus=params['address']['bus'], target='0', unit=params['address']['unit'])) - if ignore_src: + if len(params['path']) == 0: return (dev, ET.tostring(disk, encoding='utf-8', pretty_print=True)) if src_type == 'network': -- 1.9.3

The disk type can be also be identified by the file path. So move it to the common place to handle guest disk XML manipulation. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 33 ++++----------------------------- src/kimchi/xmlutils/disk.py | 33 ++++++++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 4c4682c..70240b0 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -17,8 +17,6 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -import os -import stat import string from lxml import etree @@ -28,7 +26,6 @@ from kimchi.exception import OperationFailed from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.model.storagevolumes import StorageVolumeModel 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_disks @@ -44,24 +41,6 @@ def _get_device_bus(dev_type, dom): return lookup(distro, version)[dev_type+'_bus'] -def _check_path(path): - if check_url_path(path): - src_type = 'network' - # Check if path is a valid local path - elif os.path.exists(path): - if os.path.isfile(path): - src_type = 'file' - else: - r_path = os.path.realpath(path) - if not stat.S_ISBLK(os.stat(r_path).st_mode): - raise InvalidParameter("KCHVMSTOR0003E", {'value': path}) - - src_type = 'block' - else: - raise InvalidParameter("KCHVMSTOR0003E", {'value': path}) - return src_type - - class VMStoragesModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] @@ -144,7 +123,7 @@ class VMStoragesModel(object): 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(params) try: conn = self.conn.get() dom = conn.lookupByName(vm_name) @@ -190,20 +169,16 @@ class VMStorageModel(object): raise OperationFailed("KCHVMSTOR0010E", {'error': e.message}) def update(self, vm_name, dev_name, params): - path = params.get('path', '') - params['path'] = path - if len(path) != 0: - src_type = _check_path(path) - else: - src_type = 'file' dom = VMModel.get_vm(vm_name, self.conn) dev_info = self.lookup(vm_name, dev_name) if dev_info['type'] != 'cdrom': raise InvalidOperation("KCHVMSTOR0006E") + params['path'] = params.get('path', '') dev_info.update(params) - dev, xml = get_disk_xml(src_type, dev_info, ignore_source) + + dev, xml = get_disk_xml(dev_info) try: dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: diff --git a/src/kimchi/xmlutils/disk.py b/src/kimchi/xmlutils/disk.py index f40f34f..f066dbe 100644 --- a/src/kimchi/xmlutils/disk.py +++ b/src/kimchi/xmlutils/disk.py @@ -18,20 +18,23 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import lxml.etree as ET +import os import socket +import stat import string import urlparse from lxml import objectify from lxml.builder import E -from kimchi.exception import NotFoundError +from kimchi.exception import InvalidParameter, NotFoundError +from kimchi.utils import check_url_path BUS_TO_DEV_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'} DEV_TYPE_SRC_ATTR_MAP = {'file': 'file', 'block': 'dev'} -def get_disk_xml(src_type, params): +def get_disk_xml(params): """ <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> @@ -42,7 +45,9 @@ def get_disk_xml(src_type, params): <readonly/> </disk> """ - disk = E.disk(type=src_type, device=params['type']) + path = params['path'] + disk_type = _get_disk_type(path) if len(path) > 0 else 'file' + disk = E.disk(type=disk_type, device=params['type']) disk.append(E.driver(name='qemu', type=params['format'])) # Get device name according to bus and index values @@ -60,7 +65,7 @@ def get_disk_xml(src_type, params): if len(params['path']) == 0: return (dev, ET.tostring(disk, encoding='utf-8', pretty_print=True)) - if src_type == 'network': + if disk_type == 'network': """ <source protocol='%(protocol)s' name='%(url_path)s'> <host name='%(hostname)s' port='%(port)s'/> @@ -76,12 +81,30 @@ def get_disk_xml(src_type, params): <source file='%(src)s' /> """ source = E.source() - source.set(DEV_TYPE_SRC_ATTR_MAP[src_type], params['path']) + source.set(DEV_TYPE_SRC_ATTR_MAP[disk_type], params['path']) disk.append(source) return (dev, ET.tostring(disk, encoding='utf-8', pretty_print=True)) +def _get_disk_type(path): + if check_url_path(path): + return 'network' + + if not os.path.exists(path): + raise InvalidParameter("KCHVMSTOR0003E", {'value': path}) + + # Check if path is a valid local path + if os.path.isfile(path): + return 'file' + + r_path = os.path.realpath(path) + if stat.S_ISBLK(os.stat(r_path).st_mode): + return 'block' + + raise InvalidParameter("KCHVMSTOR0003E", {'value': path}) + + def get_device_node(dom, dev_name): xml = dom.XMLDesc(0) devices = objectify.fromstring(xml).devices -- 1.9.3

When QEMU is not able to solve hostname for a remote CDROM path, we need to convert the hostname to IP to avoid problems. Add this logic to get_disk_xml() too. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/xmlutils/disk.py | 7 ++++++- tests/test_model.py | 11 ++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/kimchi/xmlutils/disk.py b/src/kimchi/xmlutils/disk.py index f066dbe..adc45da 100644 --- a/src/kimchi/xmlutils/disk.py +++ b/src/kimchi/xmlutils/disk.py @@ -28,6 +28,7 @@ from lxml import objectify from lxml.builder import E from kimchi.exception import InvalidParameter, NotFoundError +from kimchi.model.config import CapabilitiesModel from kimchi.utils import check_url_path BUS_TO_DEV_MAP = {'ide': 'hd', 'virtio': 'vd', 'scsi': 'sd'} @@ -74,8 +75,12 @@ def get_disk_xml(params): output = urlparse.urlparse(params['path']) port = str(output.port or socket.getservbyname(output.scheme)) + hostname = output.hostname + if not CapabilitiesModel().qemu_stream_dns: + hostname = socket.gethostbyname(hostname) + source = E.source(protocol=output.scheme, name=output.path) - source.append(E.host(name=output.hostname, port=port)) + source.append(E.host(name=hostname, port=port)) else: """ <source file='%(src)s' /> diff --git a/tests/test_model.py b/tests/test_model.py index e407fe5..f094ed7 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -25,9 +25,11 @@ import psutil import pwd import re import shutil +import socket import tempfile import threading import time +import urlparse import unittest import uuid @@ -433,7 +435,14 @@ class ModelTests(unittest.TestCase): {'path': valid_remote_iso_path}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) cur_cdrom_path = re.sub(":80/", '/', cdrom_info['path']) - self.assertEquals(valid_remote_iso_path, cur_cdrom_path) + + # As Kimchi server is not running during this test case + # CapabilitiesModel.qemu_stream_dns will be always False + # so we need to convert the hostname to IP + output = urlparse.urlparse(valid_remote_iso_path) + hostname = socket.gethostbyname(output.hostname) + url = valid_remote_iso_path.replace(output.hostname, hostname) + self.assertEquals(url, cur_cdrom_path) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_storage_provisioning(self): -- 1.9.3

It is the first step to have vmtemplate.py using the same functions used by model/vmstorages.py The disk generation will also use get_disk_xml() but it will be done in a separated patch. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 1 - src/kimchi/vmtemplate.py | 78 +++++++++++++----------------------------------- 2 files changed, 20 insertions(+), 59 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 1089464..51d2348 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -209,7 +209,6 @@ class VMsModel(object): stream_protocols = self.caps.libvirt_stream_protocols xml = t.to_vm_xml(name, vm_uuid, libvirt_stream_protocols=stream_protocols, - qemu_stream_dns=self.caps.qemu_stream_dns, graphics=graphics, volumes=vol_list) diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index 5f22db9..aede24f 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -19,23 +19,20 @@ import os import string -import socket import time import urlparse import uuid - from distutils.version import LooseVersion - +from lxml import etree +from lxml.builder import E from kimchi import osinfo from kimchi.exception import InvalidParameter, IsoFormatError, MissingParameter from kimchi.imageinfo import probe_image, probe_img_info from kimchi.isoinfo import IsoImage from kimchi.utils import check_url_path, pool_name_from_uri -from lxml import etree -from lxml.builder import E - +from kimchi.xmlutils.disk import get_disk_xml QEMU_NAMESPACE = "xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'" @@ -122,69 +119,36 @@ class VMTemplate(object): except IsoFormatError: raise InvalidParameter("KCHISO0001E", {'filename': iso}) - def _get_cdrom_xml(self, libvirt_stream_protocols, qemu_stream_dns): + def _get_cdrom_xml(self, libvirt_stream_protocols): if 'cdrom' not in self.info: return '' - bus = self.info['cdrom_bus'] - dev = "%s%s" % (self._bus_to_dev[bus], - string.lowercase[self.info['cdrom_index']]) - local_file = """ - <disk type='file' device='cdrom'> - <driver name='qemu' type='raw'/> - <source file='%(src)s' /> - <target dev='%(dev)s' bus='%(bus)s'/> - <readonly/> - </disk> - """ - - network_file = """ - <disk type='network' device='cdrom'> - <driver name='qemu' type='raw'/> - <source protocol='%(protocol)s' name='%(url_path)s'> - <host name='%(hostname)s' port='%(port)s'/> - </source> - <target dev='%(dev)s' bus='%(bus)s'/> - <readonly/> - </disk> - """ + params = {} + params['type'] = 'cdrom' + params['format'] = 'raw' + params['bus'] = self.info['cdrom_bus'] + params['index'] = self.info['cdrom_index'] + params['path'] = self.info['cdrom'] qemu_stream_cmdline = """ <qemu:commandline> <qemu:arg value='-drive'/> - <qemu:arg value='file=%(url)s,if=none,id=drive-%(bus)s0-1-0,\ -readonly=on,format=raw'/> + <qemu:arg value='file=%(path)s,if=none,id=drive-%(bus)s0-1-0,\ +readonly=on,format=%(format)s'/> <qemu:arg value='-device'/> <qemu:arg value='%(bus)s-cd,bus=%(bus)s.1,unit=0,\ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> </qemu:commandline> """ - if not self.info.get('iso_stream', False): - params = {'src': self.info['cdrom'], 'dev': dev, 'bus': bus} - return local_file % (params) - - output = urlparse.urlparse(self.info['cdrom']) - port = output.port - protocol = output.scheme - hostname = output.hostname - url_path = output.path - - if port is None: - port = socket.getservbyname(protocol) + if self.info.get('iso_stream', False): + protocol = urlparse.urlparse(params['path']).scheme + if protocol not in libvirt_stream_protocols: + # return qemucmdline XML + return qemu_stream_cmdline % params - url = self.info['cdrom'] - if not qemu_stream_dns: - hostname = socket.gethostbyname(hostname) - url = protocol + "://" + hostname + ":" + str(port) + url_path - - if protocol not in libvirt_stream_protocols: - return qemu_stream_cmdline % {'url': url, 'bus': bus} - - params = {'protocol': protocol, 'url_path': url_path, - 'hostname': hostname, 'port': port, 'dev': dev, 'bus': bus} - - return network_file % (params) + dev, xml = get_disk_xml(params) + return xml def _get_disks_xml(self, vm_uuid): storage_path = self._get_storage_path() @@ -380,10 +344,8 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> else: params['disks'] = self._get_disks_xml(vm_uuid) - qemu_stream_dns = kwargs.get('qemu_stream_dns', False) libvirt_stream_protocols = kwargs.get('libvirt_stream_protocols', []) - cdrom_xml = self._get_cdrom_xml(libvirt_stream_protocols, - qemu_stream_dns) + cdrom_xml = self._get_cdrom_xml(libvirt_stream_protocols) if not urlparse.urlparse(self.info.get('cdrom', "")).scheme in \ libvirt_stream_protocols and \ -- 1.9.3
participants (1)
-
Aline Manera