External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 13:45:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

virpcivpd: Revert error reporting from PCI VPD parser

Browse Source 
The VPD parsing is fragile and depends on hardware vendor's adherance to
standards. Since libvirt only ever uses this data to report it in the
nodedev XML which ignores any errors there's no much point in having
error reporting which I've added recently.

Turn the errors into VIR_DEBUG statements in preparation for upcoming
patch which completely removes the expectation to report errors.

This effectively reverts commits dfc85658bd and f85a382a0e.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Peter Krempa 2024-03-27 13:52:54 +01:00
parent 5afaf127c0
commit 34f7ca668f
3 changed files with 27 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
po/POTFILES
View File

@ -313,7 +313,6 @@ src/util/virnuma.c
src/util/virnvme.c src/util/virnvme.c
src/util/virobject.c src/util/virobject.c
src/util/virpci.c src/util/virpci.c
src/util/virpcivpd.c
src/util/virperf.c src/util/virperf.c
src/util/virpidfile.c src/util/virpidfile.c
src/util/virpolkit.c src/util/virpolkit.c

8
src/util/virpci.c
View File

@ -3104,6 +3104,7 @@ virPCIVPDResource *
virPCIDeviceGetVPD(virPCIDevice *dev) virPCIDeviceGetVPD(virPCIDevice *dev)
{ {
g_autofree char *vpdPath = virPCIFile(dev->name, "vpd"); g_autofree char *vpdPath = virPCIFile(dev->name, "vpd");
virPCIVPDResource *ret = NULL;
VIR_AUTOCLOSE fd = -1; VIR_AUTOCLOSE fd = -1;
if (!virPCIDeviceHasVPD(dev)) { if (!virPCIDeviceHasVPD(dev)) {
@ -3117,7 +3118,12 @@ virPCIDeviceGetVPD(virPCIDevice *dev)
return NULL; return NULL;
} }
return virPCIVPDParse(fd); if (!(ret = virPCIVPDParse(fd))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to parse device VPD data"));
return NULL;
}
return ret;
} }
#else #else

50
src/util/virpcivpd.c
View File

@ -361,9 +361,9 @@ virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res,
* @offset: The offset at which bytes need to be read. * @offset: The offset at which bytes need to be read.
* @csum: A pointer to a byte containing the current checksum value. Mutated by this function. * @csum: A pointer to a byte containing the current checksum value. Mutated by this function.
* *
* Returns 0 if exactly @count bytes were read from @vpdFileFd. The csum value is * Returns 0 if exactly @count bytes were read from @vpdFileFd. The csum value
* also modified as bytes are read. If an error occurs while reading data from the VPD file * is also modified as bytes are read. If an error occurs while reading data
* descriptor, it is reported and -1 is returned to the caller. * from the VPD file descriptor -1 is returned to the caller.
*/ */
static int static int
virPCIVPDReadVPDBytes(int vpdFileFd, virPCIVPDReadVPDBytes(int vpdFileFd,
@ -375,8 +375,7 @@ virPCIVPDReadVPDBytes(int vpdFileFd,
ssize_t numRead = pread(vpdFileFd, buf, count, offset); ssize_t numRead = pread(vpdFileFd, buf, count, offset);
if (numRead != count) { if (numRead != count) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("read='%zd' expected='%zu'", numRead, count);
_("failed to read the PCI VPD data"));
return -1; return -1;
} }
@ -447,8 +446,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd,
case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR: case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR:
if (readOnly) { if (readOnly) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("unexpected RW keyword in read-only section");
_("failed to read the PCI VPD data: unexpected RW keyword in read-only section"));
return -1; return -1;
} }
@ -457,8 +455,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd,
case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD: case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD:
if (!readOnly) { if (!readOnly) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("unexpected RV keyword in read-write section");
_("failed to read the PCI VPD data: unexpected RV keyword in read-write section"));
return -1; return -1;
} }
/* Only need one byte to be read and accounted towards /* Only need one byte to be read and accounted towards
@ -477,8 +474,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd,
if (resPos + resDataLen < fieldPos + fieldDataLen) { if (resPos + resDataLen < fieldPos + fieldDataLen) {
/* In this case the field cannot simply be skipped since the position of the /* In this case the field cannot simply be skipped since the position of the
* next field is determined based on the length of a previous field. */ * next field is determined based on the length of a previous field. */
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("data field length invalid");
_("failed to read the PCI VPD data: data field length invalid"));
return -1; return -1;
} }
@ -517,8 +513,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd,
case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD: case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD:
if (*csum) { if (*csum) {
/* All bytes up to and including the checksum byte should add up to 0. */ /* All bytes up to and including the checksum byte should add up to 0. */
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("invalid checksum");
_("failed to read the PCI VPD data: invalid checksum"));
return -1; return -1;
} }
hasChecksum = true; hasChecksum = true;
@ -546,14 +541,12 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd,
* they were not the last fields in the section. */ * they were not the last fields in the section. */
if ((fieldPos < resPos + resDataLen)) { if ((fieldPos < resPos + resDataLen)) {
/* unparsed data still present */ /* unparsed data still present */
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("parsing ended prematurely");
_("failed to read the PCI VPD data: parsing ended prematurely"));
return -1; return -1;
} }
if (readOnly && !hasChecksum) { if (readOnly && !hasChecksum) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("missing mandatory checksum");
_("failed to read the PCI VPD data: missing mandatory checksum"));
return -1; return -1;
} }
@ -568,7 +561,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd,
* @resDataLen: A length of the data portion of a resource. * @resDataLen: A length of the data portion of a resource.
* @csum: A pointer to a 1-byte checksum. * @csum: A pointer to a 1-byte checksum.
* *
* Returns: 0 on success -1 and an error on failure * Returns: 0 on success -1 on failure. No libvirt errors are reported.
*/ */
static int static int
virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos, virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos,
@ -584,8 +577,7 @@ virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos,
resValue = g_strdup(g_strstrip(buf)); resValue = g_strdup(g_strstrip(buf));
if (!virPCIVPDResourceIsValidTextValue(resValue)) { if (!virPCIVPDResourceIsValidTextValue(resValue)) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("PCI VPD string contains invalid characters");
_("failed to parse PCI VPD string value with invalid characters"));
return -1; return -1;
} }
res->name = g_steal_pointer(&resValue); res->name = g_steal_pointer(&resValue);
@ -599,7 +591,7 @@ virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos,
* Parse a PCI device's Vital Product Data (VPD) contained in a file descriptor. * Parse a PCI device's Vital Product Data (VPD) contained in a file descriptor.
* *
* Returns: a pointer to a GList of VPDResource types which needs to be freed by the caller or * Returns: a pointer to a GList of VPDResource types which needs to be freed by the caller or
* NULL if getting it failed for some reason. * NULL if getting it failed for some reason. No libvirt errors are reported.
*/ */
virPCIVPDResource * virPCIVPDResource *
virPCIVPDParse(int vpdFileFd) virPCIVPDParse(int vpdFileFd)
@ -629,7 +621,8 @@ virPCIVPDParse(int vpdFileFd)
if (tag & PCI_VPD_LARGE_RESOURCE_FLAG) { if (tag & PCI_VPD_LARGE_RESOURCE_FLAG) {
if (resPos > PCI_VPD_ADDR_MASK + 1 - 3) { if (resPos > PCI_VPD_ADDR_MASK + 1 - 3) {
/* Bail if the large resource starts at the position where the end tag should be. */ /* Bail if the large resource starts at the position where the end tag should be. */
goto malformed; VIR_DEBUG("expected end tag, got more data");
return NULL;
} }
/* Read the two length bytes of the large resource record. */ /* Read the two length bytes of the large resource record. */
@ -652,8 +645,7 @@ virPCIVPDParse(int vpdFileFd)
if (tag == PCI_VPD_RESOURCE_END_TAG) { if (tag == PCI_VPD_RESOURCE_END_TAG) {
/* Stop VPD traversal since the end tag was encountered. */ /* Stop VPD traversal since the end tag was encountered. */
if (!hasReadOnly) { if (!hasReadOnly) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s", VIR_DEBUG("missing read-only section");
_("failed to read the PCI VPD data: missing read-only section"));
return NULL; return NULL;
} }
@ -662,7 +654,8 @@ virPCIVPDParse(int vpdFileFd)
if (resDataLen > PCI_VPD_ADDR_MASK + 1 - resPos) { if (resDataLen > PCI_VPD_ADDR_MASK + 1 - resPos) {
/* Bail if the resource is too long to fit into the VPD address space. */ /* Bail if the resource is too long to fit into the VPD address space. */
goto malformed; VIR_DEBUG("VPD resource too long");
return NULL;
} }
switch (tag) { switch (tag) {
@ -698,9 +691,7 @@ virPCIVPDParse(int vpdFileFd)
resPos += resDataLen; resPos += resDataLen;
} }
malformed: VIR_DEBUG("unexpected end of VPD data, expected end tag");
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("failed to read the PCI VPD data: malformed data"));
return NULL; return NULL;
} }
@ -709,8 +700,7 @@ virPCIVPDParse(int vpdFileFd)
virPCIVPDResource * virPCIVPDResource *
virPCIVPDParse(int vpdFileFd G_GNUC_UNUSED) virPCIVPDParse(int vpdFileFd G_GNUC_UNUSED)
{ {
virReportError(VIR_ERR_NO_SUPPORT, "%s", VIR_DEBUG("not implemented");
_("PCI VPD reporting not available on this platform"));
return NULL; return NULL;
} }