[PATCH 0/2] conf: Fix formatting of 'associativity' and 'policy' cache attributes
Peter Krempa (2): virDomainNumaDefNodeCacheParseXML: Refactor parsing of cache XML conf: numa: Allow formatting 'none' values for 'associativity' and 'policy' of cache src/conf/numa_conf.c | 73 +++++-------------- .../numatune-hmat-none.x86_64-latest.args | 49 +++++++++++++ tests/qemuxml2argvdata/numatune-hmat-none.xml | 54 ++++++++++++++ tests/qemuxml2argvtest.c | 1 + .../numatune-hmat-none.x86_64-latest.xml | 55 ++++++++++++++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 178 insertions(+), 55 deletions(-) create mode 100644 tests/qemuxml2argvdata/numatune-hmat-none.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/numatune-hmat-none.xml create mode 100644 tests/qemuxml2xmloutdata/numatune-hmat-none.x86_64-latest.xml -- 2.40.1
Use virXMLProp* helpers to simplify the code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 60 +++++++++++--------------------------------- 1 file changed, 14 insertions(+), 46 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index b21671f587..c2e3045280 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -815,68 +815,36 @@ virDomainNumaDefNodeCacheParseXML(virDomainNuma *def, for (i = 0; i < n; i++) { VIR_XPATH_NODE_AUTORESTORE(ctxt) virNumaCache *cache = &def->mem_nodes[cur_cell].caches[i]; - g_autofree char *tmp = NULL; - unsigned int level; - int associativity; - int policy; - unsigned long long size; unsigned long long line; - if (!(tmp = virXMLPropString(nodes[i], "level"))) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing 'level' attribute in cache element for NUMA node %1$d"), - cur_cell); - return -1; - } - - if (virStrToLong_uip(tmp, NULL, 10, &level) < 0 || - level == 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid 'level' attribute in cache element for NUMA node %1$d"), - cur_cell); - return -1; - } - VIR_FREE(tmp); - - if (!(tmp = virXMLPropString(nodes[i], "associativity"))) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing 'associativity' attribute in cache element for NUMA node %1$d"), - cur_cell); + if (virXMLPropUInt(nodes[i], "level", 10, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &cache->level) < 0) return -1; - } - if ((associativity = virNumaCacheAssociativityTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid cache associativity '%1$s'"), - tmp); + if (virXMLPropEnum(nodes[i], "associativity", + virNumaCacheAssociativityTypeFromString, + VIR_XML_PROP_REQUIRED, + &cache->associativity) < 0) return -1; - } - VIR_FREE(tmp); - - if (!(tmp = virXMLPropString(nodes[i], "policy"))) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing 'policy' attribute in cache element for NUMA node %1$d"), - cur_cell); - } - if ((policy = virNumaCachePolicyTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid cache policy '%1$s'"), - tmp); + if (virXMLPropEnum(nodes[i], "policy", + virNumaCachePolicyTypeFromString, + VIR_XML_PROP_REQUIRED, + &cache->policy) < 0) return -1; - } - VIR_FREE(tmp); ctxt->node = nodes[i]; if (virDomainParseMemory("./size/@value", "./size/unit", - ctxt, &size, true, false) < 0) + ctxt, &cache->size, true, false) < 0) return -1; if (virParseScaledValue("./line/@value", "./line/unit", ctxt, &line, 1, ULLONG_MAX, true) < 0) return -1; - *cache = (virNumaCache){level, size, line, associativity, policy}; + cache->line = line; + def->mem_nodes[cur_cell].ncaches++; } -- 2.40.1
The parser makes the values mandatory and also the qemu code implements actions for those values. The formatter skips them though. Since format+parse is used to copy the XML at startup a definition with those values can't be started. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2203709 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 13 ++--- .../numatune-hmat-none.x86_64-latest.args | 49 +++++++++++++++++ tests/qemuxml2argvdata/numatune-hmat-none.xml | 54 ++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + .../numatune-hmat-none.x86_64-latest.xml | 55 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/numatune-hmat-none.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/numatune-hmat-none.xml create mode 100644 tests/qemuxml2xmloutdata/numatune-hmat-none.x86_64-latest.xml diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index c2e3045280..be0c4572c5 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -1708,15 +1708,10 @@ virNumaCacheFormat(virBuffer *buf, g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); virBufferAsprintf(&attrBuf, " level='%u'", cache->level); - if (cache->associativity) { - virBufferAsprintf(&attrBuf, " associativity='%s'", - virNumaCacheAssociativityTypeToString(cache->associativity)); - } - - if (cache->policy) { - virBufferAsprintf(&attrBuf, " policy='%s'", - virNumaCachePolicyTypeToString(cache->policy)); - } + virBufferAsprintf(&attrBuf, " associativity='%s'", + virNumaCacheAssociativityTypeToString(cache->associativity)); + virBufferAsprintf(&attrBuf, " policy='%s'", + virNumaCachePolicyTypeToString(cache->policy)); virBufferAsprintf(&childBuf, "<size value='%llu' unit='KiB'/>\n", diff --git a/tests/qemuxml2argvdata/numatune-hmat-none.x86_64-latest.args b/tests/qemuxml2argvdata/numatune-hmat-none.x86_64-latest.args new file mode 100644 index 0000000000..d61fa1fc70 --- /dev/null +++ b/tests/qemuxml2argvdata/numatune-hmat-none.x86_64-latest.args @@ -0,0 +1,49 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,hmat=on,acpi=on \ +-accel tcg \ +-cpu qemu64 \ +-m 12288 \ +-overcommit mem-lock=off \ +-smp 12,sockets=12,cores=1,threads=1 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node0","size":2147483648}' \ +-numa node,nodeid=0,cpus=0-3,initiator=0,memdev=ram-node0 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node1","size":2147483648}' \ +-numa node,nodeid=1,cpus=4-7,initiator=1,memdev=ram-node1 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node2","size":2147483648}' \ +-numa node,nodeid=2,cpus=8-11,initiator=2,memdev=ram-node2 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node3","size":2147483648}' \ +-numa node,nodeid=3,initiator=0,memdev=ram-node3 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node4","size":2147483648}' \ +-numa node,nodeid=4,initiator=0,memdev=ram-node4 \ +-object '{"qom-type":"memory-backend-ram","id":"ram-node5","size":2147483648}' \ +-numa node,nodeid=5,initiator=0,memdev=ram-node5 \ +-numa hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-latency,latency=5 \ +-numa hmat-lb,initiator=0,target=0,hierarchy=first-level,data-type=access-latency,latency=10 \ +-numa hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=204800K \ +-numa hmat-lb,initiator=0,target=0,hierarchy=first-level,data-type=access-bandwidth,bandwidth=208896K \ +-numa hmat-cache,node-id=0,size=10K,level=1,associativity=none,policy=none,line=8 \ +-uuid c7a5fdb2-cdaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x3"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/numatune-hmat-none.xml b/tests/qemuxml2argvdata/numatune-hmat-none.xml new file mode 100644 index 0000000000..a848397826 --- /dev/null +++ b/tests/qemuxml2argvdata/numatune-hmat-none.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</currentMemory> + <vcpu placement='static'>12</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0-3' memory='2097152' unit='KiB'> + <cache level='1' associativity='none' policy='none'> + <size value='10' unit='KiB'/> + <line value='8' unit='B'/> + </cache> + </cell> + <cell id='1' cpus='4-7' memory='2097152' unit='KiB'/> + <cell id='2' cpus='8-11' memory='2097152' unit='KiB'/> + <cell id='3' memory='2097152' unit='KiB'/> + <cell id='4' memory='2097152' unit='KiB'/> + <cell id='5' memory='2097152' unit='KiB'/> + <interconnects> + <latency initiator='0' target='0' type='access' value='5'/> + <latency initiator='0' target='0' cache='1' type='access' value='10'/> + <bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/> + <bandwidth initiator='0' target='0' cache='1' type='access' value='208896' unit='KiB'/> + </interconnects> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3100078b54..438bd5f5ec 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1875,6 +1875,7 @@ mymain(void) DO_TEST_NOCAPS("numatune-distances"); DO_TEST_NOCAPS("numatune-no-vcpu"); DO_TEST_CAPS_LATEST("numatune-hmat"); + DO_TEST_CAPS_LATEST("numatune-hmat-none"); DO_TEST_NOCAPS("numatune-auto-nodeset-invalid"); DO_TEST_NOCAPS("numatune-auto-prefer"); diff --git a/tests/qemuxml2xmloutdata/numatune-hmat-none.x86_64-latest.xml b/tests/qemuxml2xmloutdata/numatune-hmat-none.x86_64-latest.xml new file mode 100644 index 0000000000..185da4bb7b --- /dev/null +++ b/tests/qemuxml2xmloutdata/numatune-hmat-none.x86_64-latest.xml @@ -0,0 +1,55 @@ +<domain type='qemu'> + <name>QEMUGuest</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</currentMemory> + <vcpu placement='static'>12</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + <numa> + <cell id='0' cpus='0-3' memory='2097152' unit='KiB'> + <cache level='1' associativity='none' policy='none'> + <size value='10' unit='KiB'/> + <line value='8' unit='B'/> + </cache> + </cell> + <cell id='1' cpus='4-7' memory='2097152' unit='KiB'/> + <cell id='2' cpus='8-11' memory='2097152' unit='KiB'/> + <cell id='3' memory='2097152' unit='KiB'/> + <cell id='4' memory='2097152' unit='KiB'/> + <cell id='5' memory='2097152' unit='KiB'/> + <interconnects> + <latency initiator='0' target='0' type='access' value='5'/> + <latency initiator='0' target='0' cache='1' type='access' value='10'/> + <bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/> + <bandwidth initiator='0' target='0' cache='1' type='access' value='208896' unit='KiB'/> + </interconnects> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 93202e8e18..3917fe44ca 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -855,6 +855,7 @@ mymain(void) DO_TEST_NOCAPS("numatune-distances"); DO_TEST_NOCAPS("numatune-no-vcpu"); DO_TEST("numatune-hmat", QEMU_CAPS_NUMA_HMAT); + DO_TEST_CAPS_LATEST("numatune-hmat-none"); DO_TEST_CAPS_LATEST("numatune-memnode-restrictive-mode"); DO_TEST_CAPS_LATEST("firmware-manual-bios"); -- 2.40.1
On 5/16/23 10:33, Peter Krempa wrote:
Peter Krempa (2): virDomainNumaDefNodeCacheParseXML: Refactor parsing of cache XML conf: numa: Allow formatting 'none' values for 'associativity' and 'policy' of cache
src/conf/numa_conf.c | 73 +++++-------------- .../numatune-hmat-none.x86_64-latest.args | 49 +++++++++++++ tests/qemuxml2argvdata/numatune-hmat-none.xml | 54 ++++++++++++++ tests/qemuxml2argvtest.c | 1 + .../numatune-hmat-none.x86_64-latest.xml | 55 ++++++++++++++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 178 insertions(+), 55 deletions(-) create mode 100644 tests/qemuxml2argvdata/numatune-hmat-none.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/numatune-hmat-none.xml create mode 100644 tests/qemuxml2xmloutdata/numatune-hmat-none.x86_64-latest.xml
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa