[PATCH V2] nodedev: Fix failing to parse PCI address for non-PCI network devices

[Jim Fehlig <jfehlig@xxxxxxxx>]

Based loosely on a patch from Fei Li <fli@xxxxxxxx>.

Commit 8708ca01c added virNetDevSwitchdevFeature() to check if a network
device has Switchdev capabilities. virNetDevSwitchdevFeature() attempts
to retrieve the PCI device associated with the network device, ignoring
non-PCI devices. It does so via the following call chain

  virNetDevSwitchdevFeature()->virNetDevGetPCIDevice()->
  virPCIGetDeviceAddressFromSysfsLink()

For non-PCI network devices (qeth, Xen vif, etc),
virPCIGetDeviceAddressFromSysfsLink() will report an error when
virPCIDeviceAddressParse() fails. virPCIDeviceAddressParse() also
logs an error. After commit 8708ca01c there are now two errors reported
for each non-PCI network device even though the errors are harmless.

To avoid changing virPCIGetDeviceAddressFromSysfsLink(),
virPCIDeviceAddressParse() and related code that has quite a few
call sites, add a function to check if a network device is a
PCI device and use it in virNetDevSwitchdevFeature() before attempting
to retrieve the PCI device.
---

Suggestions welcome on a better name for virPCIIsPCIDevice, or even
a better approach to solving this issue. Currently virPCIIsPCIDevice
assumes the device is PCI if its subsystem starts with 'pci'. I'd be
happy to hear if there is a better way to determine if a network
device is PCI.

 src/libvirt_private.syms |  1 +
 src/util/virnetdev.c     | 25 ++++++++++++++++++++++---
 src/util/virpci.c        | 32 ++++++++++++++++++++++++++++++++
 src/util/virpci.h        |  3 +++
 4 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a705fa846..bdf98ded1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2458,6 +2458,7 @@ virPCIGetVirtualFunctionInfo;
 virPCIGetVirtualFunctions;
 virPCIHeaderTypeFromString;
 virPCIHeaderTypeToString;
+virPCIIsPCIDevice;
 virPCIIsVirtualFunction;
 virPCIStubDriverTypeFromString;
 virPCIStubDriverTypeToString;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index eb2d119bf..a9af08797 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1148,6 +1148,21 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname,
 }
 
 
+static bool
+virNetDevIsPCIDevice(const char *devName)
+{
+    char *vfSysfsDevicePath = NULL;
+    bool ret;
+
+    if (virNetDevSysfsFile(&vfSysfsDevicePath, devName, "device/subsystem") < 0)
+        return false;
+
+    ret = virPCIIsPCIDevice(vfSysfsDevicePath);
+    VIR_FREE(vfSysfsDevicePath);
+    return ret;
+}
+
+
 static virPCIDevicePtr
 virNetDevGetPCIDevice(const char *devName)
 {
@@ -3236,14 +3251,18 @@ virNetDevSwitchdevFeature(const char *ifname,
     if (is_vf == 1 && virNetDevGetPhysicalFunction(ifname, &pfname) < 0)
         goto cleanup;
 
-    pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) :
-                              virNetDevGetPCIDevice(ifname);
+    if (pfname == NULL && VIR_STRDUP(pfname, ifname) < 0)
+        goto cleanup;
+
     /* No PCI device, then no feature bit to check/add */
-    if (pci_device_ptr == NULL) {
+    if (!virNetDevIsPCIDevice(pfname)) {
         ret = 0;
         goto cleanup;
     }
 
+    if ((pci_device_ptr = virNetDevGetPCIDevice(pfname)) == NULL)
+        goto cleanup;
+
     if (!(nl_msg = nlmsg_alloc_simple(family_id,
                                       NLM_F_REQUEST | NLM_F_ACK))) {
         virReportOOMError();
diff --git a/src/util/virpci.c b/src/util/virpci.c
index fe57bef32..f22d89cd7 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2651,6 +2651,38 @@ virPCIGetDeviceAddressFromSysfsLink(const char *device_link)
     return bdf;
 }
 
+/**
+ * virPCIIsPCIDevice:
+ * @device_link: sysfs path for the device
+ *
+ * Returns true if the device specified in @device_link sysfs
+ * path is a PCI device, otherwise returns false.
+ */
+bool
+virPCIIsPCIDevice(const char *device_link)
+{
+    char *device_path = NULL;
+    char *subsys = NULL;
+    bool ret;
+
+    if (!virFileExists(device_link)) {
+        VIR_DEBUG("'%s' does not exist", device_link);
+        return false;
+    }
+
+    device_path = canonicalize_file_name(device_link);
+    if (device_path == NULL) {
+        VIR_DEBUG("Failed to resolve device link '%s'", device_link);
+        return false;
+    }
+
+    subsys = last_component(device_path);
+    ret = STRPREFIX(subsys, "pci");
+
+    VIR_FREE(device_path);
+    return ret;
+}
+
 /**
  * virPCIGetPhysicalFunction:
  * @vf_sysfs_path: sysfs path for the virtual function
diff --git a/src/util/virpci.h b/src/util/virpci.h
index f1fbe39e6..489d8a777 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -190,6 +190,9 @@ int virPCIDeviceWaitForCleanup(virPCIDevicePtr dev, const char *matcher);
 virPCIDeviceAddressPtr
 virPCIGetDeviceAddressFromSysfsLink(const char *device_link);
 
+bool
+virPCIIsPCIDevice(const char *device_link);
+
 int virPCIGetPhysicalFunction(const char *vf_sysfs_path,
                               virPCIDeviceAddressPtr *pf);
 
-- 
2.15.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list