[libvirt] [PATCH 0/6] node_device: update sriov/iommu info before dumpxml of a device
Patch 5/6 resolves this bug: https://bugzilla.redhat.com/show_bug.cgi?id=981546 (filed against RHEL7, but existing in every version of libvirt that supports reporting of SRIOV virtual function info via the NodeDevice APIs - 0.7.5, believe it or not). The rest of the series is there to make the bugfix less of a hack (and make it easier to fix future similar bugs). Laine Stump (6): conf: make virNodeDevCapData an official type nodedev: change if-else if in update_caps to switch node device: prepare node_device_linux_sysfs.c to add more functions node_device: new functions to get sriov/iommu info from sysfs node_device: update sriov/iommu info before dumpxml of a device node_device: replace duplicated code in hal and udev backends src/Makefile.am | 5 +- src/conf/node_device_conf.c | 50 ++++++++-------- src/conf/node_device_conf.h | 16 +++-- src/libxl/libxl_driver.c | 4 +- src/node_device/node_device_driver.c | 50 ++++++++++++---- src/node_device/node_device_driver.h | 2 - src/node_device/node_device_hal.c | 39 +++++------- src/node_device/node_device_linux_sysfs.c | 99 +++++++++++++++++++++++++++++-- src/node_device/node_device_linux_sysfs.h | 32 ++++++++++ src/node_device/node_device_udev.c | 70 +++++++--------------- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 6 +- src/xen/xen_driver.c | 4 +- 13 files changed, 247 insertions(+), 132 deletions(-) create mode 100644 src/node_device/node_device_linux_sysfs.h -- 2.1.0
For some reason a union (_virNodeDevCapData) that had only been declared inside the toplevel struct virNodeDevCapsDef was being used as an argument to functions all over the place. Since it was only a union, the "type" attribute wasn't necessarily sent with it. While this works, it just seems wrong. This patch creates a toplevel typedef for virNodeDevCapData and virNodeDevCapDataPtr, making it a struct that has the type attribute as a member, along with an anonymous union of everything that used to be in union _virNodeDevCapData. This way we only have to change the following: s/union _virNodeDevCapData */virNodeDevCapDataPtr / and s/caps->type/caps->data.type/ This will make me feel less guilty when adding functions that need a pointer to one of these. --- src/conf/node_device_conf.c | 50 +++++++++++++++---------------- src/conf/node_device_conf.h | 16 ++++++---- src/libxl/libxl_driver.c | 4 +-- src/node_device/node_device_driver.c | 14 ++++----- src/node_device/node_device_driver.h | 2 +- src/node_device/node_device_hal.c | 20 ++++++------- src/node_device/node_device_linux_sysfs.c | 6 ++-- src/node_device/node_device_udev.c | 30 +++++++++---------- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 6 ++-- src/xen/xen_driver.c | 4 +-- 11 files changed, 79 insertions(+), 75 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index feae3d4..e6f3f27 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1,7 +1,7 @@ /* * node_device_conf.c: config handling for node devices * - * Copyright (C) 2009-2014 Red Hat, Inc. + * Copyright (C) 2009-2015 Red Hat, Inc. * Copyright (C) 2008 Virtual Iron Software, Inc. * Copyright (C) 2008 David F. Lively * @@ -76,9 +76,9 @@ int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap) { virNodeDevCapsDefPtr caps = dev->def->caps; while (caps) { - if (STREQ(cap, virNodeDevCapTypeToString(caps->type))) + if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type))) return 1; - else if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST) + else if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) if ((STREQ(cap, "fc_host") && (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) || @@ -285,12 +285,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) for (caps = def->caps; caps; caps = caps->next) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - union _virNodeDevCapData *data = &caps->data; + virNodeDevCapDataPtr data = &caps->data; virBufferAsprintf(&buf, "<capability type='%s'>\n", - virNodeDevCapTypeToString(caps->type)); + virNodeDevCapTypeToString(caps->data.type)); virBufferAdjustIndent(&buf, 2); - switch (caps->type) { + switch (caps->data.type) { case VIR_NODE_DEV_CAP_SYSTEM: if (data->system.product_name) virBufferEscapeString(&buf, "<product>%s</product>\n", @@ -661,7 +661,7 @@ static int virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode, *nodes = NULL; size_t i; @@ -755,7 +755,7 @@ static int virNodeDevCapSCSIParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode; int ret = -1; @@ -800,7 +800,7 @@ static int virNodeDevCapSCSITargetParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode; int ret = -1; @@ -828,7 +828,7 @@ static int virNodeDevCapSCSIHostParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data, + virNodeDevCapDataPtr data, int create, const char *virt_type) { @@ -932,7 +932,7 @@ static int virNodeDevCapNetParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode, lnk; size_t i = -1; @@ -1010,7 +1010,7 @@ static int virNodeDevCapUSBInterfaceParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode; int ret = -1; @@ -1077,7 +1077,7 @@ static int virNodeDevCapUSBDevParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode; int ret = -1; @@ -1121,7 +1121,7 @@ virNodeDevCapUSBDevParseXML(xmlXPathContextPtr ctxt, static int virNodeDevCapPCIDevIommuGroupParseXML(xmlXPathContextPtr ctxt, xmlNodePtr iommuGroupNode, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr origNode = ctxt->node; xmlNodePtr *addrNodes = NULL; @@ -1259,7 +1259,7 @@ static int virNodeDevCapPCIDevParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode, iommuGroupNode, pciExpress; int ret = -1; @@ -1344,7 +1344,7 @@ static int virNodeDevCapSystemParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + virNodeDevCapDataPtr data) { xmlNodePtr orignode; int ret = -1; @@ -1411,10 +1411,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, VIR_FREE(tmp); goto error; } - caps->type = val; + caps->data.type = val; VIR_FREE(tmp); - switch (caps->type) { + switch (caps->data.type) { case VIR_NODE_DEV_CAP_SYSTEM: ret = virNodeDevCapSystemParseXML(ctxt, def, node, &caps->data); break; @@ -1451,7 +1451,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, case VIR_NODE_DEV_CAP_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown capability type '%d' for '%s'"), - caps->type, def->name); + caps->data.type, def->name); ret = -1; break; } @@ -1607,7 +1607,7 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, cap = def->caps; while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST && cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { if (VIR_STRDUP(*wwnn, cap->data.scsi_host.wwnn) < 0 || VIR_STRDUP(*wwpn, cap->data.scsi_host.wwpn) < 0) { @@ -1656,7 +1656,7 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, cap = parent->def->caps; while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST && (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { *parent_host = cap->data.scsi_host.host; @@ -1683,9 +1683,9 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs, void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) { size_t i = 0; - union _virNodeDevCapData *data = &caps->data; + virNodeDevCapDataPtr data = &caps->data; - switch (caps->type) { + switch (caps->data.type) { case VIR_NODE_DEV_CAP_SYSTEM: VIR_FREE(data->system.product_name); VIR_FREE(data->system.hardware.vendor_name); @@ -1771,10 +1771,10 @@ virNodeDeviceCapMatch(virNodeDeviceObjPtr devobj, virNodeDevCapsDefPtr cap = NULL; for (cap = devobj->def->caps; cap; cap = cap->next) { - if (type == cap->type) + if (type == cap->data.type) return true; - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { if (type == VIR_NODE_DEV_CAP_FC_HOST && (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 38c6d45..7dd39ca 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -1,7 +1,7 @@ /* * node_device_conf.h: config handling for node devices * - * Copyright (C) 2009-2014 Red Hat, Inc. + * Copyright (C) 2009-2015 Red Hat, Inc. * Copyright (C) 2008 Virtual Iron Software, Inc. * Copyright (C) 2008 David F. Lively * @@ -82,11 +82,9 @@ typedef enum { VIR_NODE_DEV_CAP_FLAG_PCIE = (1 << 2), } virNodeDevPCICapFlags; -typedef struct _virNodeDevCapsDef virNodeDevCapsDef; -typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; -struct _virNodeDevCapsDef { +typedef struct _virNodeDevCapData { virNodeDevCapType type; - union _virNodeDevCapData { + union { struct { char *product_name; struct { @@ -181,7 +179,13 @@ struct _virNodeDevCapsDef { struct { char *path; } sg; /* SCSI generic device */ - } data; + }; +} virNodeDevCapData, *virNodeDevCapDataPtr; + +typedef struct _virNodeDevCapsDef virNodeDevCapsDef; +typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; +struct _virNodeDevCapsDef { + virNodeDevCapData data; virNodeDevCapsDefPtr next; /* next capability */ }; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 60c139e..8a8bc85 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1,7 +1,7 @@ /* * libxl_driver.c: core driver methods for managing libxenlight domains * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2011-2015 SUSE LINUX Products GmbH, Nuernberg, Germany. * Copyright (C) 2011 Univention GmbH. * @@ -4620,7 +4620,7 @@ libxlNodeDeviceGetPCIInfo(virNodeDeviceDefPtr def, cap = def->caps; while (cap) { - if (cap->type == VIR_NODE_DEV_CAP_PCI_DEV) { + if (cap->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { *domain = cap->data.pci_dev.domain; *bus = cap->data.pci_dev.bus; *slot = cap->data.pci_dev.slot; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b8d9f4f..a14191c 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -1,7 +1,7 @@ /* * node_device_driver.c: node device enumeration * - * Copyright (C) 2010-2014 Red Hat, Inc. + * Copyright (C) 2010-2015 Red Hat, Inc. * Copyright (C) 2008 Virtual Iron Software, Inc. * Copyright (C) 2008 David F. Lively * @@ -50,9 +50,9 @@ static int update_caps(virNodeDeviceObjPtr dev) virNodeDevCapsDefPtr cap = dev->def->caps; while (cap) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) + if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) detect_scsi_host_caps(&dev->def->caps->data); - if (cap->type == VIR_NODE_DEV_CAP_NET && + if (cap->data.type == VIR_NODE_DEV_CAP_NET && virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0) return -1; @@ -262,7 +262,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, cap = obj->def->caps; while (cap) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { detect_scsi_host_caps(&cap->data); if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { @@ -386,7 +386,7 @@ nodeDeviceNumOfCaps(virNodeDevicePtr dev) for (caps = obj->def->caps; caps; caps = caps->next) { ++ncaps; - if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { if (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) ncaps++; @@ -429,10 +429,10 @@ nodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames) goto cleanup; for (caps = obj->def->caps; caps && ncaps < maxnames; caps = caps->next) { - if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->type)) < 0) + if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->data.type)) < 0) goto cleanup; - if (caps->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { if (ncaps < maxnames && caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 79583cd..81bca98 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -44,7 +44,7 @@ extern virNodeDeviceDriverStatePtr driver; int nodedevRegister(void); -int detect_scsi_host_caps(union _virNodeDevCapData *d); +int detect_scsi_host_caps(virNodeDevCapDataPtr d); int nodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags); int nodeListDevices(virConnectPtr conn, const char *cap, char **const names, diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index fa37787..38e09c4 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -139,7 +139,7 @@ get_uint64_prop(LibHalContext *ctxt, const char *udi, static int gather_pci_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { char *sysfs_path; @@ -182,7 +182,7 @@ gather_pci_cap(LibHalContext *ctx, const char *udi, static int gather_usb_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { (void)get_int_prop(ctx, udi, "usb.interface.number", (int *)&d->usb_if.number); @@ -200,7 +200,7 @@ gather_usb_cap(LibHalContext *ctx, const char *udi, static int gather_usb_device_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { (void)get_int_prop(ctx, udi, "usb_device.bus_number", (int *)&d->usb_dev.bus); @@ -222,7 +222,7 @@ gather_usb_device_cap(LibHalContext *ctx, const char *udi, static int gather_net_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { unsigned long long dummy; (void)get_str_prop(ctx, udi, "net.interface", &d->net.ifname); @@ -242,7 +242,7 @@ gather_net_cap(LibHalContext *ctx, const char *udi, static int gather_scsi_host_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { int retval = 0; @@ -260,7 +260,7 @@ gather_scsi_host_cap(LibHalContext *ctx, const char *udi, static int gather_scsi_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { (void)get_int_prop(ctx, udi, "scsi.host", (int *)&d->scsi.host); (void)get_int_prop(ctx, udi, "scsi.bus", (int *)&d->scsi.bus); @@ -273,7 +273,7 @@ gather_scsi_cap(LibHalContext *ctx, const char *udi, static int gather_storage_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { int val; (void)get_str_prop(ctx, udi, "block.device", &d->storage.block); @@ -301,7 +301,7 @@ gather_storage_cap(LibHalContext *ctx, const char *udi, static int gather_scsi_generic_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { (void)get_str_prop(ctx, udi, "scsi_generic.device", &d->sg.path); return 0; @@ -310,7 +310,7 @@ gather_scsi_generic_cap(LibHalContext *ctx, const char *udi, static int gather_system_cap(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *d) + virNodeDevCapDataPtr d) { char *uuidstr; @@ -340,7 +340,7 @@ struct _caps_tbl_entry { virNodeDevCapType type; int (*gather_fn)(LibHalContext *ctx, const char *udi, - union _virNodeDevCapData *data); + virNodeDevCapDataPtr data); }; typedef struct _caps_tbl_entry caps_tbl_entry; diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 4a95b2b..d4c2a3c 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -2,7 +2,7 @@ * node_device_linux_sysfs.c: Linux specific code to gather device data * not available through HAL. * - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -41,7 +41,7 @@ VIR_LOG_INIT("node_device.node_device_linux_sysfs"); int -detect_scsi_host_caps(union _virNodeDevCapData *d) +detect_scsi_host_caps(virNodeDevCapDataPtr d) { char *max_vports = NULL; char *vports = NULL; @@ -139,7 +139,7 @@ detect_scsi_host_caps(union _virNodeDevCapData *d) #else int -detect_scsi_host_caps(union _virNodeDevCapData *d ATTRIBUTE_UNUSED) +detect_scsi_host_caps(virNodeDevCapDataPtr d ATTRIBUTE_UNUSED) { return -1; } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 846f570..b6619bd 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -411,7 +411,7 @@ static int udevProcessPCI(struct udev_device *device, virNodeDeviceDefPtr def) { const char *syspath = NULL; - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; virPCIDeviceAddress addr; virPCIEDeviceInfoPtr pci_express = NULL; virPCIDevicePtr pciDev = NULL; @@ -567,7 +567,7 @@ static int udevProcessPCI(struct udev_device *device, static int udevProcessUSBDevice(struct udev_device *device, virNodeDeviceDefPtr def) { - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; int ret = -1; int err; @@ -639,7 +639,7 @@ static int udevProcessUSBInterface(struct udev_device *device, virNodeDeviceDefPtr def) { int ret = -1; - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; if (udevGetUintSysfsAttr(device, "bInterfaceNumber", @@ -684,7 +684,7 @@ static int udevProcessNetworkInterface(struct udev_device *device, { int ret = -1; const char *devtype = udev_device_get_devtype(device); - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; if (devtype && STREQ(devtype, "wlan")) { data->net.subtype = VIR_NODE_DEV_CAP_NET_80211; @@ -731,7 +731,7 @@ static int udevProcessSCSIHost(struct udev_device *device ATTRIBUTE_UNUSED, virNodeDeviceDefPtr def) { int ret = -1; - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; char *filename = NULL; filename = last_component(def->sysfs_path); @@ -766,7 +766,7 @@ static int udevProcessSCSITarget(struct udev_device *device ATTRIBUTE_UNUSED, { int ret = -1; const char *sysname = NULL; - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; sysname = udev_device_get_sysname(device); @@ -846,7 +846,7 @@ static int udevProcessSCSIDevice(struct udev_device *device ATTRIBUTE_UNUSED, { int ret = -1; unsigned int tmp = 0; - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; char *filename = NULL, *p = NULL; filename = last_component(def->sysfs_path); @@ -905,7 +905,7 @@ static int udevProcessSCSIDevice(struct udev_device *device ATTRIBUTE_UNUSED, static int udevProcessDisk(struct udev_device *device, virNodeDeviceDefPtr def) { - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; int ret = 0; if (udevGetUint64SysfsAttr(device, @@ -933,7 +933,7 @@ static int udevProcessRemoveableMedia(struct udev_device *device, virNodeDeviceDefPtr def, int has_media) { - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; int tmp_int = 0, ret = 0; if ((udevGetIntSysfsAttr(device, "removable", &tmp_int, 0) == PROPERTY_FOUND) && @@ -1025,7 +1025,7 @@ static int udevProcessFloppy(struct udev_device *device, static int udevProcessSD(struct udev_device *device, virNodeDeviceDefPtr def) { - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; int ret = 0; if (udevGetUint64SysfsAttr(device, @@ -1098,7 +1098,7 @@ static void udevStripSpaces(char *s) static int udevProcessStorage(struct udev_device *device, virNodeDeviceDefPtr def) { - union _virNodeDevCapData *data = &def->caps->data; + virNodeDevCapDataPtr data = &def->caps->data; int ret = -1; const char* devnode; @@ -1281,7 +1281,7 @@ static int udevGetDeviceDetails(struct udev_device *device, { int ret = 0; - switch (def->caps->type) { + switch (def->caps->data.type) { case VIR_NODE_DEV_CAP_SYSTEM: /* There's no libudev equivalent of system, so ignore it. */ break; @@ -1313,7 +1313,7 @@ static int udevGetDeviceDetails(struct udev_device *device, ret = udevProcessSCSIGeneric(device, def); break; default: - VIR_ERROR(_("Unknown device type %d"), def->caps->type); + VIR_ERROR(_("Unknown device type %d"), def->caps->data.type); ret = -1; break; } @@ -1414,7 +1414,7 @@ static int udevAddOneDevice(struct udev_device *device) if (VIR_ALLOC(def->caps) != 0) goto out; - if (udevGetDeviceType(device, &def->caps->type) != 0) + if (udevGetDeviceType(device, &def->caps->data.type) != 0) goto out; if (udevGetDeviceDetails(device, def) != 0) @@ -1588,7 +1588,7 @@ static void udevEventHandleCallback(int watch ATTRIBUTE_UNUSED, /* DMI is intel-compatible specific */ #if defined(__x86_64__) || defined(__i386__) || defined(__amd64__) static void -udevGetDMIData(union _virNodeDevCapData *data) +udevGetDMIData(virNodeDevCapDataPtr data) { struct udev *udev = NULL; struct udev_device *device = NULL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2668011..8c00cdc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13091,7 +13091,7 @@ qemuNodeDeviceGetPCIInfo(virNodeDeviceDefPtr def, cap = def->caps; while (cap) { - if (cap->type == VIR_NODE_DEV_CAP_PCI_DEV) { + if (cap->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { *domain = cap->data.pci_dev.domain; *bus = cap->data.pci_dev.bus; *slot = cap->data.pci_dev.slot; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 51ab2e0..038b2b8 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1,7 +1,7 @@ /* * test_driver.c: A "mock" hypervisor for use by application unit tests * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -5798,7 +5798,7 @@ testNodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames) } for (caps = obj->def->caps; caps && ncaps < maxnames; caps = caps->next) { - if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->type)) < 0) + if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps->data.type)) < 0) goto cleanup; } ret = ncaps; @@ -5856,7 +5856,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, * since this would also come from the backend */ caps = def->caps; while (caps) { - if (caps->type != VIR_NODE_DEV_CAP_SCSI_HOST) + if (caps->data.type != VIR_NODE_DEV_CAP_SCSI_HOST) continue; caps->data.scsi_host.host = virRandomBits(10); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 7711f33..da9e6f4 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1,7 +1,7 @@ /* * xen_driver.c: Unified Xen driver. * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -2488,7 +2488,7 @@ xenUnifiedNodeDeviceGetPCIInfo(virNodeDevicePtr dev, cap = def->caps; while (cap) { - if (cap->type == VIR_NODE_DEV_CAP_PCI_DEV) { + if (cap->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { *domain = cap->data.pci_dev.domain; *bus = cap->data.pci_dev.bus; *slot = cap->data.pci_dev.slot; -- 2.1.0
Makes it nicer as update bits are added for different cap types. --- src/node_device/node_device_driver.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index a14191c..a6b32fe 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -50,12 +50,31 @@ static int update_caps(virNodeDeviceObjPtr dev) virNodeDevCapsDefPtr cap = dev->def->caps; while (cap) { - if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) + switch (cap->data.type) { + case VIR_NODE_DEV_CAP_SCSI_HOST: detect_scsi_host_caps(&dev->def->caps->data); - if (cap->data.type == VIR_NODE_DEV_CAP_NET && - virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0) - return -1; + break; + case VIR_NODE_DEV_CAP_NET: + if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0) + return -1; + break; + /* all types that (supposedly) don't require any updates + * relative to what's in the cache. + */ + case VIR_NODE_DEV_CAP_SYSTEM: + case VIR_NODE_DEV_CAP_PCI_DEV: + case VIR_NODE_DEV_CAP_USB_DEV: + case VIR_NODE_DEV_CAP_USB_INTERFACE: + case VIR_NODE_DEV_CAP_SCSI_TARGET: + case VIR_NODE_DEV_CAP_SCSI: + case VIR_NODE_DEV_CAP_STORAGE: + case VIR_NODE_DEV_CAP_FC_HOST: + case VIR_NODE_DEV_CAP_VPORTS: + case VIR_NODE_DEV_CAP_SCSI_GENERIC: + case VIR_NODE_DEV_CAP_LAST: + break; + } cap = cap->next; } -- 2.1.0
This file contains only a single function, detect_scsi_host_caps(), which is declared in node_device_driver.h and called from both the hal and udev backends. Other things common to the hal and udev drivers can be placed in that file though. As a prelude to adding further functions, this patch renames the existing function to something closer in line with other internal libvirt function names (nodeDeviceSysfsGetSCSIHostCaps()), and puts the declarations into a separate .h file. --- src/Makefile.am | 5 +++-- src/node_device/node_device_driver.c | 7 ++++--- src/node_device/node_device_driver.h | 2 -- src/node_device/node_device_hal.c | 7 ++++--- src/node_device/node_device_linux_sysfs.c | 7 ++++--- src/node_device/node_device_linux_sysfs.h | 30 ++++++++++++++++++++++++++++++ src/node_device/node_device_udev.c | 9 +++++---- 7 files changed, 50 insertions(+), 17 deletions(-) create mode 100644 src/node_device/node_device_linux_sysfs.h diff --git a/src/Makefile.am b/src/Makefile.am index d28a8ed..579421d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2014 Red Hat, Inc. +## Copyright (C) 2005-2015 Red Hat, Inc. ## ## This library is free software; you can redistribute it and/or ## modify it under the terms of the GNU Lesser General Public @@ -966,7 +966,8 @@ ACCESS_DRIVER_POLKIT_POLICY = \ NODE_DEVICE_DRIVER_SOURCES = \ node_device/node_device_driver.c \ node_device/node_device_driver.h \ - node_device/node_device_linux_sysfs.c + node_device/node_device_linux_sysfs.c \ + node_device/node_device_linux_sysfs.h NODE_DEVICE_DRIVER_HAL_SOURCES = \ node_device/node_device_hal.c \ diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index a6b32fe..c9db00a 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -35,8 +35,9 @@ #include "virfile.h" #include "virstring.h" #include "node_device_conf.h" -#include "node_device_hal.h" #include "node_device_driver.h" +#include "node_device_hal.h" +#include "node_device_linux_sysfs.h" #include "virutil.h" #include "viraccessapicheck.h" #include "virnetdev.h" @@ -52,7 +53,7 @@ static int update_caps(virNodeDeviceObjPtr dev) while (cap) { switch (cap->data.type) { case VIR_NODE_DEV_CAP_SCSI_HOST: - detect_scsi_host_caps(&dev->def->caps->data); + nodeDeviceSysfsGetSCSIHostCaps(&dev->def->caps->data); break; case VIR_NODE_DEV_CAP_NET: if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0) @@ -282,7 +283,7 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, while (cap) { if (cap->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { - detect_scsi_host_caps(&cap->data); + nodeDeviceSysfsGetSCSIHostCaps(&cap->data); if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { if (STREQ(cap->data.scsi_host.wwnn, wwnn) && diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 81bca98..0f4ea57 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -44,8 +44,6 @@ extern virNodeDeviceDriverStatePtr driver; int nodedevRegister(void); -int detect_scsi_host_caps(virNodeDevCapDataPtr d); - int nodeNumOfDevices(virConnectPtr conn, const char *cap, unsigned int flags); int nodeListDevices(virConnectPtr conn, const char *cap, char **const names, int maxnames, unsigned int flags); diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 38e09c4..2379e88 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -1,7 +1,7 @@ /* * node_device_hal.c: node device enumeration - HAL-based implementation * - * Copyright (C) 2011-2014 Red Hat, Inc. + * Copyright (C) 2011-2015 Red Hat, Inc. * Copyright (C) 2008 Virtual Iron Software, Inc. * Copyright (C) 2008 David F. Lively * @@ -29,7 +29,9 @@ #include <libhal.h> #include "node_device_conf.h" +#include "node_device_driver.h" #include "node_device_hal.h" +#include "node_device_linux_sysfs.h" #include "virerror.h" #include "driver.h" #include "datatypes.h" @@ -37,7 +39,6 @@ #include "viruuid.h" #include "virpci.h" #include "virlog.h" -#include "node_device_driver.h" #include "virdbus.h" #include "virstring.h" @@ -248,7 +249,7 @@ gather_scsi_host_cap(LibHalContext *ctx, const char *udi, (void)get_int_prop(ctx, udi, "scsi_host.host", (int *)&d->scsi_host.host); - retval = detect_scsi_host_caps(d); + retval = nodeDeviceSysfsGetSCSIHostCaps(d); if (retval == -1) goto out; diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index d4c2a3c..fc82b32 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -1,6 +1,6 @@ /* * node_device_linux_sysfs.c: Linux specific code to gather device data - * not available through HAL. + * that is available from sysfs (but not from UDEV or HAL). * * Copyright (C) 2009-2015 Red Hat, Inc. * @@ -28,6 +28,7 @@ #include "node_device_driver.h" #include "node_device_hal.h" +#include "node_device_linux_sysfs.h" #include "virerror.h" #include "viralloc.h" #include "virlog.h" @@ -41,7 +42,7 @@ VIR_LOG_INIT("node_device.node_device_linux_sysfs"); int -detect_scsi_host_caps(virNodeDevCapDataPtr d) +nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d) { char *max_vports = NULL; char *vports = NULL; @@ -139,7 +140,7 @@ detect_scsi_host_caps(virNodeDevCapDataPtr d) #else int -detect_scsi_host_caps(virNodeDevCapDataPtr d ATTRIBUTE_UNUSED) +nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d ATTRIBUTE_UNUSED) { return -1; } diff --git a/src/node_device/node_device_linux_sysfs.h b/src/node_device/node_device_linux_sysfs.h new file mode 100644 index 0000000..307a8aa --- /dev/null +++ b/src/node_device/node_device_linux_sysfs.h @@ -0,0 +1,30 @@ +/* + * node_device_linux_sysfs.h: Linux specific code to gather device data + * that is available from sysfs (but not from UDEV or HAL). + * + * Copyright (C) 2015 Red Hat, Inc. + * + * 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, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIR_NODE_DEVICE_LINUX_SYSFS_H__ +# define __VIR_NODE_DEVICE_LINUX_SYSFS_H__ + +# include "node_device_conf.h" + +int nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d); + +#endif /* __VIR_NODE_DEVICE_LINUX_SYSFS_H__ */ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index b6619bd..29da036 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1,7 +1,7 @@ /* * node_device_udev.c: node device enumeration - libudev implementation * - * Copyright (C) 2009-2014 Red Hat, Inc. + * Copyright (C) 2009-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -27,10 +27,11 @@ #include <c-ctype.h> #include "dirname.h" -#include "node_device_udev.h" -#include "virerror.h" #include "node_device_conf.h" #include "node_device_driver.h" +#include "node_device_linux_sysfs.h" +#include "node_device_udev.h" +#include "virerror.h" #include "driver.h" #include "datatypes.h" #include "virlog.h" @@ -749,7 +750,7 @@ static int udevProcessSCSIHost(struct udev_device *device ATTRIBUTE_UNUSED, goto out; } - detect_scsi_host_caps(&def->caps->data); + nodeDeviceSysfsGetSCSIHostCaps(&def->caps->data); if (udevGenerateDeviceName(device, def, NULL) != 0) goto out; -- 2.1.0
The udev and hal drivers both already call the same functions as these new functions added to node_device_linux_sysfs.c, but 1) we need to call them from node_device_driver.c, and 2) it would be nice to eliminate the duplicated code from the hal and udev backends. --- src/node_device/node_device_linux_sysfs.c | 90 +++++++++++++++++++++++++++++++ src/node_device/node_device_linux_sysfs.h | 2 + 2 files changed, 92 insertions(+) diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index fc82b32..6d5a406 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -137,6 +137,96 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d) return ret; } + +static int +nodeDeviceSysfsGetPCISRIOVCaps(const char *sysfsPath, + virNodeDevCapDataPtr data) +{ + size_t i; + int ret; + + /* this could be a refresh, so clear out the old data */ + for (i = 0; i < data->pci_dev.num_virtual_functions; i++) + VIR_FREE(data->pci_dev.virtual_functions[i]); + VIR_FREE(data->pci_dev.virtual_functions); + data->pci_dev.num_virtual_functions = 0; + data->pci_dev.flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; + data->pci_dev.flags &= ~VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + + if (!virPCIGetPhysicalFunction(sysfsPath, &data->pci_dev.physical_function)) + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + + ret = virPCIGetVirtualFunctions(sysfsPath, &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions); + if (ret < 0) + return ret; + + if (data->pci_dev.num_virtual_functions > 0) + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; + + return ret; +} + + +static int +nodeDeviceSysfsGetPCIIOMMUGroupCaps(virNodeDevCapDataPtr data) +{ + size_t i; + int tmpGroup, ret = -1; + virPCIDeviceAddress addr; + + /* this could be a refresh, so clear out the old data */ + for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++) + VIR_FREE(data->pci_dev.iommuGroupDevices[i]); + VIR_FREE(data->pci_dev.iommuGroupDevices); + data->pci_dev.nIommuGroupDevices = 0; + data->pci_dev.iommuGroupNumber = 0; + + addr.domain = data->pci_dev.domain; + addr.bus = data->pci_dev.bus; + addr.slot = data->pci_dev.slot; + addr.function = data->pci_dev.function; + tmpGroup = virPCIDeviceAddressGetIOMMUGroupNum(&addr); + if (tmpGroup == -1) { + /* error was already reported */ + goto cleanup; + } + if (tmpGroup == -2) { + /* -2 return means there is no iommu_group data */ + ret = 0; + goto cleanup; + } + if (tmpGroup >= 0) { + if (virPCIDeviceAddressGetIOMMUGroupAddresses(&addr, &data->pci_dev.iommuGroupDevices, + &data->pci_dev.nIommuGroupDevices) < 0) + goto cleanup; + data->pci_dev.iommuGroupNumber = tmpGroup; + } + + ret = 0; + cleanup: + return ret; +} + + +/* nodeDeviceSysfsGetPCIRelatedCaps() get info that is stored in sysfs + * about devices related to this device, i.e. things that can change + * without this device itself changing. These must be refreshed + * anytime full XML of the device is requested, because they can + * change with no corresponding notification from the kernel/udev. + */ +int +nodeDeviceSysfsGetPCIRelatedDevCaps(const char *sysfsPath, + virNodeDevCapDataPtr data) +{ + if (nodeDeviceSysfsGetPCISRIOVCaps(sysfsPath, data) < 0) + return -1; + if (nodeDeviceSysfsGetPCIIOMMUGroupCaps(data) < 0) + return -1; + return 0; +} + + #else int diff --git a/src/node_device/node_device_linux_sysfs.h b/src/node_device/node_device_linux_sysfs.h index 307a8aa..e4afdd7 100644 --- a/src/node_device/node_device_linux_sysfs.h +++ b/src/node_device/node_device_linux_sysfs.h @@ -26,5 +26,7 @@ # include "node_device_conf.h" int nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d); +int nodeDeviceSysfsGetPCIRelatedDevCaps(const char *sysfsPath, + virNodeDevCapDataPtr data); #endif /* __VIR_NODE_DEVICE_LINUX_SYSFS_H__ */ -- 2.1.0
Because reloading a PF driver with a different number of VFs doesn't result in any sort of event sent from udev to the libvirt node_device driver, libvirt's cache of that info can be out of date when a request arrives for the info about a device. To fix this, we refresh that data at the time of the dumpxml request, similar to what is already done for netdev link info and SCSI host capabilities. Since the same is true for iommu group information (for example, some other device in the same iommu group could have been detached from the host), we also create a function to update the iommu group info from sysfs, and a common function that does both. (a later patch will call this common function from the udev and hal backends). This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=981546 --- src/node_device/node_device_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index c9db00a..34ba1fa 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -59,12 +59,16 @@ static int update_caps(virNodeDeviceObjPtr dev) if (virNetDevGetLinkInfo(cap->data.net.ifname, &cap->data.net.lnk) < 0) return -1; break; + case VIR_NODE_DEV_CAP_PCI_DEV: + if (nodeDeviceSysfsGetPCIRelatedDevCaps(dev->def->sysfs_path, + &dev->def->caps->data) < 0) + return -1; + break; /* all types that (supposedly) don't require any updates * relative to what's in the cache. */ case VIR_NODE_DEV_CAP_SYSTEM: - case VIR_NODE_DEV_CAP_PCI_DEV: case VIR_NODE_DEV_CAP_USB_DEV: case VIR_NODE_DEV_CAP_USB_INTERFACE: case VIR_NODE_DEV_CAP_SCSI_TARGET: -- 2.1.0
Both the hal and udev drivers call virPCI*() functions to the the SRIOV VF/PF info about PCI devices, and the UDEV backend calls virPCI*() to get IOMMU group info. Since there is now a single function call in node_device_linux_sysfs.c to do all of this, replace all that code in the two backends with calls to nodeDeviceSysfsGetPCIRelatedDevCaps(). Note that this results in the HAL driver (probably) unnecessarily calling virPCIDevieAddressGetIOMMUGroupNum(), but in the case that the host doesn't support IOMMU groups, that function turns into a NOP (it returns -2, which causes the caller to skip the call to virPCIDeviceAddressGetIOMMUGroupAddresses()). So in the worst case it is a few extra cycles spent, and in the best case a mythical platform that supported IOMMU groups but used HAL rather than UDEV would gain proper reporting of IOMMU group info. --- src/node_device/node_device_hal.c | 12 +----------- src/node_device/node_device_udev.c | 31 ++----------------------------- 2 files changed, 3 insertions(+), 40 deletions(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 2379e88..2d3bc17 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -153,20 +153,10 @@ gather_pci_cap(LibHalContext *ctx, const char *udi, ignore_value(virStrToLong_ui(p+1, &p, 16, &d->pci_dev.function)); } - if (!virPCIGetPhysicalFunction(sysfs_path, - &d->pci_dev.physical_function)) - d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - - int ret = virPCIGetVirtualFunctions(sysfs_path, - &d->pci_dev.virtual_functions, - &d->pci_dev.num_virtual_functions); - if (ret < 0) { + if (nodeDeviceSysfsGetPCIRelatedDevCaps(sysfs_path, d) < 0) { VIR_FREE(sysfs_path); return -1; } - - if (d->pci_dev.num_virtual_functions > 0) - d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 29da036..100b44d 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -413,11 +413,10 @@ static int udevProcessPCI(struct udev_device *device, { const char *syspath = NULL; virNodeDevCapDataPtr data = &def->caps->data; - virPCIDeviceAddress addr; virPCIEDeviceInfoPtr pci_express = NULL; virPCIDevicePtr pciDev = NULL; udevPrivate *priv = driver->privateData; - int tmpGroup, ret = -1; + int ret = -1; char *p; int rc; @@ -496,34 +495,8 @@ static int udevProcessPCI(struct udev_device *device, data->pci_dev.numa_node = -1; } - if (!virPCIGetPhysicalFunction(syspath, &data->pci_dev.physical_function)) - data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - - rc = virPCIGetVirtualFunctions(syspath, - &data->pci_dev.virtual_functions, - &data->pci_dev.num_virtual_functions); - /* Out of memory */ - if (rc < 0) - goto out; - else if (!rc && (data->pci_dev.num_virtual_functions > 0)) - data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; - - /* iommu group */ - addr.domain = data->pci_dev.domain; - addr.bus = data->pci_dev.bus; - addr.slot = data->pci_dev.slot; - addr.function = data->pci_dev.function; - tmpGroup = virPCIDeviceAddressGetIOMMUGroupNum(&addr); - if (tmpGroup == -1) { - /* error was already reported */ + if (nodeDeviceSysfsGetPCIRelatedDevCaps(syspath, data) < 0) goto out; - /* -2 return means there is no iommu_group data */ - } else if (tmpGroup >= 0) { - if (virPCIDeviceAddressGetIOMMUGroupAddresses(&addr, &data->pci_dev.iommuGroupDevices, - &data->pci_dev.nIommuGroupDevices) < 0) - goto out; - data->pci_dev.iommuGroupNumber = tmpGroup; - } if (!(pciDev = virPCIDeviceNew(data->pci_dev.domain, data->pci_dev.bus, -- 2.1.0
On 15.05.2015 17:04, Laine Stump wrote:
Patch 5/6 resolves this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=981546
(filed against RHEL7, but existing in every version of libvirt that supports reporting of SRIOV virtual function info via the NodeDevice APIs - 0.7.5, believe it or not).
The rest of the series is there to make the bugfix less of a hack (and make it easier to fix future similar bugs).
Laine Stump (6): conf: make virNodeDevCapData an official type nodedev: change if-else if in update_caps to switch node device: prepare node_device_linux_sysfs.c to add more functions node_device: new functions to get sriov/iommu info from sysfs node_device: update sriov/iommu info before dumpxml of a device node_device: replace duplicated code in hal and udev backends
src/Makefile.am | 5 +- src/conf/node_device_conf.c | 50 ++++++++-------- src/conf/node_device_conf.h | 16 +++-- src/libxl/libxl_driver.c | 4 +- src/node_device/node_device_driver.c | 50 ++++++++++++---- src/node_device/node_device_driver.h | 2 - src/node_device/node_device_hal.c | 39 +++++------- src/node_device/node_device_linux_sysfs.c | 99 +++++++++++++++++++++++++++++-- src/node_device/node_device_linux_sysfs.h | 32 ++++++++++ src/node_device/node_device_udev.c | 70 +++++++--------------- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 6 +- src/xen/xen_driver.c | 4 +- 13 files changed, 247 insertions(+), 132 deletions(-) create mode 100644 src/node_device/node_device_linux_sysfs.h
ACK Michal
On 05/18/2015 06:07 AM, Michal Privoznik wrote:
On 15.05.2015 17:04, Laine Stump wrote:
Patch 5/6 resolves this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=981546
(filed against RHEL7, but existing in every version of libvirt that supports reporting of SRIOV virtual function info via the NodeDevice APIs - 0.7.5, believe it or not).
ACK
Thanks! I've pushed this now.
participants (2)
-
Laine Stump -
Michal Privoznik