[libvirt PATCH v2 0/3] PCI VPD: Handle More Edge Cases

Dmitrii Shcherbakov posted 3 patches 2 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211029185718.338025-1-dmitrii.shcherbakov@canonical.com
src/util/virpcivpd.c  |  63 +++++++---
tests/virpcivpdtest.c | 263 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 296 insertions(+), 30 deletions(-)
[libvirt PATCH v2 0/3] PCI VPD: Handle More Edge Cases
Posted by Dmitrii Shcherbakov 2 years, 5 months ago
This patch set improves edge case testing:

* The parser is now more strict about checking boundary conditions when
  parsing fields: an invalid field length is a possibility which is
	now being accounted for;
* The parser will now make sure that RV and RW fields are the last
  in their section by making sure that no more data is left to read
	after those;
* The RW field in the read-write section is not considered a VPD format
  violation even though it is a violation of the spec since it does not
  prevent Libvirt from parsing valid data for presenting it to a user.
  This is a policy decision made by Libvirt in favor of usability with
  hardware that does not strictly follow the PCI/PCIe VPD spec.

Invalid field values are now skipped instead of halting further parsing
completely.

Some vendors use 0xFF as placeholders in VPD-W since those values do not
correspond to printable ASCII characters, they will be discarded,
however, parsing will continue beyond that point.

Also, it turns out that some vendors use printable ASCII characters not
present in the alphanumeric range. Following a mailing list discussion
Libvirt will accept printable ASCII characters to avoid cases where
useful data is discarded.

https://listman.redhat.com/archives/libvir-list/2021-October/msg01043.html

Higher-level software needs to account for this character set and act
accordingly.

For example, the outcome of this is that one may get "N/A" as a value
for a serial number that is supposed to be unique, however, there is
no way for Libvirt to validate serial number uniqueness anyway even if
it was a different character sequence.

https://gitlab.com/dmitriis/libvirt/-/pipelines/398517951
(x86_64 only, have not set up arch-specific runners yet and over the
limit of what gitlab provides)

Dmitrii Shcherbakov (3):
  PCI VPD: handle additional edge cases
  PCI VPD: Skip fields with invalid values
  PCI VPD: Fix a wrong return code in a test case

 src/util/virpcivpd.c  |  63 +++++++---
 tests/virpcivpdtest.c | 263 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 296 insertions(+), 30 deletions(-)

-- 
2.32.0


Re: [libvirt PATCH v2 0/3] PCI VPD: Handle More Edge Cases
Posted by Daniel Henrique Barboza 2 years, 5 months ago

On 10/29/21 15:57, Dmitrii Shcherbakov wrote:
> This patch set improves edge case testing:
> 
> * The parser is now more strict about checking boundary conditions when
>    parsing fields: an invalid field length is a possibility which is
> 	now being accounted for;
> * The parser will now make sure that RV and RW fields are the last
>    in their section by making sure that no more data is left to read
> 	after those;
> * The RW field in the read-write section is not considered a VPD format
>    violation even though it is a violation of the spec since it does not
>    prevent Libvirt from parsing valid data for presenting it to a user.
>    This is a policy decision made by Libvirt in favor of usability with
>    hardware that does not strictly follow the PCI/PCIe VPD spec.


Libvirtd in a Power9 host is throwing these warnings on every startup:


Running 'src/libvirtd'...
2021-10-31 23:56:40.637+0000: 140468: info : libvirt version: 7.9.0
2021-10-31 23:56:40.637+0000: 140468: info : hostname: ltc-boston118
2021-10-31 23:56:40.804+0000: 140520: error : virPCIVPDParseVPDLargeResourceFields:588 : internal error: VPD-W does not contain the mandatory RW field
2021-10-31 23:56:40.808+0000: 140520: error : virPCIVPDParseVPDLargeResourceFields:588 : internal error: VPD-W does not contain the mandatory RW field
2021-10-31 23:56:40.813+0000: 140520: error : virPCIVPDParseVPDLargeResourceFields:588 : internal error: VPD-W does not contain the mandatory RW field
2021-10-31 23:56:40.817+0000: 140520: error : virPCIVPDParseVPDLargeResourceFields:588 : internal error: VPD-W does not contain the mandatory RW field


With this series the warnings are now gone.


All patches:

Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>




> 
> Invalid field values are now skipped instead of halting further parsing
> completely.
> 
> Some vendors use 0xFF as placeholders in VPD-W since those values do not
> correspond to printable ASCII characters, they will be discarded,
> however, parsing will continue beyond that point.
> 
> Also, it turns out that some vendors use printable ASCII characters not
> present in the alphanumeric range. Following a mailing list discussion
> Libvirt will accept printable ASCII characters to avoid cases where
> useful data is discarded.
> 
> https://listman.redhat.com/archives/libvir-list/2021-October/msg01043.html
> 
> Higher-level software needs to account for this character set and act
> accordingly.
> 
> For example, the outcome of this is that one may get "N/A" as a value
> for a serial number that is supposed to be unique, however, there is
> no way for Libvirt to validate serial number uniqueness anyway even if
> it was a different character sequence.
> 
> https://gitlab.com/dmitriis/libvirt/-/pipelines/398517951
> (x86_64 only, have not set up arch-specific runners yet and over the
> limit of what gitlab provides)
> 
> Dmitrii Shcherbakov (3):
>    PCI VPD: handle additional edge cases
>    PCI VPD: Skip fields with invalid values
>    PCI VPD: Fix a wrong return code in a test case
> 
>   src/util/virpcivpd.c  |  63 +++++++---
>   tests/virpcivpdtest.c | 263 ++++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 296 insertions(+), 30 deletions(-)
>