From 0e566614ff3884f280b6219ee6dd4dd940b11d09 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 26 Mar 2024 15:06:58 +0100
Subject: [PATCH] pci: Remove error reporting from PCI VPD parsing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The PCI VPD (Vital Product Data) may be missing or the kernel can report
presence but not actually have the data. Also the data is specified by
the device vendor and thus may be invalid in some cases.

To avoid log spamming, since the only usage in the node device driver is
ignoring errors, remove all error reporting.

Closes: https://gitlab.com/libvirt/libvirt/-/issues/607
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/conf/node_device_conf.c | 13 +++++--------
 src/libvirt_private.syms    |  1 -
 src/util/virpci.c           | 39 ++++++-------------------------------
 src/util/virpci.h           |  1 -
 4 files changed, 11 insertions(+), 43 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 48140b17ba..32b69aae84 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -3066,15 +3066,12 @@ virNodeDeviceGetPCIVPDDynamicCap(virNodeDevCapPCIDev *devCapPCIDev)
     if (!(pciDev = virPCIDeviceNew(&devAddr)))
         return -1;
 
-    if (virPCIDeviceHasVPD(pciDev)) {
-        /* VPD is optional in PCI(e) specs. If it is there, attempt to add it. */
-        if ((res = virPCIDeviceGetVPD(pciDev))) {
-            devCapPCIDev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VPD;
-            devCapPCIDev->vpd = g_steal_pointer(&res);
-        } else {
-            virResetLastError();
-        }
+    /* VPD is optional in PCI(e) specs. If it is there, attempt to add it. */
+    if ((res = virPCIDeviceGetVPD(pciDev))) {
+        devCapPCIDev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VPD;
+        devCapPCIDev->vpd = g_steal_pointer(&res);
     }
+
     return 0;
 }
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6b6bcc368a..84e30b711c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3107,7 +3107,6 @@ virPCIDeviceGetUnbindFromStub;
 virPCIDeviceGetUsedBy;
 virPCIDeviceGetVPD;
 virPCIDeviceHasPCIExpressLink;
-virPCIDeviceHasVPD;
 virPCIDeviceIsAssignable;
 virPCIDeviceIsPCIExpress;
 virPCIDeviceListAdd;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 8becec4aa5..289c0b330b 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -3078,24 +3078,14 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
 }
 
 
-bool
-virPCIDeviceHasVPD(virPCIDevice *dev)
-{
-    g_autofree char *vpdPath = virPCIFile(dev->name, "vpd");
-    bool ret = virFileIsRegular(vpdPath);
-
-    VIR_DEBUG("path='%s', exists='%d'", vpdPath, ret);
-
-    return ret;
-}
-
 /**
  * virPCIDeviceGetVPD:
  * @dev: a PCI device to get a PCI VPD for.
  *
  * Obtain a PCI device's Vital Product Data (VPD). VPD is optional in
  * both PCI Local Bus and PCIe specifications so there is no guarantee it
- * will be there for a particular device.
+ * will be there for a particular device. The VPD data is returned in @vpd if
+ * it's available or otherwise NULL is set.
  *
  * Returns: a pointer to virPCIVPDResource which needs to be freed by the caller
  * or NULL if getting it failed for some reason (e.g. invalid format, I/O error).
@@ -3104,26 +3094,16 @@ virPCIVPDResource *
 virPCIDeviceGetVPD(virPCIDevice *dev)
 {
     g_autofree char *vpdPath = virPCIFile(dev->name, "vpd");
-    virPCIVPDResource *ret = NULL;
     VIR_AUTOCLOSE fd = -1;
 
-    if (!virPCIDeviceHasVPD(dev)) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, _("Device %1$s does not have a VPD"),
-                       virPCIDeviceGetName(dev));
-        return NULL;
-    }
+    fd = open(vpdPath, O_RDONLY);
 
-    if ((fd = open(vpdPath, O_RDONLY)) < 0) {
-        virReportSystemError(errno, _("Failed to open a VPD file '%1$s'"), vpdPath);
-        return NULL;
-    }
+    VIR_DEBUG("dev='%s' path='%s' fd='%d'", virPCIDeviceGetName(dev), vpdPath, fd);
 
-    if (!(ret = virPCIVPDParse(fd))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to parse device VPD data"));
+    if (fd < 0)
         return NULL;
-    }
 
-    return ret;
+    return virPCIVPDParse(fd);
 }
 
 #else
@@ -3200,17 +3180,10 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path G_GNUC_UNUSED,
     return -1;
 }
 
-bool
-virPCIDeviceHasVPD(virPCIDevice *dev G_GNUC_UNUSED)
-{
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
-    return NULL;
-}
 
 virPCIVPDResource *
 virPCIDeviceGetVPD(virPCIDevice *dev G_GNUC_UNUSED)
 {
-    virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
     return NULL;
 }
 #endif /* __linux__ */
diff --git a/src/util/virpci.h b/src/util/virpci.h
index a5bfe9c35d..ba5e0ae6f1 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -270,7 +270,6 @@ int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
                                  char **pfname,
                                  int *vf_index);
 
-bool virPCIDeviceHasVPD(virPCIDevice *dev);
 virPCIVPDResource * virPCIDeviceGetVPD(virPCIDevice *dev);
 
 int virPCIDeviceUnbind(virPCIDevice *dev);