On Mon, Oct 11, 2021 at 05:04:42PM +0300, Dmitrii Shcherbakov wrote: > Add support for deserializing the binary PCI/PCIe VPD format and storing > results in memory. > > The VPD format is specified in "I.3. VPD Definitions" in PCI specs > (2.2+) and "6.28.1 VPD Format" PCIe 4.0. As section 6.28 in PCIe 4.0 > notes, the PCI Local Bus and PCIe VPD formats are binary compatible > and PCIe 4.0 merely started incorporating what was already present in > PCI specs. > > Linux kernel exposes a binary blob in the VPD format via sysfs since > v2.6.26 (commit 94e6108803469a37ee1e3c92dafdd1d59298602f) which requires > a parser to interpret. > > Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@xxxxxxxxxxxxx> > --- > build-aux/syntax-check.mk | 4 +- > po/POTFILES.in | 1 + > src/libvirt_private.syms | 18 + > src/util/meson.build | 1 + > src/util/virpcivpd.c | 743 ++++++++++++++++++++++++++++++++++++ > src/util/virpcivpd.h | 111 ++++++ > src/util/virpcivpdpriv.h | 48 +++ > tests/meson.build | 1 + > tests/testutils.c | 35 ++ > tests/testutils.h | 4 + > tests/virpcivpdtest.c | 768 ++++++++++++++++++++++++++++++++++++++ > 11 files changed, 1732 insertions(+), 2 deletions(-) > create mode 100644 src/util/virpcivpd.c > create mode 100644 src/util/virpcivpd.h > create mode 100644 src/util/virpcivpdpriv.h > create mode 100644 tests/virpcivpdtest.c > diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c > new file mode 100644 > index 0000000000..5caede815f > --- /dev/null > +++ b/src/util/virpcivpd.c > @@ -0,0 +1,743 @@ > +/* > + * virpcivpd.c: helper APIs for working with the PCI/PCIe VPD capability > + * > + * Copyright (C) 2021 Canonical Ltd. > + * > + * 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/>. > + */ > +void > +virPCIVPDResourceFree(virPCIVPDResource *res) > +{ > + if (!res) { > + return; > + } > + virPCIVPDResourceROFree(res->ro); > + virPCIVPDResourceRWFree(res->rw); g_free(res); > +} > + > +virPCIVPDResourceRO * > +virPCIVPDResourceRONew(void) > +{ > + g_autoptr(virPCIVPDResourceRO) ro = g_new0(virPCIVPDResourceRO, 1); > + ro->vendor_specific = g_ptr_array_new_full(0, (GDestroyNotify)virPCIVPDResourceCustomFree); > + return g_steal_pointer(&ro); > +} > + > +void > +virPCIVPDResourceROFree(virPCIVPDResourceRO *ro) > +{ > + if (!ro) { > + return; > + } > + g_free(ro->change_level); > + g_free(ro->manufacture_id); > + g_free(ro->part_number); > + g_free(ro->serial_number); > + g_ptr_array_unref(ro->vendor_specific); g_free(ro); > +} > + > +virPCIVPDResourceRW * > +virPCIVPDResourceRWNew(void) > +{ > + g_autoptr(virPCIVPDResourceRW) rw = g_new0(virPCIVPDResourceRW, 1); > + rw->vendor_specific = g_ptr_array_new_full(0, (GDestroyNotify)virPCIVPDResourceCustomFree); > + rw->system_specific = g_ptr_array_new_full(0, (GDestroyNotify)virPCIVPDResourceCustomFree); > + return g_steal_pointer(&rw); > +} > + > +void > +virPCIVPDResourceRWFree(virPCIVPDResourceRW *rw) > +{ > + if (!rw) { > + return; > + } > + g_free(rw->asset_tag); > + g_ptr_array_unref(rw->vendor_specific); > + g_ptr_array_unref(rw->system_specific); > +} g_free(rw); > + > +void > +virPCIVPDResourceCustomFree(virPCIVPDResourceCustom *custom) > +{ > + g_free(custom->value); > + g_free(custom); > +} > +/** > + * virPCIVPDResourceUpdateKeyword: > + * @res: A non-NULL pointer to a virPCIVPDResource where a keyword will be updated. > + * @readOnly: A bool specifying which section to update (in-memory): read-only or read-write. > + * @keyword: A non-NULL pointer to a name of the keyword that will be updated. > + * @value: A pointer to the keyword value or NULL. The value is copied on successful update. > + * > + * The caller is responsible for initializing the relevant RO or RW sections of the resource, > + * otherwise, false will be returned. > + * > + * Keyword names are either 2-byte keywords from the spec or their human-readable alternatives > + * used in XML elements. For vendor-specific and system-specific keywords only V%s and Y%s > + * (except "YA" which is an asset tag) formatted values are accepted. > + * > + * Returns: true if a keyword has been updated successfully, false otherwise. > + */ > +bool > +virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, > + const char *const keyword, const char *const value) > +{ > + if (!res) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Cannot update the resource: a NULL resource pointer has been provided.")); > + return false; > + } else if (!keyword) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Cannot update the resource: a NULL keyword pointer has been provided.")); > + return false; > + } > + > + if (readOnly) { > + if (!res->ro) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Cannot update the read-only keyword: RO section not initialized.")); > + return false; > + } > + > + if (STREQ("EC", keyword) || STREQ("change_level", keyword)) { > + res->ro->change_level = g_strdup(value); > + return true; > + } else if (STREQ("MN", keyword) || STREQ("manufacture_id", keyword)) { > + res->ro->manufacture_id = g_strdup(value); > + return true; > + } else if (STREQ("PN", keyword) || STREQ("part_number", keyword)) { > + res->ro->part_number = g_strdup(value); > + return true; > + } else if (STREQ("SN", keyword) || STREQ("serial_number", keyword)) { > + res->ro->serial_number = g_strdup(value); > + return true; > + } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { > + if (!virPCIVPDResourceCustomUpsertValue(res->ro->vendor_specific, keyword[1], value)) { > + return false; > + } > + return true; > + } else if (STREQ("FG", keyword) || STREQ("LC", keyword) || STREQ("PG", keyword)) { > + /* Legacy PICMIG keywords are skipped on purpose. */ > + return true; > + } else if (STREQ("CP", keyword)) { > + /* The CP keyword is currently not supported and is skipped. */ > + return true; > + } > + > + } else { > + if (!res->rw) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _ > + ("Cannot update the read-write keyword: read-write section not initialized.")); > + return false; > + } > + > + if (STREQ("YA", keyword) || STREQ("asset_tag", keyword)) { > + res->rw->asset_tag = g_strdup(value); > + return true; > + } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { > + if (!virPCIVPDResourceCustomUpsertValue(res->rw->vendor_specific, keyword[1], value)) { > + return false; > + } > + return true; > + } else if (virPCIVPDResourceIsSystemKeyword(keyword)) { > + if (!virPCIVPDResourceCustomUpsertValue(res->rw->system_specific, keyword[1], value)) { > + return false; > + } > + return true; > + } > + } > + /* Unsupported keyword. */ Earlier we have some genuine fatal errors we return false for. I'd say we should VIR_WARN("Unexpected keyword ....") here and then 'return true' to indicate success, since it is harmless..... > + return false; > +} > + /* The field format, keyword and value are determined. Attempt to update the resource. */ > + if (!virPCIVPDResourceUpdateKeyword(res, readOnly, fieldKeyword, fieldValue)) { > + /* Write an error but attempt to handle it gracefully by continuing. > + * Unexpected keywords may be a result of a spec extension in the future. */ > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Could not update the VPD resource keyword: %s"), fieldKeyword); ...so we can honour the failure status here, and not repeat virReportError which has already been done by the method we called. > diff --git a/src/util/virpcivpd.h b/src/util/virpcivpd.h > new file mode 100644 > index 0000000000..25aee5dffe > --- /dev/null > +++ b/src/util/virpcivpd.h > @@ -0,0 +1,111 @@ > +/* > + * virpcivpd.h: helper APIs for working with the PCI/PCIe VPD capability > + * > + * Copyright (C) 2021 Canonical Ltd. > + * > + * 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/>. > + */ > + > +#pragma once > + > +#include "internal.h" > + > +/* > + * PCI Local bus (2.2+, Appendix I) and PCIe 4.0+ (7.9.19 VPD Capability) define > + * the VPD capability structure (8 bytes in total) and VPD registers that can be used to access > + * VPD data including: > + * bit 31 of the first 32-bit DWORD: data transfer completion flag (between the VPD data register > + * and the VPD data storage hardware); > + * bits 30:16 of the first 32-bit DWORD: VPD address of the first VPD data byte to be accessed; > + * bits 31:0 of the second 32-bit DWORD: VPD data bytes with LSB being pointed to by the VPD address. > + * > + * Given that only 15 bits (30:16) are allocated for VPD address its mask is 0x7fff. > +*/ > +#define PCI_VPD_ADDR_MASK 0x7FFF > + > +/* > + * VPD data consists of small and large resource data types. Information within a resource type > + * consists of a 2-byte keyword, 1-byte length and data bytes (up to 255). > +*/ > +#define PCI_VPD_MAX_FIELD_SIZE 255 > +#define PCI_VPD_LARGE_RESOURCE_FLAG 0x80 > +#define PCI_VPD_STRING_RESOURCE_FLAG 0x02 > +#define PCI_VPD_READ_ONLY_LARGE_RESOURCE_FLAG 0x10 > +#define PCI_VPD_READ_WRITE_LARGE_RESOURCE_FLAG 0x11 > +#define PCI_VPD_RESOURCE_END_TAG 0x0F > +#define PCI_VPD_RESOURCE_END_VAL PCI_VPD_RESOURCE_END_TAG << 3 > + > +typedef enum { > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_TEXT = 1, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_BINARY, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR, > + VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_LAST > +} virPCIVPDResourceFieldValueFormat; > + > +virPCIVPDResourceFieldValueFormat virPCIVPDResourceGetFieldValueFormat(const char *value); THe pieces above this point are only needed by the parser, so can all go in the .c file, so the .h is only stuff needed by the external callers. > + > +typedef struct virPCIVPDResourceCustom virPCIVPDResourceCustom; > +struct virPCIVPDResourceCustom { > + char idx; > + char *value; > +}; > + > +typedef struct virPCIVPDResourceRO virPCIVPDResourceRO; > +struct virPCIVPDResourceRO { > + char *part_number; > + char *change_level; > + char *manufacture_id; > + char *serial_number; > + GPtrArray *vendor_specific; > +}; > + > +typedef struct virPCIVPDResourceRW virPCIVPDResourceRW; > +struct virPCIVPDResourceRW { > + char *asset_tag; > + GPtrArray *vendor_specific; > + GPtrArray *system_specific; > +}; > + > +typedef struct virPCIVPDResource virPCIVPDResource; > +struct virPCIVPDResource { > + char *name; > + virPCIVPDResourceRO *ro; > + virPCIVPDResourceRW *rw; > +}; > + > + > +virPCIVPDResource *virPCIVPDParse(int vpdFileFd); > +void virPCIVPDResourceFree(virPCIVPDResource *res); > + > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVPDResource, virPCIVPDResourceFree); > + > +virPCIVPDResourceRO *virPCIVPDResourceRONew(void); > +void virPCIVPDResourceROFree(virPCIVPDResourceRO *ro); > + > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVPDResourceRO, virPCIVPDResourceROFree); > + > +virPCIVPDResourceRW *virPCIVPDResourceRWNew(void); > +void virPCIVPDResourceRWFree(virPCIVPDResourceRW *rw); > + > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVPDResourceRW, virPCIVPDResourceRWFree); > + > +bool > +virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, > + const char *const keyword, const char *const value); > + > +void virPCIVPDResourceCustomFree(virPCIVPDResourceCustom *custom); > + > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVPDResourceCustom, virPCIVPDResourceCustomFree); > +static int > +testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) > +{ > + size_t i = 0; > + g_autoptr(virPCIVPDResourceRO) ro = virPCIVPDResourceRONew(); > + g_autoptr(virPCIVPDResourceRW) rw = virPCIVPDResourceRWNew(); > + const TestPCIVPDKeywordValue readOnlyCases[] = { > + {.keyword = "EC", .value = "level1", .actual = &ro->change_level}, > + {.keyword = "EC", .value = "level2", .actual = &ro->change_level}, > + {.keyword = "change_level", .value = "level3", .actual = &ro->change_level}, > + {.keyword = "PN", .value = "number1", .actual = &ro->part_number}, > + {.keyword = "PN", .value = "number2", .actual = &ro->part_number}, > + {.keyword = "part_number", .value = "number3", .actual = &ro->part_number}, > + {.keyword = "MN", .value = "id1", .actual = &ro->manufacture_id}, > + {.keyword = "MN", .value = "id2", .actual = &ro->manufacture_id}, > + {.keyword = "manufacture_id", .value = "id3", &ro->manufacture_id}, > + {.keyword = "SN", .value = "serial1", .actual = &ro->serial_number}, > + {.keyword = "SN", .value = "serial2", .actual = &ro->serial_number}, > + {.keyword = "serial_number", .value = "serial3", .actual = &ro->serial_number}, > + }; These repeated values of the same keyword are causing memory leaks as the struct only stores a single value. We should free the existong value if we see repeats to avoid te leak > + const TestPCIVPDKeywordValue readWriteCases[] = { > + {.keyword = "YA", .value = "tag1", .actual = &ro->change_level}, > + {.keyword = "YA", .value = "tag2", .actual = &ro->change_level}, > + {.keyword = "asset_tag", .value = "tag3", .actual = &ro->change_level}, > + }; > + size_t numROCases = sizeof(readOnlyCases) / sizeof(TestPCIVPDKeywordValue); > + size_t numRWCases = sizeof(readWriteCases) / sizeof(TestPCIVPDKeywordValue); > + g_autoptr(virPCIVPDResource) res = g_new0(virPCIVPDResource, 1); > + virPCIVPDResourceCustom *custom = NULL; > + > + g_autofree char *val = g_strdup("testval"); > + res->name = g_steal_pointer(&val); > + > + /* RO has not been initialized - make sure updates fail. */ > + for (i = 0; i < numROCases; ++i) { > + if (virPCIVPDResourceUpdateKeyword(res, true, > + readOnlyCases[i].keyword, > + readOnlyCases[i].value)) { > + return -1; > + } > + } > + /* RW has not been initialized - make sure updates fail. */ > + for (i = 0; i < numRWCases; ++i) { > + if (virPCIVPDResourceUpdateKeyword(res, false, > + readWriteCases[i].keyword, > + readWriteCases[i].value)) { > + return -1; > + } > + } > + /* Initialize RO */ > + res->ro = g_steal_pointer(&ro); > + > + /* Update keywords one by one and compare actual values with the expected ones. */ > + for (i = 0; i < numROCases; ++i) { > + if (!virPCIVPDResourceUpdateKeyword(res, true, > + readOnlyCases[i].keyword, > + readOnlyCases[i].value)) { > + return -1; > + } > + if (STRNEQ(readOnlyCases[i].value, *readOnlyCases[i].actual)) { > + return -1; > + } > + } > + > + /* Do a basic vendor field check. */ > + if (!virPCIVPDResourceUpdateKeyword(res, true, "V0", "vendor0")) { > + return -1; > + } > + if (res->ro->vendor_specific->len != 1) { > + return -1; > + } > + custom = g_ptr_array_index(res->ro->vendor_specific, 0); > + if (custom->idx != '0' || STRNEQ(custom->value, "vendor0")) { > + return -1; > + } > + > + /* Check that RW updates fail if RW has not been initialized. */ > + if (virPCIVPDResourceUpdateKeyword(res, false, "YA", "tag1")) { > + return -1; > + } > + if (virPCIVPDResourceUpdateKeyword(res, false, "asset_tag", "tag1")) { > + return -1; > + } > + > + /* Initialize RW */ > + res->rw = g_steal_pointer(&rw); > + if (!virPCIVPDResourceUpdateKeyword(res, false, "YA", "tag1") > + || STRNEQ(res->rw->asset_tag, "tag1")) { > + return -1; > + } > + if (!virPCIVPDResourceUpdateKeyword(res, false, "asset_tag", "tag2") > + || STRNEQ(res->rw->asset_tag, "tag2")) { > + return -1; > + } > + > + /* Do a basic system field check. */ > + if (!virPCIVPDResourceUpdateKeyword(res, false, "Y0", "system0")) { > + return -1; > + } > + if (res->rw->system_specific->len != 1) { > + return -1; > + } > + custom = g_ptr_array_index(res->rw->system_specific, 0); > + if (custom->idx != '0' || STRNEQ(custom->value, "system0")) { > + return -1; > + } > + > + /* Just make sure the name has not been changed during keyword updates. */ > + if (!STREQ_NULLABLE(res->name, "testval")) { > + return -1; > + } > + return 0; > +} > +static int > +testVirPCIVPDParseFullVPD(const void *opaque G_GNUC_UNUSED) > +{ > + int fd = -1; > + size_t dataLen = 0; > + int ret = 0; > + > + virPCIVPDResource *res = NULL; g_autoptr > + virPCIVPDResourceCustom *custom = NULL; > + > + const uint8_t fullVPDExample[] = { > + VPD_STRING_RESOURCE_EXAMPLE_HEADER, VPD_STRING_RESOURCE_EXAMPLE_DATA, > + VPD_R_FIELDS_EXAMPLE_HEADER, VPD_R_FIELDS_EXAMPLE_DATA, > + VPD_W_FIELDS_EXAMPLE_HEADER, VPD_W_EXAMPLE_FIELDS, > + PCI_VPD_RESOURCE_END_VAL > + }; > + > + dataLen = sizeof(fullVPDExample) / sizeof(uint8_t); > + fd = virCreateAnonymousFile(fullVPDExample, dataLen); > + res = virPCIVPDParse(fd); > + VIR_FORCE_CLOSE(fd); > + > + if (!res) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + "The resource pointer is NULL after parsing which is unexpected"); > + return ret; > + } > + > + if (!res->ro) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + "Read-only keywords are missing from the VPD resource."); > + return -1; > + } else if (!res->rw) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + "Read-write keywords are missing from the VPD resource."); > + return -1; > + } > + > + if (testVirPCIVPDValidateExampleReadOnlyFields(res)) { > + return -1; > + } > + > + if (STRNEQ_NULLABLE(res->rw->asset_tag, "ID42")) { > + return -1; > + } > + > + if (!res->rw->vendor_specific) { > + return -1; > + } > + custom = g_ptr_array_index(res->rw->vendor_specific, 0); > + if (custom->idx != 'Z' || STRNEQ_NULLABLE(custom->value, "42")) { > + return -1; > + } > + > + if (!res->rw->system_specific) { > + return -1; > + } > + > + custom = g_ptr_array_index(res->rw->system_specific, 0); > + if (custom->idx != 'F' || STRNEQ_NULLABLE(custom->value, "EX")) { > + return -1; > + } > + > + custom = g_ptr_array_index(res->rw->system_specific, 1); > + if (custom->idx != 'E' || STRNEQ_NULLABLE(custom->value, "")) { > + return -1; > + } > + return ret; > +} > + > +static int > +testVirPCIVPDParseFullVPDSkipInvalidKeywords(const void *opaque G_GNUC_UNUSED) > +{ > + int fd = -1; > + size_t dataLen = 0; > + > + virPCIVPDResource *res = NULL; g_autoptr > + > + const uint8_t fullVPDExample[] = { > + VPD_STRING_RESOURCE_EXAMPLE_HEADER, > + VPD_STRING_RESOURCE_EXAMPLE_DATA, > + PCI_VPD_LARGE_RESOURCE_FLAG | PCI_VPD_READ_ONLY_LARGE_RESOURCE_FLAG, 0x25, 0x00, > + VPD_R_EXAMPLE_FIELDS, > + /* The keywords below (except for "RV") are invalid but will be skipped by the parser */ > + 0x07, 'A', 0x02, 0x00, 0x00, > + 'V', 0x07, 0x02, 0x00, 0x00, > + 'e', 'x', 0x02, 0x00, 0x00, > + 'R', 'V', 0x02, 0x9A, 0x00, > + PCI_VPD_RESOURCE_END_VAL > + }; > + > + dataLen = sizeof(fullVPDExample) / sizeof(uint8_t); > + fd = virCreateAnonymousFile(fullVPDExample, dataLen); > + res = virPCIVPDParse(fd); > + VIR_FORCE_CLOSE(fd); > + > + if (!res) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + "The resource pointer is NULL after parsing which is unexpected."); > + return -1; > + } > + if (!res->ro) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + "The RO portion of the VPD resource is NULL."); > + return -1; > + } > + > + if (testVirPCIVPDValidateExampleReadOnlyFields(res)) { > + return -1; > + } > + return 0; > +} > + > +static int > +mymain(void) > +{ > + int ret = 0; > + > + if (virTestRun("Basic functionality of virPCIVPDResource ", testPCIVPDResourceBasic, NULL) < 0) > + ret = -1; > + if (virTestRun("Custom field index comparison", > + testPCIVPDResourceCustomCompareIndex, NULL) < 0) > + ret = -1; > + if (virTestRun("Custom field value insertion and updates ", > + testPCIVPDResourceCustomUpsertValue, NULL) < 0) > + ret = -1; > + if (virTestRun("Valid text values ", testPCIVPDIsValidTextValue, NULL) < 0) > + ret = -1; > + if (virTestRun("Determining a field value format by a key ", > + testPCIVPDGetFieldValueFormat, NULL) < 0) > + ret = -1; > + if (virTestRun("Reading VPD bytes ", testVirPCIVPDReadVPDBytes, NULL) < 0) > + ret = -1; > + if (virTestRun("Parsing VPD string resources ", testVirPCIVPDParseVPDStringResource, NULL) < 0) > + ret = -1; > + if (virTestRun("Parsing a VPD resource with an invalid keyword ", > + testVirPCIVPDParseFullVPDSkipInvalidKeywords, NULL) < 0) > + ret = -1; > + if (virTestRun("Parsing VPD resources from a full VPD ", testVirPCIVPDParseFullVPD, NULL) < 0) > + ret = -1; > + if (virTestRun("Parsing invalid VPD records ", testVirPCIVPDParseFullVPDInvalid, NULL) < 0) > + ret = -1; > + > + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; > +} > + > +VIR_TEST_MAIN(mymain) > +#endif running valgrind --leak-check=full virpcivpdtest reports some errors. I pointed out most of them above I think, but might be some more to catch. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
