So udevGetDeviceDetails was one those functions using an enum in a switch, but since it had a 'default' case, compiler didn't warn about an unhandled enum. Moreover, the error about an unsupported device type reported in the default case is unnecessary, since by the time we get there, udevGetDeviceType (which was called before) already made sure that any unrecognized device types had been handled properly. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/node_device/node_device_udev.c | 45 +++++++++++++------------------------- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index bcae444d8..591da8db2 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1043,50 +1043,35 @@ udevGetDeviceType(struct udev_device *device, static int udevGetDeviceDetails(struct udev_device *device, virNodeDeviceDefPtr def) { - int ret = 0; - switch (def->caps->data.type) { - case VIR_NODE_DEV_CAP_SYSTEM: - /* There's no libudev equivalent of system, so ignore it. */ - break; case VIR_NODE_DEV_CAP_PCI_DEV: - ret = udevProcessPCI(device, def); - break; + return udevProcessPCI(device, def); case VIR_NODE_DEV_CAP_USB_DEV: - ret = udevProcessUSBDevice(device, def); - break; + return udevProcessUSBDevice(device, def); case VIR_NODE_DEV_CAP_USB_INTERFACE: - ret = udevProcessUSBInterface(device, def); - break; + return udevProcessUSBInterface(device, def); case VIR_NODE_DEV_CAP_NET: - ret = udevProcessNetworkInterface(device, def); - break; + return udevProcessNetworkInterface(device, def); case VIR_NODE_DEV_CAP_SCSI_HOST: - ret = udevProcessSCSIHost(device, def); - break; + return udevProcessSCSIHost(device, def); case VIR_NODE_DEV_CAP_SCSI_TARGET: - ret = udevProcessSCSITarget(device, def); - break; + return udevProcessSCSITarget(device, def); case VIR_NODE_DEV_CAP_SCSI: - ret = udevProcessSCSIDevice(device, def); - break; + return udevProcessSCSIDevice(device, def); case VIR_NODE_DEV_CAP_STORAGE: - ret = udevProcessStorage(device, def); - break; + return udevProcessStorage(device, def); case VIR_NODE_DEV_CAP_SCSI_GENERIC: - ret = udevProcessSCSIGeneric(device, def); - break; + return udevProcessSCSIGeneric(device, def); case VIR_NODE_DEV_CAP_DRM: - ret = udevProcessDRMDevice(device, def); - break; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown device type %d"), def->caps->data.type); - ret = -1; + return udevProcessDRMDevice(device, def); + case VIR_NODE_DEV_CAP_SYSTEM: + case VIR_NODE_DEV_CAP_FC_HOST: + case VIR_NODE_DEV_CAP_VPORTS: + case VIR_NODE_DEV_CAP_LAST: break; } - return ret; + return 0; } -- 2.12.2

Make the code look cleaner by moving the capability specific bits into separate functions. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/conf/node_device_conf.c | 578 ++++++++++++++++++++++++-------------------- 1 file changed, 322 insertions(+), 256 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index d2ad6352b..85cfd8396 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -155,6 +155,320 @@ virPCIEDeviceInfoFormat(virBufferPtr buf, } +static void +virNodeDeviceCapSystemDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (data->system.product_name) + virBufferEscapeString(buf, "<product>%s</product>\n", + data->system.product_name); + virBufferAddLit(buf, "<hardware>\n"); + virBufferAdjustIndent(buf, 2); + if (data->system.hardware.vendor_name) + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", + data->system.hardware.vendor_name); + if (data->system.hardware.version) + virBufferEscapeString(buf, "<version>%s</version>\n", + data->system.hardware.version); + if (data->system.hardware.serial) + virBufferEscapeString(buf, "<serial>%s</serial>\n", + data->system.hardware.serial); + virUUIDFormat(data->system.hardware.uuid, uuidstr); + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</hardware>\n"); + + virBufferAddLit(buf, "<firmware>\n"); + virBufferAdjustIndent(buf, 2); + if (data->system.firmware.vendor_name) + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", + data->system.firmware.vendor_name); + if (data->system.firmware.version) + virBufferEscapeString(buf, "<version>%s</version>\n", + data->system.firmware.version); + if (data->system.firmware.release_date) + virBufferEscapeString(buf, "<release_date>%s</release_date>\n", + data->system.firmware.release_date); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</firmware>\n"); +} + + +static void +virNodeDeviceCapPCIDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + size_t i; + + virBufferAsprintf(buf, "<domain>%d</domain>\n", + data->pci_dev.domain); + virBufferAsprintf(buf, "<bus>%d</bus>\n", data->pci_dev.bus); + virBufferAsprintf(buf, "<slot>%d</slot>\n", + data->pci_dev.slot); + virBufferAsprintf(buf, "<function>%d</function>\n", + data->pci_dev.function); + virBufferAsprintf(buf, "<product id='0x%04x'", + data->pci_dev.product); + if (data->pci_dev.product_name) + virBufferEscapeString(buf, ">%s</product>\n", + data->pci_dev.product_name); + else + virBufferAddLit(buf, " />\n"); + virBufferAsprintf(buf, "<vendor id='0x%04x'", + data->pci_dev.vendor); + if (data->pci_dev.vendor_name) + virBufferEscapeString(buf, ">%s</vendor>\n", + data->pci_dev.vendor_name); + else + virBufferAddLit(buf, " />\n"); + if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION) { + virBufferAddLit(buf, "<capability type='phys_function'>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, + "<address domain='0x%.4x' bus='0x%.2x' " + "slot='0x%.2x' function='0x%.1x'/>\n", + data->pci_dev.physical_function->domain, + data->pci_dev.physical_function->bus, + data->pci_dev.physical_function->slot, + data->pci_dev.physical_function->function); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</capability>\n"); + } + if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION) { + virBufferAddLit(buf, "<capability type='virt_functions'"); + if (data->pci_dev.max_virtual_functions) + virBufferAsprintf(buf, " maxCount='%u'", + data->pci_dev.max_virtual_functions); + if (data->pci_dev.num_virtual_functions == 0) { + virBufferAddLit(buf, "/>\n"); + } else { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < data->pci_dev.num_virtual_functions; i++) { + virBufferAsprintf(buf, + "<address domain='0x%.4x' bus='0x%.2x' " + "slot='0x%.2x' function='0x%.1x'/>\n", + data->pci_dev.virtual_functions[i]->domain, + data->pci_dev.virtual_functions[i]->bus, + data->pci_dev.virtual_functions[i]->slot, + data->pci_dev.virtual_functions[i]->function); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</capability>\n"); + } + } + if (data->pci_dev.hdrType) { + virBufferAsprintf(buf, "<capability type='%s'/>\n", + virPCIHeaderTypeToString(data->pci_dev.hdrType)); + } + if (data->pci_dev.nIommuGroupDevices) { + virBufferAsprintf(buf, "<iommuGroup number='%d'>\n", + data->pci_dev.iommuGroupNumber); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++) { + virBufferAsprintf(buf, + "<address domain='0x%.4x' bus='0x%.2x' " + "slot='0x%.2x' function='0x%.1x'/>\n", + data->pci_dev.iommuGroupDevices[i]->domain, + data->pci_dev.iommuGroupDevices[i]->bus, + data->pci_dev.iommuGroupDevices[i]->slot, + data->pci_dev.iommuGroupDevices[i]->function); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</iommuGroup>\n"); + } + if (data->pci_dev.numa_node >= 0) + virBufferAsprintf(buf, "<numa node='%d'/>\n", + data->pci_dev.numa_node); + + if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCIE) + virPCIEDeviceInfoFormat(buf, data->pci_dev.pci_express); +} + + +static void +virNodeDeviceCapUSBDevDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + virBufferAsprintf(buf, "<bus>%d</bus>\n", data->usb_dev.bus); + virBufferAsprintf(buf, "<device>%d</device>\n", + data->usb_dev.device); + virBufferAsprintf(buf, "<product id='0x%04x'", + data->usb_dev.product); + if (data->usb_dev.product_name) + virBufferEscapeString(buf, ">%s</product>\n", + data->usb_dev.product_name); + else + virBufferAddLit(buf, " />\n"); + virBufferAsprintf(buf, "<vendor id='0x%04x'", + data->usb_dev.vendor); + if (data->usb_dev.vendor_name) + virBufferEscapeString(buf, ">%s</vendor>\n", + data->usb_dev.vendor_name); + else + virBufferAddLit(buf, " />\n"); +} + + +static void +virNodeDeviceCapUSBInterfaceDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + virBufferAsprintf(buf, "<number>%d</number>\n", + data->usb_if.number); + virBufferAsprintf(buf, "<class>%d</class>\n", + data->usb_if._class); + virBufferAsprintf(buf, "<subclass>%d</subclass>\n", + data->usb_if.subclass); + virBufferAsprintf(buf, "<protocol>%d</protocol>\n", + data->usb_if.protocol); + if (data->usb_if.description) + virBufferEscapeString(buf, + "<description>%s</description>\n", + data->usb_if.description); +} + + +static void +virNodeDeviceCapNetDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + size_t i; + + virBufferEscapeString(buf, "<interface>%s</interface>\n", + data->net.ifname); + if (data->net.address) + virBufferEscapeString(buf, "<address>%s</address>\n", + data->net.address); + virInterfaceLinkFormat(buf, &data->net.lnk); + if (data->net.features) { + for (i = 0; i < VIR_NET_DEV_FEAT_LAST; i++) { + if (virBitmapIsBitSet(data->net.features, i)) { + virBufferAsprintf(buf, "<feature name='%s'/>\n", + virNetDevFeatureTypeToString(i)); + } + } + } + if (data->net.subtype != VIR_NODE_DEV_CAP_NET_LAST) { + const char *subtyp = + virNodeDevNetCapTypeToString(data->net.subtype); + virBufferEscapeString(buf, "<capability type='%s'/>\n", + subtyp); + } +} + + +static void +virNodeDeviceCapSCSIHostDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + virBufferAsprintf(buf, "<host>%d</host>\n", + data->scsi_host.host); + if (data->scsi_host.unique_id != -1) + virBufferAsprintf(buf, "<unique_id>%d</unique_id>\n", + data->scsi_host.unique_id); + if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + virBufferAddLit(buf, "<capability type='fc_host'>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<wwnn>%s</wwnn>\n", + data->scsi_host.wwnn); + virBufferEscapeString(buf, "<wwpn>%s</wwpn>\n", + data->scsi_host.wwpn); + virBufferEscapeString(buf, "<fabric_wwn>%s</fabric_wwn>\n", + data->scsi_host.fabric_wwn); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</capability>\n"); + } + if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { + virBufferAddLit(buf, "<capability type='vport_ops'>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<max_vports>%d</max_vports>\n", + data->scsi_host.max_vports); + virBufferAsprintf(buf, "<vports>%d</vports>\n", + data->scsi_host.vports); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</capability>\n"); + } +} + + +static void +virNodeDeviceCapSCSIDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + virBufferAsprintf(buf, "<host>%d</host>\n", data->scsi.host); + virBufferAsprintf(buf, "<bus>%d</bus>\n", data->scsi.bus); + virBufferAsprintf(buf, "<target>%d</target>\n", + data->scsi.target); + virBufferAsprintf(buf, "<lun>%d</lun>\n", data->scsi.lun); + if (data->scsi.type) + virBufferEscapeString(buf, "<type>%s</type>\n", + data->scsi.type); +} + + +static void +virNodeDeviceCapStorageDefFormat(virBufferPtr buf, + const virNodeDevCapData *data) +{ + virBufferEscapeString(buf, "<block>%s</block>\n", + data->storage.block); + if (data->storage.bus) + virBufferEscapeString(buf, "<bus>%s</bus>\n", + data->storage.bus); + if (data->storage.drive_type) + virBufferEscapeString(buf, "<drive_type>%s</drive_type>\n", + data->storage.drive_type); + if (data->storage.model) + virBufferEscapeString(buf, "<model>%s</model>\n", + data->storage.model); + if (data->storage.vendor) + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", + data->storage.vendor); + if (data->storage.serial) + virBufferEscapeString(buf, "<serial>%s</serial>\n", + data->storage.serial); + if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE) { + int avl = data->storage.flags & + VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE; + virBufferAddLit(buf, "<capability type='removable'>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<media_available>%d" + "</media_available>\n", avl ? 1 : 0); + virBufferAsprintf(buf, "<media_size>%llu</media_size>\n", + data->storage.removable_media_size); + if (data->storage.media_label) + virBufferEscapeString(buf, + "<media_label>%s</media_label>\n", + data->storage.media_label); + if (data->storage.logical_block_size > 0) + virBufferAsprintf(buf, "<logical_block_size>%llu" + "</logical_block_size>\n", + data->storage.logical_block_size); + if (data->storage.num_blocks > 0) + virBufferAsprintf(buf, + "<num_blocks>%llu</num_blocks>\n", + data->storage.num_blocks); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</capability>\n"); + } else { + virBufferAsprintf(buf, "<size>%llu</size>\n", + data->storage.size); + if (data->storage.logical_block_size > 0) + virBufferAsprintf(buf, "<logical_block_size>%llu" + "</logical_block_size>\n", + data->storage.logical_block_size); + if (data->storage.num_blocks > 0) + virBufferAsprintf(buf, "<num_blocks>%llu</num_blocks>\n", + data->storage.num_blocks); + } + if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE) + virBufferAddLit(buf, "<capability type='hotpluggable'/>\n"); +} + + char * virNodeDeviceDefFormat(const virNodeDeviceDef *def) { @@ -185,7 +499,6 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) } for (caps = def->caps; caps; caps = caps->next) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; virNodeDevCapDataPtr data = &caps->data; virBufferAsprintf(&buf, "<capability type='%s'>\n", @@ -193,279 +506,32 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferAdjustIndent(&buf, 2); switch (caps->data.type) { case VIR_NODE_DEV_CAP_SYSTEM: - if (data->system.product_name) - virBufferEscapeString(&buf, "<product>%s</product>\n", - data->system.product_name); - virBufferAddLit(&buf, "<hardware>\n"); - virBufferAdjustIndent(&buf, 2); - if (data->system.hardware.vendor_name) - virBufferEscapeString(&buf, "<vendor>%s</vendor>\n", - data->system.hardware.vendor_name); - if (data->system.hardware.version) - virBufferEscapeString(&buf, "<version>%s</version>\n", - data->system.hardware.version); - if (data->system.hardware.serial) - virBufferEscapeString(&buf, "<serial>%s</serial>\n", - data->system.hardware.serial); - virUUIDFormat(data->system.hardware.uuid, uuidstr); - virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuidstr); - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</hardware>\n"); - - virBufferAddLit(&buf, "<firmware>\n"); - virBufferAdjustIndent(&buf, 2); - if (data->system.firmware.vendor_name) - virBufferEscapeString(&buf, "<vendor>%s</vendor>\n", - data->system.firmware.vendor_name); - if (data->system.firmware.version) - virBufferEscapeString(&buf, "<version>%s</version>\n", - data->system.firmware.version); - if (data->system.firmware.release_date) - virBufferEscapeString(&buf, "<release_date>%s</release_date>\n", - data->system.firmware.release_date); - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</firmware>\n"); + virNodeDeviceCapSystemDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_PCI_DEV: - virBufferAsprintf(&buf, "<domain>%d</domain>\n", - data->pci_dev.domain); - virBufferAsprintf(&buf, "<bus>%d</bus>\n", data->pci_dev.bus); - virBufferAsprintf(&buf, "<slot>%d</slot>\n", - data->pci_dev.slot); - virBufferAsprintf(&buf, "<function>%d</function>\n", - data->pci_dev.function); - virBufferAsprintf(&buf, "<product id='0x%04x'", - data->pci_dev.product); - if (data->pci_dev.product_name) - virBufferEscapeString(&buf, ">%s</product>\n", - data->pci_dev.product_name); - else - virBufferAddLit(&buf, " />\n"); - virBufferAsprintf(&buf, "<vendor id='0x%04x'", - data->pci_dev.vendor); - if (data->pci_dev.vendor_name) - virBufferEscapeString(&buf, ">%s</vendor>\n", - data->pci_dev.vendor_name); - else - virBufferAddLit(&buf, " />\n"); - if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION) { - virBufferAddLit(&buf, "<capability type='phys_function'>\n"); - virBufferAdjustIndent(&buf, 2); - virBufferAsprintf(&buf, - "<address domain='0x%.4x' bus='0x%.2x' " - "slot='0x%.2x' function='0x%.1x'/>\n", - data->pci_dev.physical_function->domain, - data->pci_dev.physical_function->bus, - data->pci_dev.physical_function->slot, - data->pci_dev.physical_function->function); - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</capability>\n"); - } - if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION) { - virBufferAddLit(&buf, "<capability type='virt_functions'"); - if (data->pci_dev.max_virtual_functions) - virBufferAsprintf(&buf, " maxCount='%u'", - data->pci_dev.max_virtual_functions); - if (data->pci_dev.num_virtual_functions == 0) { - virBufferAddLit(&buf, "/>\n"); - } else { - virBufferAddLit(&buf, ">\n"); - virBufferAdjustIndent(&buf, 2); - for (i = 0; i < data->pci_dev.num_virtual_functions; i++) { - virBufferAsprintf(&buf, - "<address domain='0x%.4x' bus='0x%.2x' " - "slot='0x%.2x' function='0x%.1x'/>\n", - data->pci_dev.virtual_functions[i]->domain, - data->pci_dev.virtual_functions[i]->bus, - data->pci_dev.virtual_functions[i]->slot, - data->pci_dev.virtual_functions[i]->function); - } - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</capability>\n"); - } - } - if (data->pci_dev.hdrType) { - virBufferAsprintf(&buf, "<capability type='%s'/>\n", - virPCIHeaderTypeToString(data->pci_dev.hdrType)); - } - if (data->pci_dev.nIommuGroupDevices) { - virBufferAsprintf(&buf, "<iommuGroup number='%d'>\n", - data->pci_dev.iommuGroupNumber); - virBufferAdjustIndent(&buf, 2); - for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++) { - virBufferAsprintf(&buf, - "<address domain='0x%.4x' bus='0x%.2x' " - "slot='0x%.2x' function='0x%.1x'/>\n", - data->pci_dev.iommuGroupDevices[i]->domain, - data->pci_dev.iommuGroupDevices[i]->bus, - data->pci_dev.iommuGroupDevices[i]->slot, - data->pci_dev.iommuGroupDevices[i]->function); - } - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</iommuGroup>\n"); - } - if (data->pci_dev.numa_node >= 0) - virBufferAsprintf(&buf, "<numa node='%d'/>\n", - data->pci_dev.numa_node); - - if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCIE) - virPCIEDeviceInfoFormat(&buf, data->pci_dev.pci_express); + virNodeDeviceCapPCIDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_USB_DEV: - virBufferAsprintf(&buf, "<bus>%d</bus>\n", data->usb_dev.bus); - virBufferAsprintf(&buf, "<device>%d</device>\n", - data->usb_dev.device); - virBufferAsprintf(&buf, "<product id='0x%04x'", - data->usb_dev.product); - if (data->usb_dev.product_name) - virBufferEscapeString(&buf, ">%s</product>\n", - data->usb_dev.product_name); - else - virBufferAddLit(&buf, " />\n"); - virBufferAsprintf(&buf, "<vendor id='0x%04x'", - data->usb_dev.vendor); - if (data->usb_dev.vendor_name) - virBufferEscapeString(&buf, ">%s</vendor>\n", - data->usb_dev.vendor_name); - else - virBufferAddLit(&buf, " />\n"); + virNodeDeviceCapUSBDevDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_USB_INTERFACE: - virBufferAsprintf(&buf, "<number>%d</number>\n", - data->usb_if.number); - virBufferAsprintf(&buf, "<class>%d</class>\n", - data->usb_if._class); - virBufferAsprintf(&buf, "<subclass>%d</subclass>\n", - data->usb_if.subclass); - virBufferAsprintf(&buf, "<protocol>%d</protocol>\n", - data->usb_if.protocol); - if (data->usb_if.description) - virBufferEscapeString(&buf, - "<description>%s</description>\n", - data->usb_if.description); + virNodeDeviceCapUSBInterfaceDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_NET: - virBufferEscapeString(&buf, "<interface>%s</interface>\n", - data->net.ifname); - if (data->net.address) - virBufferEscapeString(&buf, "<address>%s</address>\n", - data->net.address); - virInterfaceLinkFormat(&buf, &data->net.lnk); - if (data->net.features) { - for (i = 0; i < VIR_NET_DEV_FEAT_LAST; i++) { - if (virBitmapIsBitSet(data->net.features, i)) { - virBufferAsprintf(&buf, "<feature name='%s'/>\n", - virNetDevFeatureTypeToString(i)); - } - } - } - if (data->net.subtype != VIR_NODE_DEV_CAP_NET_LAST) { - const char *subtyp = - virNodeDevNetCapTypeToString(data->net.subtype); - virBufferEscapeString(&buf, "<capability type='%s'/>\n", - subtyp); - } + virNodeDeviceCapNetDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_SCSI_HOST: - virBufferAsprintf(&buf, "<host>%d</host>\n", - data->scsi_host.host); - if (data->scsi_host.unique_id != -1) - virBufferAsprintf(&buf, "<unique_id>%d</unique_id>\n", - data->scsi_host.unique_id); - if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { - virBufferAddLit(&buf, "<capability type='fc_host'>\n"); - virBufferAdjustIndent(&buf, 2); - virBufferEscapeString(&buf, "<wwnn>%s</wwnn>\n", - data->scsi_host.wwnn); - virBufferEscapeString(&buf, "<wwpn>%s</wwpn>\n", - data->scsi_host.wwpn); - virBufferEscapeString(&buf, "<fabric_wwn>%s</fabric_wwn>\n", - data->scsi_host.fabric_wwn); - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</capability>\n"); - } - if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { - virBufferAddLit(&buf, "<capability type='vport_ops'>\n"); - virBufferAdjustIndent(&buf, 2); - virBufferAsprintf(&buf, "<max_vports>%d</max_vports>\n", - data->scsi_host.max_vports); - virBufferAsprintf(&buf, "<vports>%d</vports>\n", - data->scsi_host.vports); - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</capability>\n"); - } - + virNodeDeviceCapSCSIHostDefFormat(&buf, data); break; - case VIR_NODE_DEV_CAP_SCSI_TARGET: virBufferEscapeString(&buf, "<target>%s</target>\n", data->scsi_target.name); break; - case VIR_NODE_DEV_CAP_SCSI: - virBufferAsprintf(&buf, "<host>%d</host>\n", data->scsi.host); - virBufferAsprintf(&buf, "<bus>%d</bus>\n", data->scsi.bus); - virBufferAsprintf(&buf, "<target>%d</target>\n", - data->scsi.target); - virBufferAsprintf(&buf, "<lun>%d</lun>\n", data->scsi.lun); - if (data->scsi.type) - virBufferEscapeString(&buf, "<type>%s</type>\n", - data->scsi.type); + virNodeDeviceCapSCSIDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_STORAGE: - virBufferEscapeString(&buf, "<block>%s</block>\n", - data->storage.block); - if (data->storage.bus) - virBufferEscapeString(&buf, "<bus>%s</bus>\n", - data->storage.bus); - if (data->storage.drive_type) - virBufferEscapeString(&buf, "<drive_type>%s</drive_type>\n", - data->storage.drive_type); - if (data->storage.model) - virBufferEscapeString(&buf, "<model>%s</model>\n", - data->storage.model); - if (data->storage.vendor) - virBufferEscapeString(&buf, "<vendor>%s</vendor>\n", - data->storage.vendor); - if (data->storage.serial) - virBufferEscapeString(&buf, "<serial>%s</serial>\n", - data->storage.serial); - if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE) { - int avl = data->storage.flags & - VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE; - virBufferAddLit(&buf, "<capability type='removable'>\n"); - virBufferAdjustIndent(&buf, 2); - virBufferAsprintf(&buf, "<media_available>%d" - "</media_available>\n", avl ? 1 : 0); - virBufferAsprintf(&buf, "<media_size>%llu</media_size>\n", - data->storage.removable_media_size); - if (data->storage.media_label) - virBufferEscapeString(&buf, - "<media_label>%s</media_label>\n", - data->storage.media_label); - if (data->storage.logical_block_size > 0) - virBufferAsprintf(&buf, "<logical_block_size>%llu" - "</logical_block_size>\n", - data->storage.logical_block_size); - if (data->storage.num_blocks > 0) - virBufferAsprintf(&buf, - "<num_blocks>%llu</num_blocks>\n", - data->storage.num_blocks); - virBufferAdjustIndent(&buf, -2); - virBufferAddLit(&buf, "</capability>\n"); - } else { - virBufferAsprintf(&buf, "<size>%llu</size>\n", - data->storage.size); - if (data->storage.logical_block_size > 0) - virBufferAsprintf(&buf, "<logical_block_size>%llu" - "</logical_block_size>\n", - data->storage.logical_block_size); - if (data->storage.num_blocks > 0) - virBufferAsprintf(&buf, "<num_blocks>%llu</num_blocks>\n", - data->storage.num_blocks); - } - if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE) - virBufferAddLit(&buf, "<capability type='hotpluggable'/>\n"); + virNodeDeviceCapStorageDefFormat(&buf, data); break; case VIR_NODE_DEV_CAP_SCSI_GENERIC: virBufferEscapeString(&buf, "<char>%s</char>\n", -- 2.12.2

Since we have that information provided by @def which is not a private object, there is really no need for the variable. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/node_device/node_device_udev.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 591da8db2..6e706a10b 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -316,7 +316,6 @@ static int udevTranslatePCIIds(unsigned int vendor, static int udevProcessPCI(struct udev_device *device, virNodeDeviceDefPtr def) { - const char *syspath = NULL; virNodeDevCapPCIDevPtr pci_dev = &def->caps->data.pci_dev; virPCIEDeviceInfoPtr pci_express = NULL; virPCIDevicePtr pciDev = NULL; @@ -324,19 +323,17 @@ static int udevProcessPCI(struct udev_device *device, int ret = -1; char *p; - syspath = udev_device_get_syspath(device); - if (udevGetUintProperty(device, "PCI_CLASS", &pci_dev->class, 16) < 0) goto cleanup; - if ((p = strrchr(syspath, '/')) == NULL || + if ((p = strrchr(def->sysfs_path, '/')) == NULL || virStrToLong_ui(p + 1, &p, 16, &pci_dev->domain) < 0 || p == NULL || virStrToLong_ui(p + 1, &p, 16, &pci_dev->bus) < 0 || p == NULL || virStrToLong_ui(p + 1, &p, 16, &pci_dev->slot) < 0 || p == NULL || virStrToLong_ui(p + 1, &p, 16, &pci_dev->function) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse the PCI address from sysfs path: '%s'"), - syspath); + def->sysfs_path); goto cleanup; } @@ -363,8 +360,7 @@ static int udevProcessPCI(struct udev_device *device, &pci_dev->numa_node, 10) < 0) goto cleanup; - if (nodeDeviceSysfsGetPCIRelatedDevCaps(syspath, - &def->caps->data.pci_dev) < 0) + if (nodeDeviceSysfsGetPCIRelatedDevCaps(def->sysfs_path, pci_dev) < 0) goto cleanup; if (!(pciDev = virPCIDeviceNew(pci_dev->domain, -- 2.12.2

Since we do have this template at hand, why not using it wherever possible (list of supported pool types and remote access section). Also, perform some stylistic micro adjustments. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- docs/remote.html.in | 106 +++++++++++++++------------------------------------ docs/storage.html.in | 62 ++++++------------------------ 2 files changed, 41 insertions(+), 127 deletions(-) diff --git a/docs/remote.html.in b/docs/remote.html.in index 443683d51..117ee3477 100644 --- a/docs/remote.html.in +++ b/docs/remote.html.in @@ -7,57 +7,11 @@ Libvirt allows you to access hypervisors running on remote machines through authenticated and encrypted connections. </p> - <ul> - <li> - <a href="#Remote_basic_usage">Basic usage</a> - </li> - <li> - <a href="#Remote_transports">Transports</a> - </li> - <li> - <a href="#Remote_URI_reference">Remote URIs</a> - <ul> - <li> - <a href="#Remote_URI_parameters">Extra parameters</a> - </li> - </ul> - </li> - <li> - <a href="#Remote_certificates">Generating TLS certificates</a> - <ul> - <li> - <a href="#Remote_PKI">Public Key Infrastructure set up</a> - </li> - <li> - <a href="#Remote_TLS_background">Background to TLS certificates</a> - </li> - <li> - <a href="#Remote_TLS_CA">Setting up a Certificate Authority (CA)</a> - </li> - <li> - <a href="#Remote_TLS_server_certificates">Issuing server certificates</a> - </li> - <li> - <a href="#Remote_TLS_client_certificates">Issuing client certificates</a> - </li> - <li> - <a href="#Remote_TLS_troubleshooting">Troubleshooting TLS certificate problems</a> - </li> - </ul> - </li> - <li> - <a href="#Remote_libvirtd_configuration">libvirtd configuration file</a> - </li> - <li> - <a href="#Remote_IPv6">IPv6 support</a> - </li> - <li> - <a href="#Remote_limitations">Limitations</a> - </li> - </ul> - <h3> + <ul id="toc"></ul> + + <h2> <a name="Remote_basic_usage">Basic usage</a> - </h3> + </h2> <p> On the remote machine, <code>libvirtd</code> should be running in general. See <a href="#Remote_libvirtd_configuration">the section @@ -95,9 +49,9 @@ relating to failures in the remote transport itself. </li> <li> Remote calls are handled synchronously, so they will be much slower than, say, direct hypervisor calls. </li> </ul> - <h3> + <h2> <a name="Remote_transports">Transports</a> - </h3> + </h2> <p> Remote libvirt supports a range of transports: </p> @@ -156,9 +110,9 @@ netcat is required on the remote side.</dd> <p> The default transport, if no other is specified, is <code>tls</code>. </p> - <h3> + <h2> <a name="Remote_URI_reference">Remote URIs</a> - </h3> + </h2> <p> See also: <a href="uri.html">documentation on ordinary ("local") URIs</a>. </p> @@ -203,9 +157,9 @@ and use a different known_hosts file.</li> Connect to a remote host using a ssh connection with the libssh driver and use a different known_hosts file.</li> </ul> - <h4> + <h3> <a name="Remote_URI_parameters">Extra parameters</a> - </h4> + </h3> <p> Extra parameters can be added to remote URIs as part of the query string (the part following <q><code>?</code></q>). @@ -409,12 +363,12 @@ Note that parameter values must be <td> Example: <code>sshauth=privkey,agent</code> </td> </tr> </table> - <h3> + <h2> <a name="Remote_certificates">Generating TLS certificates</a> - </h3> - <h4> + </h2> + <h3> <a name="Remote_PKI">Public Key Infrastructure set up</a> - </h4> + </h3> <p> If you are unsure how to create TLS certificates, skip to the next section. @@ -517,9 +471,9 @@ next section. </li> <li> For the root user, the global default locations will always be used.</li> </ul> - <h4> + <h3> <a name="Remote_TLS_background">Background to TLS certificates</a> - </h4> + </h3> <p> Libvirt supports TLS certificates for verifying the identity of the server and clients. There are two distinct checks involved: @@ -552,9 +506,9 @@ they have a valid certificate issued by the CA for their own IP address. You may want to change this to make it less (or more) permissive, depending on your needs. </p> - <h4> + <h3> <a name="Remote_TLS_CA">Setting up a Certificate Authority (CA)</a> - </h4> + </h3> <p> You will need the <a href="http://www.gnu.org/software/gnutls/manual/html_node/Invoking-c... certtool program documented here</a>. In Fedora, it is in the @@ -623,9 +577,9 @@ This is all that is required to set up your CA. Keep the CA's private key carefully as you will need it when you come to issue certificates for your clients and servers. </p> - <h4> + <h3> <a name="Remote_TLS_server_certificates">Issuing server certificates</a> - </h4> + </h3> <p> For each server (libvirtd) you need to issue a certificate with the X.509 CommonName (CN) field set to the hostname @@ -706,9 +660,9 @@ which can be installed on the server as <code>/etc/pki/libvirt/servercert.pem</code>. </li> </ul> - <h4> + <h3> <a name="Remote_TLS_client_certificates">Issuing client certificates</a> - </h4> + </h3> <p> For each client (ie. any program linked with libvirt, such as <a href="http://virt-manager.et.redhat.com/">virt-manager</a>) @@ -759,9 +713,9 @@ cp clientcert.pem /etc/pki/libvirt/clientcert.pem </pre> </li> </ol> - <h4> + <h3> <a name="Remote_TLS_troubleshooting">Troubleshooting TLS certificate problems</a> - </h4> + </h3> <dl> <dt> failed to verify client's certificate </dt> <dd> @@ -777,9 +731,9 @@ tell you enough to diagnose the problem. to analyze the setup on the client or server machines, preferably as root. It will try to point out the possible problems and provide solutions to fix the set up up to a point where you have secure remote access.</p> - <h3> + <h2> <a name="Remote_libvirtd_configuration">libvirtd configuration file</a> - </h3> + </h2> <p> Libvirtd (the remote daemon) is configured from a file called <code>/etc/libvirt/libvirtd.conf</code>, or specified on @@ -945,9 +899,9 @@ Blank lines and comments beginning with <code>#</code> are ignored. </td> </tr> </table> - <h3> + <h2> <a name="Remote_IPv6">IPv6 support</a> - </h3> + </h2> <p> The libvirtd service and libvirt remote client driver both use the <code>getaddrinfo()</code> functions for name resolution and are @@ -958,9 +912,9 @@ address resolved for a service is reachable over IPv6, then an IPv6 connection will be made, otherwise IPv4 will be used. In summary it should just 'do the right thing(tm)'. </p> - <h3> + <h2> <a name="Remote_limitations">Limitations</a> - </h3> + </h2> <ul> <li> Fine-grained authentication: libvirt in general, but in particular the remote case should support more diff --git a/docs/storage.html.in b/docs/storage.html.in index 5e18f02c5..89ebb7097 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -83,47 +83,7 @@ <p> Libvirt supports the following storage pool types: </p> - <ul> - <li> - <a href="#StorageBackendDir">Directory backend</a> - </li> - <li> - <a href="#StorageBackendFS">Local filesystem backend</a> - </li> - <li> - <a href="#StorageBackendNetFS">Network filesystem backend</a> - </li> - <li> - <a href="#StorageBackendLogical">Logical backend</a> - </li> - <li> - <a href="#StorageBackendDisk">Disk backend</a> - </li> - <li> - <a href="#StorageBackendISCSI">iSCSI backend</a> - </li> - <li> - <a href="#StorageBackendSCSI">SCSI backend</a> - </li> - <li> - <a href="#StorageBackendMultipath">Multipath backend</a> - </li> - <li> - <a href="#StorageBackendRBD">RBD (RADOS Block Device) backend</a> - </li> - <li> - <a href="#StorageBackendSheepdog">Sheepdog backend</a> - </li> - <li> - <a href="#StorageBackendGluster">Gluster backend</a> - </li> - <li> - <a href="#StorageBackendZFS">ZFS backend</a> - </li> - <li> - <a href="#StorageBackendVstorage">Virtuozzo storage backend</a> - </li> - </ul> + <ul id="toc"></ul> <h2><a name="StorageBackendDir">Directory pool</a></h2> <p> @@ -306,7 +266,7 @@ </p> - <h2><a name="StorageBackendLogical">Logical volume pools</a></h2> + <h2><a name="StorageBackendLogical">Logical volume pool</a></h2> <p> This provides a pool based on an LVM volume group. For a pre-defined LVM volume group, simply providing the group @@ -343,7 +303,7 @@ </p> - <h2><a name="StorageBackendDisk">Disk volume pools</a></h2> + <h2><a name="StorageBackendDisk">Disk pool</a></h2> <p> This provides a pool based on a physical disk. Volumes are created by adding partitions to the disk. Disk pools have constraints @@ -434,7 +394,7 @@ </ul> - <h2><a name="StorageBackendISCSI">iSCSI volume pools</a></h2> + <h2><a name="StorageBackendISCSI">iSCSI pool</a></h2> <p> This provides a pool based on an iSCSI target. Volumes must be pre-allocated on the iSCSI server, and cannot be created via @@ -473,7 +433,7 @@ The iSCSI volume pool does not use the volume format type element. </p> - <h2><a name="StorageBackendSCSI">SCSI volume pools</a></h2> + <h2><a name="StorageBackendSCSI">SCSI pool</a></h2> <p> This provides a pool based on a SCSI HBA. Volumes are preexisting SCSI LUNs, and cannot be created via the libvirt APIs. Since /dev/XXX names @@ -505,7 +465,7 @@ The SCSI volume pool does not use the volume format type element. </p> - <h2><a name="StorageBackendMultipath">Multipath pools</a></h2> + <h2><a name="StorageBackendMultipath">Multipath pool</a></h2> <p> This provides a pool that contains all the multipath devices on the host. Therefore, only one Multipath pool may be configured per host. @@ -538,7 +498,7 @@ The Multipath volume pool does not use the volume format type element. </p> - <h2><a name="StorageBackendRBD">RBD pools</a></h2> + <h2><a name="StorageBackendRBD">RBD pool</a></h2> <p> This storage driver provides a pool which contains all RBD images in a RADOS pool. RBD (RADOS Block Device) is part @@ -611,7 +571,7 @@ The RBD pool does not use the volume format type element. </p> - <h2><a name="StorageBackendSheepdog">Sheepdog pools</a></h2> + <h2><a name="StorageBackendSheepdog">Sheepdog pool</a></h2> <p> This provides a pool based on a Sheepdog Cluster. Sheepdog is a distributed storage system for QEMU/KVM. @@ -670,7 +630,7 @@ The Sheepdog pool does not use the volume format type element. </p> - <h2><a name="StorageBackendGluster">Gluster pools</a></h2> + <h2><a name="StorageBackendGluster">Gluster pool</a></h2> <p> This provides a pool based on native Gluster access. Gluster is a distributed file system that can be exposed to the user via @@ -756,7 +716,7 @@ pool type. </p> - <h2><a name="StorageBackendZFS">ZFS pools</a></h2> + <h2><a name="StorageBackendZFS">ZFS pool</a></h2> <p> This provides a pool based on the ZFS filesystem. Initially it was developed for FreeBSD, and <span class="since">since 1.3.2</span> experimental support @@ -794,7 +754,7 @@ <p> The ZFS volume pool does not use the volume format type element. </p> - <h2><a name="StorageBackendVstorage">Vstorage pools</a></h2> + <h2><a name="StorageBackendVstorage">Vstorage pool</a></h2> <p> This provides a pool based on Virtuozzo storage. Virtuozzo Storage is a highly available distributed software-defined storage with built-in -- 2.12.2

On Thu, Apr 20, 2017 at 03:05:55PM +0200, Erik Skultety wrote: You can go even further and split this function into two separate functions for "physical function" and "virtual function". If you drop the sriov_cap the code can be cleaner by simply passing the VIR_NODE_DEV_CAP_FLAG_PCI_* flag directly as parameter and if you decide to split the virNodeDevPCICapSRIOVParseXML() you will not need to pass it at all.) Pavel

[...] Oops, I forgot something in here. Consider this bit squashed in (otherwise the parent device's output would lack some tiny bits) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index a04009110..349d51f69 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -427,6 +427,7 @@ udevPCIGetMdevCaps(struct udev_device *device, VIR_STEAL_PTR(pcidata->mdevs, mdevs); pcidata->nmdevs = nmdevs; + pcidata->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_MDEV; nmdevs = 0; ret = 0; cleanup: Erik

Start discovering the mediated devices on the host system and format the attributes for the child 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 mdevs at the moment. This patch introduces both the formatting and parsing routines, updates nodedev.rng schema, adding a testcase for mdev child device 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 | 43 +++++++++++++++++++- src/node_device/node_device_udev.c | 46 ++++++++++++++++++++++ .../mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml | 8 ++++ tests/nodedevxml2xmltest.c | 1 + 5 files changed, 114 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 4b5dca777..8466d504d 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> @@ -578,6 +579,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 fe4f1bc60..757e4bed7 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -582,9 +582,13 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def) case VIR_NODE_DEV_CAP_DRM: 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_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: - case VIR_NODE_DEV_CAP_MDEV: case VIR_NODE_DEV_CAP_LAST: break; } @@ -1645,6 +1649,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, @@ -1716,6 +1753,8 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_SCSI_GENERIC: case VIR_NODE_DEV_CAP_MDEV: + ret = virNodeDevCapMdevParseXML(ctxt, def, node, &caps->data.mdev); + break; case VIR_NODE_DEV_CAP_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown capability type '%d' for '%s'"), @@ -2035,6 +2074,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) VIR_FREE(data->sg.path); break; case VIR_NODE_DEV_CAP_MDEV: + virNodeDevCapMdevClear(&data->mdev); + break; case VIR_NODE_DEV_CAP_DRM: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 79f1537d9..a04009110 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1083,6 +1083,51 @@ udevProcessSCSIGeneric(struct udev_device *dev, } static int +udevProcessMediatedDevice(struct udev_device *dev, + virNodeDeviceDefPtr def) +{ + int ret = -1; + const char *uuidstr = NULL; + int iommugrp = -1; + int model = -1; + char *path = NULL; + virMediatedDevicePtr mdev = NULL; + virNodeDevCapMdevPtr data = &def->caps->data.mdev; + + if (virAsprintf(&path, "%s/mdev_type", udev_device_get_syspath(dev)) < 0) + goto cleanup; + + if (udevGetMdevCaps(dev, path, data) < 0) + goto cleanup; + + if ((model = virMediatedDeviceModelTypeFromString(data->device_api)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Device API '%s' not supported yet"), + data->device_api); + goto cleanup; + } + + uuidstr = udev_device_get_sysname(dev); + if (!(mdev = virMediatedDeviceNew(uuidstr, model))) + goto cleanup; + + if ((iommugrp = virMediatedDeviceGetIOMMUGroupNum(mdev)) < 0) + goto cleanup; + + if (udevGenerateDeviceName(dev, def, NULL) != 0) + goto cleanup; + + data->iommuGroupNumber = iommugrp; + + ret = 0; + cleanup: + VIR_FREE(path); + virMediatedDeviceFree(mdev); + return ret; + +} + +static int udevGetDeviceNodes(struct udev_device *device, virNodeDeviceDefPtr def) { @@ -1199,6 +1244,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_SYSTEM: case VIR_NODE_DEV_CAP_FC_HOST: case VIR_NODE_DEV_CAP_VPORTS: 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 f023d8a13..88c75ea78 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("mdev_3627463d_b7f0_4fea_b468_f1da537d301b"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.12.2

On Thu, Apr 20, 2017 at 03:05:59PM +0200, Erik Skultety wrote: ^ Extra parenthesis ------------------------------------------------------------------' Jan

On Mon, Apr 24, 2017 at 04:04:01PM +0200, Pavel Hrdina wrote: If you remove all MDEV references you can push it right away.

Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- docs/drvnodedev.html.in | 164 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 162 insertions(+), 2 deletions(-) diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in index ed185c3df..6dece3806 100644 --- a/docs/drvnodedev.html.in +++ b/docs/drvnodedev.html.in @@ -71,7 +71,7 @@ <code>storage</code> (<span class="since">Since 1.0.4</span>), <code>scsi_generic</code> (<span class="since">Since 1.0.7</span>), <code>drm</code> (<span class="since">Since 3.1.0</span>), and - <code>mdev</code> (<span class="since">Since 3.2.0</span>). + <code>mdev</code> (<span class="since">Since 3.3.0</span>). This element can be nested in which case it further specifies a device's capability. Refer to specific device types to see more values for the <code>type</code> attribute which are exclusive. @@ -168,7 +168,7 @@ &lt;/capability&gt;</pre> <p> A SR-IOV child device on the other hand, would then report its top level - capability type as a physical function instead: + capability type as <code>phys_function</code> instead: </p> <pre> @@ -180,5 +180,165 @@ ... &lt;device&gt;</pre> + <h3><a name="MDEVCap">MDEV capability</a></h3> + <p> + A PCI device capable of creating mediated devices will include a nested + capability mdev which enumerates all the supported mdev types on the + physical device, along with the type attributes available through sysfs: + </p> + + <dl> + <dt><code>type</code></dt> + <dd> + This element describes a mediated device type which acts as an + abstract template defining a resource allocation for instances of this + device type. The element has one attribute <code>id</code> which holds + an official vendor-supplied identifier for the type. + <span class="since">Since 3.3.0</span> + </dd> + + <dt><code>name</code></dt> + <dd> + The <code>name</code> element holds a vendor-supplied code name for + the given mediated device type. This is an optional element. + <span class="since">Since 3.3.0</span> + </dd> + + <dt><code>deviceAPI</code></dt> + <dd> + The value of this element describes how an instance of the given type + will be presented to the guest by the VFIO framework. + <span class="since">Since 3.3.0</span> + </dd> + + <dt><code>availableInstances</code></dt> + <dd> + This element reports the current state of resource allocation. In other + words, how many instances of the given type can still be successfully + created on the physical device. + <span class="since">Since 3.3.0</span> + </dd> + </dl> + + <p> + For a more info about mediated devices, refer to the + <a href="#MDEV">paragraph below</a>. + </p> + +<pre> +&lt;device&gt; +... + &lt;driver&gt; + &lt;name&gt;nvidia&lt;/name&gt; + &lt;/driver&gt; + &lt;capability type='pci'&gt; +... + &lt;capability type='mdev'&gt; + &lt;type id='nvidia-11'&gt; + &lt;name&gt;GRID M60-0B&lt;/name&gt; + &lt;deviceAPI&gt;vfio-pci&lt;/deviceAPI&gt; + &lt;availableInstances&gt;16&lt;/availableInstances&gt; + &lt;/type&gt; + &lt;!-- Here would come the rest of the available mdev types --&gt; + &lt;/capability&gt; +... + &lt;/capability&gt; +&lt;/device&gt;</pre> + + <h2><a name="MDEV">Mediated devices (MDEVs)</a></h2> + <p> + Mediated devices (<span class="since">Since 3.3.0</span>) are software + devices defining resource allocation on the backing physical device which + in turn allows the parent physical device's resources to be divided into + several mediated devices, thus sharing the physical device's performance + among multiple guests. Unlike SR-IOV however, where a PCIe device appears + as multiple separate PCIe devices on the host's PCI bus, mediated devices + only appear on the mdev virtual bus. Therefore, no detach/reattach + procedure from/to the host driver procedure is involved even though + mediated devices are used in a direct device assignment manner.<br/> + + The following sub-elements and attributes are exposed within the + <code>capability</code> element: + </p> + + <dl> + <dt><code>type</code></dt> + <dd> + This element describes a mediated device type which acts as an + abstract template defining a resource allocation for instances of this + device type. The element has one attribute <code>id</code> which holds + an official vendor-supplied identifier for the type. + <span class="since">Since 3.3.0</span> + </dd> + + <dt><code>iommuGroup</code></dt> + <dd> + This element supports a single attribute <code>number</code> which holds + the IOMMU group number the mediated device belongs to. + <span class="since">Since 3.3.0</span> + </dd> + </dl> + + <h3>Example of a mediated device</h3> + <pre> +&lt;device&gt; + &lt;name&gt;mdev_4b20d080_1b54_4048_85b3_a6a62d165c01&lt;/name&gt; + &lt;path&gt;/sys/devices/pci0000:00/0000:00:02.0/4b20d080-1b54-4048-85b3-a6a62d165c01&lt;/path&gt; + &lt;parent&gt;pci_0000_06_00_0&lt;/parent&gt; + &lt;driver&gt; + &lt;name&gt;vfio_mdev&lt;/name&gt; + &lt;/driver&gt; + &lt;capability type='mdev'&gt; + &lt;type id='nvidia-11'/&gt; + &lt;iommuGroup number='12'/&gt; + &lt;capability/&gt; +&lt;device/&gt;</pre> + + <p> + The support of mediated device's framework in libvirt's node device driver + covers the following features: + </p> + + <ul> + <li> + list available mediated devices on the host + (<span class="since">Since 3.3.0</span>) + </li> + <li> + display device details + (<span class="since">Since 3.3.0</span>) + </li> + </ul> + + <p> + Because mediated devices are instantiated from vendor specific templates, + simply called 'types', information describing these types is contained + within the parent device's capabilities + (see the example in <a href="#PCI">PCI host devices</a>).<br/><br/> + + To see the supported mediated device types on a specific physical device + use the following: + </p> + + <pre> +$ ls /sys/class/mdev_bus/&lt;device&gt;/mdev_supported_types</pre> + + <p> + To manually instantiate a mediated device, use one of the following as a + reference: + </p> + + <pre> +$ uuidgen &gt; /sys/class/mdev_bus/&lt;device&gt;/mdev_supported_types/&lt;type&gt;/create +... +$ echo &lt;UUID&gt; &gt; /sys/class/mdev_bus/&lt;device&gt;/mdev_supported_types/&lt;type&gt;/create</pre> + + <p> + Manual removal of a mediated device is then performed as follows: + </p> + + <pre> +$ echo 1 &gt; /sys/bus/mdev/devices/&lt;uuid&gt;/remove</pre> + </body> </html> -- 2.12.2