Namely, this patch is about virMediatedDeviceGetIOMMUGroup{Dev,Num} functions. There's no compelling reason why these functions should take an object, on the contrary, having to create an object every time one needs to query the IOMMU group number, discarding the object afterwards, seems odd. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- There's more than this, e.g. all hostdevs do it like that, but that's a refactor for another day. src/qemu/qemu_domain.c | 8 +------- src/security/security_apparmor.c | 10 +--------- src/security/security_dac.c | 20 ++------------------ src/security/security_selinux.c | 20 ++------------------ src/util/virmdev.c | 21 +++++++++++++-------- src/util/virmdev.h | 4 ++-- 6 files changed, 21 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 00b0b4ac4..73c7447d7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7102,7 +7102,6 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, virUSBDevicePtr usb = NULL; virSCSIDevicePtr scsi = NULL; virSCSIVHostDevicePtr host = NULL; - virMediatedDevicePtr mdev = NULL; char *tmpPath = NULL; bool freeTmpPath = false; bool includeVFIO = false; @@ -7203,11 +7202,7 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, } case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: - if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr, - mdevsrc->model))) - goto cleanup; - - if (!(tmpPath = virMediatedDeviceGetIOMMUGroupDev(mdev))) + if (!(tmpPath = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) goto cleanup; freeTmpPath = true; @@ -7263,7 +7258,6 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, virUSBDeviceFree(usb); virSCSIDeviceFree(scsi); virSCSIVHostDeviceFree(host); - virMediatedDeviceFree(mdev); if (freeTmpPath) VIR_FREE(tmpPath); return ret; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index fc5581526..62672b0af 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -905,21 +905,13 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { char *vfiodev = NULL; - virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr, - mdevsrc->model); - if (!mdev) + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) goto done; - if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) { - virMediatedDeviceFree(mdev); - goto done; - } - ret = AppArmorSetSecurityHostdevLabelHelper(vfiodev, ptr); VIR_FREE(vfiodev); - virMediatedDeviceFree(mdev); break; } diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 922e48494..7dcf4c15f 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -968,21 +968,13 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { char *vfiodev = NULL; - virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr, - mdevsrc->model); - if (!mdev) + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) goto done; - if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) { - virMediatedDeviceFree(mdev); - goto done; - } - ret = virSecurityDACSetHostdevLabelHelper(vfiodev, &cbdata); VIR_FREE(vfiodev); - virMediatedDeviceFree(mdev); break; } @@ -1144,21 +1136,13 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { char *vfiodev = NULL; - virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr, - mdevsrc->model); - if (!mdev) + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) goto done; - if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) { - virMediatedDeviceFree(mdev); - goto done; - } - ret = virSecurityDACRestoreFileLabel(virSecurityManagerGetPrivateData(mgr), vfiodev); VIR_FREE(vfiodev); - virMediatedDeviceFree(mdev); break; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index df7c96833..c7a2dfe98 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1843,21 +1843,13 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { char *vfiodev = NULL; - virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr, - mdevsrc->model); - if (!mdev) + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) goto done; - if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) { - virMediatedDeviceFree(mdev); - goto done; - } - ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, &data); VIR_FREE(vfiodev); - virMediatedDeviceFree(mdev); break; } @@ -2092,21 +2084,13 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: { char *vfiodev = NULL; - virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr, - mdevsrc->model); - if (!mdev) + if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr))) goto done; - if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) { - virMediatedDeviceFree(mdev); - goto done; - } - ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev); VIR_FREE(vfiodev); - virMediatedDeviceFree(mdev); break; } diff --git a/src/util/virmdev.c b/src/util/virmdev.c index 2a1ade739..85a3fec16 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -209,43 +209,48 @@ virMediatedDeviceGetPath(virMediatedDevicePtr dev) * for freeing the result. */ char * -virMediatedDeviceGetIOMMUGroupDev(virMediatedDevicePtr dev) +virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr) { - char *resultpath = NULL; + char *result_path = NULL; char *iommu_path = NULL; char *vfio_path = NULL; + char *dev_path = virMediatedDeviceGetSysfsPath(uuidstr); - if (virAsprintf(&iommu_path, "%s/iommu_group", dev->path) < 0) + if (!dev_path) return NULL; + if (virAsprintf(&iommu_path, "%s/iommu_group", dev_path) < 0) + goto cleanup; + if (!virFileExists(iommu_path)) { virReportSystemError(errno, _("failed to access '%s'"), iommu_path); goto cleanup; } - if (virFileResolveLink(iommu_path, &resultpath) < 0) { + if (virFileResolveLink(iommu_path, &result_path) < 0) { virReportSystemError(errno, _("failed to resolve '%s'"), iommu_path); goto cleanup; } - if (virAsprintf(&vfio_path, "/dev/vfio/%s", last_component(resultpath)) < 0) + if (virAsprintf(&vfio_path, "/dev/vfio/%s", last_component(result_path)) < 0) goto cleanup; cleanup: - VIR_FREE(resultpath); + VIR_FREE(result_path); VIR_FREE(iommu_path); + VIR_FREE(dev_path); return vfio_path; } int -virMediatedDeviceGetIOMMUGroupNum(virMediatedDevicePtr dev) +virMediatedDeviceGetIOMMUGroupNum(const char *uuidstr) { char *vfio_path = NULL; char *group_num_str = NULL; unsigned int group_num = -1; - if (!(vfio_path = virMediatedDeviceGetIOMMUGroupDev(dev))) + if (!(vfio_path = virMediatedDeviceGetIOMMUGroupDev(uuidstr))) return -1; group_num_str = last_component(vfio_path); diff --git a/src/util/virmdev.h b/src/util/virmdev.h index 2f3d6bb84..e2da8f5ab 100644 --- a/src/util/virmdev.h +++ b/src/util/virmdev.h @@ -65,10 +65,10 @@ virMediatedDeviceSetUsedBy(virMediatedDevicePtr dev, const char *domname); char * -virMediatedDeviceGetIOMMUGroupDev(virMediatedDevicePtr dev); +virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr); int -virMediatedDeviceGetIOMMUGroupNum(virMediatedDevicePtr dev); +virMediatedDeviceGetIOMMUGroupNum(const char *uuidstr); char * virMediatedDeviceGetSysfsPath(const char *uuidstr); -- 2.12.2

On Wed, Apr 26, 2017 at 04:55:29PM +0200, Erik Skultety wrote: Wrong indentation of the function body. Pavel

The parent device needs to report the generic stuff about the supported mediated devices types, like device API, available instances, type name, etc. Therefore this patch introduces a new nested capability element of type 'mdev_types' with the resulting XML of the following format: <device> ... <capability type='pci'> ... <capability type='mdev_types'> <type id='vendor_supplied_id'> <name>optional_vendor_supplied_codename</name> <deviceAPI>vfio-pci</deviceAPI> <availableInstances>NUM</availableInstances> </type> ... <type> ... </type> </capability> </capability> ... </device> Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- docs/schemas/nodedev.rng | 26 +++++ src/conf/node_device_conf.c | 104 ++++++++++++++++++ src/conf/node_device_conf.h | 15 +++ src/conf/virnodedeviceobj.c | 7 ++ src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 119 +++++++++++++++++++++ .../pci_0000_02_10_7_mdev_types.xml | 32 ++++++ tests/nodedevxml2xmltest.c | 1 + 8 files changed, 305 insertions(+) create mode 100644 tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 0f90a73c8..e0a2c5032 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -205,6 +205,32 @@ </optional> <optional> + <element name='capability'> + <attribute name='type'> + <value>mdev_types</value> + </attribute> + <oneOrMore> + <element name='type'> + <attribute name='id'> + <data type='string'/> + </attribute> + <optional> + <element name='name'><text/></element> + </optional> + <element name='deviceAPI'> + <choice> + <value>vfio-pci</value> + </choice> + </element> + <element name='availableInstances'> + <ref name='unsignedInt'/> + </element> + </element> + </oneOrMore> + </element> + </optional> + + <optional> <element name='iommuGroup'> <attribute name='number'> <ref name='unsignedInt'/> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 24cb6d66f..b3012821a 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -88,6 +88,26 @@ virNodeDevCapsDefParseString(const char *xpath, } +static void +virNodeDevCapMdevTypeClear(virNodeDevCapMdevTypePtr type) +{ + VIR_FREE(type->id); + VIR_FREE(type->name); + VIR_FREE(type->device_api); +} + + +void +virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type) +{ + if (!type) + return; + + virNodeDevCapMdevTypeClear(type); + VIR_FREE(type); +} + + void virNodeDeviceDefFree(virNodeDeviceDefPtr def) { @@ -265,6 +285,27 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<capability type='%s'/>\n", virPCIHeaderTypeToString(data->pci_dev.hdrType)); } + if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) { + virBufferAddLit(buf, "<capability type='mdev_types'>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < data->pci_dev.nmdev_types; i++) { + virNodeDevCapMdevTypePtr type = data->pci_dev.mdev_types[i]; + virBufferEscapeString(buf, "<type id='%s'>\n", type->id); + virBufferAdjustIndent(buf, 2); + if (type->name) + virBufferAsprintf(buf, "<name>%s</name>\n", + type->name); + virBufferAsprintf(buf, "<deviceAPI>%s</deviceAPI>\n", + type->device_api); + virBufferAsprintf(buf, + "<availableInstances>%u</availableInstances>\n", + type->available_instances); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</type>\n"); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</capability>\n"); + } if (data->pci_dev.nIommuGroupDevices) { virBufferAsprintf(buf, "<iommuGroup number='%d'>\n", data->pci_dev.iommuGroupNumber); @@ -1365,6 +1406,63 @@ virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr ctxt, static int +virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr ctxt, + virNodeDevCapPCIDevPtr pci_dev) +{ + int ret = -1; + xmlNodePtr orignode = NULL; + xmlNodePtr *nodes = NULL; + int nmdev_types = virXPathNodeSet("./type", ctxt, &nodes); + virNodeDevCapMdevTypePtr type = NULL; + size_t i; + + orignode = ctxt->node; + for (i = 0; i < nmdev_types; i++) { + ctxt->node = nodes[i]; + + if (VIR_ALLOC(type) < 0) + goto cleanup; + + if (!(type->id = virXPathString("string(./@id[1])", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'id' attribute for mediated device's " + "<type> element")); + goto cleanup; + } + + if (!(type->device_api = virXPathString("string(./deviceAPI[1])", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing device API for mediated device type '%s'"), + type->id); + goto cleanup; + } + + if (virXPathUInt("number(./availableInstances)", ctxt, + &type->available_instances) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("missing number of available instances for " + "mediated device type '%s'"), + type->id); + goto cleanup; + } + + type->name = virXPathString("string(./name)", ctxt); + + if (VIR_APPEND_ELEMENT(pci_dev->mdev_types, + pci_dev->nmdev_types, type) < 0) + goto cleanup; + } + + pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; + ret = 0; + cleanup: + virNodeDevCapMdevTypeFree(type); + ctxt->node = orignode; + return ret; +} + + +static int virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, virNodeDevCapPCIDevPtr pci_dev) @@ -1386,6 +1484,9 @@ virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt, } else if (STREQ(type, "virt_functions") && virNodeDevPCICapSRIOVVirtualParseXML(ctxt, pci_dev) < 0) { goto cleanup; + } if (STREQ(type, "mdev_types") && + virNodeDevPCICapMdevTypesParseXML(ctxt, pci_dev)) { + goto cleanup; } else { int hdrType = virPCIHeaderTypeFromString(type); @@ -1899,6 +2000,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) VIR_FREE(data->pci_dev.iommuGroupDevices[i]); VIR_FREE(data->pci_dev.iommuGroupDevices); virPCIEDeviceInfoFree(data->pci_dev.pci_express); + for (i = 0; i < data->pci_dev.nmdev_types; i++) + virNodeDevCapMdevTypeFree(data->pci_dev.mdev_types[i]); + VIR_FREE(data->pci_dev.mdev_types); break; case VIR_NODE_DEV_CAP_USB_DEV: VIR_FREE(data->usb_dev.product_name); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 273d49f76..629f732e6 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -95,6 +95,7 @@ typedef enum { VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0), VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1), VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2), + VIR_NODE_DEV_CAP_FLAG_PCI_MDEV = (1 << 3), } virNodeDevPCICapFlags; typedef enum { @@ -133,6 +134,15 @@ struct _virNodeDevCapSystem { virNodeDevCapSystemFirmware firmware; }; +typedef struct _virNodeDevCapMdevType virNodeDevCapMdevType; +typedef virNodeDevCapMdevType *virNodeDevCapMdevTypePtr; +struct _virNodeDevCapMdevType { + char *id; + char *name; + char *device_api; + unsigned int available_instances; +}; + typedef struct _virNodeDevCapPCIDev virNodeDevCapPCIDev; typedef virNodeDevCapPCIDev *virNodeDevCapPCIDevPtr; struct _virNodeDevCapPCIDev { @@ -156,6 +166,8 @@ struct _virNodeDevCapPCIDev { int numa_node; virPCIEDeviceInfoPtr pci_express; int hdrType; /* enum virPCIHeaderType or -1 */ + virNodeDevCapMdevTypePtr *mdev_types; + size_t nmdev_types; }; typedef struct _virNodeDevCapUSBDev virNodeDevCapUSBDev; @@ -340,6 +352,9 @@ virNodeDeviceDefFree(virNodeDeviceDefPtr def); void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps); +void +virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type); + # define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP \ (VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV | \ diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index 181d2efe1..fc5f3708f 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -468,6 +468,13 @@ virNodeDeviceCapMatch(virNodeDeviceObjPtr devobj, VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) return true; } + + if (cap->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { + if (type == VIR_NODE_DEV_CAP_MDEV_TYPES && + (cap->data.pci_dev.flags & + VIR_NODE_DEV_CAP_FLAG_PCI_MDEV)) + return true; + } } return false; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 83e979a2b..b22952960 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -665,6 +665,7 @@ virNetDevIPRouteParseXML; # conf/node_device_conf.h +virNodeDevCapMdevTypeFree; virNodeDevCapsDefFree; virNodeDevCapTypeFromString; virNodeDevCapTypeToString; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 1ddb55c80..4c75dec2b 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -314,6 +314,119 @@ static int udevTranslatePCIIds(unsigned int vendor, } +static int +udevFillMdevType(struct udev_device *device, + const char *dir, + virNodeDevCapMdevTypePtr type) +{ + int ret = -1; + char *attrpath = NULL; + +#define MDEV_GET_SYSFS_ATTR(attr_name, cb, ...) \ + do { \ + if (virAsprintf(&attrpath, "%s/%s", dir, #attr_name) < 0) \ + goto cleanup; \ + \ + if (cb(device, attrpath, __VA_ARGS__) < 0) \ + goto cleanup; \ + \ + VIR_FREE(attrpath); \ + } while (0) \ + + if (VIR_STRDUP(type->id, last_component(dir)) < 0) + goto cleanup; + + /* query udev for the attributes under subdirectories using the relative + * path stored in @dir, i.e. 'mdev_supported_types/<type_id>' + */ + MDEV_GET_SYSFS_ATTR(name, udevGetStringSysfsAttr, &type->name); + MDEV_GET_SYSFS_ATTR(device_api, udevGetStringSysfsAttr, &type->device_api); + MDEV_GET_SYSFS_ATTR(available_instances, udevGetUintSysfsAttr, + &type->available_instances, 10); + +#undef MDEV_GET_SYSFS_ATTR + + ret = 0; + cleanup: + VIR_FREE(attrpath); + return ret; +} + + +static int +udevPCIGetMdevTypesCap(struct udev_device *device, + virNodeDevCapPCIDevPtr pcidata) +{ + int ret = -1; + int dirret = -1; + DIR *dir = NULL; + struct dirent *entry; + char *path = NULL; + char *tmppath = NULL; + virNodeDevCapMdevTypePtr type = NULL; + virNodeDevCapMdevTypePtr *types = NULL; + size_t ntypes = 0; + size_t i; + + if (virAsprintf(&path, "%s/mdev_supported_types", + udev_device_get_syspath(device)) < 0) + return -1; + + if ((dirret = virDirOpenIfExists(&dir, path)) < 0) + goto cleanup; + + if (dirret == 0) { + ret = 0; + goto cleanup; + } + + if (VIR_ALLOC(types) < 0) + goto cleanup; + + /* UDEV doesn't report attributes under subdirectories by default but is + * able to query them if the path to the attribute is relative to the + * device's base path, e.g. /sys/devices/../0000:00:01.0/ is the device's + * base path as udev reports it, but we're interested in attributes under + * /sys/devices/../0000:00:01.0/mdev_supported_types/<type>/. So, we need to + * scan the subdirectories ourselves. + */ + while ((dirret = virDirRead(dir, &entry, path)) > 0) { + if (VIR_ALLOC(type) < 0) + goto cleanup; + + /* construct the relative mdev type path bit for udev */ + if (virAsprintf(&tmppath, "mdev_supported_types/%s", entry->d_name) < 0) + goto cleanup; + + if (udevFillMdevType(device, tmppath, type) < 0) + goto cleanup; + + if (VIR_APPEND_ELEMENT(types, ntypes, type) < 0) + goto cleanup; + + VIR_FREE(tmppath); + } + + if (dirret < 0) + goto cleanup; + + VIR_STEAL_PTR(pcidata->mdev_types, types); + pcidata->nmdev_types = ntypes; + pcidata->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; + ntypes = 0; + ret = 0; + cleanup: + virNodeDevCapMdevTypeFree(type); + for (i = 0; i < ntypes; i++) + virNodeDevCapMdevTypeFree(types[i]); + VIR_FREE(types); + VIR_FREE(path); + VIR_FREE(tmppath); + VIR_DIR_CLOSE(dir); + return ret; +} + + static int udevProcessPCI(struct udev_device *device, virNodeDeviceDefPtr def) { @@ -400,6 +513,12 @@ static int udevProcessPCI(struct udev_device *device, } } + /* check whether the device is mediated devices framework capable, if so, + * process it + */ + if (udevPCIGetMdevTypesCap(device, pci_dev) < 0) + goto cleanup; + ret = 0; cleanup: diff --git a/tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml b/tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml new file mode 100644 index 000000000..a2d57569a --- /dev/null +++ b/tests/nodedevschemadata/pci_0000_02_10_7_mdev_types.xml @@ -0,0 +1,32 @@ +<device> + <name>pci_0000_02_10_7</name> + <parent>pci_0000_00_04_0</parent> + <capability type='pci'> + <domain>0</domain> + <bus>2</bus> + <slot>16</slot> + <function>7</function> + <product id='0x10ca'>82576 Virtual Function</product> + <vendor id='0x8086'>Intel Corporation</vendor> + <capability type='mdev_types'> + <type id='foo1'> + <name>bar1</name> + <deviceAPI>vfio-pci</deviceAPI> + <availableInstances>1</availableInstances> + </type> + <type id='foo2'> + <name>bar2</name> + <deviceAPI>vfio-pci</deviceAPI> + <availableInstances>2</availableInstances> + </type> + </capability> + <iommuGroup number='31'> + <address domain='0x0000' bus='0x02' slot='0x10' function='0x7'/> + </iommuGroup> + <numa node='0'/> + <pci-express> + <link validity='cap' port='0' speed='2.5' width='4'/> + <link validity='sta' width='0'/> + </pci-express> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index f023d8a13..e3a77646c 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -101,6 +101,7 @@ mymain(void) DO_TEST("pci_0000_02_10_7_sriov_pf_vfs_all"); DO_TEST("pci_0000_02_10_7_sriov_pf_vfs_all_header_type"); DO_TEST("drm_renderD129"); + DO_TEST("pci_0000_02_10_7_mdev_types"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.12.2

Start discovering the mediated devices on the host system and format the attributes for the mediated device into the XML. Compared to the parent device which reports generic information about the abstract mediated devices types, a child device only reports the type name it has been instantiated from and the IOMMU group number, since that's device specific compared to the rest of the info that can be gathered about mediated devices at the moment. This patch introduces both the formatting and parsing routines, updates nodedev.rng schema, adding a testcase as well. The resulting mdev child device XML: <device> <name>mdev_4b20d080_1b54_4048_85b3_a6a62d165c01</name> <path>/sys/devices/.../4b20d080-1b54-4048-85b3-a6a62d165c01</path> <parent>pci_0000_06_00_0</parent> <driver> <name>vfio_mdev</name> </driver> <capability type='mdev'> <type id='vendor_supplied_type_id'/> <iommuGroup number='NUM'/> <capability/> <device/> Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- docs/schemas/nodedev.rng | 17 +++++++++ src/conf/node_device_conf.c | 41 +++++++++++++++++++++ src/conf/node_device_conf.h | 8 ++++ src/node_device/node_device_udev.c | 43 +++++++++++++++++++++- .../mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml | 8 ++++ tests/nodedevxml2xmltest.c | 1 + 6 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 tests/nodedevschemadata/mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index e0a2c5032..924f73861 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -83,6 +83,7 @@ <ref name="capscsi"/> <ref name="capstorage"/> <ref name="capdrm"/> + <ref name="capmdev"/> </choice> </element> </define> @@ -580,6 +581,22 @@ </element> </define> + <define name='capmdev'> + <attribute name='type'> + <value>mdev</value> + </attribute> + <element name='type'> + <attribute name='id'> + <data type='string'/> + </attribute> + </element> + <element name='iommuGroup'> + <attribute name='number'> + <ref name='unsignedInt'/> + </attribute> + </element> + </define> + <define name='address'> <element name='address'> <attribute name='domain'><ref name='hexuint'/></attribute> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index b3012821a..01b48324c 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -584,6 +584,10 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferEscapeString(&buf, "<type>%s</type>\n", virNodeDevDRMTypeToString(data->drm.type)); break; case VIR_NODE_DEV_CAP_MDEV: + virBufferEscapeString(&buf, "<type id='%s'/>\n", data->mdev.type); + virBufferAsprintf(&buf, "<iommuGroup number='%u'/>\n", + data->mdev.iommuGroupNumber); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -1650,6 +1654,39 @@ virNodeDevCapSystemParseXML(xmlXPathContextPtr ctxt, } +static int +virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt, + virNodeDeviceDefPtr def, + xmlNodePtr node, + virNodeDevCapMdevPtr mdev) +{ + xmlNodePtr orignode; + int ret = -1; + + orignode = ctxt->node; + ctxt->node = node; + + if (!(mdev->type = virXPathString("string(./type[1]/@id)", ctxt))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing type id attribute for '%s'"), def->name); + goto out; + } + + if (virNodeDevCapsDefParseULong("number(./iommuGroup[1]/@number)", ctxt, + &mdev->iommuGroupNumber, def, + _("missing iommuGroup number atribute for " + "'%s'"), + _("invalid iommuGroup number attribute for " + "'%s'")) < 0) + goto out; + + ret = 0; + out: + ctxt->node = orignode; + return ret; +} + + static virNodeDevCapsDefPtr virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, @@ -1718,6 +1755,8 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, ret = virNodeDevCapDRMParseXML(ctxt, def, node, &caps->data.drm); break; case VIR_NODE_DEV_CAP_MDEV: + ret = virNodeDevCapMdevParseXML(ctxt, def, node, &caps->data.mdev); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: @@ -2041,6 +2080,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) VIR_FREE(data->sg.path); break; case VIR_NODE_DEV_CAP_MDEV: + VIR_FREE(data->mdev.type); + break; case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_DRM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 629f732e6..cea9247d2 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -143,6 +143,13 @@ struct _virNodeDevCapMdevType { unsigned int available_instances; }; +typedef struct _virNodeDevCapMdev virNodeDevCapMdev; +typedef virNodeDevCapMdev *virNodeDevCapMdevPtr; +struct _virNodeDevCapMdev { + char *type; + unsigned int iommuGroupNumber; +}; + typedef struct _virNodeDevCapPCIDev virNodeDevCapPCIDev; typedef virNodeDevCapPCIDev *virNodeDevCapPCIDevPtr; struct _virNodeDevCapPCIDev { @@ -276,6 +283,7 @@ struct _virNodeDevCapData { virNodeDevCapStorage storage; virNodeDevCapSCSIGeneric sg; virNodeDevCapDRM drm; + virNodeDevCapMdev mdev; }; }; diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 4c75dec2b..8288be1cb 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1069,6 +1069,42 @@ udevProcessSCSIGeneric(struct udev_device *dev, } static int +udevProcessMediatedDevice(struct udev_device *dev, + virNodeDeviceDefPtr def) +{ + int ret = -1; + const char *uuidstr = NULL; + int iommugrp = -1; + char *linkpath = NULL; + char *realpath = NULL; + virNodeDevCapMdevPtr data = &def->caps->data.mdev; + + if (virAsprintf(&linkpath, "%s/mdev_type", udev_device_get_syspath(dev)) < 0) + goto cleanup; + + if (virFileResolveLink(linkpath, &realpath) < 0) + goto cleanup; + + if (VIR_STRDUP(data->type, last_component(realpath)) < 0) + goto cleanup; + + uuidstr = udev_device_get_sysname(dev); + if ((iommugrp = virMediatedDeviceGetIOMMUGroupNum(uuidstr)) < 0) + goto cleanup; + + if (udevGenerateDeviceName(dev, def, NULL) != 0) + goto cleanup; + + data->iommuGroupNumber = iommugrp; + + ret = 0; + cleanup: + VIR_FREE(linkpath); + VIR_FREE(realpath); + return ret; +} + +static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) { @@ -1136,12 +1172,16 @@ udevGetDeviceType(struct udev_device *device, if (udevHasDeviceProperty(device, "INTERFACE")) *type = VIR_NODE_DEV_CAP_NET; - /* SCSI generic device doesn't set DEVTYPE property */ + /* Neither SCSI generic devices nor mediated devices set DEVTYPE + * property, therefore we need to rely on the SUBSYSTEM property */ if (udevGetStringProperty(device, "SUBSYSTEM", &subsystem) < 0) return -1; if (STREQ_NULLABLE(subsystem, "scsi_generic")) *type = VIR_NODE_DEV_CAP_SCSI_GENERIC; + else if (STREQ_NULLABLE(subsystem, "mdev")) + *type = VIR_NODE_DEV_CAP_MDEV; + VIR_FREE(subsystem); } @@ -1181,6 +1221,7 @@ static int udevGetDeviceDetails(struct udev_device *device, case VIR_NODE_DEV_CAP_DRM: return udevProcessDRMDevice(device, def); case VIR_NODE_DEV_CAP_MDEV: + return udevProcessMediatedDevice(device, def); case VIR_NODE_DEV_CAP_MDEV_TYPES: case VIR_NODE_DEV_CAP_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: diff --git a/tests/nodedevschemadata/mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml b/tests/nodedevschemadata/mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml new file mode 100644 index 000000000..470e5917e --- /dev/null +++ b/tests/nodedevschemadata/mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml @@ -0,0 +1,8 @@ +<device> + <name>mdev_3627463d_b7f0_4fea_b468_f1da537d301b</name> + <parent>computer</parent> + <capability type='mdev'> + <type id='mtty-1'/> + <iommuGroup number='12'/> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index e3a77646c..a2aad518d 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -102,6 +102,7 @@ mymain(void) DO_TEST("pci_0000_02_10_7_sriov_pf_vfs_all_header_type"); DO_TEST("drm_renderD129"); DO_TEST("pci_0000_02_10_7_mdev_types"); + DO_TEST("mdev_3627463d_b7f0_4fea_b468_f1da537d301b"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.12.2

On Wed, Apr 26, 2017 at 04:55:33PM +0200, Erik Skultety wrote: s/3.3.0/3.4.0/ for the whole patch. Please add the "mdev" capability into the list of valid capability types for virsh nodedev-list command. Replace the <br/> by </p><p>. Replace the <br/> by </p><p>.