diff --git a/libvirt-Don-t-overwrite-error-message-from-virXPathNodeSet.patch b/libvirt-Don-t-overwrite-error-message-from-virXPathNodeSet.patch new file mode 100644 index 0000000..9848939 --- /dev/null +++ b/libvirt-Don-t-overwrite-error-message-from-virXPathNodeSet.patch @@ -0,0 +1,679 @@ +From 9a31f486329e36bbe6f6156eb89d4d455fc0a7d8 Mon Sep 17 00:00:00 2001 +Message-ID: <9a31f486329e36bbe6f6156eb89d4d455fc0a7d8.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 15:50:27 +0100 +Subject: [PATCH] Don't overwrite error message from 'virXPathNodeSet' +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +'virXPathNodeSet' returns -1 only when 'ctxt' or 'xpath' are NULL or +when the 'xpath' string is invalid. Both are programming errors. It +doesn't make sense for the code to overwrite the error message for +anything supposedly more relevant. + +The majority of calls to 'virXPathNodeSet' already didn't do this, so +this patch fixes the rest to prevent it from spreading again. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit a9f76d6ab7278864150d9f4776750ea22d7ef508) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/conf/domain_conf.c | 78 +++++++------------------------ + src/conf/network_conf.c | 80 ++++++++------------------------ + src/conf/node_device_conf.c | 17 ++----- + src/conf/numa_conf.c | 15 +++--- + src/cpu/cpu_ppc64.c | 5 +- + src/qemu/qemu_capabilities.c | 30 +++--------- + src/qemu/qemu_domain.c | 23 +++------ + src/qemu/qemu_migration_cookie.c | 5 +- + src/qemu/qemu_nbdkit.c | 5 +- + src/vz/vz_sdk.c | 5 +- + 10 files changed, 72 insertions(+), 191 deletions(-) + +diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c +index ac06fa39f6..52a5796ad2 100644 +--- a/src/conf/domain_conf.c ++++ b/src/conf/domain_conf.c +@@ -17765,11 +17765,8 @@ virDomainResctrlMonDefParse(virDomainDef *def, + + ctxt->node = node; + +- if ((n = virXPathNodeSet("./monitor", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("Cannot extract monitor nodes")); ++ if ((n = virXPathNodeSet("./monitor", ctxt, &nodes)) < 0) + goto cleanup; +- } + + for (i = 0; i < n; i++) { + domresmon = g_new0(virDomainResctrlMonDef, 1); +@@ -17897,11 +17894,8 @@ virDomainCachetuneDefParse(virDomainDef *def, + if (virBitmapIsAllClear(vcpus)) + return 0; + +- if ((n = virXPathNodeSet("./cache", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("Cannot extract cache nodes under cachetune")); ++ if ((n = virXPathNodeSet("./cache", ctxt, &nodes)) < 0) + return -1; +- } + + if (virDomainResctrlVcpuMatch(def, vcpus, &resctrl) < 0) + return -1; +@@ -18167,11 +18161,8 @@ virDomainDefParseMemory(virDomainDef *def, + + if (virXPathNode("./memoryBacking/hugepages", ctxt)) { + /* hugepages will be used */ +- if ((n = virXPathNodeSet("./memoryBacking/hugepages/page", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract hugepages nodes")); ++ if ((n = virXPathNodeSet("./memoryBacking/hugepages/page", ctxt, &nodes)) < 0) + return -1; +- } + + if (n) { + def->mem.hugepages = g_new0(virDomainHugePage, n); +@@ -18255,11 +18246,8 @@ virDomainMemorytuneDefParse(virDomainDef *def, + if (virBitmapIsAllClear(vcpus)) + return 0; + +- if ((n = virXPathNodeSet("./node", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("Cannot extract memory nodes under memorytune")); ++ if ((n = virXPathNodeSet("./node", ctxt, &nodes)) < 0) + return -1; +- } + + if (virDomainResctrlVcpuMatch(def, vcpus, &resctrl) < 0) + return -1; +@@ -18326,11 +18314,9 @@ virDomainDefTunablesParse(virDomainDef *def, + &def->blkio.weight) < 0) + def->blkio.weight = 0; + +- if ((n = virXPathNodeSet("./blkiotune/device", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "%s", _("cannot extract blkiotune nodes")); ++ if ((n = virXPathNodeSet("./blkiotune/device", ctxt, &nodes)) < 0) + return -1; +- } ++ + if (n) + def->blkio.devices = g_new0(virBlkioDevice, n); + +@@ -18441,11 +18427,8 @@ virDomainDefTunablesParse(virDomainDef *def, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./cputune/emulatorpin", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract emulatorpin nodes")); ++ if ((n = virXPathNodeSet("./cputune/emulatorpin", ctxt, &nodes)) < 0) + return -1; +- } + + if (n) { + if (n > 1) { +@@ -18460,11 +18443,8 @@ virDomainDefTunablesParse(virDomainDef *def, + VIR_FREE(nodes); + + +- if ((n = virXPathNodeSet("./cputune/iothreadpin", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract iothreadpin nodes")); ++ if ((n = virXPathNodeSet("./cputune/iothreadpin", ctxt, &nodes)) < 0) + return -1; +- } + + for (i = 0; i < n; i++) { + if (virDomainIOThreadPinDefParseXML(nodes[i], def) < 0) +@@ -18472,11 +18452,8 @@ virDomainDefTunablesParse(virDomainDef *def, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./cputune/vcpusched", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract vcpusched nodes")); ++ if ((n = virXPathNodeSet("./cputune/vcpusched", ctxt, &nodes)) < 0) + return -1; +- } + + for (i = 0; i < n; i++) { + if (virDomainVcpuThreadSchedParse(nodes[i], def) < 0) +@@ -18484,11 +18461,8 @@ virDomainDefTunablesParse(virDomainDef *def, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./cputune/iothreadsched", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract iothreadsched nodes")); ++ if ((n = virXPathNodeSet("./cputune/iothreadsched", ctxt, &nodes)) < 0) + return -1; +- } + + for (i = 0; i < n; i++) { + if (virDomainIOThreadSchedParse(nodes[i], def) < 0) +@@ -18496,11 +18470,8 @@ virDomainDefTunablesParse(virDomainDef *def, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./cputune/emulatorsched", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract emulatorsched nodes")); ++ if ((n = virXPathNodeSet("./cputune/emulatorsched", ctxt, &nodes)) < 0) + return -1; +- } + + if (n) { + if (n > 1) { +@@ -18514,11 +18485,8 @@ virDomainDefTunablesParse(virDomainDef *def, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./cputune/cachetune", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract cachetune nodes")); ++ if ((n = virXPathNodeSet("./cputune/cachetune", ctxt, &nodes)) < 0) + return -1; +- } + + for (i = 0; i < n; i++) { + if (virDomainCachetuneDefParse(def, ctxt, nodes[i], flags) < 0) +@@ -18526,11 +18494,8 @@ virDomainDefTunablesParse(virDomainDef *def, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./cputune/memorytune", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract memorytune nodes")); ++ if ((n = virXPathNodeSet("./cputune/memorytune", ctxt, &nodes)) < 0) + return -1; +- } + + for (i = 0; i < n; i++) { + if (virDomainMemorytuneDefParse(def, ctxt, nodes[i], flags) < 0) +@@ -18834,11 +18799,8 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, + !virDomainIOThreadIDArrayHasPin(def)) + def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO; + +- if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "%s", _("cannot extract resource nodes")); ++ if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) + return NULL; +- } + + if (n > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", +@@ -18886,11 +18848,9 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, + return NULL; + + /* analysis of the resource leases */ +- if ((n = virXPathNodeSet("./devices/lease", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "%s", _("cannot extract device leases")); ++ if ((n = virXPathNodeSet("./devices/lease", ctxt, &nodes)) < 0) + return NULL; +- } ++ + if (n) + def->leases = g_new0(virDomainLeaseDef *, n); + for (i = 0; i < n; i++) { +@@ -19009,11 +18969,9 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./devices/console", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "%s", _("cannot extract console devices")); ++ if ((n = virXPathNodeSet("./devices/console", ctxt, &nodes)) < 0) + return NULL; +- } ++ + if (n) + def->consoles = g_new0(virDomainChrDef *, n); + +diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c +index 6f8a0d2d0b..52c90e53f8 100644 +--- a/src/conf/network_conf.c ++++ b/src/conf/network_conf.c +@@ -892,13 +892,9 @@ virNetworkDNSDefParseXML(const char *networkName, + &def->forwardPlainNames) < 0) + return -1; + +- nfwds = virXPathNodeSet("./forwarder", ctxt, &fwdNodes); +- if (nfwds < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((nfwds = virXPathNodeSet("./forwarder", ctxt, &fwdNodes)) < 0) + return -1; +- } ++ + if (nfwds > 0) { + def->forwarders = g_new0(virNetworkDNSForwarder, nfwds); + +@@ -922,13 +918,9 @@ virNetworkDNSDefParseXML(const char *networkName, + } + } + +- nhosts = virXPathNodeSet("./host", ctxt, &hostNodes); +- if (nhosts < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((nhosts = virXPathNodeSet("./host", ctxt, &hostNodes)) < 0) + return -1; +- } ++ + if (nhosts > 0) { + def->hosts = g_new0(virNetworkDNSHostDef, nhosts); + +@@ -941,13 +933,9 @@ virNetworkDNSDefParseXML(const char *networkName, + } + } + +- nsrvs = virXPathNodeSet("./srv", ctxt, &srvNodes); +- if (nsrvs < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((nsrvs = virXPathNodeSet("./srv", ctxt, &srvNodes)) < 0) + return -1; +- } ++ + if (nsrvs > 0) { + def->srvs = g_new0(virNetworkDNSSrvDef, nsrvs); + +@@ -960,13 +948,9 @@ virNetworkDNSDefParseXML(const char *networkName, + } + } + +- ntxts = virXPathNodeSet("./txt", ctxt, &txtNodes); +- if (ntxts < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((ntxts = virXPathNodeSet("./txt", ctxt, &txtNodes)) < 0) + return -1; +- } ++ + if (ntxts > 0) { + def->txts = g_new0(virNetworkDNSTxtDef, ntxts); + +@@ -1222,13 +1206,10 @@ virNetworkForwardNatDefParseXML(const char *networkName, + return -1; + + /* addresses for SNAT */ +- nNatAddrs = virXPathNodeSet("./address", ctxt, &natAddrNodes); +- if (nNatAddrs < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid
element found in of network %1$s"), +- networkName); ++ if ((nNatAddrs = virXPathNodeSet("./address", ctxt, &natAddrNodes)) < 0) + return -1; +- } else if (nNatAddrs > 1) { ++ ++ if (nNatAddrs > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one
element is allowed in in in network %1$s"), + networkName); +@@ -1284,13 +1265,10 @@ virNetworkForwardNatDefParseXML(const char *networkName, + } + + /* ports for SNAT and MASQUERADE */ +- nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes); +- if (nNatPorts < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes)) < 0) + return -1; +- } else if (nNatPorts > 1) { ++ ++ if (nNatPorts > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one element is allowed in in in network %1$s"), + networkName); +@@ -1358,37 +1336,19 @@ virNetworkForwardDefParseXML(const char *networkName, + } + + /* bridge and hostdev modes can use a pool of physical interfaces */ +- nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes); +- if (nForwardIfs < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes)) < 0) + return -1; +- } + +- nForwardAddrs = virXPathNodeSet("./address", ctxt, &forwardAddrNodes); +- if (nForwardAddrs < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid
element found in of network %1$s"), +- networkName); ++ if ((nForwardAddrs = virXPathNodeSet("./address", ctxt, &forwardAddrNodes)) < 0) + return -1; +- } + +- nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes); +- if (nForwardPfs < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes)) < 0) + return -1; +- } + +- nForwardNats = virXPathNodeSet("./nat", ctxt, &forwardNatNodes); +- if (nForwardNats < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("invalid element found in of network %1$s"), +- networkName); ++ if ((nForwardNats = virXPathNodeSet("./nat", ctxt, &forwardNatNodes)) < 0) + return -1; +- } else if (nForwardNats > 1) { ++ ++ if (nForwardNats > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one element is allowed in of network %1$s"), + networkName); +diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c +index 95de77abe9..dd174d3020 100644 +--- a/src/conf/node_device_conf.c ++++ b/src/conf/node_device_conf.c +@@ -960,11 +960,9 @@ virNodeDeviceCapVPDParseCustomFields(xmlXPathContextPtr ctxt, virPCIVPDResource + g_autofree xmlNodePtr *nodes = NULL; + size_t i = 0; + +- if ((nfields = virXPathNodeSet("./vendor_field[@index]", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _("failed to evaluate elements")); ++ if ((nfields = virXPathNodeSet("./vendor_field[@index]", ctxt, &nodes)) < 0) + return -1; +- } ++ + for (i = 0; i < nfields; i++) { + g_autofree char *value = NULL; + g_autofree char *index = NULL; +@@ -989,11 +987,9 @@ virNodeDeviceCapVPDParseCustomFields(xmlXPathContextPtr ctxt, virPCIVPDResource + VIR_FREE(nodes); + + if (!readOnly) { +- if ((nfields = virXPathNodeSet("./system_field[@index]", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _("failed to evaluate elements")); ++ if ((nfields = virXPathNodeSet("./system_field[@index]", ctxt, &nodes)) < 0) + return -1; +- } ++ + for (i = 0; i < nfields; i++) { + g_autofree char *value = NULL; + g_autofree char *index = NULL; +@@ -1074,11 +1070,8 @@ virNodeDeviceCapVPDParseXML(xmlXPathContextPtr ctxt, virPCIVPDResource **res) + return -1; + } + +- if ((nfields = virXPathNodeSet("./fields[@access]", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _("no VPD elements with an access type attribute found")); ++ if ((nfields = virXPathNodeSet("./fields[@access]", ctxt, &nodes)) < 0) + return -1; +- } + + for (i = 0; i < nfields; i++) { + g_autofree char *access = NULL; +diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c +index bcd7838e00..d8120de6d2 100644 +--- a/src/conf/numa_conf.c ++++ b/src/conf/numa_conf.c +@@ -135,11 +135,8 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa, + size_t i = 0; + g_autofree xmlNodePtr *nodes = NULL; + +- if ((n = virXPathNodeSet("./numatune/memnode", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("Cannot extract memnode nodes")); ++ if ((n = virXPathNodeSet("./numatune/memnode", ctxt, &nodes)) < 0) + return -1; +- } + + if (!n) + return 0; +@@ -700,7 +697,10 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, + if (!virXPathNode("./distances[1]", ctxt)) + return 0; + +- if ((sibling = virXPathNodeSet("./distances[1]/sibling", ctxt, &nodes)) <= 0) { ++ if ((sibling = virXPathNodeSet("./distances[1]/sibling", ctxt, &nodes)) < 0) ++ goto cleanup; ++ ++ if (sibling == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("NUMA distances defined without siblings")); + goto cleanup; +@@ -852,7 +852,10 @@ virDomainNumaDefParseXML(virDomainNuma *def, + if (!virXPathNode("./cpu/numa[1]", ctxt)) + return 0; + +- if ((n = virXPathNodeSet("./cpu/numa[1]/cell", ctxt, &cell)) <= 0) { ++ if ((n = virXPathNodeSet("./cpu/numa[1]/cell", ctxt, &cell)) < 0) ++ return -1; ++ ++ if (n == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("NUMA topology defined without NUMA cells")); + return -1; +diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c +index e13cdbdf6b..448a0a7d85 100644 +--- a/src/cpu/cpu_ppc64.c ++++ b/src/cpu/cpu_ppc64.c +@@ -334,7 +334,10 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, + } + } + +- if ((n = virXPathNodeSet("./pvr", ctxt, &nodes)) <= 0) { ++ if ((n = virXPathNodeSet("./pvr", ctxt, &nodes)) < 0) ++ return -1; ++ ++ if (n == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing PVR information for CPU model %1$s"), + model->name); +diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c +index e13df2b27d..10090e0986 100644 +--- a/src/qemu/qemu_capabilities.c ++++ b/src/qemu/qemu_capabilities.c +@@ -4015,11 +4015,8 @@ virQEMUCapsLoadCPUModels(virArch arch, + int n; + xmlNodePtr node; + +- if ((n = virXPathNodeSet(xpath, ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse qemu capabilities cpus")); ++ if ((n = virXPathNodeSet(xpath, ctxt, &nodes)) < 0) + return -1; +- } + + if (n == 0) + return 0; +@@ -4057,11 +4054,8 @@ virQEMUCapsLoadCPUModels(virArch arch, + nblockers = virXPathNodeSet("./blocker", ctxt, &blockerNodes); + ctxt->node = node; + +- if (nblockers < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse CPU blockers in QEMU capabilities")); ++ if (nblockers < 0) + return -1; +- } + + if (nblockers > 0) { + size_t j; +@@ -4100,11 +4094,8 @@ virQEMUCapsLoadMachines(virQEMUCapsAccel *caps, + size_t i; + int n; + +- if ((n = virXPathNodeSet(xpath, ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse qemu capabilities machines")); ++ if ((n = virXPathNodeSet(xpath, ctxt, &nodes)) < 0) + return -1; +- } + + if (n == 0) + return 0; +@@ -4317,11 +4308,8 @@ virQEMUCapsParseSGXInfo(virQEMUCaps *qemuCaps, + ctxt->node = sgxSections; + nSgxSections = virXPathNodeSet("./section", ctxt, §ionNodes); + +- if (nSgxSections < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse SGX sections in QEMU capabilities cache")); ++ if (nSgxSections < 0) + return -1; +- } + + sgx->nSgxSections = nSgxSections; + sgx->sgxSections = g_new0(virSGXSection, nSgxSections); +@@ -4404,11 +4392,8 @@ virQEMUCapsParseFlags(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) + size_t i; + int n; + +- if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse qemu capabilities flags")); ++ if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) + return -1; +- } + + VIR_DEBUG("Got flags %d", n); + for (i = 0; i < n; i++) { +@@ -4442,11 +4427,8 @@ virQEMUCapsParseGIC(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt) + size_t i; + int n; + +- if ((n = virXPathNodeSet("./gic", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse qemu capabilities gic")); ++ if ((n = virXPathNodeSet("./gic", ctxt, &nodes)) < 0) + return -1; +- } + + if (n > 0) { + unsigned int uintValue; +diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c +index e2a1bf2c13..97520bb49c 100644 +--- a/src/qemu/qemu_domain.c ++++ b/src/qemu/qemu_domain.c +@@ -3155,11 +3155,8 @@ qemuDomainObjPrivateXMLParseSlirpFeatures(xmlNodePtr featuresNode, + + ctxt->node = featuresNode; + +- if ((n = virXPathNodeSet("./feature", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "%s", _("failed to parse slirp-helper features")); ++ if ((n = virXPathNodeSet("./feature", ctxt, &nodes)) < 0) + return -1; +- } + + for (i = 0; i < n; i++) { + g_autofree char *str = virXMLPropString(nodes[i], "name"); +@@ -3273,11 +3270,9 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./qemuCaps/flag", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "%s", _("failed to parse qemu capabilities flags")); ++ if ((n = virXPathNodeSet("./qemuCaps/flag", ctxt, &nodes)) < 0) + return -1; +- } ++ + if (n > 0) { + qemuCaps = virQEMUCapsNew(); + +@@ -3305,11 +3300,9 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, + + priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1; + +- if ((n = virXPathNodeSet("./devices/device", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse qemu device list")); ++ if ((n = virXPathNodeSet("./devices/device", ctxt, &nodes)) < 0) + return -1; +- } ++ + if (n > 0) { + /* NULL-terminated list */ + priv->qemuDevices = g_new0(char *, n + 1); +@@ -3325,11 +3318,9 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, + } + VIR_FREE(nodes); + +- if ((n = virXPathNodeSet("./slirp/helper", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse slirp helper list")); ++ if ((n = virXPathNodeSet("./slirp/helper", ctxt, &nodes)) < 0) + return -1; +- } ++ + for (i = 0; i < n; i++) { + g_autofree char *alias = virXMLPropString(nodes[i], "alias"); + g_autofree char *pid = virXMLPropString(nodes[i], "pid"); +diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c +index 5505fdaf22..4361949cca 100644 +--- a/src/qemu/qemu_migration_cookie.c ++++ b/src/qemu/qemu_migration_cookie.c +@@ -947,11 +947,8 @@ qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt) + g_autofree xmlNodePtr *interfaces = NULL; + VIR_XPATH_NODE_AUTORESTORE(ctxt) + +- if ((n = virXPathNodeSet("./network/interface", ctxt, &interfaces)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "%s", _("missing interface information")); ++ if ((n = virXPathNodeSet("./network/interface", ctxt, &interfaces)) < 0) + return NULL; +- } + + optr->nnets = n; + optr->net = g_new0(qemuMigrationCookieNetData, optr->nnets); +diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c +index 85e61be44c..3343241aaf 100644 +--- a/src/qemu/qemu_nbdkit.c ++++ b/src/qemu/qemu_nbdkit.c +@@ -400,11 +400,8 @@ qemuNbdkitCapsParseFlags(qemuNbdkitCaps *nbdkitCaps, + size_t i; + int n; + +- if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("failed to parse qemu capabilities flags")); ++ if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) + return -1; +- } + + VIR_DEBUG("Got flags %d", n); + for (i = 0; i < n; i++) { +diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c +index 6a15d60577..ce4586a3f5 100644 +--- a/src/vz/vz_sdk.c ++++ b/src/vz/vz_sdk.c +@@ -4612,11 +4612,8 @@ prlsdkParseSnapshotTree(const char *treexml) + "ParallelsSavedStates", &ctxt, NULL, false))) + goto cleanup; + +- if ((n = virXPathNodeSet("//SavedStateItem", ctxt, &nodes)) < 0) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- _("cannot extract snapshot nodes")); ++ if ((n = virXPathNodeSet("//SavedStateItem", ctxt, &nodes)) < 0) + goto cleanup; +- } + + for (i = 0; i < n; i++) { + if (nodes[i]->parent == xmlDocGetRootElement(xml)) +-- +2.43.0 diff --git a/libvirt-conf-Introduce-dynamicMemslots-attribute-for-virtio-mem.patch b/libvirt-conf-Introduce-dynamicMemslots-attribute-for-virtio-mem.patch new file mode 100644 index 0000000..1ae2cc8 --- /dev/null +++ b/libvirt-conf-Introduce-dynamicMemslots-attribute-for-virtio-mem.patch @@ -0,0 +1,152 @@ +From de94232ffb9eef84bb72631979f59bbadfc3cb9e Mon Sep 17 00:00:00 2001 +Message-ID: +From: Michal Privoznik +Date: Thu, 4 Jan 2024 10:03:36 +0100 +Subject: [PATCH] conf: Introduce dynamicMemslots attribute for virtio-mem + +Introduced in v8.2.0-rc0~74^2~2, QEMU now allows setting +.dynamic-memslots attribute for virtio-mem-pci devices. When +turned on, it allows memory exposed to guest to be split into +multiple memslots and thus smaller memory footprint (see the +original commit for detailed explanation). + +Therefore, introduce new attribute which will control +that QEMU knob. + +Signed-off-by: Michal Privoznik +Reviewed-by: Peter Krempa +(cherry picked from commit 53258205854e649bc82310542373df004a4734ab) +Resolves: https://issues.redhat.com/browse/RHEL-15316 +Signed-off-by: Michal Privoznik +--- + docs/formatdomain.rst | 13 +++++++++++++ + src/conf/domain_conf.c | 18 +++++++++++++++++- + src/conf/domain_conf.h | 1 + + src/conf/schemas/domaincommon.rng | 5 +++++ + .../memory-hotplug-virtio-mem.xml | 2 +- + 5 files changed, 37 insertions(+), 2 deletions(-) + +diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst +index 298ad46a45..34b2564909 100644 +--- a/docs/formatdomain.rst ++++ b/docs/formatdomain.rst +@@ -8437,6 +8437,19 @@ Example: usage of the memory devices + The ``node`` subelement configures the guest NUMA node to attach the memory + to. The element shall be used only if the guest has NUMA nodes configured. + ++ For ``virtio-mem`` optional attribute ``dynamicMemslots`` can be specified ++ (accepted values "yes"/"no") which allows hypervisor to spread memory into ++ multiple memory slots (allocate them dynamically based on the amount of ++ memory exposed to the guest), resulting in smaller memory footprint. But be ++ aware this may affect vhost-user devices. When enabled, older vhost-user ++ device implementations (such as virtiofs) may refuse to initialize resulting ++ in failed domain startup or device hotplug. When only modern vhost-user ++ based devices will be used or when no vhost-user devices are expected to be ++ used it's beneficial to enable this feature. The current default is ++ hypervisor dependant (for QEMU is "no"). If the default changes and you are ++ having difficulties with vhost-user devices, try toggling this to "no". ++ :since:`Since 10.1.0 and QEMU 8.2.0` ++ + The following optional elements may be used: + + ``label`` +diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c +index 6211d2a51b..ac06fa39f6 100644 +--- a/src/conf/domain_conf.c ++++ b/src/conf/domain_conf.c +@@ -13543,6 +13543,10 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, + &def->target.virtio_mem.requestedsize, false, false) < 0) + return -1; + ++ if (virXMLPropTristateBool(node, "dynamicMemslots", VIR_XML_PROP_NONE, ++ &def->target.virtio_mem.dynamicMemslots) < 0) ++ return -1; ++ + addrNode = virXPathNode("./address", ctxt); + addr = &def->target.virtio_mem.address; + break; +@@ -21217,6 +21221,12 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDef *src, + src->target.virtio_mem.address); + return false; + } ++ ++ if (src->target.virtio_mem.dynamicMemslots != dst->target.virtio_mem.dynamicMemslots) { ++ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", ++ _("Target memory device 'dynamicMemslots' property doesn't match source memory device")); ++ return false; ++ } + break; + + case VIR_DOMAIN_MEMORY_MODEL_DIMM: +@@ -25432,6 +25442,7 @@ virDomainMemoryTargetDefFormat(virBuffer *buf, + unsigned int flags) + { + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); ++ g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&childBuf, "%llu\n", def->size); + if (def->targetNode >= 0) +@@ -25471,6 +25482,11 @@ virDomainMemoryTargetDefFormat(virBuffer *buf, + if (def->target.virtio_mem.address) + virBufferAsprintf(&childBuf, "
\n", + def->target.virtio_mem.address); ++ ++ if (def->target.virtio_mem.dynamicMemslots) { ++ virBufferAsprintf(&attrBuf, " dynamicMemslots='%s'", ++ virTristateBoolTypeToString(def->target.virtio_mem.dynamicMemslots)); ++ } + break; + + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: +@@ -25480,7 +25496,7 @@ virDomainMemoryTargetDefFormat(virBuffer *buf, + break; + } + +- virXMLFormatElement(buf, "target", NULL, &childBuf); ++ virXMLFormatElement(buf, "target", &attrBuf, &childBuf); + } + + static int +diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h +index d176bda5f8..bd283d42df 100644 +--- a/src/conf/domain_conf.h ++++ b/src/conf/domain_conf.h +@@ -2676,6 +2676,7 @@ struct _virDomainMemoryDef { + unsigned long long currentsize; /* kibibytes, valid for an active + domain only and parsed */ + unsigned long long address; /* address where memory is mapped */ ++ virTristateBool dynamicMemslots; + } virtio_mem; + struct { + } sgx_epc; +diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng +index a34427c330..df44cd9857 100644 +--- a/src/conf/schemas/domaincommon.rng ++++ b/src/conf/schemas/domaincommon.rng +@@ -7268,6 +7268,11 @@ + + + ++ ++ ++ ++ ++ + + + +diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml +index 52fa6b14e9..20282a131b 100644 +--- a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml ++++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.xml +@@ -60,7 +60,7 @@ + 1-3 + 2048 + +- ++ + 2097152 + 0 + 2048 +-- +2.43.0 diff --git a/libvirt-conf-node_device-Refactor-virNodeDeviceCapVPDParseCustomFields-to-fix-error-reporting.patch b/libvirt-conf-node_device-Refactor-virNodeDeviceCapVPDParseCustomFields-to-fix-error-reporting.patch new file mode 100644 index 0000000..eb91e09 --- /dev/null +++ b/libvirt-conf-node_device-Refactor-virNodeDeviceCapVPDParseCustomFields-to-fix-error-reporting.patch @@ -0,0 +1,138 @@ +From 8d84e4af4cbb93d73f4d9967f517552257f025f5 Mon Sep 17 00:00:00 2001 +Message-ID: <8d84e4af4cbb93d73f4d9967f517552257f025f5.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 18:26:29 +0100 +Subject: [PATCH] conf: node_device: Refactor + 'virNodeDeviceCapVPDParseCustomFields' to fix error reporting +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The errors raised in virNodeDeviceCapVPDParseCustomFields were actually +ignored by continuing the parse rather than raised. + +Rather than just replace 'continue' by 'return -1' this patch refactors +the whole parser to simplify it as well as report reasonable errors. + +Parsing of individual fields is done without XPath and is extracted into +a common helper. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit ea8d864d9ecf3ee72910ccc1497244f1ef74e948) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/conf/node_device_conf.c | 81 ++++++++++++++++++------------------- + 1 file changed, 40 insertions(+), 41 deletions(-) + +diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c +index d7e1a23034..0f2c341967 100644 +--- a/src/conf/node_device_conf.c ++++ b/src/conf/node_device_conf.c +@@ -953,63 +953,62 @@ virNodeDevCapMdevTypesParseXML(xmlXPathContextPtr ctxt, + return ret; + } + ++ + static int +-virNodeDeviceCapVPDParseCustomFields(xmlXPathContextPtr ctxt, virPCIVPDResource *res, bool readOnly) ++virNodeDeviceCapVPDParseCustomFieldOne(xmlNodePtr node, ++ virPCIVPDResource *res, ++ bool read_only, ++ const char keyword_prefix) ++{ ++ g_autofree char *value = NULL; ++ g_autofree char *index = NULL; ++ g_autofree char *keyword = NULL; ++ ++ if (!(index = virXMLPropStringRequired(node, "index"))) ++ return -1; ++ ++ if (strlen(index) != 1) { ++ virReportError(VIR_ERR_XML_ERROR, ++ _("'%1$s' 'index' value '%2$s' malformed"), ++ node->name, index); ++ return -1; ++ } ++ ++ keyword = g_strdup_printf("%c%c", keyword_prefix, index[0]); ++ ++ if (!(value = virXMLNodeContentString(node))) ++ return -1; ++ ++ virPCIVPDResourceUpdateKeyword(res, read_only, keyword, value); ++ return 0; ++} ++ ++ ++static int ++virNodeDeviceCapVPDParseCustomFields(xmlXPathContextPtr ctxt, ++ virPCIVPDResource *res, ++ bool readOnly) + { + int nfields = -1; + g_autofree xmlNodePtr *nodes = NULL; + size_t i = 0; + +- if ((nfields = virXPathNodeSet("./vendor_field[@index]", ctxt, &nodes)) < 0) ++ if ((nfields = virXPathNodeSet("./vendor_field", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < nfields; i++) { +- g_autofree char *value = NULL; +- g_autofree char *index = NULL; +- VIR_XPATH_NODE_AUTORESTORE(ctxt) +- g_autofree char *keyword = NULL; +- +- ctxt->node = nodes[i]; +- if (!(index = virXPathString("string(./@index[1])", ctxt)) || +- strlen(index) > 1) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _(" evaluation has failed")); +- continue; +- } +- if (!(value = virXPathString("string(./text())", ctxt))) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _(" value evaluation has failed")); +- continue; +- } +- keyword = g_strdup_printf("V%c", index[0]); +- virPCIVPDResourceUpdateKeyword(res, readOnly, keyword, value); ++ if (virNodeDeviceCapVPDParseCustomFieldOne(nodes[i], res, readOnly, 'V') < 0) ++ return -1; + } + VIR_FREE(nodes); + + if (!readOnly) { +- if ((nfields = virXPathNodeSet("./system_field[@index]", ctxt, &nodes)) < 0) ++ if ((nfields = virXPathNodeSet("./system_field", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < nfields; i++) { +- g_autofree char *value = NULL; +- g_autofree char *index = NULL; +- g_autofree char *keyword = NULL; +- VIR_XPATH_NODE_AUTORESTORE(ctxt); +- +- ctxt->node = nodes[i]; +- if (!(index = virXPathString("string(./@index[1])", ctxt)) || +- strlen(index) > 1) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _(" evaluation has failed")); +- continue; +- } +- if (!(value = virXPathString("string(./text())", ctxt))) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _(" value evaluation has failed")); +- continue; +- } +- keyword = g_strdup_printf("Y%c", index[0]); +- virPCIVPDResourceUpdateKeyword(res, readOnly, keyword, value); ++ if (virNodeDeviceCapVPDParseCustomFieldOne(nodes[i], res, readOnly, 'Y') < 0) ++ return -1; + } + } + +-- +2.43.0 diff --git a/libvirt-conf-virNodeDeviceCapVPDParse-Remove-pointless-NULL-checks.patch b/libvirt-conf-virNodeDeviceCapVPDParse-Remove-pointless-NULL-checks.patch new file mode 100644 index 0000000..58c94cb --- /dev/null +++ b/libvirt-conf-virNodeDeviceCapVPDParse-Remove-pointless-NULL-checks.patch @@ -0,0 +1,47 @@ +From 15145b5ecb2e9186e42bbb295e1d44f93ff25cfb Mon Sep 17 00:00:00 2001 +Message-ID: <15145b5ecb2e9186e42bbb295e1d44f93ff25cfb.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 16:27:35 +0100 +Subject: [PATCH] conf: virNodeDeviceCapVPDParse*: Remove pointless NULL checks +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The function are never called with NULL argument so the checks can be +removed. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit fb69acf5c255f6baedefe2a2535325af8088ced5) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/conf/node_device_conf.c | 6 ------ + 1 file changed, 6 deletions(-) + +diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c +index dd174d3020..d7e1a23034 100644 +--- a/src/conf/node_device_conf.c ++++ b/src/conf/node_device_conf.c +@@ -1023,9 +1023,6 @@ virNodeDeviceCapVPDParseReadOnlyFields(xmlXPathContextPtr ctxt, virPCIVPDResourc + "serial_number", "part_number", NULL}; + size_t i = 0; + +- if (res == NULL) +- return -1; +- + res->ro = virPCIVPDResourceRONew(); + + while (keywords[i]) { +@@ -1061,9 +1058,6 @@ virNodeDeviceCapVPDParseXML(xmlXPathContextPtr ctxt, virPCIVPDResource **res) + size_t i = 0; + g_autoptr(virPCIVPDResource) newres = g_new0(virPCIVPDResource, 1); + +- if (res == NULL) +- return -1; +- + if (!(newres->name = virXPathString("string(./name)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Could not read a device name from the element")); +-- +2.43.0 diff --git a/libvirt-qemu-migration-Properly-handle-reservation-of-manually-specified-NBD-port.patch b/libvirt-qemu-migration-Properly-handle-reservation-of-manually-specified-NBD-port.patch new file mode 100644 index 0000000..ef06cf2 --- /dev/null +++ b/libvirt-qemu-migration-Properly-handle-reservation-of-manually-specified-NBD-port.patch @@ -0,0 +1,100 @@ +From 52036c598d2670b4d103c923be1fdd95c096be4e Mon Sep 17 00:00:00 2001 +Message-ID: <52036c598d2670b4d103c923be1fdd95c096be4e.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Tue, 16 Jan 2024 15:52:25 +0100 +Subject: [PATCH] qemu: migration: Properly handle reservation of manually + specified NBD port + +Originally the migration code didn't register the NBD disk port with the +port allocator when it was manually specified. Later when commit +e74d627bb3bc2684cbe3 refactored the code and started registering it, the +old logic which was clearing 'priv->nbdPort' in case when it was manually +specified was not removed. + +This caused following problems: + - the port was not released after successful migration + - the port was released even when it was not allocated on failures + regarding the NBD server start + - the port was not released on other failures of the migration after + NBD server startup + +To address this we remove the assumption that 'priv->nbdPort' is used +only for auto-allocated port and fill it only once the port is +allocated and make the caller of qemuMigrationDstStartNBDServer +responsible for releasing it. + +Fixes: e74d627bb3bc2684cbe3edc1e2f7cc745b4e1ff3 +Resolves: https://issues.redhat.com/browse/RHEL-21543 +Signed-off-by: Peter Krempa +Reviewed-by: Andrea Bolognani +(cherry picked from commit 43f027b57c4d885fc076ffb8829d525a3c343c6f) +--- + src/qemu/qemu_migration.c | 22 +++++++--------------- + 1 file changed, 7 insertions(+), 15 deletions(-) + +diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c +index 25dc16a9e9..6f8b830969 100644 +--- a/src/qemu/qemu_migration.c ++++ b/src/qemu/qemu_migration.c +@@ -527,6 +527,8 @@ qemuMigrationDstPrepareStorage(virDomainObj *vm, + * arguments in 'migrate' monitor command. + * Error is reported here. + * ++ * Caller is responsible for releasing 'priv->nbdPort' from the port allocator. ++ * + * Returns 0 on success, -1 otherwise. + */ + static int +@@ -627,6 +629,9 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, + + server.port = port; + } ++ ++ /* caller will release the port */ ++ priv->nbdPort = server.port; + } + + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_MIGRATION_IN) < 0) +@@ -643,14 +648,9 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, + qemuDomainObjExitMonitor(vm); + } + +- if (server.transport == VIR_STORAGE_NET_HOST_TRANS_TCP) +- priv->nbdPort = server.port; +- + ret = 0; + + cleanup: +- if (ret < 0) +- virPortAllocatorRelease(server.port); + return ret; + + exit_monitor: +@@ -3261,11 +3261,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, + virDomainAuditStart(vm, "migrated", false); + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + VIR_ASYNC_JOB_MIGRATION_IN, stopFlags); +- /* release if port is auto selected which is not the case if +- * it is given in parameters +- */ +- if (nbdPort == 0) +- virPortAllocatorRelease(priv->nbdPort); ++ virPortAllocatorRelease(priv->nbdPort); + priv->nbdPort = 0; + } + goto cleanup; +@@ -3425,11 +3421,7 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, + + if (autoPort) + priv->migrationPort = port; +- /* in this case port is not auto selected and we don't need to manage it +- * anymore after cookie is baked +- */ +- if (nbdPort != 0) +- priv->nbdPort = 0; ++ + ret = 0; + + cleanup: +-- +2.43.0 diff --git a/libvirt-qemuMigrationDstStartNBDServer-Refactor-cleanup.patch b/libvirt-qemuMigrationDstStartNBDServer-Refactor-cleanup.patch new file mode 100644 index 0000000..549a3b4 --- /dev/null +++ b/libvirt-qemuMigrationDstStartNBDServer-Refactor-cleanup.patch @@ -0,0 +1,84 @@ +From d968a490b2fb8b4c7af2c835288e6f693ea1cc67 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Tue, 16 Jan 2024 16:22:03 +0100 +Subject: [PATCH] qemuMigrationDstStartNBDServer: Refactor cleanup + +There's nothing under the 'cleanup:' label thus the whole code can be +simplified. + +Signed-off-by: Peter Krempa +Reviewed-by: Andrea Bolognani +(cherry picked from commit 36e11cca83c6617a81528969c27579a1ab891443) +https://issues.redhat.com/browse/RHEL-21543 +--- + src/qemu/qemu_migration.c | 18 +++++++----------- + 1 file changed, 7 insertions(+), 11 deletions(-) + +diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c +index 6f8b830969..01ab803842 100644 +--- a/src/qemu/qemu_migration.c ++++ b/src/qemu/qemu_migration.c +@@ -541,7 +541,6 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, + const char *nbdURI, + const char *tls_alias) + { +- int ret = -1; + qemuDomainObjPrivate *priv = vm->privateData; + size_t i; + virStorageNetHostDef server = { +@@ -610,22 +609,22 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot migrate empty or read-only disk %1$s"), + disk->dst); +- goto cleanup; ++ return -1; + } + + if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk))) +- goto cleanup; ++ return -1; + + if (!server_started && + server.transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { + if (server.port) { + if (virPortAllocatorSetUsed(server.port) < 0) +- goto cleanup; ++ return -1; + } else { + unsigned short port = 0; + + if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0) +- goto cleanup; ++ return -1; + + server.port = port; + } +@@ -635,7 +634,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, + } + + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_MIGRATION_IN) < 0) +- goto cleanup; ++ return -1; + + if (!server_started) { + if (qemuMonitorNBDServerStart(priv->mon, &server, tls_alias) < 0) +@@ -648,14 +647,11 @@ qemuMigrationDstStartNBDServer(virQEMUDriver *driver, + qemuDomainObjExitMonitor(vm); + } + +- ret = 0; +- +- cleanup: +- return ret; ++ return 0; + + exit_monitor: + qemuDomainObjExitMonitor(vm); +- goto cleanup; ++ return -1; + } + + +-- +2.43.0 diff --git a/libvirt-qemu_capabilities-Add-QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS-capability.patch b/libvirt-qemu_capabilities-Add-QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS-capability.patch new file mode 100644 index 0000000..5d0e429 --- /dev/null +++ b/libvirt-qemu_capabilities-Add-QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS-capability.patch @@ -0,0 +1,94 @@ +From 7dd85500450b1889a81d574337331e080b218c9f Mon Sep 17 00:00:00 2001 +Message-ID: <7dd85500450b1889a81d574337331e080b218c9f.1707394626.git.jdenemar@redhat.com> +From: Michal Privoznik +Date: Thu, 4 Jan 2024 10:57:12 +0100 +Subject: [PATCH] qemu_capabilities: Add + QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS capability + +Starting from v8.2.0-rc0~74^2~2 QEMU has .dynamic-memslots +attribute for virtio-mem-pci device. Introduce a capability which +reflects that. + +Signed-off-by: Michal Privoznik +Reviewed-by: Peter Krempa +(cherry picked from commit 497cab753b801c7a66e2a480482e5144665ecbf4) +Resolves: https://issues.redhat.com/browse/RHEL-15316 +Signed-off-by: Michal Privoznik +--- + src/qemu/qemu_capabilities.c | 2 ++ + src/qemu/qemu_capabilities.h | 1 + + tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml | 1 + + tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml | 1 + + tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 1 + + 5 files changed, 6 insertions(+) + +diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c +index a4d42b40ed..e13df2b27d 100644 +--- a/src/qemu/qemu_capabilities.c ++++ b/src/qemu/qemu_capabilities.c +@@ -700,6 +700,7 @@ VIR_ENUM_IMPL(virQEMUCaps, + "virtio-blk-vhost-vdpa", /* QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA */ + "virtio-blk.iothread-mapping", /* QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING */ + "smp-clusters", /* QEMU_CAPS_SMP_CLUSTERS */ ++ "virtio-mem-pci.dynamic-memslots", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS */ + ); + + +@@ -1519,6 +1520,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVhostUserFS[] = + + static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioMemPCI[] = { + { "prealloc", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_PREALLOC, NULL }, ++ { "dynamic-memslots", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS, NULL }, + }; + + static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioIOMMU[] = { +diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h +index a353750670..82ae4b738b 100644 +--- a/src/qemu/qemu_capabilities.h ++++ b/src/qemu/qemu_capabilities.h +@@ -679,6 +679,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ + QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA, /* virtio-blk-vhost-vdpa block driver */ + QEMU_CAPS_VIRTIO_BLK_IOTHREAD_MAPPING, /* virtio-blk supports per-virtqueue iothread mapping */ + QEMU_CAPS_SMP_CLUSTERS, /* -smp clusters= */ ++ QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS, /* -device virtio-mem-pci.dynamic-memslots= */ + + QEMU_CAPS_LAST /* this must always be the last item */ + } virQEMUCapsFlags; +diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml +index 54fd349365..03c9343da5 100644 +--- a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml ++++ b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml +@@ -163,6 +163,7 @@ + + + ++ + 8002000 + 61700246 + v8.2.0 +diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml +index 8a6527810a..d16cd88720 100644 +--- a/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml ++++ b/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml +@@ -200,6 +200,7 @@ + + + ++ + 8002000 + 43100246 + v8.2.0 +diff --git a/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml +index b4c3b1bae3..65eaa08cd4 100644 +--- a/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml ++++ b/tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml +@@ -201,6 +201,7 @@ + + + ++ + 8002050 + 43100245 + v8.2.0-196-g7425b6277f +-- +2.43.0 diff --git a/libvirt-qemu_command-Generate-cmd-line-for-virtio-mem-dynamicMemslots.patch b/libvirt-qemu_command-Generate-cmd-line-for-virtio-mem-dynamicMemslots.patch new file mode 100644 index 0000000..ed142af --- /dev/null +++ b/libvirt-qemu_command-Generate-cmd-line-for-virtio-mem-dynamicMemslots.patch @@ -0,0 +1,62 @@ +From 866ec16d8264b3ef2533b276d161e6dc1db470a0 Mon Sep 17 00:00:00 2001 +Message-ID: <866ec16d8264b3ef2533b276d161e6dc1db470a0.1707394627.git.jdenemar@redhat.com> +From: Michal Privoznik +Date: Thu, 4 Jan 2024 10:49:06 +0100 +Subject: [PATCH] qemu_command: Generate cmd line for virtio-mem + dynamicMemslots + +This is pretty straightforward. + +Resolves: https://issues.redhat.com/browse/RHEL-15316 +Signed-off-by: Michal Privoznik +Reviewed-by: Peter Krempa +(cherry picked from commit dab99eedcd15d135e287185ce03eb05338ce225d) +Signed-off-by: Michal Privoznik +--- + src/qemu/qemu_command.c | 3 +++ + .../memory-hotplug-virtio-mem.x86_64-latest.args | 2 +- + 2 files changed, 4 insertions(+), 1 deletion(-) + +diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c +index 712feb7b81..4d5a202c7d 100644 +--- a/src/qemu/qemu_command.c ++++ b/src/qemu/qemu_command.c +@@ -3653,6 +3653,7 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, + unsigned long long requestedsize = 0; + unsigned long long address = 0; + bool prealloc = false; ++ virTristateBool dynamicMemslots = VIR_TRISTATE_BOOL_ABSENT; + + if (!mem->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +@@ -3694,6 +3695,7 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, + blocksize = mem->target.virtio_mem.blocksize; + requestedsize = mem->target.virtio_mem.requestedsize; + address = mem->target.virtio_mem.address; ++ dynamicMemslots = mem->target.virtio_mem.dynamicMemslots; + break; + + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: +@@ -3716,6 +3718,7 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg, + "s:memdev", memdev, + "B:prealloc", prealloc, + "P:memaddr", address, ++ "T:dynamic-memslots", dynamicMemslots, + "s:id", mem->info.alias, + NULL) < 0) + return NULL; +diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args +index dbe96ae21d..36cff6ec13 100644 +--- a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args ++++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args +@@ -32,7 +32,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ + -object '{"qom-type":"memory-backend-ram","id":"memvirtiomem0","reserve":false,"size":1073741824}' \ + -device '{"driver":"virtio-mem-pci","node":0,"block-size":2097152,"requested-size":536870912,"memdev":"memvirtiomem0","id":"virtiomem0","bus":"pci.0","addr":"0x2"}' \ + -object '{"qom-type":"memory-backend-file","id":"memvirtiomem1","mem-path":"/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1","reserve":false,"size":2147483648,"host-nodes":[1,2,3],"policy":"bind"}' \ +--device '{"driver":"virtio-mem-pci","node":0,"block-size":2097152,"requested-size":1073741824,"memdev":"memvirtiomem1","prealloc":true,"memaddr":5637144576,"id":"virtiomem1","bus":"pci.1","addr":"0x1"}' \ ++-device '{"driver":"virtio-mem-pci","node":0,"block-size":2097152,"requested-size":1073741824,"memdev":"memvirtiomem1","prealloc":true,"memaddr":5637144576,"dynamic-memslots":true,"id":"virtiomem1","bus":"pci.1","addr":"0x1"}' \ + -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ + -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ + -device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \ +-- +2.43.0 diff --git a/libvirt-qemu_snapshot-create-don-t-require-disk-only-flag-for-offline-external-snapshot.patch b/libvirt-qemu_snapshot-create-don-t-require-disk-only-flag-for-offline-external-snapshot.patch new file mode 100644 index 0000000..e307d29 --- /dev/null +++ b/libvirt-qemu_snapshot-create-don-t-require-disk-only-flag-for-offline-external-snapshot.patch @@ -0,0 +1,67 @@ +From 743afd9422e59bc6cbda6b4a904394a045eb9aec Mon Sep 17 00:00:00 2001 +Message-ID: <743afd9422e59bc6cbda6b4a904394a045eb9aec.1707394627.git.jdenemar@redhat.com> +From: Pavel Hrdina +Date: Tue, 30 Jan 2024 12:13:32 +0100 +Subject: [PATCH] qemu_snapshot: create: don't require disk-only flag for + offline external snapshot + +Historically creating offline external snapshot required disk-only flag +as well. Now when user requests new snapshot for offline VM and at least +one disk is specified to use external snapshot we will no longer require +disk-only flag as all other not specified disk will use external +snapshots as well. + +Resolves: https://issues.redhat.com/browse/RHEL-22797 +Signed-off-by: Pavel Hrdina +Reviewed-by: Peter Krempa +(cherry picked from commit 189fdeff10b85786a495a8fcf67ba1428d9fc482) + +Signed-off-by: Pavel Hrdina +--- + src/qemu/qemu_snapshot.c | 16 ++++++++++++++-- + 1 file changed, 14 insertions(+), 2 deletions(-) + +diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c +index 871e63c9cd..0cac0c4146 100644 +--- a/src/qemu/qemu_snapshot.c ++++ b/src/qemu/qemu_snapshot.c +@@ -1582,15 +1582,27 @@ qemuSnapshotCreateXMLValidateDef(virDomainObj *vm, + * to internal snapshots. + */ + static bool +-qemuSnapshotCreateUseExternal(virDomainSnapshotDef *def, ++qemuSnapshotCreateUseExternal(virDomainObj *vm, ++ virDomainSnapshotDef *def, + unsigned int flags) + { ++ size_t i; ++ + if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) + return true; + + if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) + return true; + ++ if (!virDomainObjIsActive(vm)) { ++ /* No need to check all disks as function qemuSnapshotPrepare() guarantees ++ * that we don't have a combination of internal and external location. */ ++ for (i = 0; i < def->ndisks; i++) { ++ if (def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) ++ return true; ++ } ++ } ++ + return false; + } + +@@ -1623,7 +1635,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm, + return -1; + } + +- if (qemuSnapshotCreateUseExternal(def, flags)) { ++ if (qemuSnapshotCreateUseExternal(vm, def, flags)) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + def->state = virDomainObjGetState(vm, NULL); + +-- +2.43.0 diff --git a/libvirt-qemu_snapshot-create-refactor-external-snapshot-detection.patch b/libvirt-qemu_snapshot-create-refactor-external-snapshot-detection.patch new file mode 100644 index 0000000..ffe4ece --- /dev/null +++ b/libvirt-qemu_snapshot-create-refactor-external-snapshot-detection.patch @@ -0,0 +1,88 @@ +From b12f3837ddde95373b39843b8526b55e40d96500 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Pavel Hrdina +Date: Tue, 30 Jan 2024 11:33:23 +0100 +Subject: [PATCH] qemu_snapshot: create: refactor external snapshot detection + +Introduce new function qemuSnapshotCreateUseExternal() that will return +true if we will use external snapshots as default location. + +Signed-off-by: Pavel Hrdina +Reviewed-by: Peter Krempa +(cherry picked from commit faa2e3bb541e50d719d05e58a129b4443c0c737c) + +Resolves: https://issues.redhat.com/browse/RHEL-22797 +Signed-off-by: Pavel Hrdina +--- + src/qemu/qemu_snapshot.c | 39 ++++++++++++++++++++++++++++++--------- + 1 file changed, 30 insertions(+), 9 deletions(-) + +diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c +index af5f995b0d..871e63c9cd 100644 +--- a/src/qemu/qemu_snapshot.c ++++ b/src/qemu/qemu_snapshot.c +@@ -1576,6 +1576,25 @@ qemuSnapshotCreateXMLValidateDef(virDomainObj *vm, + } + + ++/** ++ * Check if libvirt should use external snapshots as default align_location ++ * that will be used by virDomainSnapshotAlignDisks(). Otherwise we default ++ * to internal snapshots. ++ */ ++static bool ++qemuSnapshotCreateUseExternal(virDomainSnapshotDef *def, ++ unsigned int flags) ++{ ++ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) ++ return true; ++ ++ if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) ++ return true; ++ ++ return false; ++} ++ ++ + static int + qemuSnapshotCreateAlignDisks(virDomainObj *vm, + virDomainSnapshotDef *def, +@@ -1584,7 +1603,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm, + { + g_autofree char *xml = NULL; + qemuDomainObjPrivate *priv = vm->privateData; +- virDomainSnapshotLocation align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; ++ virDomainSnapshotLocation align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT; + + /* Easiest way to clone inactive portion of vm->def is via + * conversion in and back out of xml. */ +@@ -1604,17 +1623,19 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm, + return -1; + } + +- if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) { ++ if (qemuSnapshotCreateUseExternal(def, flags)) { + align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; +- if (virDomainObjIsActive(vm)) +- def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT; +- else +- def->state = VIR_DOMAIN_SNAPSHOT_SHUTOFF; +- def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NO; +- } else if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { + def->state = virDomainObjGetState(vm, NULL); +- align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; ++ ++ if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) { ++ if (virDomainObjIsActive(vm)) ++ def->state = VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT; ++ else ++ def->state = VIR_DOMAIN_SNAPSHOT_SHUTOFF; ++ def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NO; ++ } + } else { ++ align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; + def->state = virDomainObjGetState(vm, NULL); + + if (virDomainObjIsActive(vm) && +-- +2.43.0 diff --git a/libvirt-qemu_snapshot-fix-detection-if-non-leaf-snapshot-isn-t-in-active-chain.patch b/libvirt-qemu_snapshot-fix-detection-if-non-leaf-snapshot-isn-t-in-active-chain.patch new file mode 100644 index 0000000..ff725f4 --- /dev/null +++ b/libvirt-qemu_snapshot-fix-detection-if-non-leaf-snapshot-isn-t-in-active-chain.patch @@ -0,0 +1,66 @@ +From aa70508df0626a00e4ed7c0ecb11b985beeb92cd Mon Sep 17 00:00:00 2001 +Message-ID: +From: Pavel Hrdina +Date: Tue, 30 Jan 2024 13:05:22 +0100 +Subject: [PATCH] qemu_snapshot: fix detection if non-leaf snapshot isn't in + active chain + +The condition was completely wrong. As per the comment for function +virDomainMomentIsAncestor() it checks that the first argument is +descendant of the second argument. + +Consider the following snapshot tree for VM: + + s1 + | + +- s2 + | | + | +- s3 + | + +- s4 + | + +- s5 (current) + +When deleting s2 with the original code we checked if +virDomainMomentIsAncestor(s2, s5) which would return false basically for +any snapshot as s5 is leaf snapshot so no children. + +When deleting s2 with fixed code we check if +virDomainMomentIsAncestor(s5, s2) which still returns false but when +deleting s4 it will correctly return true. + +Before this fix it fails with the following error: + + error: Failed to delete snapshot s2 + error: invalid argument: could not find base disk source in disk source chain + +After the fix it fails with correct error: + + error: Failed to delete snapshot s2 + error: unsupported configuration: deletion of non-leaf external snapshot that is not in active chain is not supported + +Resolves: https://issues.redhat.com/browse/RHEL-23212 +Signed-off-by: Pavel Hrdina +Reviewed-by: Peter Krempa +(cherry picked from commit 7143c4e1f95b4dc804f67cc5de98fba746193892) + +Signed-off-by: Pavel Hrdina +--- + src/qemu/qemu_snapshot.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c +index 73ff533827..af5f995b0d 100644 +--- a/src/qemu/qemu_snapshot.c ++++ b/src/qemu/qemu_snapshot.c +@@ -3815,7 +3815,7 @@ qemuSnapshotDeleteValidate(virDomainObj *vm, + } + + if (snap != current && snap->nchildren != 0 && +- virDomainMomentIsAncestor(snap, current)) { ++ !virDomainMomentIsAncestor(current, snap)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("deletion of non-leaf external snapshot that is not in active chain is not supported")); + return -1; +-- +2.43.0 diff --git a/libvirt-qemu_validate-Check-capability-for-virtio-mem-dynamicMemslots.patch b/libvirt-qemu_validate-Check-capability-for-virtio-mem-dynamicMemslots.patch new file mode 100644 index 0000000..9397f52 --- /dev/null +++ b/libvirt-qemu_validate-Check-capability-for-virtio-mem-dynamicMemslots.patch @@ -0,0 +1,40 @@ +From 9f35c04add8d64748671e76171683332bd454317 Mon Sep 17 00:00:00 2001 +Message-ID: <9f35c04add8d64748671e76171683332bd454317.1707394627.git.jdenemar@redhat.com> +From: Michal Privoznik +Date: Thu, 4 Jan 2024 11:04:51 +0100 +Subject: [PATCH] qemu_validate: Check capability for virtio-mem + dynamicMemslots + +The QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS reflects +whether QEMU is capable of .dynamic-memslots for virtio-mem. +Use it when validating domain configuration. + +Signed-off-by: Michal Privoznik +Reviewed-by: Peter Krempa +(cherry picked from commit 6be07af817e0a8d48613296af66873e62a73339a) +Resolves: https://issues.redhat.com/browse/RHEL-15316 +Signed-off-by: Michal Privoznik +--- + src/qemu/qemu_validate.c | 7 +++++++ + 1 file changed, 7 insertions(+) + +diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c +index b22d3618fe..fe8f7ae8cc 100644 +--- a/src/qemu/qemu_validate.c ++++ b/src/qemu/qemu_validate.c +@@ -5066,6 +5066,13 @@ qemuValidateDomainDeviceDefMemory(virDomainMemoryDef *mem, + _("virtio-mem isn't supported by this QEMU binary")); + return -1; + } ++ ++ if (mem->target.virtio_mem.dynamicMemslots == VIR_TRISTATE_BOOL_YES && ++ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS)) { ++ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", ++ _("virtio-mem does not support dynamicMemslots")); ++ return -1; ++ } + break; + + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: +-- +2.43.0 diff --git a/libvirt-remoteDispatchAuthPolkit-Fix-lock-ordering-deadlock-if-client-closes-connection-during-auth.patch b/libvirt-remoteDispatchAuthPolkit-Fix-lock-ordering-deadlock-if-client-closes-connection-during-auth.patch new file mode 100644 index 0000000..14e34df --- /dev/null +++ b/libvirt-remoteDispatchAuthPolkit-Fix-lock-ordering-deadlock-if-client-closes-connection-during-auth.patch @@ -0,0 +1,142 @@ +From 1b87b9821afe39c2af5c1893b11cb7f452c61014 Mon Sep 17 00:00:00 2001 +Message-ID: <1b87b9821afe39c2af5c1893b11cb7f452c61014.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 17 Jan 2024 15:55:35 +0100 +Subject: [PATCH] remoteDispatchAuthPolkit: Fix lock ordering deadlock if + client closes connection during auth + +Locks in following text: +A: virNetServer +B: virNetServerClient +C: daemonClientPrivate + +'virNetServerSetClientAuthenticated' locks A then B + +'remoteDispatchAuthPolkit' calls 'virNetServerSetClientAuthenticated' +while holding C. + +If a client closes its connection 'virNetServerProcessClients' with the +lock A and B locked will call 'virNetServerClientCloseLocked' which will +try to dispose of the 'client' private data by: + + ref(b); + unlock(b); + remoteClientFreePrivateCallbacks(); + lock(b); + unref(b); + +Unfortunately remoteClientFreePrivateCallbacks() tries lock C. + +Thus the locks are held in the following order: + + polkit auth: C -> A + connection close: A -> C + +causing a textbook-example deadlock. To resolve it we can simply drop +lock 'C' before calling 'virNetServerSetClientAuthenticated' as the lock +is not needed any more. + +Resolves: https://issues.redhat.com/browse/RHEL-20337 +Signed-off-by: Peter Krempa +Reviewed-by: Martin Kletzander +(cherry picked from commit c697aff8a1b5542d51c0b4a10046ad37089d12d5) +--- + src/remote/remote_daemon_dispatch.c | 76 +++++++++++++++-------------- + 1 file changed, 39 insertions(+), 37 deletions(-) + +diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c +index 7daf503b51..aaabd1e56c 100644 +--- a/src/remote/remote_daemon_dispatch.c ++++ b/src/remote/remote_daemon_dispatch.c +@@ -3979,50 +3979,52 @@ remoteDispatchAuthPolkit(virNetServer *server, + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + int rv; +- VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock); +- +- action = virNetServerClientGetReadonly(client) ? +- "org.libvirt.unix.monitor" : +- "org.libvirt.unix.manage"; + +- VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); +- if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { +- VIR_ERROR(_("client tried invalid PolicyKit init request")); +- goto authfail; +- } ++ VIR_WITH_MUTEX_LOCK_GUARD(&priv->lock) { ++ action = virNetServerClientGetReadonly(client) ? ++ "org.libvirt.unix.monitor" : ++ "org.libvirt.unix.manage"; + +- if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid, +- &callerPid, ×tamp) < 0) { +- goto authfail; +- } ++ VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); ++ if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { ++ VIR_ERROR(_("client tried invalid PolicyKit init request")); ++ goto authfail; ++ } + +- if (timestamp == 0) { +- VIR_WARN("Failing polkit auth due to missing client (pid=%lld) start time", +- (long long)callerPid); +- goto authfail; +- } ++ if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid, ++ &callerPid, ×tamp) < 0) { ++ goto authfail; ++ } + +- VIR_INFO("Checking PID %lld running as %d", +- (long long) callerPid, callerUid); ++ if (timestamp == 0) { ++ VIR_WARN("Failing polkit auth due to missing client (pid=%lld) start time", ++ (long long)callerPid); ++ goto authfail; ++ } + +- rv = virPolkitCheckAuth(action, +- callerPid, +- timestamp, +- callerUid, +- NULL, +- true); +- if (rv == -1) +- goto authfail; +- else if (rv == -2) +- goto authdeny; ++ VIR_INFO("Checking PID %lld running as %d", ++ (long long) callerPid, callerUid); + +- PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW, +- "client=%p auth=%d identity=%s", +- client, REMOTE_AUTH_POLKIT, ident); +- VIR_INFO("Policy allowed action %s from pid %lld, uid %d", +- action, (long long) callerPid, callerUid); +- ret->complete = 1; ++ rv = virPolkitCheckAuth(action, ++ callerPid, ++ timestamp, ++ callerUid, ++ NULL, ++ true); ++ if (rv == -1) ++ goto authfail; ++ else if (rv == -2) ++ goto authdeny; ++ ++ PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW, ++ "client=%p auth=%d identity=%s", ++ client, REMOTE_AUTH_POLKIT, ident); ++ VIR_INFO("Policy allowed action %s from pid %lld, uid %d", ++ action, (long long) callerPid, callerUid); ++ ret->complete = 1; ++ } + ++ /* this must be called with the private data mutex unlocked */ + virNetServerSetClientAuthenticated(server, client); + return 0; + +-- +2.43.0 diff --git a/libvirt-remote_driver-Restore-special-behavior-of-remoteDomainGetBlockIoTune.patch b/libvirt-remote_driver-Restore-special-behavior-of-remoteDomainGetBlockIoTune.patch new file mode 100644 index 0000000..40adc63 --- /dev/null +++ b/libvirt-remote_driver-Restore-special-behavior-of-remoteDomainGetBlockIoTune.patch @@ -0,0 +1,45 @@ +From 5821f93bf9b42e3732fe168cdae85054e9a3ac61 Mon Sep 17 00:00:00 2001 +Message-ID: <5821f93bf9b42e3732fe168cdae85054e9a3ac61.1707394626.git.jdenemar@redhat.com> +From: Michal Privoznik +Date: Mon, 29 Jan 2024 10:07:05 +0100 +Subject: [PATCH] remote_driver: Restore special behavior of + remoteDomainGetBlockIoTune() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +In v9.10.0-rc1~103 the remote driver was switched to g_auto() for +client RPC return parameters. But whilst doing so a small bug +slipped in: previously, when virDomainGetBlockIoTune() was called +with *nparams == 0, the function set *nparams to the number of +supported params and zero was returned (so that client can +allocate memory and call the API second time). IOW - the usual, +old style of APIs where we didn't want to allocate memory on +caller's behalf. But because of this bug, a negative one is +returned instead. + +Fixes: 501825011c1fe80f458820c7efe5a198e0af9be5 +Signed-off-by: Michal Privoznik +Reviewed-by: Ján Tomko +(cherry picked from commit 3a3f73ea9f1925ca5e256fa54c5aa451ddeaa19e) +Resolves: https://issues.redhat.com/browse/RHEL-22800 +Signed-off-by: Michal Privoznik +--- + src/remote/remote_driver.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c +index 392377deae..bedf2cb833 100644 +--- a/src/remote/remote_driver.c ++++ b/src/remote/remote_driver.c +@@ -2570,7 +2570,7 @@ static int remoteDomainGetBlockIoTune(virDomainPtr domain, + */ + if (*nparams == 0) { + *nparams = ret.nparams; +- return -1; ++ return 0; + } + + if (virTypedParamsDeserialize((struct _virTypedParameterRemote *) ret.params.params_val, +-- +2.43.0 diff --git a/libvirt-schema-nodedev-Adjust-allowed-characters-in-vpdFieldValueFormat.patch b/libvirt-schema-nodedev-Adjust-allowed-characters-in-vpdFieldValueFormat.patch new file mode 100644 index 0000000..30bb078 --- /dev/null +++ b/libvirt-schema-nodedev-Adjust-allowed-characters-in-vpdFieldValueFormat.patch @@ -0,0 +1,39 @@ +From 4c070fe8db9456a0cd33910d37e613a045a1ec77 Mon Sep 17 00:00:00 2001 +Message-ID: <4c070fe8db9456a0cd33910d37e613a045a1ec77.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 16:12:09 +0100 +Subject: [PATCH] schema: nodedev: Adjust allowed characters in + 'vpdFieldValueFormat' +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The check in 'virPCIVPDResourceIsValidTextValue' allows any printable +characters, thus the XML schema should do the same. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit edaa1112ffef253013dcc3318794cebfaa2a6cb7) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +https://issues.redhat.com/browse/RHEL-22400 [9.3.z] +https://issues.redhat.com/browse/RHEL-22399 [9.2.z] +--- + src/conf/schemas/nodedev.rng | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/conf/schemas/nodedev.rng b/src/conf/schemas/nodedev.rng +index fba4021754..ff07313968 100644 +--- a/src/conf/schemas/nodedev.rng ++++ b/src/conf/schemas/nodedev.rng +@@ -869,7 +869,7 @@ + + + +- [0-9a-zA-F -_,.:;=]{0,255} ++ .{0,255} + + + +-- +2.43.0 diff --git a/libvirt-tests-Test-the-previously-mishandled-PCI-VPD-characters.patch b/libvirt-tests-Test-the-previously-mishandled-PCI-VPD-characters.patch new file mode 100644 index 0000000..8a301ee --- /dev/null +++ b/libvirt-tests-Test-the-previously-mishandled-PCI-VPD-characters.patch @@ -0,0 +1,88 @@ +From a7c145e58b5de35554004f5a779091cec7d03be1 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Tue, 23 Jan 2024 16:40:34 +0100 +Subject: [PATCH] tests: Test the previously mishandled PCI VPD characters +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Modify the test data to validate '<>' and other characters. +Unfortunately the test suite doesn't have a proper end-to-end test, thus +we just add a XML->XML variant and also add data to the binary parser. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 9eda33161f49fcf3ba07d648bd80d2a9a2388479) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +https://issues.redhat.com/browse/RHEL-22400 [9.3.z] +https://issues.redhat.com/browse/RHEL-22399 [9.2.z] +--- + tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml | 4 ++-- + tests/virpcimock.c | 4 ++-- + tests/virpcivpdtest.c | 4 ++-- + 3 files changed, 6 insertions(+), 6 deletions(-) + +diff --git a/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml +index 8b56e4f6b4..c9a2901381 100644 +--- a/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml ++++ b/tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml +@@ -15,7 +15,7 @@ + B1 + foobar + MBF2H332A-AEEOT +- MT2113X00000 ++ MT2113X00000>< + PCIeGen4 x8 + MBF2H332A-AEEOT + 3c53d07eec484d8aab34dabd24fe575aa +@@ -25,7 +25,7 @@ + fooasset + vendorfield0 + vendorfield2 +- vendorfieldA ++ !@#$./>< + systemfieldB + systemfield0 + +diff --git a/tests/virpcimock.c b/tests/virpcimock.c +index 13b37bb23d..2f98b0cf13 100644 +--- a/tests/virpcimock.c ++++ b/tests/virpcimock.c +@@ -966,9 +966,9 @@ init_env(void) + 't', 'e', 's', 't', 'n', 'a', 'm', 'e', + PCI_VPD_LARGE_RESOURCE_FLAG | PCI_VPD_READ_ONLY_LARGE_RESOURCE_FLAG, 0x16, 0x00, + 'P', 'N', 0x02, '4', '2', +- 'E', 'C', 0x04, '4', '2', '4', '2', ++ 'E', 'C', 0x04, '4', '<', '>', '2', + 'V', 'A', 0x02, 'E', 'X', +- 'R', 'V', 0x02, 0x31, 0x00, ++ 'R', 'V', 0x02, 0x1D, 0x00, + PCI_VPD_RESOURCE_END_VAL + }; + struct pciVPD exampleVPD = { +diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c +index b4dd68b7aa..ae5772d3f5 100644 +--- a/tests/virpcivpdtest.c ++++ b/tests/virpcivpdtest.c +@@ -424,7 +424,7 @@ testPCIVPDGetFieldValueFormat(const void *data G_GNUC_UNUSED) + + # define VPD_W_EXAMPLE_FIELDS \ + 'V', 'Z', 0x02, '4', '2', \ +- 'Y', 'A', 0x04, 'I', 'D', '4', '2', \ ++ 'Y', 'A', 0x04, '!', '<', '>', ':', \ + 'Y', 'F', 0x02, 'E', 'X', \ + 'Y', 'E', 0x00, \ + 'R', 'W', 0x02, 0x00, 0x00 +@@ -579,7 +579,7 @@ testVirPCIVPDParseFullVPD(const void *opaque G_GNUC_UNUSED) + if (testVirPCIVPDValidateExampleReadOnlyFields(res)) + return -1; + +- if (STRNEQ_NULLABLE(res->rw->asset_tag, "ID42")) ++ if (STRNEQ_NULLABLE(res->rw->asset_tag, "!<>:")) + return -1; + + if (!res->rw->vendor_specific) +-- +2.43.0 diff --git a/libvirt-tests-virpcivpd-Remove-testVirPCIVPDParseVPDStringResource-case.patch b/libvirt-tests-virpcivpd-Remove-testVirPCIVPDParseVPDStringResource-case.patch new file mode 100644 index 0000000..71ce319 --- /dev/null +++ b/libvirt-tests-virpcivpd-Remove-testVirPCIVPDParseVPDStringResource-case.patch @@ -0,0 +1,81 @@ +From 7c634eb7244604521b0f2a00f3a7e2e65a6a8399 Mon Sep 17 00:00:00 2001 +Message-ID: <7c634eb7244604521b0f2a00f3a7e2e65a6a8399.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 17:55:06 +0100 +Subject: [PATCH] tests: virpcivpd: Remove + 'testVirPCIVPDParseVPDStringResource' case +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The test case excercises 'virPCIVPDParseVPDLargeResourceString' which is +also tested by other cases which parse the whole VPD block. Remove the +specific test case as it's not adding any additional value. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 78e17cd550f0ee1f200557d496ef43724319c17e) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + tests/virpcivpdtest.c | 38 -------------------------------------- + 1 file changed, 38 deletions(-) + +diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c +index aadd1b222b..fddb42f52c 100644 +--- a/tests/virpcivpdtest.c ++++ b/tests/virpcivpdtest.c +@@ -429,42 +429,6 @@ testPCIVPDGetFieldValueFormat(const void *data G_GNUC_UNUSED) + 'Y', 'E', 0x00, \ + 'R', 'W', 0x02, 0x00, 0x00 + +-static int +-testVirPCIVPDParseVPDStringResource(const void *opaque G_GNUC_UNUSED) +-{ +- VIR_AUTOCLOSE fd = -1; +- uint8_t csum = 0; +- size_t dataLen = 0; +- bool result = false; +- +- g_autoptr(virPCIVPDResource) res = g_new0(virPCIVPDResource, 1); +- const char *expectedValue = "testname"; +- +- const uint8_t stringResExample[] = { +- VPD_STRING_RESOURCE_EXAMPLE_DATA +- }; +- +- dataLen = G_N_ELEMENTS(stringResExample); +- if ((fd = virCreateAnonymousFile(stringResExample, dataLen)) < 0) +- return -1; +- +- result = virPCIVPDParseVPDLargeResourceString(fd, 0, dataLen, &csum, res); +- +- if (!result) { +- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +- "Could not parse the example resource."); +- return -1; +- } +- +- if (STRNEQ(expectedValue, res->name)) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "Unexpected string resource value: %s, expected: %s", +- res->name, expectedValue); +- return -1; +- } +- return 0; +-} +- + static int + testVirPCIVPDValidateExampleReadOnlyFields(virPCIVPDResource *res) + { +@@ -964,8 +928,6 @@ mymain(void) + if (virTestRun("Determining a field value format by a key ", + testPCIVPDGetFieldValueFormat, NULL) < 0) + ret = -1; +- if (virTestRun("Parsing VPD string resources ", testVirPCIVPDParseVPDStringResource, NULL) < 0) +- ret = -1; + if (virTestRun("Parsing a VPD resource with a zero-length RW ", + testVirPCIVPDParseZeroLengthRW, NULL) < 0) + ret = -1; +-- +2.43.0 diff --git a/libvirt-tests-virpcivpdtest-Remove-testVirPCIVPDReadVPDBytes-case.patch b/libvirt-tests-virpcivpdtest-Remove-testVirPCIVPDReadVPDBytes-case.patch new file mode 100644 index 0000000..9682b99 --- /dev/null +++ b/libvirt-tests-virpcivpdtest-Remove-testVirPCIVPDReadVPDBytes-case.patch @@ -0,0 +1,83 @@ +From c4b66437f7b829efa0ab6c6007347061ca257719 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Wed, 24 Jan 2024 14:55:47 +0100 +Subject: [PATCH] tests: virpcivpdtest: Remove 'testVirPCIVPDReadVPDBytes' case +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The case checks only the 'virPCIVPDReadVPDBytes' which is also tested +multiple times via 'virPCIVPDParse' as it's used to read the data, thus +having a special case for this is pointless. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 1a994a9dc6424ff7ea9d0f5d325059ffd234408b) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + tests/virpcivpdtest.c | 41 ----------------------------------------- + 1 file changed, 41 deletions(-) + +diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c +index ae5772d3f5..aadd1b222b 100644 +--- a/tests/virpcivpdtest.c ++++ b/tests/virpcivpdtest.c +@@ -429,45 +429,6 @@ testPCIVPDGetFieldValueFormat(const void *data G_GNUC_UNUSED) + 'Y', 'E', 0x00, \ + 'R', 'W', 0x02, 0x00, 0x00 + +-static int +-testVirPCIVPDReadVPDBytes(const void *opaque G_GNUC_UNUSED) +-{ +- VIR_AUTOCLOSE fd = -1; +- g_autofree uint8_t *buf = NULL; +- uint8_t csum = 0; +- size_t readBytes = 0; +- size_t dataLen = 0; +- +- /* An example of a valid VPD record with one VPD-R resource and 2 fields. */ +- uint8_t fullVPDExample[] = { +- VPD_STRING_RESOURCE_EXAMPLE_HEADER, VPD_STRING_RESOURCE_EXAMPLE_DATA, +- VPD_R_FIELDS_EXAMPLE_HEADER, VPD_R_FIELDS_EXAMPLE_DATA, +- PCI_VPD_RESOURCE_END_VAL +- }; +- dataLen = G_N_ELEMENTS(fullVPDExample) - 2; +- buf = g_malloc0(dataLen); +- +- if ((fd = virCreateAnonymousFile(fullVPDExample, dataLen)) < 0) +- return -1; +- +- readBytes = virPCIVPDReadVPDBytes(fd, buf, dataLen, 0, &csum); +- +- if (readBytes != dataLen) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "The number of bytes read %zu is lower than expected %zu ", +- readBytes, dataLen); +- return -1; +- } +- +- if (csum) { +- virReportError(VIR_ERR_INTERNAL_ERROR, +- "The sum of all VPD bytes up to and including the checksum byte" +- "is equal to zero: 0x%02x", csum); +- return -1; +- } +- return 0; +-} +- + static int + testVirPCIVPDParseVPDStringResource(const void *opaque G_GNUC_UNUSED) + { +@@ -1003,8 +964,6 @@ mymain(void) + 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 a zero-length RW ", +-- +2.43.0 diff --git a/libvirt-util-pcivpd-Refactor-virPCIVPDResourceIsValidTextValue.patch b/libvirt-util-pcivpd-Refactor-virPCIVPDResourceIsValidTextValue.patch new file mode 100644 index 0000000..0d6d92c --- /dev/null +++ b/libvirt-util-pcivpd-Refactor-virPCIVPDResourceIsValidTextValue.patch @@ -0,0 +1,62 @@ +From b28e30bd2b1b40fb3bec3064e883cc9f3abff7c5 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Wed, 24 Jan 2024 15:53:39 +0100 +Subject: [PATCH] util: pcivpd: Refactor virPCIVPDResourceIsValidTextValue +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The function is never called with NULL argument. Remove the check and +refactor the rest including the debug statement. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit eb3844009dc3bdd50274954618b8cd9962218317) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +https://issues.redhat.com/browse/RHEL-22400 [9.3.z] +https://issues.redhat.com/browse/RHEL-22399 [9.2.z] +--- + src/util/virpcivpd.c | 23 +++++++++-------------- + 1 file changed, 9 insertions(+), 14 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 248a9b2790..81c7c317b3 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -175,23 +175,18 @@ virPCIVPDResourceGetFieldValueFormat(const char *keyword) + bool + virPCIVPDResourceIsValidTextValue(const char *value) + { +- size_t i = 0; ++ const char *v; ++ bool ret = true; + +- if (value == NULL) +- return false; +- +- /* An empty string is a valid value. */ +- if (STREQ(value, "")) +- return true; +- +- while (i < strlen(value)) { +- if (!g_ascii_isprint(value[i])) { +- VIR_DEBUG("The provided value contains non-ASCII printable characters: %s", value); +- return false; ++ for (v = value; *v; v++) { ++ if (!g_ascii_isprint(*v)) { ++ ret = false; ++ break; + } +- ++i; + } +- return true; ++ ++ VIR_DEBUG("val='%s' ret='%d'", value, ret); ++ return ret; + } + + void +-- +2.43.0 diff --git a/libvirt-util-pcivpd-Unexport-virPCIVPDParseVPDLargeResourceFields.patch b/libvirt-util-pcivpd-Unexport-virPCIVPDParseVPDLargeResourceFields.patch new file mode 100644 index 0000000..1d3646b --- /dev/null +++ b/libvirt-util-pcivpd-Unexport-virPCIVPDParseVPDLargeResourceFields.patch @@ -0,0 +1,82 @@ +From 44db6e745e039dd10c1f4256047eaef8be61ecd6 Mon Sep 17 00:00:00 2001 +Message-ID: <44db6e745e039dd10c1f4256047eaef8be61ecd6.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 14:40:38 +0100 +Subject: [PATCH] util: pcivpd: Unexport virPCIVPDParseVPDLargeResourceFields +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The function is not used in other files. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit d395d7a20f218d5c1af956c70fae43e8e9626436) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/libvirt_private.syms | 1 - + src/util/virpcivpd.c | 15 +-------------- + src/util/virpcivpdpriv.h | 3 --- + 3 files changed, 1 insertion(+), 18 deletions(-) + +diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms +index dbc4e26d79..89b0d01de6 100644 +--- a/src/libvirt_private.syms ++++ b/src/libvirt_private.syms +@@ -3700,7 +3700,6 @@ virVHBAPathExists; + # util/virpcivpd.h + + virPCIVPDParse; +-virPCIVPDParseVPDLargeResourceFields; + virPCIVPDParseVPDLargeResourceString; + virPCIVPDResourceCustomCompareIndex; + virPCIVPDResourceCustomFree; +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 373321a836..510be65cb6 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -436,7 +436,7 @@ virPCIVPDReadVPDBytes(int vpdFileFd, uint8_t *buf, size_t count, off_t offset, u + * Returns: a pointer to a VPDResource which needs to be freed by the caller or + * NULL if getting it failed for some reason. + */ +-bool ++static bool + virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t resDataLen, + bool readOnly, uint8_t *csum, virPCIVPDResource *res) + { +@@ -744,19 +744,6 @@ virPCIVPDParseVPDLargeResourceString(int vpdFileFd G_GNUC_UNUSED, + return false; + } + +-bool +-virPCIVPDParseVPDLargeResourceFields(int vpdFileFd G_GNUC_UNUSED, +- uint16_t resPos G_GNUC_UNUSED, +- uint16_t resDataLen G_GNUC_UNUSED, +- bool readOnly G_GNUC_UNUSED, +- uint8_t *csum G_GNUC_UNUSED, +- virPCIVPDResource *res G_GNUC_UNUSED) +-{ +- virReportError(VIR_ERR_NO_SUPPORT, "%s", +- _("PCI VPD reporting not available on this platform")); +- return false; +-} +- + virPCIVPDResource * + virPCIVPDParse(int vpdFileFd G_GNUC_UNUSED) + { +diff --git a/src/util/virpcivpdpriv.h b/src/util/virpcivpdpriv.h +index 17e6e14ab7..d84f1e9c8a 100644 +--- a/src/util/virpcivpdpriv.h ++++ b/src/util/virpcivpdpriv.h +@@ -69,8 +69,5 @@ virPCIVPDResourceCustomCompareIndex(virPCIVPDResourceCustom *a, virPCIVPDResourc + bool + virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const value); + +-bool virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t resDataLen, +- bool readOnly, uint8_t *csum, virPCIVPDResource *res); +- + bool virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos, uint16_t resDataLen, + uint8_t *csum, virPCIVPDResource *res); +-- +2.43.0 diff --git a/libvirt-util-virPCIVPDResourceUpdateKeyword-Remove-impossible-checks.patch b/libvirt-util-virPCIVPDResourceUpdateKeyword-Remove-impossible-checks.patch new file mode 100644 index 0000000..decc210 --- /dev/null +++ b/libvirt-util-virPCIVPDResourceUpdateKeyword-Remove-impossible-checks.patch @@ -0,0 +1,63 @@ +From 3c01b1ab89ad4c16862582a05394f056616925a2 Mon Sep 17 00:00:00 2001 +Message-ID: <3c01b1ab89ad4c16862582a05394f056616925a2.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 16:42:45 +0100 +Subject: [PATCH] util: virPCIVPDResourceUpdateKeyword: Remove impossible + checks +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +All callers satisfy these checks as they are just for programming +errors. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit dd328cd48a469d81e91eaf56fad832aa8bd288d6) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 19 ------------------- + 1 file changed, 19 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index f198faaf42..3beb405252 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -313,20 +313,7 @@ bool + virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, + const char *const keyword, const char *const value) + { +- if (!res) { +- VIR_INFO("Cannot update the resource: a NULL resource pointer has been provided."); +- return false; +- } else if (!keyword) { +- VIR_INFO("Cannot update the resource: a NULL keyword pointer has been provided."); +- return false; +- } +- + if (readOnly) { +- if (!res->ro) { +- VIR_INFO("Cannot update the read-only keyword: RO section not initialized."); +- return false; +- } +- + if (STREQ("EC", keyword) || STREQ("change_level", keyword)) { + g_free(res->ro->change_level); + res->ro->change_level = g_strdup(value); +@@ -353,13 +340,7 @@ virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, + /* The CP keyword is currently not supported and is skipped. */ + return true; + } +- + } else { +- if (!res->rw) { +- VIR_INFO("Cannot update the read-write keyword: read-write section not initialized."); +- return false; +- } +- + if (STREQ("YA", keyword) || STREQ("asset_tag", keyword)) { + g_free(res->rw->asset_tag); + res->rw->asset_tag = g_strdup(value); +-- +2.43.0 diff --git a/libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceCustomUpsertValue.patch b/libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceCustomUpsertValue.patch new file mode 100644 index 0000000..5fd3ff1 --- /dev/null +++ b/libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceCustomUpsertValue.patch @@ -0,0 +1,143 @@ +From 509e56de7e817de1edbd76da806eb79dca6d45e3 Mon Sep 17 00:00:00 2001 +Message-ID: <509e56de7e817de1edbd76da806eb79dca6d45e3.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 16:11:24 +0100 +Subject: [PATCH] util: virpcivpd: Remove return value from + virPCIVPDResourceCustomUpsertValue +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +None of the callers pass NULL, so the NULL check is pointless. Remove it +an remove the return value. + +The function is exported only for use in 'virpcivpdtest' thus marking +the arguments as NONNULL is unnecessary. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit d36da8ea4a107d129bdc701f95b1b131bc3df01d) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 18 ++++-------------- + src/util/virpcivpdpriv.h | 2 +- + tests/virpcivpdtest.c | 12 ++++-------- + 3 files changed, 9 insertions(+), 23 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 67065dec46..f198faaf42 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -270,7 +270,7 @@ virPCIVPDResourceCustomCompareIndex(virPCIVPDResourceCustom *a, virPCIVPDResourc + * + * Returns: true if a value has been updated successfully, false otherwise. + */ +-bool ++void + virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const value) + { + g_autoptr(virPCIVPDResourceCustom) custom = NULL; +@@ -278,9 +278,6 @@ virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const + guint pos = 0; + bool found = false; + +- if (arr == NULL || value == NULL) +- return false; +- + custom = g_new0(virPCIVPDResourceCustom, 1); + custom->idx = index; + custom->value = g_strdup(value); +@@ -294,7 +291,6 @@ virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const + } else { + g_ptr_array_add(arr, g_steal_pointer(&custom)); + } +- return true; + } + + /** +@@ -348,9 +344,7 @@ virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, + res->ro->serial_number = g_strdup(value); + return true; + } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { +- if (!virPCIVPDResourceCustomUpsertValue(res->ro->vendor_specific, keyword[1], value)) { +- return false; +- } ++ virPCIVPDResourceCustomUpsertValue(res->ro->vendor_specific, keyword[1], value); + return true; + } else if (STREQ("FG", keyword) || STREQ("LC", keyword) || STREQ("PG", keyword)) { + /* Legacy PICMIG keywords are skipped on purpose. */ +@@ -371,14 +365,10 @@ virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, + res->rw->asset_tag = g_strdup(value); + return true; + } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { +- if (!virPCIVPDResourceCustomUpsertValue(res->rw->vendor_specific, keyword[1], value)) { +- return false; +- } ++ virPCIVPDResourceCustomUpsertValue(res->rw->vendor_specific, keyword[1], value); + return true; + } else if (virPCIVPDResourceIsSystemKeyword(keyword)) { +- if (!virPCIVPDResourceCustomUpsertValue(res->rw->system_specific, keyword[1], value)) { +- return false; +- } ++ virPCIVPDResourceCustomUpsertValue(res->rw->system_specific, keyword[1], value); + return true; + } + } +diff --git a/src/util/virpcivpdpriv.h b/src/util/virpcivpdpriv.h +index 617991930b..f26b64139d 100644 +--- a/src/util/virpcivpdpriv.h ++++ b/src/util/virpcivpdpriv.h +@@ -66,5 +66,5 @@ bool virPCIVPDResourceIsValidTextValue(const char *value); + gboolean + virPCIVPDResourceCustomCompareIndex(virPCIVPDResourceCustom *a, virPCIVPDResourceCustom *b); + +-bool ++void + virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const value); +diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c +index fddb42f52c..8a2f337e85 100644 +--- a/tests/virpcivpdtest.c ++++ b/tests/virpcivpdtest.c +@@ -244,8 +244,7 @@ testPCIVPDResourceCustomUpsertValue(const void *data G_GNUC_UNUSED) + { + g_autoptr(GPtrArray) arr = g_ptr_array_new_full(0, (GDestroyNotify)virPCIVPDResourceCustomFree); + virPCIVPDResourceCustom *custom = NULL; +- if (!virPCIVPDResourceCustomUpsertValue(arr, 'A', "testval")) +- return -1; ++ virPCIVPDResourceCustomUpsertValue(arr, 'A', "testval"); + + if (arr->len != 1) + return -1; +@@ -255,8 +254,7 @@ testPCIVPDResourceCustomUpsertValue(const void *data G_GNUC_UNUSED) + return -1; + + /* Idempotency */ +- if (!virPCIVPDResourceCustomUpsertValue(arr, 'A', "testval")) +- return -1; ++ virPCIVPDResourceCustomUpsertValue(arr, 'A', "testval"); + + if (arr->len != 1) + return -1; +@@ -266,8 +264,7 @@ testPCIVPDResourceCustomUpsertValue(const void *data G_GNUC_UNUSED) + return -1; + + /* Existing value updates. */ +- if (!virPCIVPDResourceCustomUpsertValue(arr, 'A', "testvalnew")) +- return -1; ++ virPCIVPDResourceCustomUpsertValue(arr, 'A', "testvalnew"); + + if (arr->len != 1) + return -1; +@@ -277,8 +274,7 @@ testPCIVPDResourceCustomUpsertValue(const void *data G_GNUC_UNUSED) + return -1; + + /* Inserting multiple values */ +- if (!virPCIVPDResourceCustomUpsertValue(arr, '1', "42")) +- return -1; ++ virPCIVPDResourceCustomUpsertValue(arr, '1', "42"); + + if (arr->len != 2) + return -1; +-- +2.43.0 diff --git a/libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceUpdateKeyword.patch b/libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceUpdateKeyword.patch new file mode 100644 index 0000000..28e9d7f --- /dev/null +++ b/libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceUpdateKeyword.patch @@ -0,0 +1,206 @@ +From 1fe79c11dab8f390efd2035ba20c194987ba4dee Mon Sep 17 00:00:00 2001 +Message-ID: <1fe79c11dab8f390efd2035ba20c194987ba4dee.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 17:15:10 +0100 +Subject: [PATCH] util: virpcivpd: Remove return value from + virPCIVPDResourceUpdateKeyword +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The function always succeeded and after the removal of programing error +checks doesn't even have a 'return false' case. Additionally one of the +tests in 'virpcivpdtest' tested that this function never failed on wrong +data. Embrace this logic and remove the return value and adjust logging +to VIR_DEBUG level to avoid spamming logs. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 9aa303a948a3eb98b727a333e2571d19ae0a81ce) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 31 +++++++++++-------------------- + src/util/virpcivpd.h | 8 +++++--- + tests/virpcivpdtest.c | 38 ++++++++++++++++++-------------------- + 3 files changed, 34 insertions(+), 43 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 3beb405252..0021a88f2d 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -307,54 +307,48 @@ virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const + * 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. ++ * Unknown or malformed values are ignored. + */ +-bool +-virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, +- const char *const keyword, const char *const value) ++void ++virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, ++ const bool readOnly, ++ const char *const keyword, ++ const char *const value) + { + if (readOnly) { + if (STREQ("EC", keyword) || STREQ("change_level", keyword)) { + g_free(res->ro->change_level); + res->ro->change_level = g_strdup(value); +- return true; + } else if (STREQ("MN", keyword) || STREQ("manufacture_id", keyword)) { + g_free(res->ro->manufacture_id); + res->ro->manufacture_id = g_strdup(value); +- return true; + } else if (STREQ("PN", keyword) || STREQ("part_number", keyword)) { + g_free(res->ro->part_number); + res->ro->part_number = g_strdup(value); +- return true; + } else if (STREQ("SN", keyword) || STREQ("serial_number", keyword)) { + g_free(res->ro->serial_number); + res->ro->serial_number = g_strdup(value); +- return true; + } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { + virPCIVPDResourceCustomUpsertValue(res->ro->vendor_specific, keyword[1], value); +- 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 { ++ VIR_DEBUG("unhandled PCI VPD r/o keyword '%s'(val='%s')", keyword, value); + } + } else { + if (STREQ("YA", keyword) || STREQ("asset_tag", keyword)) { + g_free(res->rw->asset_tag); + res->rw->asset_tag = g_strdup(value); +- return true; + } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { + virPCIVPDResourceCustomUpsertValue(res->rw->vendor_specific, keyword[1], value); +- return true; + } else if (virPCIVPDResourceIsSystemKeyword(keyword)) { + virPCIVPDResourceCustomUpsertValue(res->rw->system_specific, keyword[1], value); +- return true; ++ } else { ++ VIR_DEBUG("unhandled PCI VPD r/w keyword '%s'(val='%s')", keyword, value); + } + } +- VIR_WARN("Tried to update an unsupported keyword %s: skipping.", keyword); +- return true; + } + + #ifdef __linux__ +@@ -527,10 +521,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + res->rw = virPCIVPDResourceRWNew(); + } + /* The field format, keyword and value are determined. Attempt to update the resource. */ +- if (!virPCIVPDResourceUpdateKeyword(res, readOnly, fieldKeyword, fieldValue)) { +- VIR_INFO("Could not update the VPD resource keyword: %s", fieldKeyword); +- return false; +- } ++ virPCIVPDResourceUpdateKeyword(res, readOnly, fieldKeyword, fieldValue); + } + + /* May have exited the loop prematurely in case RV or RW were encountered and +diff --git a/src/util/virpcivpd.h b/src/util/virpcivpd.h +index 9bfec43e03..d8d3dd3075 100644 +--- a/src/util/virpcivpd.h ++++ b/src/util/virpcivpd.h +@@ -67,9 +67,11 @@ 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 ++virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, ++ const bool readOnly, ++ const char *const keyword, ++ const char *const value); + + void virPCIVPDResourceCustomFree(virPCIVPDResourceCustom *custom); + +diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c +index 20545759d5..a6311bfe76 100644 +--- a/tests/virpcivpdtest.c ++++ b/tests/virpcivpdtest.c +@@ -84,17 +84,15 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) + + /* 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; ++ virPCIVPDResourceUpdateKeyword(res, true, ++ readOnlyCases[i].keyword, ++ readOnlyCases[i].value); + if (STRNEQ(readOnlyCases[i].value, *readOnlyCases[i].actual)) + return -1; + } + + /* Do a basic vendor field check. */ +- if (!virPCIVPDResourceUpdateKeyword(res, true, "V0", "vendor0")) +- return -1; ++ virPCIVPDResourceUpdateKeyword(res, true, "V0", "vendor0"); + + if (res->ro->vendor_specific->len != 1) + return -1; +@@ -105,25 +103,23 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) + + /* Make sure unsupported RO keyword updates are not fatal. */ + for (i = 0; i < numUnsupportedCases; ++i) { +- if (!virPCIVPDResourceUpdateKeyword(res, true, +- unsupportedFieldCases[i].keyword, +- unsupportedFieldCases[i].value)) +- return -1; ++ virPCIVPDResourceUpdateKeyword(res, true, ++ unsupportedFieldCases[i].keyword, ++ unsupportedFieldCases[i].value); + } + + /* Initialize RW */ + res->rw = g_steal_pointer(&rw); +- if (!virPCIVPDResourceUpdateKeyword(res, false, "YA", "tag1") +- || STRNEQ(res->rw->asset_tag, "tag1")) ++ virPCIVPDResourceUpdateKeyword(res, false, "YA", "tag1"); ++ if (STRNEQ(res->rw->asset_tag, "tag1")) + return -1; + +- if (!virPCIVPDResourceUpdateKeyword(res, false, "asset_tag", "tag2") +- || STRNEQ(res->rw->asset_tag, "tag2")) ++ virPCIVPDResourceUpdateKeyword(res, false, "asset_tag", "tag2"); ++ if (STRNEQ(res->rw->asset_tag, "tag2")) + return -1; + + /* Do a basic system field check. */ +- if (!virPCIVPDResourceUpdateKeyword(res, false, "Y0", "system0")) +- return -1; ++ virPCIVPDResourceUpdateKeyword(res, false, "Y0", "system0"); + + if (res->rw->system_specific->len != 1) + return -1; +@@ -134,10 +130,12 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) + + /* Make sure unsupported RW keyword updates are not fatal. */ + for (i = 0; i < numUnsupportedCases; ++i) { +- if (!virPCIVPDResourceUpdateKeyword(res, false, +- unsupportedFieldCases[i].keyword, +- unsupportedFieldCases[i].value)) +- return -1; ++ /* This test is deliberately left in despite ++ * virPCIVPDResourceUpdateKeyword always succeeding to prevent ++ * possible regressions if the function is ever rewritten */ ++ virPCIVPDResourceUpdateKeyword(res, false, ++ unsupportedFieldCases[i].keyword, ++ unsupportedFieldCases[i].value); + } + + +-- +2.43.0 diff --git a/libvirt-util-virpcivpd-Unexport-virPCIVPDParseVPDLargeResourceString.patch b/libvirt-util-virpcivpd-Unexport-virPCIVPDParseVPDLargeResourceString.patch new file mode 100644 index 0000000..4ae5618 --- /dev/null +++ b/libvirt-util-virpcivpd-Unexport-virPCIVPDParseVPDLargeResourceString.patch @@ -0,0 +1,78 @@ +From 5c7b4d446b4436deef45176daeb33827007a8ef2 Mon Sep 17 00:00:00 2001 +Message-ID: <5c7b4d446b4436deef45176daeb33827007a8ef2.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 17:58:17 +0100 +Subject: [PATCH] util: virpcivpd: Unexport + 'virPCIVPDParseVPDLargeResourceString' +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 810a3ca980801a57298c245c65aa558eda5fb9d8) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/libvirt_private.syms | 1 - + src/util/virpcivpd.c | 14 +------------- + src/util/virpcivpdpriv.h | 3 --- + 3 files changed, 1 insertion(+), 17 deletions(-) + +diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms +index 89b0d01de6..035f8c7b5d 100644 +--- a/src/libvirt_private.syms ++++ b/src/libvirt_private.syms +@@ -3700,7 +3700,6 @@ virVHBAPathExists; + # util/virpcivpd.h + + virPCIVPDParse; +-virPCIVPDParseVPDLargeResourceString; + virPCIVPDResourceCustomCompareIndex; + virPCIVPDResourceCustomFree; + virPCIVPDResourceCustomUpsertValue; +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 510be65cb6..b303e161ae 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -595,7 +595,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + * Returns: a pointer to a VPDResource which needs to be freed by the caller or + * NULL if getting it failed for some reason. + */ +-bool ++static bool + virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos, + uint16_t resDataLen, uint8_t *csum, virPCIVPDResource *res) + { +@@ -732,18 +732,6 @@ virPCIVPDParse(int vpdFileFd) + + #else /* ! __linux__ */ + +-bool +-virPCIVPDParseVPDLargeResourceString(int vpdFileFd G_GNUC_UNUSED, +- uint16_t resPos G_GNUC_UNUSED, +- uint16_t resDataLen G_GNUC_UNUSED, +- uint8_t *csum G_GNUC_UNUSED, +- virPCIVPDResource *res G_GNUC_UNUSED) +-{ +- virReportError(VIR_ERR_NO_SUPPORT, "%s", +- _("PCI VPD reporting not available on this platform")); +- return false; +-} +- + virPCIVPDResource * + virPCIVPDParse(int vpdFileFd G_GNUC_UNUSED) + { +diff --git a/src/util/virpcivpdpriv.h b/src/util/virpcivpdpriv.h +index d84f1e9c8a..617991930b 100644 +--- a/src/util/virpcivpdpriv.h ++++ b/src/util/virpcivpdpriv.h +@@ -68,6 +68,3 @@ virPCIVPDResourceCustomCompareIndex(virPCIVPDResourceCustom *a, virPCIVPDResourc + + bool + virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const value); +- +-bool virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos, uint16_t resDataLen, +- uint8_t *csum, virPCIVPDResource *res); +-- +2.43.0 diff --git a/libvirt-util-virpcivpd-Unexport-virPCIVPDReadVPDBytes.patch b/libvirt-util-virpcivpd-Unexport-virPCIVPDReadVPDBytes.patch new file mode 100644 index 0000000..0a5dbc1 --- /dev/null +++ b/libvirt-util-virpcivpd-Unexport-virPCIVPDReadVPDBytes.patch @@ -0,0 +1,82 @@ +From 225998619d4e6fb87e04d48c74b1320663543312 Mon Sep 17 00:00:00 2001 +Message-ID: <225998619d4e6fb87e04d48c74b1320663543312.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 14:58:52 +0100 +Subject: [PATCH] util: virpcivpd: Unexport 'virPCIVPDReadVPDBytes' +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The function is no longer used outside of virpcivpd.c + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 4d229ef4404f277004767bec444448fa83e8be99) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/libvirt_private.syms | 1 - + src/util/virpcivpd.c | 14 +------------- + src/util/virpcivpdpriv.h | 3 --- + 3 files changed, 1 insertion(+), 17 deletions(-) + +diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms +index fc26109029..dbc4e26d79 100644 +--- a/src/libvirt_private.syms ++++ b/src/libvirt_private.syms +@@ -3702,7 +3702,6 @@ virVHBAPathExists; + virPCIVPDParse; + virPCIVPDParseVPDLargeResourceFields; + virPCIVPDParseVPDLargeResourceString; +-virPCIVPDReadVPDBytes; + virPCIVPDResourceCustomCompareIndex; + virPCIVPDResourceCustomFree; + virPCIVPDResourceCustomUpsertValue; +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 81c7c317b3..373321a836 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -401,7 +401,7 @@ virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, + * descriptor, it is reported and -1 is returned to the caller. If EOF is occurred, 0 is returned + * to the caller. + */ +-size_t ++static size_t + virPCIVPDReadVPDBytes(int vpdFileFd, uint8_t *buf, size_t count, off_t offset, uint8_t *csum) + { + ssize_t numRead = pread(vpdFileFd, buf, count, offset); +@@ -732,18 +732,6 @@ virPCIVPDParse(int vpdFileFd) + + #else /* ! __linux__ */ + +-size_t +-virPCIVPDReadVPDBytes(int vpdFileFd G_GNUC_UNUSED, +- uint8_t *buf G_GNUC_UNUSED, +- size_t count G_GNUC_UNUSED, +- off_t offset G_GNUC_UNUSED, +- uint8_t *csum G_GNUC_UNUSED) +-{ +- virReportError(VIR_ERR_NO_SUPPORT, "%s", +- _("PCI VPD reporting not available on this platform")); +- return 0; +-} +- + bool + virPCIVPDParseVPDLargeResourceString(int vpdFileFd G_GNUC_UNUSED, + uint16_t resPos G_GNUC_UNUSED, +diff --git a/src/util/virpcivpdpriv.h b/src/util/virpcivpdpriv.h +index 0f565f81ae..17e6e14ab7 100644 +--- a/src/util/virpcivpdpriv.h ++++ b/src/util/virpcivpdpriv.h +@@ -69,9 +69,6 @@ virPCIVPDResourceCustomCompareIndex(virPCIVPDResourceCustom *a, virPCIVPDResourc + bool + virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, char index, const char *const value); + +-size_t +-virPCIVPDReadVPDBytes(int vpdFileFd, uint8_t *buf, size_t count, off_t offset, uint8_t *csum); +- + bool virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t resDataLen, + bool readOnly, uint8_t *csum, virPCIVPDResource *res); + +-- +2.43.0 diff --git a/libvirt-util-virtportallocator-Add-VIR_DEBUG-statements-for-port-allocations-and-release.patch b/libvirt-util-virtportallocator-Add-VIR_DEBUG-statements-for-port-allocations-and-release.patch new file mode 100644 index 0000000..fb0655c --- /dev/null +++ b/libvirt-util-virtportallocator-Add-VIR_DEBUG-statements-for-port-allocations-and-release.patch @@ -0,0 +1,64 @@ +From 9f199b64bd22f4f740441bb690d9bd9231e8efc0 Mon Sep 17 00:00:00 2001 +Message-ID: <9f199b64bd22f4f740441bb690d9bd9231e8efc0.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Tue, 16 Jan 2024 15:10:55 +0100 +Subject: [PATCH] util: virtportallocator: Add VIR_DEBUG statements for port + allocations and release + +Add a few debug statements to be able to trace lifetime of a +reserved/allocated port. + +Signed-off-by: Peter Krempa +Reviewed-by: Andrea Bolognani +(cherry picked from commit 19eaa854386cc5bfc0f9ed15ac2a0937cb3a4421) +https://issues.redhat.com/browse/RHEL-21543 +--- + src/util/virportallocator.c | 9 +++++++++ + 1 file changed, 9 insertions(+) + +diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c +index 6d6f99778e..70393d87ee 100644 +--- a/src/util/virportallocator.c ++++ b/src/util/virportallocator.c +@@ -29,9 +29,12 @@ + #include "virthread.h" + #include "virerror.h" + #include "virutil.h" ++#include "virlog.h" + + #define VIR_FROM_THIS VIR_FROM_NONE + ++VIR_LOG_INIT("util.virportallocator"); ++ + #define VIR_PORT_ALLOCATOR_NUM_PORTS 65536 + + typedef struct _virPortAllocator virPortAllocator; +@@ -228,6 +231,8 @@ virPortAllocatorAcquire(const virPortAllocatorRange *range, + return -1; + } + *port = i; ++ VIR_DEBUG("port='%u'", *port); ++ + return 0; + } + } +@@ -247,6 +252,8 @@ virPortAllocatorRelease(unsigned short port) + if (!pa) + return -1; + ++ VIR_DEBUG("port='%u'", port); ++ + if (!port) + return 0; + +@@ -265,6 +272,8 @@ virPortAllocatorSetUsed(unsigned short port) + if (!pa) + return -1; + ++ VIR_DEBUG("port='%u'", port); ++ + if (!port) + return 0; + +-- +2.43.0 diff --git a/libvirt-virNodeDeviceCapVPDFormat-Properly-escape-system-originated-strings.patch b/libvirt-virNodeDeviceCapVPDFormat-Properly-escape-system-originated-strings.patch new file mode 100644 index 0000000..8805078 --- /dev/null +++ b/libvirt-virNodeDeviceCapVPDFormat-Properly-escape-system-originated-strings.patch @@ -0,0 +1,94 @@ +From dd11b0a672feb5932548aa72c4db859889401587 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Tue, 30 Jan 2024 17:11:37 +0100 +Subject: [PATCH] virNodeDeviceCapVPDFormat: Properly escape system-originated + strings +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Similarly to previous commit other specific fields which come from the +system data and aren't sanitized enough to be safe for XML were also +formatted via virBufferAsprintf. + +Other static and safe strings used virBufferEscapeString instead of +virBufferAddLit. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 2ccac1e42f34404e3a5af22671a31fa1dca94e94) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +https://issues.redhat.com/browse/RHEL-22400 [9.3.z] +https://issues.redhat.com/browse/RHEL-22399 [9.2.z] +--- + src/conf/node_device_conf.c | 32 +++++++++++++------------------- + 1 file changed, 13 insertions(+), 19 deletions(-) + +diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c +index 87c046e571..95de77abe9 100644 +--- a/src/conf/node_device_conf.c ++++ b/src/conf/node_device_conf.c +@@ -270,14 +270,6 @@ virNodeDeviceCapVPDFormatCustomSystemField(virPCIVPDResourceCustom *field, virBu + virNodeDeviceCapVPDFormatCustomField(buf, "system_field", field); + } + +-static inline void +-virNodeDeviceCapVPDFormatRegularField(virBuffer *buf, const char *keyword, const char *value) +-{ +- if (keyword == NULL || value == NULL) +- return; +- +- virBufferAsprintf(buf, "<%s>%s\n", keyword, value, keyword); +-} + + static void + virNodeDeviceCapVPDFormat(virBuffer *buf, virPCIVPDResource *res) +@@ -290,31 +282,33 @@ virNodeDeviceCapVPDFormat(virBuffer *buf, virPCIVPDResource *res) + virBufferEscapeString(buf, "%s\n", res->name); + + if (res->ro != NULL) { +- virBufferEscapeString(buf, "\n", "readonly"); +- ++ virBufferAddLit(buf, "\n"); + virBufferAdjustIndent(buf, 2); +- virNodeDeviceCapVPDFormatRegularField(buf, "change_level", res->ro->change_level); +- virNodeDeviceCapVPDFormatRegularField(buf, "manufacture_id", res->ro->manufacture_id); +- virNodeDeviceCapVPDFormatRegularField(buf, "part_number", res->ro->part_number); +- virNodeDeviceCapVPDFormatRegularField(buf, "serial_number", res->ro->serial_number); ++ ++ virBufferEscapeString(buf, "%s\n", res->ro->change_level); ++ virBufferEscapeString(buf, "%s\n", res->ro->manufacture_id); ++ virBufferEscapeString(buf, "%s\n", res->ro->part_number); ++ virBufferEscapeString(buf, "%s\n", res->ro->serial_number); ++ + g_ptr_array_foreach(res->ro->vendor_specific, + (GFunc)virNodeDeviceCapVPDFormatCustomVendorField, buf); +- virBufferAdjustIndent(buf, -2); + ++ virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "\n"); + } + + if (res->rw != NULL) { +- virBufferEscapeString(buf, "\n", "readwrite"); +- ++ virBufferAddLit(buf, "\n"); + virBufferAdjustIndent(buf, 2); +- virNodeDeviceCapVPDFormatRegularField(buf, "asset_tag", res->rw->asset_tag); ++ ++ virBufferEscapeString(buf, "%s\n", res->rw->asset_tag); ++ + g_ptr_array_foreach(res->rw->vendor_specific, + (GFunc)virNodeDeviceCapVPDFormatCustomVendorField, buf); + g_ptr_array_foreach(res->rw->system_specific, + (GFunc)virNodeDeviceCapVPDFormatCustomSystemField, buf); +- virBufferAdjustIndent(buf, -2); + ++ virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "\n"); + } + +-- +2.43.0 diff --git a/libvirt-virNodeDeviceCapVPDFormatCustom-Escape-unsanitized-strings.patch b/libvirt-virNodeDeviceCapVPDFormatCustom-Escape-unsanitized-strings.patch new file mode 100644 index 0000000..7c33cb0 --- /dev/null +++ b/libvirt-virNodeDeviceCapVPDFormatCustom-Escape-unsanitized-strings.patch @@ -0,0 +1,80 @@ +From 32fe728dafc85c31b34f669b11264967bfc553dd Mon Sep 17 00:00:00 2001 +Message-ID: <32fe728dafc85c31b34f669b11264967bfc553dd.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 15:15:03 +0100 +Subject: [PATCH] virNodeDeviceCapVPDFormatCustom*: Escape unsanitized strings +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The custom field data is taken from PCI device data which can contain +any printable characters, and thus must be escaped when putting into +XML. + +Originally, based on the comment and XML schema which was fixed in +previous commits the idea seemed to be that the parser would validate +that only characters which don't break the XML would be present but that +didn't seem to materialize. + +Switch to proper escaping of the XML. + +Fixes: 3954378d06a +Resolves: https://issues.redhat.com/browse/RHEL-22314 +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 5373b8c02ce44d0284bc9c60b3b7bc12bff2f867) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +https://issues.redhat.com/browse/RHEL-22400 [9.3.z] +https://issues.redhat.com/browse/RHEL-22399 [9.2.z] +--- + src/conf/node_device_conf.c | 25 +++++++++++++++++-------- + 1 file changed, 17 insertions(+), 8 deletions(-) + +diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c +index 4826be6f42..87c046e571 100644 +--- a/src/conf/node_device_conf.c ++++ b/src/conf/node_device_conf.c +@@ -242,23 +242,32 @@ virNodeDeviceCapMdevTypesFormat(virBuffer *buf, + } + + static void +-virNodeDeviceCapVPDFormatCustomVendorField(virPCIVPDResourceCustom *field, virBuffer *buf) ++virNodeDeviceCapVPDFormatCustomField(virBuffer *buf, ++ const char *fieldtype, ++ virPCIVPDResourceCustom *field) + { ++ g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; ++ g_auto(virBuffer) content = VIR_BUFFER_INITIALIZER; ++ + if (field == NULL || field->value == NULL) + return; + +- virBufferAsprintf(buf, "%s\n", field->idx, +- field->value); ++ virBufferAsprintf(&attrBuf, " index='%c'", field->idx); ++ virBufferEscapeString(&content, "%s", field->value); ++ ++ virXMLFormatElementInternal(buf, fieldtype, &attrBuf, &content, false, false); + } + + static void +-virNodeDeviceCapVPDFormatCustomSystemField(virPCIVPDResourceCustom *field, virBuffer *buf) ++virNodeDeviceCapVPDFormatCustomVendorField(virPCIVPDResourceCustom *field, virBuffer *buf) + { +- if (field == NULL || field->value == NULL) +- return; ++ virNodeDeviceCapVPDFormatCustomField(buf, "vendor_field", field); ++} + +- virBufferAsprintf(buf, "%s\n", field->idx, +- field->value); ++static void ++virNodeDeviceCapVPDFormatCustomSystemField(virPCIVPDResourceCustom *field, virBuffer *buf) ++{ ++ virNodeDeviceCapVPDFormatCustomField(buf, "system_field", field); + } + + static inline void +-- +2.43.0 diff --git a/libvirt-virNodeDeviceCapVPDParseXML-Fix-error-reporting.patch b/libvirt-virNodeDeviceCapVPDParseXML-Fix-error-reporting.patch new file mode 100644 index 0000000..ddce9fe --- /dev/null +++ b/libvirt-virNodeDeviceCapVPDParseXML-Fix-error-reporting.patch @@ -0,0 +1,74 @@ +From 9de66ba6bd07c0c5d674be4dadec1f83a39d1533 Mon Sep 17 00:00:00 2001 +Message-ID: <9de66ba6bd07c0c5d674be4dadec1f83a39d1533.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Tue, 30 Jan 2024 17:41:44 +0100 +Subject: [PATCH] virNodeDeviceCapVPDParseXML: Fix error reporting +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Don't overwrite already reported errors and improve parsing of +attributes. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit dd36db2607e40d0df986108224563295b79d969c) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/conf/node_device_conf.c | 22 +++++++--------------- + 1 file changed, 7 insertions(+), 15 deletions(-) + +diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c +index 0f2c341967..c68ac3af78 100644 +--- a/src/conf/node_device_conf.c ++++ b/src/conf/node_device_conf.c +@@ -1059,11 +1059,11 @@ virNodeDeviceCapVPDParseXML(xmlXPathContextPtr ctxt, virPCIVPDResource **res) + + if (!(newres->name = virXPathString("string(./name)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", +- _("Could not read a device name from the element")); ++ _("Could not read a device name from the element")); + return -1; + } + +- if ((nfields = virXPathNodeSet("./fields[@access]", ctxt, &nodes)) < 0) ++ if ((nfields = virXPathNodeSet("./fields", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < nfields; i++) { +@@ -1071,27 +1071,19 @@ virNodeDeviceCapVPDParseXML(xmlXPathContextPtr ctxt, virPCIVPDResource **res) + VIR_XPATH_NODE_AUTORESTORE(ctxt); + + ctxt->node = nodes[i]; +- if (!(access = virXPathString("string(./@access[1])", ctxt))) { +- virReportError(VIR_ERR_XML_ERROR, "%s", +- _("VPD fields access type parsing has failed")); ++ ++ if (!(access = virXMLPropStringRequired(nodes[i], "access"))) + return -1; +- } + + if (STREQ(access, "readonly")) { +- if (virNodeDeviceCapVPDParseReadOnlyFields(ctxt, newres) < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("Could not parse %1$s VPD resource fields"), access); ++ if (virNodeDeviceCapVPDParseReadOnlyFields(ctxt, newres) < 0) + return -1; +- } + } else if (STREQ(access, "readwrite")) { +- if (virNodeDeviceCapVPDParseReadWriteFields(ctxt, newres) < 0) { +- virReportError(VIR_ERR_XML_ERROR, +- _("Could not parse %1$s VPD resource fields"), access); ++ if (virNodeDeviceCapVPDParseReadWriteFields(ctxt, newres) < 0) + return -1; +- } + } else { + virReportError(VIR_ERR_XML_ERROR, +- _("Unsupported VPD field access type specified %1$s"), ++ _("Unsupported VPD field access type '%1$s'"), + access); + return -1; + } +-- +2.43.0 diff --git a/libvirt-virPCIDeviceGetVPD-Fix-multiple-error-handling-bugs.patch b/libvirt-virPCIDeviceGetVPD-Fix-multiple-error-handling-bugs.patch new file mode 100644 index 0000000..d8fff76 --- /dev/null +++ b/libvirt-virPCIDeviceGetVPD-Fix-multiple-error-handling-bugs.patch @@ -0,0 +1,69 @@ +From b025fc09fc5eff84bacea27f99837a013f810d60 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Mon, 29 Jan 2024 16:59:20 +0100 +Subject: [PATCH] virPCIDeviceGetVPD: Fix multiple error handling bugs +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +- fix passing of 'errno' to 'virReportSystemError' + + The 'open' syscall returns '-1' and sets 'errno' on failure. The code + passed '-fd' as 'errno' rather than errno itself, thus always reporting + EPERM. + +- don't overwrite errors when closing FD + + Use VIR_AUTOCLOSE to avoid overwriting the errors from virPCIVPDParse. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit bac86dd36e2c8c56e3d5678a94fc69f8e41a7d35) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpci.c | 19 ++++++------------- + 1 file changed, 6 insertions(+), 13 deletions(-) + +diff --git a/src/util/virpci.c b/src/util/virpci.c +index 99e6e6cbb1..780b4f9eec 100644 +--- a/src/util/virpci.c ++++ b/src/util/virpci.c +@@ -3103,28 +3103,21 @@ virPCIDeviceHasVPD(virPCIDevice *dev) + virPCIVPDResource * + virPCIDeviceGetVPD(virPCIDevice *dev) + { +- g_autofree char *vpdPath = NULL; +- int fd; +- g_autoptr(virPCIVPDResource) res = NULL; ++ g_autofree char *vpdPath = virPCIFile(dev->name, "vpd"); ++ VIR_AUTOCLOSE fd = -1; + +- vpdPath = virPCIFile(dev->name, "vpd"); + if (!virPCIDeviceHasVPD(dev)) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Device %1$s does not have a VPD"), +- virPCIDeviceGetName(dev)); +- return NULL; +- } +- if ((fd = open(vpdPath, O_RDONLY)) < 0) { +- virReportSystemError(-fd, _("Failed to open a VPD file '%1$s'"), vpdPath); ++ virPCIDeviceGetName(dev)); + return NULL; + } +- res = virPCIVPDParse(fd); + +- if (VIR_CLOSE(fd) < 0) { +- virReportSystemError(errno, _("Unable to close the VPD file, fd: %1$d"), fd); ++ if ((fd = open(vpdPath, O_RDONLY)) < 0) { ++ virReportSystemError(errno, _("Failed to open a VPD file '%1$s'"), vpdPath); + return NULL; + } + +- return g_steal_pointer(&res); ++ return virPCIVPDParse(fd); + } + + #else +-- +2.43.0 diff --git a/libvirt-virPCIDeviceGetVPD-Handle-errors-in-callers.patch b/libvirt-virPCIDeviceGetVPD-Handle-errors-in-callers.patch new file mode 100644 index 0000000..7d6b433 --- /dev/null +++ b/libvirt-virPCIDeviceGetVPD-Handle-errors-in-callers.patch @@ -0,0 +1,52 @@ +From 1af80ce930b188e4410f66bdfee71ca91e38d5a3 Mon Sep 17 00:00:00 2001 +Message-ID: <1af80ce930b188e4410f66bdfee71ca91e38d5a3.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 22:32:33 +0100 +Subject: [PATCH] virPCIDeviceGetVPD: Handle errors in callers +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Until now 'virPCIDeviceGetVPD' couldn't reallistically raise an error, +but that will change. Handle the errors by either resetting it if we'd +be ignoring it or forward it. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit e1dc851e7cbc4a525b095b0dd4fdc779a882b19c) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/conf/node_device_conf.c | 2 ++ + tests/virpcitest.c | 3 ++- + 2 files changed, 4 insertions(+), 1 deletion(-) + +diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c +index c68ac3af78..b8c91d6ecd 100644 +--- a/src/conf/node_device_conf.c ++++ b/src/conf/node_device_conf.c +@@ -3052,6 +3052,8 @@ virNodeDeviceGetPCIVPDDynamicCap(virNodeDevCapPCIDev *devCapPCIDev) + if ((res = virPCIDeviceGetVPD(pciDev))) { + devCapPCIDev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VPD; + devCapPCIDev->vpd = g_steal_pointer(&res); ++ } else { ++ virResetLastError(); + } + } + return 0; +diff --git a/tests/virpcitest.c b/tests/virpcitest.c +index d69a1b5118..017c283a44 100644 +--- a/tests/virpcitest.c ++++ b/tests/virpcitest.c +@@ -344,7 +344,8 @@ testVirPCIDeviceGetVPD(const void *opaque) + if (!dev) + return -1; + +- res = virPCIDeviceGetVPD(dev); ++ if (!(res = virPCIDeviceGetVPD(dev))) ++ return -1; + + /* Only basic checks - full parser validation is done elsewhere. */ + if (res->ro == NULL) +-- +2.43.0 diff --git a/libvirt-virPCIDeviceHasVPD-Refactor-debug-messages.patch b/libvirt-virPCIDeviceHasVPD-Refactor-debug-messages.patch new file mode 100644 index 0000000..c42e23a --- /dev/null +++ b/libvirt-virPCIDeviceHasVPD-Refactor-debug-messages.patch @@ -0,0 +1,52 @@ +From ca12c899e642a82525e038df818ade15142de02f Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Mon, 29 Jan 2024 16:53:27 +0100 +Subject: [PATCH] virPCIDeviceHasVPD: Refactor "debug" messages +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +A checker function should not raise VIR_INFO or VIR_WARN messages +especially if they contain information useful only for debugging. + +Turn the message into a VIR_DEBUG with universal meaning. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 3ca1079318b8047a32ae05e1c6e5fb2dac9fc719) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpci.c | 15 +++++---------- + 1 file changed, 5 insertions(+), 10 deletions(-) + +diff --git a/src/util/virpci.c b/src/util/virpci.c +index 6c04e57038..99e6e6cbb1 100644 +--- a/src/util/virpci.c ++++ b/src/util/virpci.c +@@ -3081,17 +3081,12 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, + bool + virPCIDeviceHasVPD(virPCIDevice *dev) + { +- g_autofree char *vpdPath = NULL; ++ g_autofree char *vpdPath = virPCIFile(dev->name, "vpd"); ++ bool ret = virFileIsRegular(vpdPath); + +- vpdPath = virPCIFile(dev->name, "vpd"); +- if (!virFileExists(vpdPath)) { +- VIR_INFO("Device VPD file does not exist %s", vpdPath); +- return false; +- } else if (!virFileIsRegular(vpdPath)) { +- VIR_WARN("VPD path does not point to a regular file %s", vpdPath); +- return false; +- } +- return true; ++ VIR_DEBUG("path='%s', exists='%d'", vpdPath, ret); ++ ++ return ret; + } + + /** +-- +2.43.0 diff --git a/libvirt-virPCIVPDParse-Do-reasonable-error-reporting.patch b/libvirt-virPCIVPDParse-Do-reasonable-error-reporting.patch new file mode 100644 index 0000000..cdc5197 --- /dev/null +++ b/libvirt-virPCIVPDParse-Do-reasonable-error-reporting.patch @@ -0,0 +1,101 @@ +From b94d438cdd0c7a91885c204fdddece35b27bcccb Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Mon, 29 Jan 2024 17:12:43 +0100 +Subject: [PATCH] virPCIVPDParse: Do reasonable error reporting +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Remove the wannabe error reporting via 'VIR_DEBUG/VIR_INFO' in favor of +proper errors. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit f85a382a0e709afea3a4021de593b1679553a35a) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 36 ++++++++++++++++++------------------ + 1 file changed, 18 insertions(+), 18 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 4a440c2aea..16df468875 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -616,7 +616,7 @@ virPCIVPDParse(int vpdFileFd) + + uint16_t resPos = 0, resDataLen; + uint8_t tag = 0; +- bool endResReached = false, hasReadOnly = false; ++ bool hasReadOnly = false; + + g_autoptr(virPCIVPDResource) res = g_new0(virPCIVPDResource, 1); + +@@ -628,9 +628,8 @@ virPCIVPDParse(int vpdFileFd) + /* 0x80 == 0b10000000 - the large resource data type flag. */ + if (tag & PCI_VPD_LARGE_RESOURCE_FLAG) { + if (resPos > PCI_VPD_ADDR_MASK + 1 - 3) { +- /* Bail if the large resource starts at the position +- * where the end tag should be. */ +- break; ++ /* Bail if the large resource starts at the position where the end tag should be. */ ++ goto malformed; + } + + /* Read the two length bytes of the large resource record. */ +@@ -649,14 +648,21 @@ virPCIVPDParse(int vpdFileFd) + /* Change the position to the byte past the byte containing tag and length bits. */ + resPos += 1; + } ++ + if (tag == PCI_VPD_RESOURCE_END_TAG) { + /* Stop VPD traversal since the end tag was encountered. */ +- endResReached = true; +- break; ++ if (!hasReadOnly) { ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: missing read-only section")); ++ return NULL; ++ } ++ ++ return g_steal_pointer(&res); + } ++ + if (resDataLen > PCI_VPD_ADDR_MASK + 1 - resPos) { + /* Bail if the resource is too long to fit into the VPD address space. */ +- break; ++ goto malformed; + } + + switch (tag) { +@@ -686,22 +692,16 @@ virPCIVPDParse(int vpdFileFd) + /* While we cannot parse unknown resource types, they can still be skipped + * based on the header and data length. */ + VIR_DEBUG("Encountered an unexpected VPD resource tag: %#x", tag); +- resPos += resDataLen; +- continue; + } + + /* Continue processing other resource records. */ + resPos += resDataLen; + } +- if (!hasReadOnly) { +- VIR_DEBUG("Encountered an invalid VPD: does not have a VPD-R record"); +- return NULL; +- } else if (!endResReached) { +- /* Does not have an end tag. */ +- VIR_DEBUG("Encountered an invalid VPD"); +- return NULL; +- } +- return g_steal_pointer(&res); ++ ++ malformed: ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: malformed data")); ++ return NULL; + } + + #else /* ! __linux__ */ +-- +2.43.0 diff --git a/libvirt-virPCIVPDParseVPDLargeResourceFields-Merge-logic-conditions.patch b/libvirt-virPCIVPDParseVPDLargeResourceFields-Merge-logic-conditions.patch new file mode 100644 index 0000000..015241c --- /dev/null +++ b/libvirt-virPCIVPDParseVPDLargeResourceFields-Merge-logic-conditions.patch @@ -0,0 +1,65 @@ +From 9f0e2b76c89b692aa935c76f9d21012c50e4575d Mon Sep 17 00:00:00 2001 +Message-ID: <9f0e2b76c89b692aa935c76f9d21012c50e4575d.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Tue, 30 Jan 2024 15:02:39 +0100 +Subject: [PATCH] virPCIVPDParseVPDLargeResourceFields: Merge logic conditions +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Merge the pre-checks with the 'switch' statement which is operating on +the same values to simplify further refactoring. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 037803a949cfe60c03824482f889cc315bb7b788) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 24 ++++++++++++++---------- + 1 file changed, 14 insertions(+), 10 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index ddd79fa8bc..ba05014e40 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -438,23 +438,27 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + fieldKeyword = g_strndup((char *)buf, 2); + fieldFormat = virPCIVPDResourceGetFieldValueFormat(fieldKeyword); + +- /* Handle special cases first */ +- if (!readOnly && fieldFormat == VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD) { +- VIR_INFO("Unexpected RV keyword in the read-write section."); +- return false; +- } else if (readOnly && fieldFormat == VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR) { +- VIR_INFO("Unexpected RW keyword in the read-only section."); +- return false; +- } +- + /* Determine how many bytes to read per field value type. */ + switch (fieldFormat) { + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_TEXT: +- case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR: + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_BINARY: + bytesToRead = fieldDataLen; + break; ++ ++ case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR: ++ if (readOnly) { ++ VIR_INFO("Unexpected RW keyword in the read-only section."); ++ return false; ++ } ++ ++ bytesToRead = fieldDataLen; ++ break; ++ + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD: ++ if (!readOnly) { ++ VIR_INFO("Unexpected RV keyword in the read-write section."); ++ return false; ++ } + /* Only need one byte to be read and accounted towards + * the checksum calculation. */ + bytesToRead = 1; +-- +2.43.0 diff --git a/libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-processing-of-read-data.patch b/libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-processing-of-read-data.patch new file mode 100644 index 0000000..7aa3329 --- /dev/null +++ b/libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-processing-of-read-data.patch @@ -0,0 +1,108 @@ +From 3c987a4fb4a4e96bd4a9af04e43862d96169767e Mon Sep 17 00:00:00 2001 +Message-ID: <3c987a4fb4a4e96bd4a9af04e43862d96169767e.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Tue, 30 Jan 2024 15:14:49 +0100 +Subject: [PATCH] virPCIVPDParseVPDLargeResourceFields: Refactor processing of + read data +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Use a 'switch' statement instead of a bunch of if/elseif statements. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 378b82dac2f7bb7f20e32c4f0f8db49ff5f36851) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 66 +++++++++++++++++++++++++------------------- + 1 file changed, 37 insertions(+), 29 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 25c4c2c5ec..60e520c46f 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -485,36 +485,43 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + /* Advance the position to the first byte of the next field. */ + fieldPos += fieldDataLen; + +- if (fieldFormat == VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_TEXT) { +- /* Trim whitespace around a retrieved value and set it to be a field's value. Cases +- * where unnecessary whitespace was present around a field value have been encountered +- * in the wild. +- */ +- fieldValue = g_strstrip(g_strndup((char *)buf, fieldDataLen)); +- if (!virPCIVPDResourceIsValidTextValue(fieldValue)) { +- /* Skip fields with invalid values - this is safe assuming field length is +- * correctly specified. */ +- VIR_DEBUG("A value for field %s contains invalid characters", fieldKeyword); ++ switch (fieldFormat) { ++ case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_TEXT: ++ /* Trim whitespace around a retrieved value and set it to be a field's value. Cases ++ * where unnecessary whitespace was present around a field value have been encountered ++ * in the wild. ++ */ ++ fieldValue = g_strstrip(g_strndup((char *)buf, fieldDataLen)); ++ if (!virPCIVPDResourceIsValidTextValue(fieldValue)) { ++ /* Skip fields with invalid values - this is safe assuming field length is ++ * correctly specified. */ ++ VIR_DEBUG("A value for field %s contains invalid characters", fieldKeyword); ++ continue; ++ } ++ break; ++ ++ case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_BINARY: ++ fieldValue = g_malloc(fieldDataLen); ++ memcpy(fieldValue, buf, fieldDataLen); ++ break; ++ ++ case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR: ++ /* Skip the read-write space since it is used for indication only. */ ++ hasRW = true; ++ goto done; ++ ++ case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD: ++ if (*csum) { ++ /* All bytes up to and including the checksum byte should add up to 0. */ ++ VIR_INFO("Checksum validation has failed"); ++ return false; ++ } ++ hasChecksum = true; ++ goto done; ++ ++ case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_LAST: ++ /* Skip unknown fields */ + continue; +- } +- } else if (fieldFormat == VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD) { +- if (*csum) { +- /* All bytes up to and including the checksum byte should add up to 0. */ +- VIR_INFO("Checksum validation has failed"); +- return false; +- } +- hasChecksum = true; +- break; +- } else if (fieldFormat == VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR) { +- /* Skip the read-write space since it is used for indication only. */ +- hasRW = true; +- break; +- } else if (fieldFormat == VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_LAST) { +- /* Skip unknown fields */ +- continue; +- } else { +- fieldValue = g_malloc(fieldDataLen); +- memcpy(fieldValue, buf, fieldDataLen); + } + + if (readOnly) { +@@ -528,6 +535,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + virPCIVPDResourceUpdateKeyword(res, readOnly, fieldKeyword, fieldValue); + } + ++ done: + /* May have exited the loop prematurely in case RV or RW were encountered and + * they were not the last fields in the section. */ + endReached = (fieldPos >= resPos + resDataLen); +-- +2.43.0 diff --git a/libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-return-logic.patch b/libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-return-logic.patch new file mode 100644 index 0000000..691b009 --- /dev/null +++ b/libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-return-logic.patch @@ -0,0 +1,105 @@ +From 72da25da974a1fa9d995b46afb76714d56f4cb9b Mon Sep 17 00:00:00 2001 +Message-ID: <72da25da974a1fa9d995b46afb76714d56f4cb9b.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Tue, 30 Jan 2024 16:45:39 +0100 +Subject: [PATCH] virPCIVPDParseVPDLargeResourceFields: Refactor return logic +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Rewrite the conditions after exiting the parser so that they are easier +to understand. This partially decreases the granularity of "error" +messages as they are not strictly necessary albeit for debugging. + +As it was already observed in this code the logic itself often does +something else than the comment claims, thus the code logic is +preserved. + +Changes: + - any case when not all data was processed is aggregated together and + gets a common "error" message + - absence of 'checksum' field is checked separately + - helper variables are removed as they are no longer needed + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit a352bcf1c64e4efaa31ff5b193ff8ff4ca9d1329) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 32 +++++++++++++------------------- + 1 file changed, 13 insertions(+), 19 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 60e520c46f..be19f7b747 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -415,10 +415,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + g_autofree uint8_t *buf = g_malloc0(PCI_VPD_MAX_FIELD_SIZE + 1); + uint16_t fieldDataLen = 0, bytesToRead = 0; + uint16_t fieldPos = resPos; +- + bool hasChecksum = false; +- bool hasRW = false; +- bool endReached = false; + + /* Note the equal sign - fields may have a zero length in which case they will + * just occupy 3 header bytes. In the in case of the RW field this may mean that +@@ -507,7 +504,9 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR: + /* Skip the read-write space since it is used for indication only. */ +- hasRW = true; ++ /* The lack of RW is allowed on purpose in the read-write section since some vendors ++ * violate the PCI/PCIe specs and do not include it, however, this does not prevent parsing ++ * of valid data. */ + goto done; + + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD: +@@ -531,6 +530,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + if (!res->rw) + res->rw = virPCIVPDResourceRWNew(); + } ++ + /* The field format, keyword and value are determined. Attempt to update the resource. */ + virPCIVPDResourceUpdateKeyword(res, readOnly, fieldKeyword, fieldValue); + } +@@ -538,27 +538,21 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + done: + /* May have exited the loop prematurely in case RV or RW were encountered and + * they were not the last fields in the section. */ +- endReached = (fieldPos >= resPos + resDataLen); +- if (readOnly && !(hasChecksum && endReached)) { +- VIR_DEBUG("VPD-R does not contain the mandatory RV field as the last field"); ++ if ((fieldPos < resPos + resDataLen)) { ++ /* unparsed data still present */ ++ VIR_DEBUG("PCI VPD data parsing failed"); ++ return false; ++ } ++ ++ if (readOnly && !hasChecksum) { ++ VIR_DEBUG("VPD-R does not contain the mandatory checksum"); + return false; +- } else if (!readOnly && !endReached) { +- /* The lack of RW is allowed on purpose in the read-write section since some vendors +- * violate the PCI/PCIe specs and do not include it, however, this does not prevent parsing +- * of valid data. If the RW is present, however, we make sure it is the last field in +- * the read-write section. */ +- if (hasRW) { +- VIR_DEBUG("VPD-W section parsing ended prematurely (RW is not the last field)."); +- return false; +- } else { +- VIR_DEBUG("VPD-W section parsing ended prematurely."); +- return false; +- } + } + + return true; + } + ++ + /** + * virPCIVPDParseVPDLargeResourceString: + * @vpdFileFd: A file descriptor associated with a file containing PCI device VPD. +-- +2.43.0 diff --git a/libvirt-virPCIVPDParseVPDLargeResourceFields-Remove-impossible-default-switch-case.patch b/libvirt-virPCIVPDParseVPDLargeResourceFields-Remove-impossible-default-switch-case.patch new file mode 100644 index 0000000..bde9bb0 --- /dev/null +++ b/libvirt-virPCIVPDParseVPDLargeResourceFields-Remove-impossible-default-switch-case.patch @@ -0,0 +1,38 @@ +From deaf496dc41a0108a77aca108d6365021e9c5ad0 Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Tue, 30 Jan 2024 15:05:20 +0100 +Subject: [PATCH] virPCIVPDParseVPDLargeResourceFields: Remove impossible + 'default' switch case +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The 'fieldFormat' variable is guaranteed to have only the proper enum +values by virPCIVPDResourceGetFieldValueFormat. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit f1deac9635352f39fdf26e5d6bc2051f787149c9) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 3 --- + 1 file changed, 3 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index ba05014e40..25c4c2c5ec 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -470,9 +470,6 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + VIR_DEBUG("Could not determine a field value format for keyword: %s", fieldKeyword); + bytesToRead = fieldDataLen; + break; +- default: +- VIR_INFO("Unexpected field value format encountered."); +- return false; + } + + if (resPos + resDataLen < fieldPos + fieldDataLen) { +-- +2.43.0 diff --git a/libvirt-virPCIVPDParseVPDLargeResourceFields-Report-proper-errors.patch b/libvirt-virPCIVPDParseVPDLargeResourceFields-Report-proper-errors.patch new file mode 100644 index 0000000..f55f22e --- /dev/null +++ b/libvirt-virPCIVPDParseVPDLargeResourceFields-Report-proper-errors.patch @@ -0,0 +1,189 @@ +From 22f89f5d9f59e0636cc10d9d86687cf369f58627 Mon Sep 17 00:00:00 2001 +Message-ID: <22f89f5d9f59e0636cc10d9d86687cf369f58627.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Tue, 30 Jan 2024 16:55:50 +0100 +Subject: [PATCH] virPCIVPDParseVPDLargeResourceFields: Report proper errors +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The code abused 'VIR_INFO' as an attempt at error reporting. Rework the +code to return the usual 0/-1 and raise proper errors. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit dfc85658bd00323d6d006ab78dc6e346cafa5ed5) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 67 +++++++++++++++++++++++--------------------- + 1 file changed, 35 insertions(+), 32 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index be19f7b747..4a440c2aea 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -404,12 +404,15 @@ virPCIVPDReadVPDBytes(int vpdFileFd, + * @csum: A pointer to a 1-byte checksum. + * @res: A pointer to virPCIVPDResource. + * +- * Returns: a pointer to a VPDResource which needs to be freed by the caller or +- * NULL if getting it failed for some reason. ++ * Returns 0 if the field was parsed sucessfully; -1 on error + */ +-static bool +-virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t resDataLen, +- bool readOnly, uint8_t *csum, virPCIVPDResource *res) ++static int ++virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, ++ uint16_t resPos, ++ uint16_t resDataLen, ++ bool readOnly, ++ uint8_t *csum, ++ virPCIVPDResource *res) + { + /* A buffer of up to one resource record field size (plus a zero byte) is needed. */ + g_autofree uint8_t *buf = g_malloc0(PCI_VPD_MAX_FIELD_SIZE + 1); +@@ -427,7 +430,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + + /* Keyword resources consist of keywords (2 ASCII bytes per the spec) and 1-byte length. */ + if (virPCIVPDReadVPDBytes(vpdFileFd, buf, 3, fieldPos, csum) < 0) +- return false; ++ return -1; + + fieldDataLen = buf[2]; + /* Change the position to the field's data portion skipping the keyword and length bytes. */ +@@ -444,8 +447,9 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RDWR: + if (readOnly) { +- VIR_INFO("Unexpected RW keyword in the read-only section."); +- return false; ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: unexpected RW keyword in read-only section")); ++ return -1; + } + + bytesToRead = fieldDataLen; +@@ -453,8 +457,9 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD: + if (!readOnly) { +- VIR_INFO("Unexpected RV keyword in the read-write section."); +- return false; ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: unexpected RV keyword in read-write section")); ++ return -1; + } + /* Only need one byte to be read and accounted towards + * the checksum calculation. */ +@@ -472,12 +477,13 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + if (resPos + resDataLen < fieldPos + fieldDataLen) { + /* 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. */ +- VIR_INFO("A field data length violates the resource length boundary."); +- return false; ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: data field length invalid")); ++ return -1; + } + + if (virPCIVPDReadVPDBytes(vpdFileFd, buf, bytesToRead, fieldPos, csum) < 0) +- return false; ++ return -1; + + /* Advance the position to the first byte of the next field. */ + fieldPos += fieldDataLen; +@@ -492,7 +498,6 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + if (!virPCIVPDResourceIsValidTextValue(fieldValue)) { + /* Skip fields with invalid values - this is safe assuming field length is + * correctly specified. */ +- VIR_DEBUG("A value for field %s contains invalid characters", fieldKeyword); + continue; + } + break; +@@ -512,8 +517,9 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + case VIR_PCI_VPD_RESOURCE_FIELD_VALUE_FORMAT_RESVD: + if (*csum) { + /* All bytes up to and including the checksum byte should add up to 0. */ +- VIR_INFO("Checksum validation has failed"); +- return false; ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: invalid checksum")); ++ return -1; + } + hasChecksum = true; + goto done; +@@ -540,16 +546,18 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + * they were not the last fields in the section. */ + if ((fieldPos < resPos + resDataLen)) { + /* unparsed data still present */ +- VIR_DEBUG("PCI VPD data parsing failed"); +- return false; ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: parsing ended prematurely")); ++ return -1; + } + + if (readOnly && !hasChecksum) { +- VIR_DEBUG("VPD-R does not contain the mandatory checksum"); +- return false; ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data: missing mandatory checksum")); ++ return -1; + } + +- return true; ++ return 0; + } + + +@@ -606,7 +614,6 @@ virPCIVPDParse(int vpdFileFd) + uint8_t csum = 0; + uint8_t headerBuf[2]; + +- bool isWellFormed = false; + uint16_t resPos = 0, resDataLen; + uint8_t tag = 0; + bool endResReached = false, hasReadOnly = false; +@@ -659,20 +666,21 @@ virPCIVPDParse(int vpdFileFd) + &csum, res) < 0) + return NULL; + +- isWellFormed = true; + break; + /* Large resource type which is also a VPD-R: 0x80 | 0x10 == 0x90 */ + case PCI_VPD_LARGE_RESOURCE_FLAG | PCI_VPD_READ_ONLY_LARGE_RESOURCE_FLAG: +- isWellFormed = virPCIVPDParseVPDLargeResourceFields(vpdFileFd, resPos, +- resDataLen, true, &csum, res); ++ if (virPCIVPDParseVPDLargeResourceFields(vpdFileFd, resPos, ++ resDataLen, true, &csum, res) < 0) ++ return NULL; + /* Encountered the VPD-R tag. The resource record parsing also validates + * the presence of the required checksum in the RV field. */ + hasReadOnly = true; + break; + /* Large resource type which is also a VPD-W: 0x80 | 0x11 == 0x91 */ + case PCI_VPD_LARGE_RESOURCE_FLAG | PCI_VPD_READ_WRITE_LARGE_RESOURCE_FLAG: +- isWellFormed = virPCIVPDParseVPDLargeResourceFields(vpdFileFd, resPos, resDataLen, +- false, &csum, res); ++ if (virPCIVPDParseVPDLargeResourceFields(vpdFileFd, resPos, resDataLen, ++ false, &csum, res) < 0) ++ return NULL; + break; + default: + /* While we cannot parse unknown resource types, they can still be skipped +@@ -682,11 +690,6 @@ virPCIVPDParse(int vpdFileFd) + continue; + } + +- if (!isWellFormed) { +- VIR_DEBUG("Encountered an invalid VPD"); +- return NULL; +- } +- + /* Continue processing other resource records. */ + resPos += resDataLen; + } +-- +2.43.0 diff --git a/libvirt-virPCIVPDParseVPDLargeResourceString-Properly-report-errors.patch b/libvirt-virPCIVPDParseVPDLargeResourceString-Properly-report-errors.patch new file mode 100644 index 0000000..968493b --- /dev/null +++ b/libvirt-virPCIVPDParseVPDLargeResourceString-Properly-report-errors.patch @@ -0,0 +1,75 @@ +From 135355109036f6c4e25df72de7d4fa3dc4ab40c1 Mon Sep 17 00:00:00 2001 +Message-ID: <135355109036f6c4e25df72de7d4fa3dc4ab40c1.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 23:42:22 +0100 +Subject: [PATCH] virPCIVPDParseVPDLargeResourceString: Properly report errors +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Replace VIR_INFO being used as form of error reporting with proper +virReportError and the usual return values. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit aa5e3cc44934d154714610104623f19f9f6d8bfe) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 21 ++++++++++++--------- + 1 file changed, 12 insertions(+), 9 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 10cabff0b9..ddd79fa8bc 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -557,10 +557,9 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + * @resDataLen: A length of the data portion of a resource. + * @csum: A pointer to a 1-byte checksum. + * +- * Returns: a pointer to a VPDResource which needs to be freed by the caller or +- * NULL if getting it failed for some reason. ++ * Returns: 0 on success -1 and an error on failure + */ +-static bool ++static int + virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos, + uint16_t resDataLen, uint8_t *csum, virPCIVPDResource *res) + { +@@ -570,15 +569,16 @@ virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos, + g_autofree char *buf = g_malloc0(resDataLen + 1); + + if (virPCIVPDReadVPDBytes(vpdFileFd, (uint8_t *)buf, resDataLen, resPos, csum) < 0) +- return false; ++ return -1; + + resValue = g_strdup(g_strstrip(buf)); + if (!virPCIVPDResourceIsValidTextValue(resValue)) { +- VIR_INFO("The string resource has invalid characters in its value"); +- return false; ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to parse PCI VPD string value with invalid characters")); ++ return -1; + } + res->name = g_steal_pointer(&resValue); +- return true; ++ return 0; + } + + /** +@@ -652,8 +652,11 @@ virPCIVPDParse(int vpdFileFd) + switch (tag) { + /* Large resource type which is also a string: 0x80 | 0x02 = 0x82 */ + case PCI_VPD_LARGE_RESOURCE_FLAG | PCI_VPD_STRING_RESOURCE_FLAG: +- isWellFormed = virPCIVPDParseVPDLargeResourceString(vpdFileFd, resPos, resDataLen, +- &csum, res); ++ if (virPCIVPDParseVPDLargeResourceString(vpdFileFd, resPos, resDataLen, ++ &csum, res) < 0) ++ return NULL; ++ ++ isWellFormed = true; + break; + /* Large resource type which is also a VPD-R: 0x80 | 0x10 == 0x90 */ + case PCI_VPD_LARGE_RESOURCE_FLAG | PCI_VPD_READ_ONLY_LARGE_RESOURCE_FLAG: +-- +2.43.0 diff --git a/libvirt-virPCIVPDReadVPDBytes-Refactor-error-handling.patch b/libvirt-virPCIVPDReadVPDBytes-Refactor-error-handling.patch new file mode 100644 index 0000000..4b9add2 --- /dev/null +++ b/libvirt-virPCIVPDReadVPDBytes-Refactor-error-handling.patch @@ -0,0 +1,156 @@ +From 93036f97d3b6da59fadc2aab57401e42e9fc148f Mon Sep 17 00:00:00 2001 +Message-ID: <93036f97d3b6da59fadc2aab57401e42e9fc148f.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Mon, 29 Jan 2024 23:33:07 +0100 +Subject: [PATCH] virPCIVPDReadVPDBytes: Refactor error handling +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Each caller was checking that the function read as many bytes as it +expected. Move the check inside virPCIVPDReadVPDBytes and make it report +a proper error rather than just a combination of VIR_DEBUG inside the +function and a random VIR_INFO in the caller. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit c15a495902bab43454341361174c8ba3dadfcdd5) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 73 +++++++++++++++++++++++--------------------- + 1 file changed, 38 insertions(+), 35 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 0021a88f2d..10cabff0b9 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -361,34 +361,40 @@ virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, + * @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. + * +- * Returns: the number of VPD bytes read from the specified file descriptor. The csum value is ++ * Returns 0 if exactly @count bytes were read from @vpdFileFd. The csum value is + * also modified as bytes are read. If an error occurs while reading data from the VPD file +- * descriptor, it is reported and -1 is returned to the caller. If EOF is occurred, 0 is returned +- * to the caller. ++ * descriptor, it is reported and -1 is returned to the caller. + */ +-static size_t +-virPCIVPDReadVPDBytes(int vpdFileFd, uint8_t *buf, size_t count, off_t offset, uint8_t *csum) ++static int ++virPCIVPDReadVPDBytes(int vpdFileFd, ++ uint8_t *buf, ++ size_t count, ++ off_t offset, ++ uint8_t *csum) + { + ssize_t numRead = pread(vpdFileFd, buf, count, offset); + +- if (numRead == -1) { +- VIR_DEBUG("Unable to read %zu bytes at offset %zd from fd: %d", +- count, (ssize_t)offset, vpdFileFd); +- } else if (numRead) { +- /* +- * Update the checksum for every byte read. Per the PCI(e) specs +- * the checksum is correct if the sum of all bytes in VPD from +- * VPD address 0 up to and including the VPD-R RV field's first +- * data byte is zero. +- */ +- while (count--) { +- *csum += *buf; +- buf++; +- } ++ if (numRead != count) { ++ virReportError(VIR_ERR_OPERATION_FAILED, "%s", ++ _("failed to read the PCI VPD data")); ++ return -1; ++ } ++ ++ /* ++ * Update the checksum for every byte read. Per the PCI(e) specs ++ * the checksum is correct if the sum of all bytes in VPD from ++ * VPD address 0 up to and including the VPD-R RV field's first ++ * data byte is zero. ++ */ ++ while (count--) { ++ *csum += *buf; ++ buf++; + } +- return numRead; ++ ++ return 0; + } + ++ + /** + * virPCIVPDParseVPDLargeResourceFields: + * @vpdFileFd: A file descriptor associated with a file containing PCI device VPD. +@@ -423,12 +429,9 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + g_autofree char *fieldValue = NULL; + + /* Keyword resources consist of keywords (2 ASCII bytes per the spec) and 1-byte length. */ +- if (virPCIVPDReadVPDBytes(vpdFileFd, buf, 3, fieldPos, csum) != 3) { +- /* Invalid field encountered which means the resource itself is invalid too. Report +- * That VPD has invalid format and bail. */ +- VIR_INFO("Could not read a resource field header - VPD has invalid format"); ++ if (virPCIVPDReadVPDBytes(vpdFileFd, buf, 3, fieldPos, csum) < 0) + return false; +- } ++ + fieldDataLen = buf[2]; + /* Change the position to the field's data portion skipping the keyword and length bytes. */ + fieldPos += 3; +@@ -474,10 +477,10 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, uint16_t resPos, uint16_t re + VIR_INFO("A field data length violates the resource length boundary."); + return false; + } +- if (virPCIVPDReadVPDBytes(vpdFileFd, buf, bytesToRead, fieldPos, csum) != bytesToRead) { +- VIR_INFO("Could not parse a resource field data - VPD has invalid format"); ++ ++ if (virPCIVPDReadVPDBytes(vpdFileFd, buf, bytesToRead, fieldPos, csum) < 0) + return false; +- } ++ + /* Advance the position to the first byte of the next field. */ + fieldPos += fieldDataLen; + +@@ -566,10 +569,9 @@ virPCIVPDParseVPDLargeResourceString(int vpdFileFd, uint16_t resPos, + /* The resource value is not NULL-terminated so add one more byte. */ + g_autofree char *buf = g_malloc0(resDataLen + 1); + +- if (virPCIVPDReadVPDBytes(vpdFileFd, (uint8_t *)buf, resDataLen, resPos, csum) != resDataLen) { +- VIR_INFO("Could not read a part of a resource - VPD has invalid format"); ++ if (virPCIVPDReadVPDBytes(vpdFileFd, (uint8_t *)buf, resDataLen, resPos, csum) < 0) + return false; +- } ++ + resValue = g_strdup(g_strstrip(buf)); + if (!virPCIVPDResourceIsValidTextValue(resValue)) { + VIR_INFO("The string resource has invalid characters in its value"); +@@ -610,8 +612,8 @@ virPCIVPDParse(int vpdFileFd) + + while (resPos <= PCI_VPD_ADDR_MASK) { + /* Read the resource data type tag. */ +- if (virPCIVPDReadVPDBytes(vpdFileFd, &tag, 1, resPos, &csum) != 1) +- break; ++ if (virPCIVPDReadVPDBytes(vpdFileFd, &tag, 1, resPos, &csum) < 0) ++ return NULL; + + /* 0x80 == 0b10000000 - the large resource data type flag. */ + if (tag & PCI_VPD_LARGE_RESOURCE_FLAG) { +@@ -620,9 +622,10 @@ virPCIVPDParse(int vpdFileFd) + * where the end tag should be. */ + break; + } ++ + /* Read the two length bytes of the large resource record. */ +- if (virPCIVPDReadVPDBytes(vpdFileFd, headerBuf, 2, resPos + 1, &csum) != 2) +- break; ++ if (virPCIVPDReadVPDBytes(vpdFileFd, headerBuf, 2, resPos + 1, &csum) < 0) ++ return NULL; + + resDataLen = headerBuf[0] + (headerBuf[1] << 8); + /* Change the position to the byte following the tag and length bytes. */ +-- +2.43.0 diff --git a/libvirt-virPCIVPDResourceGetKeywordPrefix-Fix-logging.patch b/libvirt-virPCIVPDResourceGetKeywordPrefix-Fix-logging.patch new file mode 100644 index 0000000..da56864 --- /dev/null +++ b/libvirt-virPCIVPDResourceGetKeywordPrefix-Fix-logging.patch @@ -0,0 +1,56 @@ +From c820b898998aca63d597567724011182c4fa50cd Mon Sep 17 00:00:00 2001 +Message-ID: +From: Peter Krempa +Date: Wed, 24 Jan 2024 15:13:16 +0100 +Subject: [PATCH] virPCIVPDResourceGetKeywordPrefix: Fix logging +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Use VIR_DEBUG instead of VIR_INFO as that's more appropriate and report +relevant information for debugging. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit ab3f4d1b0b9f29c924e928f8c6663b4076e49b38) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + src/util/virpcivpd.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index b303e161ae..67065dec46 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -61,20 +61,20 @@ virPCIVPDResourceGetKeywordPrefix(const char *keyword) + g_autofree char *key = NULL; + + /* Keywords must have a length of 2 bytes. */ +- if (strlen(keyword) != 2) { +- VIR_INFO("The keyword length is not 2 bytes: %s", keyword); +- return NULL; +- } else if (!(virPCIVPDResourceIsUpperOrNumber(keyword[0]) && +- virPCIVPDResourceIsUpperOrNumber(keyword[1]))) { +- VIR_INFO("The keyword is not comprised only of uppercase ASCII letters or digits"); +- return NULL; +- } ++ if (strlen(keyword) != 2 || ++ !(virPCIVPDResourceIsUpperOrNumber(keyword[0]) && ++ virPCIVPDResourceIsUpperOrNumber(keyword[1]))) ++ goto cleanup; ++ + /* Special-case the system-specific keywords since they share the "Y" prefix with "YA". */ + if (virPCIVPDResourceIsSystemKeyword(keyword) || virPCIVPDResourceIsVendorKeyword(keyword)) + key = g_strndup(keyword, 1); + else + key = g_strndup(keyword, 2); + ++ cleanup: ++ VIR_DEBUG("keyword='%s' key='%s'", keyword, NULLSTR(key)); ++ + return g_steal_pointer(&key); + } + +-- +2.43.0 diff --git a/libvirt-virPCIVPDResourceIsValidTextValue-Adjust-comment-to-reflect-actual-code.patch b/libvirt-virPCIVPDResourceIsValidTextValue-Adjust-comment-to-reflect-actual-code.patch new file mode 100644 index 0000000..b96ea1b --- /dev/null +++ b/libvirt-virPCIVPDResourceIsValidTextValue-Adjust-comment-to-reflect-actual-code.patch @@ -0,0 +1,54 @@ +From 4daeb72f2eb09db6c5ac1628c35139af4ab7653e Mon Sep 17 00:00:00 2001 +Message-ID: <4daeb72f2eb09db6c5ac1628c35139af4ab7653e.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 15:24:27 +0100 +Subject: [PATCH] virPCIVPDResourceIsValidTextValue: Adjust comment to reflect + actual code +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The function does not reject '&', '<', '>' contrary to what it actually +states. Move and adjust the comment. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit 42df6cc1b4acc40d05ff6bc8e85587e4faec6cac) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +https://issues.redhat.com/browse/RHEL-22400 [9.3.z] +https://issues.redhat.com/browse/RHEL-22399 [9.2.z] +--- + src/util/virpcivpd.c | 12 ++++-------- + 1 file changed, 4 insertions(+), 8 deletions(-) + +diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c +index 39557c7347..248a9b2790 100644 +--- a/src/util/virpcivpd.c ++++ b/src/util/virpcivpd.c +@@ -167,19 +167,15 @@ virPCIVPDResourceGetFieldValueFormat(const char *keyword) + * value or text field value. The expectations are based on the keywords specified + * in relevant sections of PCI(e) specifications + * ("I.3. VPD Definitions" in PCI specs, "6.28.1 VPD Format" PCIe 4.0). ++ * ++ * The PCI(e) specs mention alphanumeric characters when talking about text fields ++ * and the string resource but also include spaces and dashes in the provided example. ++ * Dots, commas, equal signs have also been observed in values used by major device vendors. + */ + bool + virPCIVPDResourceIsValidTextValue(const char *value) + { + size_t i = 0; +- /* +- * The PCI(e) specs mention alphanumeric characters when talking about text fields +- * and the string resource but also include spaces and dashes in the provided example. +- * Dots, commas, equal signs have also been observed in values used by major device vendors. +- * The specs do not specify a full set of allowed code points and for Libvirt it is important +- * to keep values in the ranges allowed within XML elements (mainly excluding less-than, +- * greater-than and ampersand). +- */ + + if (value == NULL) + return false; +-- +2.43.0 diff --git a/libvirt-virpcivpdtest-testPCIVPDResourceBasic-Remove-tests-for-uninitialized-ro-rw-section.patch b/libvirt-virpcivpdtest-testPCIVPDResourceBasic-Remove-tests-for-uninitialized-ro-rw-section.patch new file mode 100644 index 0000000..7f710f0 --- /dev/null +++ b/libvirt-virpcivpdtest-testPCIVPDResourceBasic-Remove-tests-for-uninitialized-ro-rw-section.patch @@ -0,0 +1,85 @@ +From 636dbbfd795583431523972a0057ede74d110a6e Mon Sep 17 00:00:00 2001 +Message-ID: <636dbbfd795583431523972a0057ede74d110a6e.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Wed, 24 Jan 2024 17:13:51 +0100 +Subject: [PATCH] virpcivpdtest: testPCIVPDResourceBasic: Remove tests for + uninitialized 'ro'/'rw' section +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This is a synthetic case which tests the behaviour if the 'ro' or 'rw' +struct members are uninitialized, basically excercising only a pointless +programming-error NULL check in 'virPCIVPDResourceUpdateKeyword' as real +usage does always pass a proper pointer. + +Signed-off-by: Peter Krempa +Reviewed-by: Ján Tomko +(cherry picked from commit e8f5edf556145cbe6bae53255ded3051d65f24f1) + +https://issues.redhat.com/browse/RHEL-22314 [9.4.0] +--- + tests/virpcivpdtest.c | 27 --------------------------- + 1 file changed, 27 deletions(-) + +diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c +index 8a2f337e85..20545759d5 100644 +--- a/tests/virpcivpdtest.c ++++ b/tests/virpcivpdtest.c +@@ -64,11 +64,6 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) + {.keyword = "SN", .value = "serial2", .actual = &ro->serial_number}, + {.keyword = "serial_number", .value = "serial3", .actual = &ro->serial_number}, + }; +- 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}, +- }; + const TestPCIVPDKeywordValue unsupportedFieldCases[] = { + {.keyword = "FG", .value = "42", .actual = NULL}, + {.keyword = "LC", .value = "42", .actual = NULL}, +@@ -77,7 +72,6 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) + {.keyword = "EX", .value = "42", .actual = NULL}, + }; + size_t numROCases = G_N_ELEMENTS(readOnlyCases); +- size_t numRWCases = G_N_ELEMENTS(readWriteCases); + size_t numUnsupportedCases = G_N_ELEMENTS(unsupportedFieldCases); + g_autoptr(virPCIVPDResource) res = g_new0(virPCIVPDResource, 1); + virPCIVPDResourceCustom *custom = NULL; +@@ -85,20 +79,6 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) + 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); + +@@ -131,13 +111,6 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) + 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") +-- +2.43.0 diff --git a/libvirt-virt-admin-Add-warning-when-connection-to-default-daemon-fails.patch b/libvirt-virt-admin-Add-warning-when-connection-to-default-daemon-fails.patch new file mode 100644 index 0000000..6faabc1 --- /dev/null +++ b/libvirt-virt-admin-Add-warning-when-connection-to-default-daemon-fails.patch @@ -0,0 +1,52 @@ +From 44eeb458ace773226f08d0807fda964014004301 Mon Sep 17 00:00:00 2001 +Message-ID: <44eeb458ace773226f08d0807fda964014004301.1707394627.git.jdenemar@redhat.com> +From: Peter Krempa +Date: Thu, 1 Feb 2024 10:40:41 +0100 +Subject: [PATCH] virt-admin: Add warning when connection to default daemon + fails + +The admin connection defaults to the system-wide 'libvirtd' daemon to +manage (libvirtd:///system). As we've now switched to modular daemons +this will not work for most users out of the box: + + $ virt-admin version + error: Failed to connect to the admin server + error: no valid connection + error: Failed to connect socket to '/run/user/1000/libvirt/libvirt-admin-sock': No such file or directory + +As we don't want to assume which daemon the user wants to manage in the +modular topology there's no reasonable default to pick. + +Give a hint to the users to use the '-c' if the connection to the +default URI fails: + + $ virt-admin version + NOTE: Connecting to default daemon. Specify daemon using '-c' (e.g. virtqemud:///system) + error: Failed to connect to the admin server + error: no valid connection + error: Failed to connect socket to '/run/user/1000/libvirt/libvirt-admin-sock': No such file or directory + +Signed-off-by: Peter Krempa +Reviewed-by: Jonathon Jongsma +(cherry picked from commit 442061583e9dc0e4e3bf314275979051345a4a93) +https://issues.redhat.com/browse/RHEL-23170 +--- + tools/virt-admin.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/tools/virt-admin.c b/tools/virt-admin.c +index 1e22a3c8a9..70b6e4916f 100644 +--- a/tools/virt-admin.c ++++ b/tools/virt-admin.c +@@ -108,6 +108,9 @@ vshAdmConnect(vshControl *ctl, unsigned int flags) + priv->conn = virAdmConnectOpen(ctl->connname, flags); + + if (!priv->conn) { ++ if (!ctl->connname) ++ vshPrintExtra(ctl, "%s", _("NOTE: Connecting to default daemon. Specify daemon using '-c' (e.g. virtqemud:///system)\n")); ++ + if (priv->wantReconnect) + vshError(ctl, "%s", _("Failed to reconnect to the admin server")); + else +-- +2.43.0 diff --git a/libvirt.spec b/libvirt.spec index c4d7827..4a2fc08 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -270,7 +270,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 10.0.0 -Release: 2%{?dist}%{?extra_release} +Release: 3%{?dist}%{?extra_release} License: GPL-2.0-or-later AND LGPL-2.1-only AND LGPL-2.1-or-later AND OFL-1.1 URL: https://libvirt.org/ @@ -290,6 +290,50 @@ Patch8: libvirt-tests-Verify-handling-of-CPU-clusters-in-QMP-data.patch Patch9: libvirt-build-Make-daemons-depend-on-generated-_protocol.-ch.patch Patch10: libvirt-domain_validate-Check-for-domain-address-conflicts-fully.patch Patch11: libvirt-qemu_hotplug-Don-t-lose-created-flag-in-qemuDomainChangeNet.patch +Patch12: libvirt-remote_driver-Restore-special-behavior-of-remoteDomainGetBlockIoTune.patch +Patch13: libvirt-conf-Introduce-dynamicMemslots-attribute-for-virtio-mem.patch +Patch14: libvirt-qemu_capabilities-Add-QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS-capability.patch +Patch15: libvirt-qemu_validate-Check-capability-for-virtio-mem-dynamicMemslots.patch +Patch16: libvirt-qemu_command-Generate-cmd-line-for-virtio-mem-dynamicMemslots.patch +Patch17: libvirt-qemu_snapshot-fix-detection-if-non-leaf-snapshot-isn-t-in-active-chain.patch +Patch18: libvirt-qemu_snapshot-create-refactor-external-snapshot-detection.patch +Patch19: libvirt-qemu_snapshot-create-don-t-require-disk-only-flag-for-offline-external-snapshot.patch +Patch20: libvirt-remoteDispatchAuthPolkit-Fix-lock-ordering-deadlock-if-client-closes-connection-during-auth.patch +Patch21: libvirt-util-virtportallocator-Add-VIR_DEBUG-statements-for-port-allocations-and-release.patch +Patch22: libvirt-qemu-migration-Properly-handle-reservation-of-manually-specified-NBD-port.patch +Patch23: libvirt-qemuMigrationDstStartNBDServer-Refactor-cleanup.patch +Patch24: libvirt-virPCIVPDResourceIsValidTextValue-Adjust-comment-to-reflect-actual-code.patch +Patch25: libvirt-util-pcivpd-Refactor-virPCIVPDResourceIsValidTextValue.patch +Patch26: libvirt-virNodeDeviceCapVPDFormatCustom-Escape-unsanitized-strings.patch +Patch27: libvirt-virNodeDeviceCapVPDFormat-Properly-escape-system-originated-strings.patch +Patch28: libvirt-schema-nodedev-Adjust-allowed-characters-in-vpdFieldValueFormat.patch +Patch29: libvirt-tests-Test-the-previously-mishandled-PCI-VPD-characters.patch +Patch30: libvirt-Don-t-overwrite-error-message-from-virXPathNodeSet.patch +Patch31: libvirt-tests-virpcivpdtest-Remove-testVirPCIVPDReadVPDBytes-case.patch +Patch32: libvirt-util-virpcivpd-Unexport-virPCIVPDReadVPDBytes.patch +Patch33: libvirt-util-pcivpd-Unexport-virPCIVPDParseVPDLargeResourceFields.patch +Patch34: libvirt-tests-virpcivpd-Remove-testVirPCIVPDParseVPDStringResource-case.patch +Patch35: libvirt-util-virpcivpd-Unexport-virPCIVPDParseVPDLargeResourceString.patch +Patch36: libvirt-virPCIVPDResourceGetKeywordPrefix-Fix-logging.patch +Patch37: libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceCustomUpsertValue.patch +Patch38: libvirt-conf-virNodeDeviceCapVPDParse-Remove-pointless-NULL-checks.patch +Patch39: libvirt-virpcivpdtest-testPCIVPDResourceBasic-Remove-tests-for-uninitialized-ro-rw-section.patch +Patch40: libvirt-util-virPCIVPDResourceUpdateKeyword-Remove-impossible-checks.patch +Patch41: libvirt-conf-node_device-Refactor-virNodeDeviceCapVPDParseCustomFields-to-fix-error-reporting.patch +Patch42: libvirt-virNodeDeviceCapVPDParseXML-Fix-error-reporting.patch +Patch43: libvirt-util-virpcivpd-Remove-return-value-from-virPCIVPDResourceUpdateKeyword.patch +Patch44: libvirt-virPCIDeviceHasVPD-Refactor-debug-messages.patch +Patch45: libvirt-virPCIDeviceGetVPD-Fix-multiple-error-handling-bugs.patch +Patch46: libvirt-virPCIDeviceGetVPD-Handle-errors-in-callers.patch +Patch47: libvirt-virPCIVPDReadVPDBytes-Refactor-error-handling.patch +Patch48: libvirt-virPCIVPDParseVPDLargeResourceString-Properly-report-errors.patch +Patch49: libvirt-virPCIVPDParseVPDLargeResourceFields-Merge-logic-conditions.patch +Patch50: libvirt-virPCIVPDParseVPDLargeResourceFields-Remove-impossible-default-switch-case.patch +Patch51: libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-processing-of-read-data.patch +Patch52: libvirt-virPCIVPDParseVPDLargeResourceFields-Refactor-return-logic.patch +Patch53: libvirt-virPCIVPDParseVPDLargeResourceFields-Report-proper-errors.patch +Patch54: libvirt-virPCIVPDParse-Do-reasonable-error-reporting.patch +Patch55: libvirt-virt-admin-Add-warning-when-connection-to-default-daemon-fails.patch Requires: libvirt-daemon = %{version}-%{release} @@ -2593,6 +2637,52 @@ exit 0 %endif %changelog +* Thu Feb 8 2024 Jiri Denemark - 10.0.0-3 +- remote_driver: Restore special behavior of remoteDomainGetBlockIoTune() (RHEL-22800) +- conf: Introduce dynamicMemslots attribute for virtio-mem (RHEL-15316) +- qemu_capabilities: Add QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_DYNAMIC_MEMSLOTS capability (RHEL-15316) +- qemu_validate: Check capability for virtio-mem dynamicMemslots (RHEL-15316) +- qemu_command: Generate cmd line for virtio-mem dynamicMemslots (RHEL-15316) +- qemu_snapshot: fix detection if non-leaf snapshot isn't in active chain (RHEL-23212) +- qemu_snapshot: create: refactor external snapshot detection (RHEL-22797) +- qemu_snapshot: create: don't require disk-only flag for offline external snapshot (RHEL-22797) +- remoteDispatchAuthPolkit: Fix lock ordering deadlock if client closes connection during auth (RHEL-20337) +- util: virtportallocator: Add VIR_DEBUG statements for port allocations and release (RHEL-21543) +- qemu: migration: Properly handle reservation of manually specified NBD port (RHEL-21543) +- qemuMigrationDstStartNBDServer: Refactor cleanup (RHEL-21543) +- virPCIVPDResourceIsValidTextValue: Adjust comment to reflect actual code (RHEL-22314) +- util: pcivpd: Refactor virPCIVPDResourceIsValidTextValue (RHEL-22314) +- virNodeDeviceCapVPDFormatCustom*: Escape unsanitized strings (RHEL-22314) +- virNodeDeviceCapVPDFormat: Properly escape system-originated strings (RHEL-22314) +- schema: nodedev: Adjust allowed characters in 'vpdFieldValueFormat' (RHEL-22314) +- tests: Test the previously mishandled PCI VPD characters (RHEL-22314) +- Don't overwrite error message from 'virXPathNodeSet' (RHEL-22314) +- tests: virpcivpdtest: Remove 'testVirPCIVPDReadVPDBytes' case (RHEL-22314) +- util: virpcivpd: Unexport 'virPCIVPDReadVPDBytes' (RHEL-22314) +- util: pcivpd: Unexport virPCIVPDParseVPDLargeResourceFields (RHEL-22314) +- tests: virpcivpd: Remove 'testVirPCIVPDParseVPDStringResource' case (RHEL-22314) +- util: virpcivpd: Unexport 'virPCIVPDParseVPDLargeResourceString' (RHEL-22314) +- virPCIVPDResourceGetKeywordPrefix: Fix logging (RHEL-22314) +- util: virpcivpd: Remove return value from virPCIVPDResourceCustomUpsertValue (RHEL-22314) +- conf: virNodeDeviceCapVPDParse*: Remove pointless NULL checks (RHEL-22314) +- virpcivpdtest: testPCIVPDResourceBasic: Remove tests for uninitialized 'ro'/'rw' section (RHEL-22314) +- util: virPCIVPDResourceUpdateKeyword: Remove impossible checks (RHEL-22314) +- conf: node_device: Refactor 'virNodeDeviceCapVPDParseCustomFields' to fix error reporting (RHEL-22314) +- virNodeDeviceCapVPDParseXML: Fix error reporting (RHEL-22314) +- util: virpcivpd: Remove return value from virPCIVPDResourceUpdateKeyword (RHEL-22314) +- virPCIDeviceHasVPD: Refactor "debug" messages (RHEL-22314) +- virPCIDeviceGetVPD: Fix multiple error handling bugs (RHEL-22314) +- virPCIDeviceGetVPD: Handle errors in callers (RHEL-22314) +- virPCIVPDReadVPDBytes: Refactor error handling (RHEL-22314) +- virPCIVPDParseVPDLargeResourceString: Properly report errors (RHEL-22314) +- virPCIVPDParseVPDLargeResourceFields: Merge logic conditions (RHEL-22314) +- virPCIVPDParseVPDLargeResourceFields: Remove impossible 'default' switch case (RHEL-22314) +- virPCIVPDParseVPDLargeResourceFields: Refactor processing of read data (RHEL-22314) +- virPCIVPDParseVPDLargeResourceFields: Refactor return logic (RHEL-22314) +- virPCIVPDParseVPDLargeResourceFields: Report proper errors (RHEL-22314) +- virPCIVPDParse: Do reasonable error reporting (RHEL-22314) +- virt-admin: Add warning when connection to default daemon fails (RHEL-23170) + * Mon Jan 29 2024 Jiri Denemark - 10.0.0-2 - tests: Add hostcpudata for machine with CPU clusters (RHEL-7043) - conf: Report CPU clusters in capabilities XML (RHEL-7043)