[libvirt PATCH v6 1/5] Add a PCI/PCIe device VPD Parser
Daniel P. Berrangé
berrange at redhat.com
Tue Oct 19 15:28:24 UTC 2021
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 at canonical.com>
> ---
> 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 :|
More information about the libvir-list
mailing list