On 04/23/2014 05:48 PM, Zhou Zheng Sheng wrote:

The schema checker needs to report an error message when the disk size is less than 1GB. Currently this message is missing, so it reports an KeyError. This patch adds the error message.

Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- src/kimchi/API.json | 6 ++++-- src/kimchi/i18n.py | 1 + 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 3360a9c..f2192b6 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -355,7 +355,8 @@ } }, "minItems": 1, - "uniqueItems": true + "uniqueItems": true, + "error": "KCHTMPL0022E" }, "storagepool": { "description": "Location of the storage pool", @@ -508,7 +509,8 @@ } }, "minItems": 1, - "uniqueItems": true + "uniqueItems": true, + "error": "KCHTMPL0022E" }, "storagepool": { "description": "Location of the storage pool", diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 89bcd02..3d3abb4 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -116,6 +116,7 @@ messages = { "KCHTMPL0019E": _("The volume: %(volume)s in not in storage pool %(pool)s"), "KCHTMPL0020E": _("Unable to create template due error: %(err)s"), "KCHTMPL0021E": _("Unable to delete template due error: %(err)s"), + "KCHTMPL0022E": _("Disk size must be greater than 1GB."),

"KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), Reviewed-by: Mark Wu<wudxw@linux.vnet.ibm.com>

The URI /host/devices only presents scsi_host (particularly fc_host) device information. To implement host pci pass through, we should list all types of host devices. This patch adds support for parsing various host devices information, and listing them on /host/devices. So the user is free to choose any listed PCI device to pass through to guest. Since the patch changes the device information dictionary format, the existing code consuming the device information is also changed accordingly.

To get all types of host device, access the following URL.

curl -u root -H "Content-Type: application/json" \ -H "Accept: application/json" \ http://127.0.0.1:8000/host/devices

To get only fc_host devices, change the URL to "http://127.0.0.1:8000/host/devices?_cap=fc_host"

To get only pci device, change the URL to "http://127.0.0.1:8000/host/devices?_cap=pci

Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- docs/API.md | 11 +- src/kimchi/hostdev.py | 307 +++++++++++++++++++++++++++++++++ src/kimchi/mockmodel.py | 7 +- src/kimchi/model/host.py | 15 +- src/kimchi/model/libvirtstoragepool.py | 18 +- tests/test_rest.py | 6 +- tests/test_storagepool.py | 7 +- 7 files changed, 335 insertions(+), 36 deletions(-) create mode 100644 src/kimchi/hostdev.py

diff --git a/docs/API.md b/docs/API.md index 716c983..009c58d 100644 --- a/docs/API.md +++ b/docs/API.md @@ -822,12 +822,11 @@ Contains the host sample data. * **GET**: Retrieve information of a single pci device. Currently only scsi_host devices are supported: * name: The name of the device. - * adapter_type: The capability type of the scsi_host device (fc_host). - Empty if pci device is not scsi_host. - * wwnn: The HBA Word Wide Node Name. - Empty if pci device is not scsi_host. - * wwpn: The HBA Word Wide Port Name - Empty if pci device is not scsi_host. + * path: Path of device in sysfs. + * adapter: Host adapter information. Empty if pci device is not scsi_host. + * type: The capability type of the scsi_host device (fc_host, vport_ops). + * wwnn: The HBA Word Wide Node Name. Empty if pci device is not fc_host. + * wwpn: The HBA Word Wide Port Name. Empty if pci device is not fc_host.

### Collection: Host Packages Update

diff --git a/src/kimchi/hostdev.py b/src/kimchi/hostdev.py new file mode 100644 index 0000000..638b654 --- /dev/null +++ b/src/kimchi/hostdev.py @@ -0,0 +1,307 @@ +# +# 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 kimchi.utils import kimchi_log +from kimchi.xmlutils import xpath_get_text + + +def _xpath(xml_str): + def get_first_text(the_path): + r = xpath_get_text(xml_str, the_path) + if r == []: + return None + r = r[0] + if r is None: + return '' + return r + return get_first_text It can serve for general purpose. So do you think it's better move to utils. + + +def get_all_host_devices(libvirt_conn): Any reason to keep an argument for libvirt connection? I mean you can get the connection inside this function. + devs = libvirt_conn.listAllDevices() + return [get_dev_info(dev) for dev in devs] + + +def get_devices_tree(dev_infos): + devs = dict([(dev_info['name'], dev_info) for dev_info in dev_infos]) + root = None + for dev_info in dev_infos: + if dev_info['parent'] is None: + root = dev_info + continue + try: + parent = devs[dev_info['parent']] + except KeyError: + continue + + try: + children = parent['children'] + except KeyError: + parent['children'] = [dev_info] + else: + children.append(dev_info) + return root + + +def get_dev_info(dev): + ''' Parse the node device XML string into dict according to + http://libvirt.org/formatnode.html. ''' + + info = {'name': dev.name(), 'parent': dev.parent()} + + xml_str = dev.XMLDesc() + xpath = _xpath(xml_str) + info['path'] = xpath('/device/path') + dev_type = xpath('/device/capability/@type') + info['type'] = dev_type + + get_dev_type_info = { + 'net': _get_net_dev_info, + 'pci': _get_pci_dev_info, + 'scsi': _get_scsi_dev_info, + 'scsi_generic': _get_scsi_generic_dev_info, + 'scsi_host': _get_scsi_host_dev_info, + 'scsi_target': _get_scsi_target_dev_info, + 'storage': _get_storage_dev_info, + 'system': _get_system_dev_info, + 'usb': _get_usb_dev_info, + 'usb_device': _get_usb_device_dev_info, + } + try: + get_info = get_dev_type_info[dev_type] + except KeyError: + kimchi_log.error("Unknown device type: %s", dev_type) + return info + + info.update(get_info(xml_str)) + return info + + +def _get_net_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'interface': xpath('/device/capability/interface'), + 'address': xpath('/device/capability/address')} + + links = {"80203": "IEEE 802.3", "80211": "IEEE 802.11"} + link_raw = xpath('/device/capability/capability/@type') + info['link_type'] = links.get(link_raw, link_raw) + + return info + + +def _get_pci_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'domain': xpath('/device/capability/domain'), + 'bus': xpath('/device/capability/bus'), + 'slot': xpath('/device/capability/slot'), + 'function': xpath('/device/capability/function'), + 'product': { + 'id': xpath('/device/capability/product/@id'), + 'description': xpath('/device/capability/product')}, + 'vendor': { + 'id': xpath('/device/capability/vendor/@id'), + 'name': xpath('/device/capability/vendor')}}

On 04/23/2014 05:48 PM, Zhou Zheng Sheng wrote: the xml path of device info can be categorized into a few patterns, then you can use helper functions to get device info for all kinds of devices. It will make the code look clean.

+ + iommuGroup_num = xpath('/device/capability/iommuGroup/@number') + if iommuGroup_num is not None: + info['iommuGroup'] = iommuGroup_num + return info + + +def _get_scsi_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'host': xpath('/device/capability/host'), + 'bus': xpath('/device/capability/bus'), + 'target': xpath('/device/capability/target'), + 'lun': xpath('/device/capability/lun'), + 'type': xpath('/device/capability/type')} + return info + + +def _get_scsi_generic_dev_info(xml_str): + # scsi_generic is not documented in libvirt official website. Try to + # parse scsi_generic according to the following libvirt path series. + # https://www.redhat.com/archives/libvir-list/2013-June/msg00014.html + xpath = _xpath(xml_str) + info = { + 'char': xpath('/device/capability/char')} + return info + + +def _get_scsi_host_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'host': xpath('/device/capability/host')} + + cap_type = xpath('/device/capability/capability/@type') + if cap_type is None: + info['adapter'] = {'type': ''} + return info + info['adapter'] = {'type': cap_type} + + if cap_type == 'fc_host': + info['adapter'] = { + 'wwnn': xpath('/device/capability/capability//wwnn'), + 'wwpn': xpath('/device/capability/capability/wwpn'), + 'fabric_wwn': xpath('/device/capability/capability/fabric_wwn')} + elif cap_type == 'vports_ops': + vport_in_use_count = xpath('/device/capability/capability/vports') + if vport_in_use_count is not None: + info['adapter']['vports'] = vport_in_use_count + max_vports = xpath('/device/capability/capability/max_vports') + if max_vports is not None: + info['adapter']['max_vports'] = max_vports + else: + kimchi_log.error("Unknown scsi host adapter type: %s", cap_type) + + return info + + +def _get_scsi_target_dev_info(xml_str): + # scsi_target is not documented in libvirt official website. Try to + # parse scsi_target according to the libvirt commit db19834a0a. + xpath = _xpath(xml_str) + info = { + 'target': xpath('/device/capability/target')} + return info + + +def _get_storage_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'block': xpath('/device/capability/block'), + 'bus': xpath('/device/capability/bus'), + 'drive_type': xpath('/device/capability/drive_type'), + 'model': xpath('/device/capability/model'), + 'vendor': xpath('/device/capability/vendor'), + 'serial': xpath('/device/capability/serial'), + 'size': xpath('/device/capability/size')} + + media_type = xpath('/device/capability/capability/@type') + if media_type is not None: + info['media'] = {'type': media_type} + if media_type == 'removable': + available = xpath('/device/capability/capability/media_available') + if available == '1': + info['media']['available'] = True + info['media'].update({ + 'media_size': + xpath('/device/capability/capability/media_size'), + 'media_label': + xpath('/device/capability/capability/media_label') + }) + else: + info['media']['available'] = False + return info + + +def _get_system_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'hardware': { + 'vendor': xpath('/device/capability/hardware/vendor'), + 'version': xpath('/device/capability/hardware/version'), + 'serial': xpath('/device/capability/hardware/serial'), + 'uuid': xpath('/device/capability/hardware/uuid')}, + 'firmware': { + 'vendor': xpath('/device/capability/firmware/vendor'), + 'version': xpath('/device/capability/firmware/version'), + 'release_date': xpath('/device/capability/firmware/version')}} should be release_date, not version. + + product = xpath('/device/capability/product') + if product is not None: + info['product'] = product + return info + + +def _get_usb_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'number': xpath('/device/capability/number'), + 'class': xpath('/device/capability/class'), + 'subclass': xpath('/device/capability/subclass'), + 'protocol': xpath('/device/capability/protocol')} + + description = xpath('/device/capability/description') + if description is not None: + info['description'] = description + return info + + +def _get_usb_device_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'bus': xpath('/device/capability/bus'), + 'device': xpath('/device/capability/device'), + 'product': { + 'id': xpath('/device/capability/product/@id'), + 'description': xpath('/device/capability/product')}, + 'vendor': { + 'id': xpath('/device/capability/vendor/@id'), + 'description': xpath('/device/capability/vendor')}} + return info + + +# For test and debug +def _print_host_dev_tree(): + from kimchi.model.libvirtconnection import LibvirtConnection + conn = LibvirtConnection('qemu:///system').get() + dev_infos = get_all_host_devices(conn) + root = get_devices_tree(dev_infos) + if root is None: + print "No device found" + return + print '-----------------' + print '\n'.join(_format_dev_node(root)) + + +def _format_dev_node(node): + from pprint import pformat + + try: + children = node['children'] + del node['children'] + except KeyError: + children = [] + + lines = [] + lines.extend([' ~' + line for line in pformat(node).split('\n')]) + + count = len(children) + for i, child in enumerate(children): + if count == 1: + lines.append(' \-----------------') + else: + lines.append(' +-----------------') + clines = _format_dev_node(child) + if i == count - 1: + p = ' ' + else: + p = ' |' + lines.extend([p + cline for cline in clines]) + lines.append('') + + return lines + + +if __name__ == '__main__': + _print_host_dev_tree() diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 866ad2c..a3e92f7 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -490,9 +490,10 @@ class MockModel(object): def device_lookup(self, nodedev_name): return { 'name': nodedev_name, - 'adapter_type': 'fc_host', - 'wwnn': uuid.uuid4().hex[:16], - 'wwpn': uuid.uuid4().hex[:16]} + 'adapter': { + 'type': 'fc_host', + 'wwnn': uuid.uuid4().hex[:16], + 'wwpn': uuid.uuid4().hex[:16]}}

def isopool_lookup(self, name): return {'state': 'active', diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py index e0ac760..112141c 100644 --- a/src/kimchi/model/host.py +++ b/src/kimchi/model/host.py @@ -28,6 +28,7 @@ import psutil from cherrypy.process.plugins import BackgroundTask

from kimchi import disks +from kimchi import hostdev from kimchi import netinfo from kimchi import xmlutils from kimchi.basemodel import Singleton @@ -265,20 +266,10 @@ class DeviceModel(object): def lookup(self, nodedev_name): conn = self.conn.get() try: - dev_xml = conn.nodeDeviceLookupByName(nodedev_name).XMLDesc(0) + dev = conn.nodeDeviceLookupByName(nodedev_name) except: raise NotFoundError('KCHHOST0003E', {'name': nodedev_name}) - cap_type = xmlutils.xpath_get_text( - dev_xml, '/device/capability/capability/@type') - wwnn = xmlutils.xpath_get_text( - dev_xml, '/device/capability/capability/wwnn') - wwpn = xmlutils.xpath_get_text( - dev_xml, '/device/capability/capability/wwpn') - return { - 'name': nodedev_name, - 'adapter_type': cap_type[0] if len(cap_type) >= 1 else '', - 'wwnn': wwnn[0] if len(wwnn) == 1 else '', - 'wwpn': wwpn[0] if len(wwpn) == 1 else ''} + return hostdev.get_dev_info(dev)

class PackagesUpdateModel(object): diff --git a/src/kimchi/model/libvirtstoragepool.py b/src/kimchi/model/libvirtstoragepool.py index 47b239b..b15bf1a 100644 --- a/src/kimchi/model/libvirtstoragepool.py +++ b/src/kimchi/model/libvirtstoragepool.py @@ -180,34 +180,34 @@ class ScsiPoolDef(StoragePoolDef): self.poolArgs['source']['name'] = tmp_name.replace('scsi_', '') # fc_host adapters type are only available in libvirt >= 1.0.5 if not self.poolArgs['fc_host_support']: - self.poolArgs['source']['adapter_type'] = 'scsi_host' + self.poolArgs['source']['adapter']['type'] = 'scsi_host' msg = "Libvirt version <= 1.0.5. Setting SCSI host name as '%s'; "\ "setting SCSI adapter type as 'scsi_host'; "\ "ignoring wwnn and wwpn." % tmp_name kimchi_log.info(msg) # Path for Fibre Channel scsi hosts self.poolArgs['path'] = '/dev/disk/by-path' - if not self.poolArgs['source']['adapter_type']: - self.poolArgs['source']['adapter_type'] = 'scsi_host' + if not self.poolArgs['source']['adapter']['type']: + self.poolArgs['source']['adapter']['type'] = 'scsi_host'

@property def xml(self): # Required parameters # name: - # source[adapter_type]: + # source[adapter][type]: # source[name]: - # source[wwnn]: - # source[wwpn]: + # source[adapter][wwnn]: + # source[adapter][wwpn]: # path:

xml = """ <pool type='scsi'> <name>{name}</name> <source> - <adapter type='{source[adapter_type]}'\ + <adapter type='{source[adapter][type]}'\ name='{source[name]}'\ - wwnn='{source[wwnn]}'\ - wwpn='{source[wwpn]}'/> + wwnn='{source[adapter][wwnn]}'\ + wwpn='{source[adapter][wwpn]}'/> </source> <target> <path>{path}</path> diff --git a/tests/test_rest.py b/tests/test_rest.py index 06396db..966718d 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -155,9 +155,9 @@ class RestTests(unittest.TestCase): nodedev = json.loads(self.request('/host/devices/scsi_host4').read()) # Mockmodel generates random wwpn and wwnn self.assertEquals('scsi_host4', nodedev['name']) - self.assertEquals('fc_host', nodedev['adapter_type']) - self.assertEquals(16, len(nodedev['wwpn'])) - self.assertEquals(16, len(nodedev['wwnn'])) + self.assertEquals('fc_host', nodedev['adapter']['type']) + self.assertEquals(16, len(nodedev['adapter']['wwpn'])) + self.assertEquals(16, len(nodedev['adapter']['wwnn']))

def test_get_vms(self): vms = json.loads(self.request('/vms').read()) diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py index 22b4943..3e3ad83 100644 --- a/tests/test_storagepool.py +++ b/tests/test_storagepool.py @@ -145,9 +145,10 @@ class storagepoolTests(unittest.TestCase): 'path': '/dev/disk/by-path', 'source': { 'name': 'scsi_host3', - 'adapter_type': 'fc_host', - 'wwpn': '0123456789abcdef', - 'wwnn': 'abcdef0123456789'}}, + 'adapter': { + 'type': 'fc_host', + 'wwpn': '0123456789abcdef', + 'wwnn': 'abcdef0123456789'}}}, 'xml': """ <pool type='scsi'>

on 2014/04/24 17:06, Mark Wu wrote:

On 04/23/2014 05:48 PM, Zhou Zheng Sheng wrote:

...

The host devices form a tree. If the user decides to pass through an HBA card to guest, all the related SCSI disks under the HBA card will be managed by the guest as well. If we can present all host devices in a tree, the user can conveniently infer the affected sub-devices of a passed through PCI device.

To get all devices in a tree, GET the following URL:

curl -u root -H "Content-Type: application/json" \ -H "Accept: application/json" \ http://127.0.0.1:8000/host/devices?_tree=1

It would print information like follow. [ { "hardware":{...}, "product":"...", "name":"computer", "parent":null, "path":null, "firmware":{...}, "type":"system", "children":[ { "slot":"0", "function":"0", "domain":"0", "vendor":{...}, "name":"pci_0000_00_00_0", "parent":"computer", "bus":"0", "product":{...}, "path":"/sys/devices/pci0000:00/0000:00:00.0", "type":"pci", "iommuGroup":"0" }, { # Another child device info ..., "children":[ { # A grand child device info ..., }, ... # Other grand children devices info ] }, ... ] } ]

The above tree is not very human friendly. In future we should parse this tree in front-end and show a nice graphical tree to the user. For now, you can get a human readable device tree using the following command (but it's just for debugging purpose). PYTHONPATH=src python -m kimchi.hostdev | less

It would print the following. ----------------- ~{'firmware': {...}, ~ 'hardware': {...}, ~ 'name': 'computer', ~ 'parent': None, ~ 'path': None, ~ 'product': '...', ~ 'type': 'system'} +----------------- | ~{'bus': '0', | ~ 'domain': '0', | ~ 'function': '0', | ~ 'iommuGroup': '0', | ~ 'name': 'pci_0000_00_00_0', | ~ 'parent': 'computer', | ~ 'path': '/sys/devices/pci0000:00/0000:00:00.0', | ~ 'product': {'description': '... DRAM Controller', | ~ 'id': '0x0104'}, | ~ 'slot': '0', | ~ 'type': 'pci', | ~ 'vendor': {'id': '0x8086', 'name': 'Intel Corporation'}} | +----------------- | ~{'bus': '0', | ~ 'domain': '0', | ~ 'function': '0', | ~ 'iommuGroup': '1', | ~ 'name': 'pci_0000_00_01_0', | ~ 'parent': 'computer', | ~ 'path': '/sys/devices/pci0000:00/0000:00:01.0', | ~ 'product': {...}, | ~ 'slot': '1', | ~ 'type': 'pci', | ~ 'vendor': {'id': '0x8086', 'name': 'Intel Corporation'}} | \----------------- | ~{'bus': '1', | ~ 'domain': '0', | ~ 'function': '0', | ~ 'iommuGroup': '1', | ~ 'name': 'pci_0000_01_00_0', | ~ 'parent': 'pci_0000_00_01_0', | ~ 'path': '/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0', | ~ 'product': {'description': 'GF119M ...', 'id': '0x1057'}, | ~ 'slot': '0', | ~ 'type': 'pci', | ~ 'vendor': {'id': '0x10de', 'name': 'NVIDIA Corporation'}} | | +----------------- | ~{'bus': '0', | ~ 'domain': '0', ...

Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- docs/API.md | 7 +++++++ src/kimchi/control/host.py | 21 +++++++++++++++++++++ src/kimchi/i18n.py | 2 ++ 3 files changed, 30 insertions(+)

diff --git a/docs/API.md b/docs/API.md index 009c58d..d89ca8c 100644 --- a/docs/API.md +++ b/docs/API.md @@ -812,6 +812,13 @@ Contains the host sample data. * Parameters: * _cap: Filter node device list with given node device capability. To list Fibre Channel SCSI Host devices, use "_cap=fc_host". + * _tree: To list the devices in a tree, use "_tree=1". The result is + like [root_device], where root_device dictionary contains all + information of the root divice, and a root_device["children"] + item. The root_device["children"] is a list, and the elements + in the list are device dicts containing device information and + "children" lists. If a device is a leaf one, it does not + contain device["children"] item.

### Resource: Device

diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py index 003c4b9..9347932 100644 --- a/src/kimchi/control/host.py +++ b/src/kimchi/control/host.py @@ -22,6 +22,7 @@ import cherrypy from kimchi.control.base import Collection, Resource, SimpleCollection from kimchi.control.utils import UrlSubNode, validate_method from kimchi.exception import OperationFailed +from kimchi import hostdev from kimchi.template import render

@@ -88,6 +89,26 @@ class Devices(Collection): super(Devices, self).__init__(model) self.resource = Device

+ def _get_resources(self, flag_filter): + try: + _tree = bool(int(flag_filter['_tree'])) + del flag_filter['_tree'] + except KeyError: + _tree = False + + devices = super(Devices, self)._get_resources(flag_filter) + + if devices and _tree: + dev_infos = [device.data for device in devices] + root = hostdev.get_devices_tree(dev_infos) + if root is None: + raise InvalidParameter('KCHTMPL0023E') + root_device = Device(self.model, root['name']) + root_device.info = root + devices = [root_device] + + return devices +

class Device(Resource): def __init__(self, model, id): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 3d3abb4..955aff2 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -117,6 +117,8 @@ messages = { "KCHTMPL0020E": _("Unable to create template due error: %(err)s"), "KCHTMPL0021E": _("Unable to delete template due error: %(err)s"), "KCHTMPL0022E": _("Disk size must be greater than 1GB."), + "KCHTMPL0023E": _("No root device found. Maybe you should stop filtering " + "devices to avoid filtering out the root device of the device tree."),

"KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), We need have a discussion with UI developers about the restriction. As you mentioned, the other devices in the same group will be affected by the passing through. We need let the user know this potential dangerous. Considering kimchi is a light weight virtualization management tool, I will suggest we only expose the leaf device with a field which indicate the devices will be affected by passing this device. And the usb controller and scsi could be filtered, we can implement the passing through of them by their own mechanism.

In next patch set I will implement three types of pass through. 1. USB device Kimchi should be able to list all USB devices available to pass through. 2. SCSI device It's supported since libvirt 1.0.6. Kimchi should be able to list all SCSI generic devices and pass through to VM. (A bit conflict with SCSI storage pool?) 3. PCI device List only leaf PCI device, such as wireless NIC, and each device entry should contain a list of device names in the same IOMMU group, because all PCI devices in the same IOMMU group should be detached at the same time. This should simplifies the front-end work. However I think a device tree is still useful. Say two PCI devices are in the same IOMMU group, one is wireless NIC and other is a USB/SD/SCSI controller. When we decide to pass through the wireless NIC to guest, the controller would also be detached. Only listing the wireless NIC and the controller's names are not enough, because the controller may be linked to children devices, and those children devices are affected too. In back-end, I think providing rich device information and organizing them in a tree is very useful. It should be the front-end's duty to show them in a human readable and friendly manner. If you think the information produced by host/devices is too much, that is because it's not for human, but for front-end code to parse. We can show all host devices in a tree in the front-end. The node in the tree contains only an icon and a title. When the mouse moves over the title, details can be shown in tool tip. Only the leaf USB, SCSI and PCI device are available for the user to pass through, and they are shown in highlight with a tick box. Other parent devices can be shown in gray color. By clicking the leaf device tick box, the code should automatically compute the affected devices. For USB or SCSI device, the affected device is itself. For PCI device, the affect devices are ones in the same IOMMU group, and the tree view should changed promptly to reflect which devices would be affected. Once the user determines the devices to pass through, the front-end code just send the device names to back-end, and back-end detect the device types automatically and generate the XML. In this way we provides a single front-end interface to pass through all kinds of host devices, and also a single back-end interface to list and pass through them.

I don't like the idea of updating template to support pci passthrough because one pci device can only be attached on one guest. This usage breaks the semantics of template. I know we have adopted this method to support SCSI LUN volume. But I think we need stop moving towards that direction. I want to propose the following way to resolve this problem: Provides an option to allow user do customization based on the selected template when he create vm from the template. Then the guest xml generation code merge the customized configuration into template xml. It makes more sense to handle vm specific configuration than updating template. On 04/23/2014 05:48 PM, Zhou Zheng Sheng wrote:

This patch enbales Kimchi's VM to use host PCI device directly, so that it greatly improves the related device performance. The user can pass through HBA card, NIC, USB controller, SCSI controller or other PCI devices to VM, as long as the host supports one of Intel VT-d, AMD IOMMU or POWER sPAPR technology and runs a recent release of Linux kernel.

To try this feature, firstly create a VM template as usual, either in the web UI or by running the curl command.

curl -u root -H "Content-Type: application/json" \ -H "Accept: application/json" -X POST \ "http://localhost:8000/templates" \ -d '{"os_distro":"fedora", "os_version":"20", "cdrom":"/iso/Fedora-Live-Desktop-x86_64-20-1.iso"}'

To determine the PCI devices to pass through, get a list of host PCI device.

curl -u root -H "Content-Type: application/json" \ -H "Accept: application/json" \ http://127.0.0.1:8000/host/devices?_cap=pci

It would print a list like following. [ { "slot":"0", "function":"0", "domain":"0", "vendor":{"id":"0x8086", "name":"Intel Corporation"}, "name":"pci_0000_00_00_0", "parent":"computer", "bus":"0", "product":{"id":"0x0104", "description":"... DRAM Controller" }, "path":"/sys/devices/pci0000:00/0000:00:00.0", "type":"pci", "iommuGroup":"0" }, { "slot":"1", "function":"0", "domain":"0", "vendor":{"id":"0x8086", "name":"Intel Corporation"}, "name":"pci_0000_00_01_0", "parent":"computer", "bus":"0", "product":{"id":"0x0101", "description":"... PCI Express Root Port"}, "path":"/sys/devices/pci0000:00/0000:00:01.0", "type":"pci", "iommuGroup":"1" }, ... ]

Actually all the devices form a tree in the host, passing through a PCI device means passing all the sub-device linked to this PCI device. To get an idea of the device tree, we can access the following URL.

curl -u root -H "Content-Type: application/json" \ -H "Accept: application/json" \ http://127.0.0.1:8000/host/devices?_tree=1

The output is not human readable, but in future the front-end should parse the output into a graph. For now, as a debugging output, you can run one of the following command to get a human readable tree.

Alternative 1: PYTHONPATH=src python -m kimchi.hostdev | less Alternative 2: virsh nodedev-list --tree

Then take down the 'name' attribute of the PCI device, such as "pci_0000_0d_00_0" or "pci_0000_03_00_0".

Now update the template to define the PCI devices to pass through.

curl -u root -H "Content-Type: application/json" \ -H "Accept: application/json" -X PUT \ "http://localhost:8000/templates/Fedora-Live-Desktop-x86_64-20-11398235858253" \ -d '{"host_pci_devs": ["pci_0000_0d_00_0", "pci_0000_03_00_0"]}'

Then create a VM from the template, and start the VM. libvirt would automatically detach the PCI devices from host, and attach them to vfio-pci or pci-stub. The guest OS should be able to use those devices, and "lspci" in guest OS should show the devices information. After the VM is powered off, libvirt would automatically re-attach the devices to their original host drivers.

The author was able to test this feature by passing through the wireless NIC and an SD card reader to the VM.

Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- src/kimchi/API.json | 18 +++++++++++-- src/kimchi/control/templates.py | 5 ++-- src/kimchi/featuretests.py | 9 +++++++ src/kimchi/i18n.py | 11 ++++++++ src/kimchi/model/config.py | 2 ++ src/kimchi/model/templates.py | 58 +++++++++++++++++++++++++++++++++++++++++ src/kimchi/vmtemplate.py | 43 ++++++++++++++++++++++++++++++ 7 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/src/kimchi/API.json b/src/kimchi/API.json index f2192b6..93478ae 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -26,6 +26,18 @@ ] } } + }, + "host_pci_devs": { + "description": "Configure host PCI devices to be passed through to VM", + "type": "array", + "uniqueItems": true, + "error": "KCHTMPL0027E", + "items": { + "description": "Name of the PCI device", + "pattern": "^[A-Za-z0-9_]+$", + "type": "string", + "error": "KCHTMPL0028E" + } } }, "properties": { @@ -376,7 +388,8 @@ "type": "array", "items": { "type": "string" } }, - "graphics": { "$ref": "#/kimchitype/graphics" } + "graphics": { "$ref": "#/kimchitype/graphics" }, + "host_pci_devs": {"$ref": "#/kimchitype/host_pci_devs"} }, "additionalProperties": false, "error": "KCHAPI0001E" @@ -530,7 +543,8 @@ "type": "array", "items": { "type": "string" } }, - "graphics": { "$ref": "#/kimchitype/graphics" } + "graphics": { "$ref": "#/kimchitype/graphics" }, + "host_pci_devs": {"$ref": "#/kimchitype/host_pci_devs"} }, "additionalProperties": false, "error": "KCHAPI0001E" diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py index a535960..7048d1b 100644 --- a/src/kimchi/control/templates.py +++ b/src/kimchi/control/templates.py @@ -34,7 +34,7 @@ class Template(Resource): self.update_params = ["name", "folder", "icon", "os_distro", "storagepool", "os_version", "cpus", "memory", "cdrom", "disks", "networks", - "graphics"] + "graphics", "host_pci_devs"] self.uri_fmt = "/templates/%s" self.clone = self.generate_action_handler('clone')

@@ -52,4 +52,5 @@ class Template(Resource): 'storagepool': self.info['storagepool'], 'networks': self.info['networks'], 'folder': self.info.get('folder', []), - 'graphics': self.info['graphics']} + 'graphics': self.info['graphics'], + 'host_pci_devs': self.info.get('host_pci_devs', [])} diff --git a/src/kimchi/featuretests.py b/src/kimchi/featuretests.py index 07d5676..c906347 100644 --- a/src/kimchi/featuretests.py +++ b/src/kimchi/featuretests.py @@ -29,6 +29,7 @@ from lxml.builder import E

from kimchi.utils import kimchi_log +from kimchi.utils import run_command

ISO_STREAM_XML = """ @@ -175,3 +176,11 @@ class FeatureTests(object): pool is None or pool.undefine() conn is None or conn.close() return True + + @staticmethod + def kernel_support_vfio(): + out, err, rc = run_command(['modprobe', 'vfio-pci']) + if rc != 0: + kimchi_log.warning("Unable to load Kernal module vfio-pci.") + return False + return True diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 955aff2..d70c863 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -119,6 +119,17 @@ messages = { "KCHTMPL0022E": _("Disk size must be greater than 1GB."), "KCHTMPL0023E": _("No root device found. Maybe you should stop filtering " "devices to avoid filtering out the root device of the device tree."), + "KCHTMPL0024E": _("Device '%(dev)s' of type '%(type)s' is not a PCI device."), + "KCHTMPL0025E": _("No IOMMU groups found. Host PCI pass through needs " + "IOMMU group to function correctly. Please enable Intel " + "VT-d or AMD IOMMU in your BIOS, then verify the Kernel " + "is compiled with IOMMU support. For Intel CPU, add " + "intel_iommu=on to your Kernel parameter in " + "/boot/grub2/grub.conf. For AMD CPU, add iommu=pt iommu=1"), + "KCHTMPL0026E": _("Unable to detach the device %(dev)s from host. It may be " + "in use by the host or other guest."), + "KCHTMPL0027E": _("PCI device names must be unique."), + "KCHTMPL0028E": _("PCI device name should contain only alphabetic letters, digits and underscore."),

"KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py index 7081373..12aea82 100644 --- a/src/kimchi/model/config.py +++ b/src/kimchi/model/config.py @@ -53,6 +53,7 @@ class CapabilitiesModel(object): self.qemu_stream_dns = False self.libvirt_stream_protocols = [] self.fc_host_support = False + self.kernel_vfio = False

# Subscribe function to set host capabilities to be run when cherrypy # server is up @@ -65,6 +66,7 @@ class CapabilitiesModel(object): self.qemu_stream_dns = FeatureTests.qemu_iso_stream_dns() self.nfs_target_probe = FeatureTests.libvirt_support_nfs_probe() self.fc_host_support = FeatureTests.libvirt_support_fc_host() + self.kernel_vfio = FeatureTests.kernel_support_vfio()

self.libvirt_stream_protocols = [] for p in ['http', 'https', 'ftp', 'ftps', 'tftp']: diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 60f4de5..0acbeb9 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -19,16 +19,20 @@

import copy import os +import glob import time

import libvirt

+from kimchi import hostdev from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed +from kimchi.model.config import CapabilitiesModel from kimchi.kvmusertests import UserTests from kimchi.utils import pool_name_from_uri from kimchi.utils import probe_file_permission_as_user +from kimchi.utils import run_command from kimchi.vmtemplate import VMTemplate from lxml import objectify

@@ -74,6 +78,9 @@ class TemplatesModel(object): except Exception: raise InvalidParameter("KCHTMPL0003E", {'network': net_name, 'template': name}) + + self.host_pci_device_validate(params.get(u'host_pci_devs', [])) + # Creates the template class with necessary information # Checkings will be done while creating this class, so any exception # will be raised here @@ -93,6 +100,19 @@ class TemplatesModel(object): with self.objstore as session: return session.get_list('template')

+ def host_pci_device_validate(self, host_pci_devs): + conn = self.conn.get() + for dev_name in host_pci_devs: + try: + dev = conn.nodeDeviceLookupByName(dev_name) + except: + raise NotFoundError('KCHHOST0003E', {'name': dev_name}) + dev_info = hostdev.get_dev_info(dev) + if dev_info['type'] != 'pci': + raise InvalidParameter("KCHTMPL0024E", + {'dev': dev_name, + 'type': dev_info['type']}) + def template_volume_validate(self, tmp_volumes, pool): kwargs = {'conn': self.conn, 'objstore': self.objstore} pool_type = xmlutils.xpath_get_text(pool.XMLDesc(0), "/pool/@type")[0] @@ -186,6 +206,9 @@ class TemplateModel(object): raise InvalidParameter("KCHTMPL0003E", {'network': net_name, 'template': name})

+ self.templates.host_pci_device_validate(new_t.get(u'host_pci_devs', + [])) + self.delete(name) try: ident = self.templates.create(new_t) @@ -225,6 +248,41 @@ class LibvirtVMTemplate(VMTemplate): names = conn.listStoragePools() + conn.listDefinedStoragePools() return sorted(map(lambda x: x.decode('utf-8'), names))

+ def _get_all_host_devs_name(self, cap=None): + conn = self.conn.get() + return conn.listDevices(cap, 0) + + def _host_pci_devs_validate(self): + ''' Validate all PCI pass through devices and return device + information. ''' + + dev_names = self.info.get('host_pci_devs', []) + if not dev_names: + return None + + if not glob.glob('/sys/kernel/iommu_groups/*'): + raise NotFoundError("KCHTMPL0025E") + + out, err, rc = run_command(['getsebool', 'virt_use_sysfs']) + if rc == 0: + if out.rstrip('\n') != "virt_use_sysfs --> on": + out, err, rc = run_command(['setsebool', '-P', + 'virt_use_sysfs=on']) + + driver = 'vfio' if CapabilitiesModel().kernel_vfio else 'kvm' + conn = self.conn.get() + dev_infos = [] + for dev_name in dev_names: + dev = conn.nodeDeviceLookupByName(dev_name) + try: + dev.detachFlags(driver) + except libvirt.libvirtError: + raise InvalidParameter('KCHTMPL0026E', {'dev': dev_name}) + dev.reAttach() + dev_infos.append(hostdev.get_dev_info(dev)) + + return {'driver': driver, 'devs': dev_infos} + def _network_validate(self): names = self.info['networks'] for name in names: diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index 8d5217a..750d236 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -299,6 +299,29 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> input_output += sound % self.info return input_output

+ def _get_host_pci_passthrough_xml(self): + r = self._host_pci_devs_validate() + if r is None: + return '' + + driver = r['driver'] + xmls = [] + for dev_info in r['devs']: + xmlstr = ''' + <hostdev mode='subsystem' type='pci' managed='yes'> + <source> + <address domain='%(domain)s' bus='%(bus)s' \ + slot='%(slot)s' function='%(function)s'/> + </source> + <driver name='%(driver)s'/> + </hostdev>''' % { + 'domain': dev_info['domain'], 'bus': dev_info['bus'], + 'slot': dev_info['slot'], 'function': dev_info['function'], + 'driver': driver} + xmls.append(xmlstr) + + return ''.join(xmls) + def to_vm_xml(self, vm_name, vm_uuid, **kwargs): params = dict(self.info) params['name'] = vm_name @@ -333,6 +356,8 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> else: params['cdroms'] = cdrom_xml

+ params['host_pci_devs'] = self._get_host_pci_passthrough_xml() + xml = """ <domain type='%(domain)s' %(qemu-namespace)s> %(qemu-stream-cmdline)s @@ -360,6 +385,7 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> %(networks)s %(graphics)s %(input_output)s + %(host_pci_devs)s <memballoon model='virtio' /> </devices> </domain> @@ -370,6 +396,7 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> self._storage_validate() self._network_validate() self._iso_validate() + self._host_pci_devs_validate()

def _iso_validate(self): pass @@ -380,6 +407,14 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> def _storage_validate(self): pass

+ def _host_pci_devs_validate(self): + ''' Should return a dictionary containing two items as following. + {"driver": "kvm"/"vfio"/..., + "devs": [{dev_info0...}, {dev_info1...}, ...]} + Returning None means no host PCI device to pass through. + ''' + pass + def fork_vm_storage(self, vm_uuid): pass

@@ -398,6 +433,9 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> def _get_all_storagepools_name(self): return []

+ def _get_all_host_devs_name(self, cap=None): + return [] + def validate_integrity(self): invalid = {} # validate networks integrity @@ -418,6 +456,11 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/> if not (os.path.isfile(iso) or check_url_path(iso)): invalid['cdrom'] = [iso]

+ invalid_pci_devs = list(set(self.info.get('host_pci_devs', [])) - + set(self._get_all_host_devs_name(cap='pci'))) + if invalid_pci_devs: + invalid['host_pci_devs'] = invalid_pci_devs + self.info['invalid'] = invalid

return self.info