July 2024 - Devel - libvirt List Archives

[PATCH] virt-aa-helper: Allow RO access to /usr/share/edk2-ovmf
by Michal Privoznik 09 Jul '24

09 Jul '24

When binary version of edk2 is distributed, the files reside under /usr/share/edk2-ovmf as can be seen from Gentoo's ebuild [1]. Allow virt-aa-helper to generate paths under that dir. 1: https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-firmware/edk2-ovmf-bin/e… Resolves: https://bugs.gentoo.org/911786 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 402cbd9602..076b98a1d7 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -475,6 +475,7 @@ valid_path(const char *path, const bool readonly) "/initrd", "/initrd.img", "/usr/share/edk2/", + "/usr/share/edk2-ovmf/", "/usr/share/OVMF/", /* for OVMF images */ "/usr/share/ovmf/", /* for OVMF images */ "/usr/share/AAVMF/", /* for AAVMF images */ -- 2.44.2

3 2

[PATCH 0/6] qemu: Fixes to firmware selection
by Andrea Bolognani 09 Jul '24

09 Jul '24

Andrea Bolognani (6): tests: Update firmware descriptors tests: Add more firmware selection coverage qemu: Filter firmware images by type qemu: ROM firmware images are always readonly tests: Add firmware descriptor for edk2 on riscv64 tests: Add test for UEFI autoselection on riscv64 src/qemu/qemu_firmware.c | 17 +++++++++ .../qemu_5.2.0-tcg-virt.riscv64.xml | 4 ++- .../qemu_5.2.0-virt.riscv64.xml | 4 ++- .../qemu_8.0.0-tcg-virt.riscv64.xml | 4 ++- .../qemu_8.0.0-virt.riscv64.xml | 4 ++- .../firmware/60-edk2-ovmf-x64-inteltdx.json | 10 +++--- .../qemu/firmware/50-edk2-riscv-qcow2.json | 33 +++++++++++++++++ .../firmware/60-edk2-ovmf-x64-inteltdx.json | 10 +++--- tests/qemufirmwaretest.c | 7 +++- ...efi-riscv64.riscv64-latest.abi-update.args | 34 ++++++++++++++++++ ...-efi-riscv64.riscv64-latest.abi-update.xml | 28 +++++++++++++++ .../firmware-auto-efi-riscv64.xml | 14 ++++++++ ...ware-auto-efi-rw-pflash.x86_64-latest.err} | 0 ...mware-auto-efi-rw-pflash.x86_64-latest.xml | 35 +++++++++++++++++++ .../firmware-auto-efi-rw-pflash.xml | 18 ++++++++++ tests/qemuxmlconftest.c | 3 +- 16 files changed, 207 insertions(+), 18 deletions(-) create mode 100644 tests/qemufirmwaredata/usr/share/qemu/firmware/50-edk2-riscv-qcow2.json create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-riscv64.riscv64-latest.abi-update.args create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-riscv64.riscv64-latest.abi-update.xml create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-riscv64.xml rename tests/qemuxmlconfdata/{firmware-auto-efi-rw.x86_64-latest.abi-update.err => firmware-auto-efi-rw-pflash.x86_64-latest.err} (100%) create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.xml -- 2.45.2

2 7

[PATCH] vmx: Do not require all ID data for VMWare Distributed Switch
by Martin Kletzander 08 Jul '24

08 Jul '24

Similarly to commit 2482801608b8 we can safely ignore connectionId, portId and portgroupId in both XML and VMX as they are only a blind pass-through between XML and VMX and an ethernet without such parameters was spotted in the wild. On top of that even our documentation says the whole VMWare Distrubuted Switch configuration is a best-effort. Resolves: https://issues.redhat.com/browse/RHEL-46099 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/conf/domain_conf.c | 11 ++++----- src/conf/schemas/domaincommon.rng | 24 ++++++++++++------- src/vmx/vmx.c | 24 ++++++++++++------- ...-portid.vmx => ethernet-vds-no-params.vmx} | 2 -- ...-portid.xml => ethernet-vds-no-params.xml} | 2 +- 5 files changed, 37 insertions(+), 26 deletions(-) rename tests/vmx2xmldata/{ethernet-vds-no-portid.vmx => ethernet-vds-no-params.vmx} (76%) rename tests/vmx2xmldata/{ethernet-vds-no-portid.xml => ethernet-vds-no-params.xml} (82%) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6080f4f90a54..bfef89e1beae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9593,15 +9593,14 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, def->data.vds.switch_id) < 0) return NULL; - if (virXMLPropLongLong(source_node, "portid", 0, VIR_XML_PROP_REQUIRED, - &def->data.vds.port_id, def->data.vds.port_id) < 0) + if (virXMLPropLongLong(source_node, "portid", 0, VIR_XML_PROP_NONE, + &def->data.vds.port_id, 0) < 0) return NULL; - if (!(def->data.vds.portgroup_id = virXMLPropStringRequired(source_node, "portgroupid"))) - return NULL; + def->data.vds.portgroup_id = virXMLPropString(source_node, "portgroupid"); - if (virXMLPropLongLong(source_node, "connectionid", 0, VIR_XML_PROP_REQUIRED, - &def->data.vds.connection_id, def->data.vds.connection_id) < 0) + if (virXMLPropLongLong(source_node, "connectionid", 0, VIR_XML_PROP_NONE, + &def->data.vds.connection_id, 0) < 0) return NULL; break; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index b163e4eece09..2d23fcf12375 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3684,15 +3684,21 @@ <attribute name="switchid"> <ref name="UUID"/> </attribute> - <attribute name="portid"> - <data type="long"/> - </attribute> - <attribute name="portgroupid"> - <data type="string"/> - </attribute> - <attribute name="connectionid"> - <data type="long"/> - </attribute> + <optional> + <attribute name="portid"> + <data type="long"/> + </attribute> + </optional> + <optional> + <attribute name="portgroupid"> + <data type="string"/> + </attribute> + </optional> + <optional> + <attribute name="connectionid"> + <data type="long"/> + </attribute> + </optional> </element> <ref name="interface-options"/> </interleave> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index d082a0766010..e5bc2d793c66 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2896,7 +2896,7 @@ virVMXParseEthernet(virConf *conf, int controller, virDomainNetDef **def) if (virVMXGetConfigString(conf, portgroupId_name, &(*def)->data.vds.portgroup_id, - false) < 0 || + true) < 0 || virVMXGetConfigLong(conf, portId_name, &(*def)->data.vds.port_id, @@ -2906,7 +2906,7 @@ virVMXParseEthernet(virConf *conf, int controller, virDomainNetDef **def) connectionId_name, &(*def)->data.vds.connection_id, 0, - false) < 0) + true) < 0) goto cleanup; } else if (connectionType == NULL && networkName == NULL) { (*def)->type = VIR_DOMAIN_NET_TYPE_NULL; @@ -4038,14 +4038,22 @@ virVMXFormatEthernet(virDomainNetDef *def, int controller, uuid[5], uuid[6], uuid[7], uuid[8], uuid[9], uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]); - virBufferAsprintf(buffer, "ethernet%d.dvs.portId = \"%lld\"\n", - controller, def->data.vds.port_id); + if (def->data.vds.port_id) { + virBufferAsprintf(buffer, "ethernet%d.dvs.portId = \"%lld\"\n", + controller, def->data.vds.port_id); + } + + if (def->data.vds.portgroup_id) { + virBufferAsprintf(buffer, "ethernet%d.dvs.", controller); + virBufferEscapeString(buffer, "portgroupId = \"%s\"\n", + def->data.vds.portgroup_id); + } - virBufferAsprintf(buffer, "ethernet%d.dvs.", controller); - virBufferEscapeString(buffer, "portgroupId = \"%s\"\n", def->data.vds.portgroup_id); + if (def->data.vds.connection_id) { + virBufferAsprintf(buffer, "ethernet%d.dvs.connectionId = \"%lld\"\n", + controller, def->data.vds.connection_id); + } - virBufferAsprintf(buffer, "ethernet%d.dvs.connectionId = \"%lld\"\n", - controller, def->data.vds.connection_id); break; } diff --git a/tests/vmx2xmldata/ethernet-vds-no-portid.vmx b/tests/vmx2xmldata/ethernet-vds-no-params.vmx similarity index 76% rename from tests/vmx2xmldata/ethernet-vds-no-portid.vmx rename to tests/vmx2xmldata/ethernet-vds-no-params.vmx index 7761accb3abc..90afbdac30cd 100644 --- a/tests/vmx2xmldata/ethernet-vds-no-portid.vmx +++ b/tests/vmx2xmldata/ethernet-vds-no-params.vmx @@ -5,6 +5,4 @@ ethernet0.virtualDev = "e1000e" ethernet0.addressType = "vpx" ethernet0.generatedAddress = "00:50:56:87:65:43" ethernet0.dvs.switchId = "50 34 26 b2 94 e9 3b 16-1d 68 87 bf ff 4a 54 40" -ethernet0.dvs.portgroupId = "dvportgroup-1285" -ethernet0.dvs.connectionId = "408217997" displayName = "test" diff --git a/tests/vmx2xmldata/ethernet-vds-no-portid.xml b/tests/vmx2xmldata/ethernet-vds-no-params.xml similarity index 82% rename from tests/vmx2xmldata/ethernet-vds-no-portid.xml rename to tests/vmx2xmldata/ethernet-vds-no-params.xml index 60fd9c99feb9..0011ba471a50 100644 --- a/tests/vmx2xmldata/ethernet-vds-no-portid.xml +++ b/tests/vmx2xmldata/ethernet-vds-no-params.xml @@ -14,7 +14,7 @@ <devices> <interface type='vds'> <mac address='00:50:56:87:65:43' type='generated'/> - <source switchid='503426b2-94e9-3b16-1d68-87bfff4a5440' portid='0' portgroupid='dvportgroup-1285' connectionid='408217997'/> + <source switchid='503426b2-94e9-3b16-1d68-87bfff4a5440'/> <model type='e1000e'/> </interface> <video> -- 2.45.1

2 1

[PATCH] test_driver: support VIR_DOMAIN_AFFECT_LIVE in testUpdateDeviceFlags()
by John Levon 04 Jul '24

04 Jul '24

Pick up some more of the qemu_driver.c code so this function supports both CONFIG and LIVE updates. Note that qemuDomainUpdateDeviceFlags() passed vm->def to virDomainDeviceDefParse() for the VIR_DOMAIN_AFFECT_CONFIG case, which is technically incorrect; in the test driver code we'll fix this. Signed-off-by: John Levon <john.levon(a)nutanix.com> --- src/test/test_driver.c | 54 +++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 712bb20563..da682da9ad 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -10239,10 +10239,10 @@ testDomainAttachDevice(virDomainPtr domain, const char *xml) static int -testDomainUpdateDeviceConfig(virDomainDef *vmdef, - virDomainDeviceDef *dev, - unsigned int parse_flags, - virDomainXMLOption *xmlopt) +testDomainUpdateDevice(virDomainDef *vmdef, + virDomainDeviceDef *dev, + unsigned int parse_flags, + virDomainXMLOption *xmlopt) { virDomainDiskDef *newDisk; virDomainDeviceDef oldDev = { .type = dev->type }; @@ -10316,12 +10316,16 @@ testDomainUpdateDeviceFlags(virDomainPtr dom, testDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; virObjectEvent *event = NULL; + virDomainDef *def = NULL; + virDomainDef *persistentDef = NULL; g_autoptr(virDomainDef) vmdef = NULL; - g_autoptr(virDomainDeviceDef) dev = NULL; + g_autoptr(virDomainDeviceDef) dev_live = NULL; + g_autoptr(virDomainDeviceDef) dev_config = NULL; int ret = -1; unsigned int parse_flags = 0; - virCheckFlags(VIR_DOMAIN_AFFECT_CONFIG, -1); + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); if (!(vm = testDomObjFromDomain(dom))) goto cleanup; @@ -10337,9 +10341,24 @@ testDomainUpdateDeviceFlags(virDomainPtr dom, parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; } - if (!(dev = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, - NULL, parse_flags))) { - goto endjob; + /* + * NB: this has diverged from qemuDomainUpdateDeviceFlags(), which uses + * vm->def in both cases; technically the qemu driver should do the same. + */ + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) + goto cleanup; + + if (def) { + if (!(dev_live = virDomainDeviceDefParse(xml, def, driver->xmlopt, + NULL, parse_flags))) + goto endjob; + } + + if (persistentDef) { + if (!(dev_config = virDomainDeviceDefParse(xml, persistentDef, + driver->xmlopt, NULL, + parse_flags))) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -10350,18 +10369,19 @@ testDomainUpdateDeviceFlags(virDomainPtr dom, /* virDomainDefCompatibleDevice call is delayed until we know the * device we're going to update. */ - if ((ret = testDomainUpdateDeviceConfig(vmdef, dev, - parse_flags, - driver->xmlopt)) < 0) + if ((ret = testDomainUpdateDevice(vmdef, dev_config, + parse_flags, + driver->xmlopt)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - ret = -1; - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("live update of device '%1$s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - goto endjob; + /* virDomainDefCompatibleDevice call is delayed until we know the + * device we're going to update. */ + if ((ret = testDomainUpdateDevice(def, dev_live, + parse_flags, + driver->xmlopt)) < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { -- 2.34.1

2 1

[PATCH] conf: Fix out-of-bounds write during cleanup of virDomainNumaDefNodeDistanceParseXML
by Rayhan Faizel 04 Jul '24

04 Jul '24

mem_nodes[i].ndistances is written outside the loop causing an out-of-bounds write leading to heap corruption. While we are at it, the entire cleanup portion can be removed as it can be handled in virDomainNumaFree. One instance of VIR_FREE is also removed and replaced with g_autofree. This patch also adds a testcase which would be picked up by ASAN, if this portion regresses. Fixes: 742494eed8dbdde8b1d05a306032334e6226beea Signed-off-by: Rayhan Faizel <rayhan.faizel(a)gmail.com> --- src/conf/numa_conf.c | 30 ++++++------------- ...ance-nonexistent-sibling.x86_64-latest.err | 1 + .../cpu-numa-distance-nonexistent-sibling.xml | 29 ++++++++++++++++++ tests/qemuxmlconftest.c | 1 + 4 files changed, 40 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index d8120de6d2..0a0e2911f7 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -343,8 +343,7 @@ virDomainNumaFree(virDomainNuma *numa) virBitmapFree(numa->mem_nodes[i].cpumask); virBitmapFree(numa->mem_nodes[i].nodeset); - if (numa->mem_nodes[i].ndistances > 0) - g_free(numa->mem_nodes[i].distances); + g_free(numa->mem_nodes[i].distances); g_free(numa->mem_nodes[i].caches); } @@ -685,9 +684,8 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, xmlXPathContextPtr ctxt, unsigned int cur_cell) { - int ret = -1; int sibling; - xmlNodePtr *nodes = NULL; + g_autofree xmlNodePtr *nodes = NULL; size_t i, ndistances = def->nmem_nodes; if (ndistances == 0) @@ -698,12 +696,12 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, return 0; if ((sibling = virXPathNodeSet("./distances[1]/sibling", ctxt, &nodes)) < 0) - goto cleanup; + return -1; if (sibling == 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("NUMA distances defined without siblings")); - goto cleanup; + return -1; } for (i = 0; i < sibling; i++) { @@ -713,19 +711,19 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, if (virXMLPropUInt(nodes[i], "id", 10, VIR_XML_PROP_REQUIRED, &sibling_id) < 0) - goto cleanup; + return -1; /* The "id" needs to be within numa/cell range */ if (sibling_id >= ndistances) { virReportError(VIR_ERR_XML_ERROR, _("'sibling_id %1$d' does not refer to a valid cell within NUMA 'cell id %2$d'"), sibling_id, cur_cell); - goto cleanup; + return -1; } if (virXMLPropUInt(nodes[i], "value", 10, VIR_XML_PROP_REQUIRED, &sibling_value) < 0) - goto cleanup; + return -1; /* Assure LOCAL_DISTANCE <= "value" <= UNREACHABLE * and correct LOCAL_DISTANCE setting if such applies. @@ -739,7 +737,7 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, virReportError(VIR_ERR_XML_ERROR, _("'value %1$d' is invalid for 'sibling id %2$d' under NUMA 'cell id %3$d'"), sibling_value, sibling_id, cur_cell); - goto cleanup; + return -1; } /* Apply the local / remote distance */ @@ -770,17 +768,7 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, rdist[cur_cell].value = sibling_value; } - ret = 0; - - cleanup: - if (ret < 0) { - for (i = 0; i < ndistances; i++) - VIR_FREE(def->mem_nodes[i].distances); - def->mem_nodes[i].ndistances = 0; - } - VIR_FREE(nodes); - - return ret; + return 0; } diff --git a/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err new file mode 100644 index 0000000000..4866ff5e80 --- /dev/null +++ b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err @@ -0,0 +1 @@ +XML error: 'sibling_id 2' does not refer to a valid cell within NUMA 'cell id 1' diff --git a/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml new file mode 100644 index 0000000000..62a6c32fbe --- /dev/null +++ b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>16</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu> + <topology sockets='2' dies='1' cores='4' threads='2'/> + <numa> + <cell id='1' cpus='8-15' memory='109550' unit='KiB'> + <distances> + <sibling id='2' value='10'/> + </distances> + </cell> + <cell id='0' cpus='0-7' memory='109550' unit='KiB'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 4a711fceeb..af49934c33 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2160,6 +2160,7 @@ mymain(void) DO_TEST_CAPS_LATEST_PARSE_ERROR("cpu-numa3"); DO_TEST_CAPS_LATEST("cpu-numa-disjoint"); DO_TEST_CAPS_LATEST("cpu-numa-memshared"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("cpu-numa-distance-nonexistent-sibling"); /* host-model cpu expansion depends on the cpu reported by qemu and thus * we invoke it for all real capability dumps we have */ -- 2.34.1

2 1

[PATCH 0/2] qemu: support logging config for swtpm
by Daniel P. Berrangé 04 Jul '24

04 Jul '24

When debugging guest problems with TPMs it is helpful to be able to have full swtpm logging. This isn't possible currently and manually restarting the swtpm process of a running guest is disruptive. Daniel P. Berrangé (2): conf: add support for 'debug' parameter on TPM emulator qemu: set swtpm log level parameter docs/formatdomain.rst | 6 ++++-- src/conf/domain_conf.c | 7 +++++++ src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 5 +++++ src/qemu/qemu_tpm.c | 6 +++++- tests/qemuxmlconfdata/tpm-emulator-tpm2.xml | 2 +- 6 files changed, 23 insertions(+), 4 deletions(-) -- 2.45.1

2 3

[PATCH v2 0/8] ch: support restore with network devices
by Purna Pavan Chandra 04 Jul '24

04 Jul '24

Current ch driver supports restore only for domains without any network configuration defined. This was because libvirt explicitly passes network fds and CH did not had support to restore with new net FDS. This support has been added recently, https://github.com/cloud-hypervisor/cloud-hypervisor/pull/6402 The changes in this patch series includes moving to socket communication for restore api, create new net fds and pass them via SCM_RIGHTS to CH. New changes in v2: * Reword of few commints * Add version checks in save/restore validations * Add use_timeout in chSocketRecv * Address Praveen Paladugu's comments v1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/PT77… Purna Pavan Chandra (8): ch: report response message instead of just code ch: Pass net ids explicitly during vm creation ch: refactor chProcessAddNetworkDevices ch: support poll with -1 in chSocketRecv ch: use monitor socket fd to send restore request ch: refactor virCHMonitorSaveVM ch: support restore with net devices ch: kill CH process if restore fails src/ch/ch_capabilities.c | 6 + src/ch/ch_capabilities.h | 1 + src/ch/ch_driver.c | 29 +++-- src/ch/ch_monitor.c | 62 +++++++---- src/ch/ch_monitor.h | 6 +- src/ch/ch_process.c | 233 +++++++++++++++++++++++++++++++-------- 6 files changed, 254 insertions(+), 83 deletions(-) -- 2.34.1

1 8

[PATCH] conf: Fix out-of-bounds write during cleanup of virDomainNumaDefNodeDistanceParseXML
by Rayhan Faizel 04 Jul '24

04 Jul '24

mem_nodes[i].ndistances is written outside the loop causing an out-of-bounds write leading to heap corruption. While we are at it, the entire cleanup portion can be removed as it can be handled in virDomainNumaFree. One instance of VIR_FREE is also removed and replaced with g_autofree. This patch also adds a testcase which would be picked up by ASAN, if this portion regresses. Signed-off-by: Rayhan Faizel <rayhan.faizel(a)gmail.com> --- src/conf/numa_conf.c | 30 ++++++------------- ...ance-nonexistent-sibling.x86_64-latest.err | 1 + .../cpu-numa-distance-nonexistent-sibling.xml | 29 ++++++++++++++++++ tests/qemuxmlconftest.c | 1 + 4 files changed, 40 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index d8120de6d2..0a0e2911f7 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -343,8 +343,7 @@ virDomainNumaFree(virDomainNuma *numa) virBitmapFree(numa->mem_nodes[i].cpumask); virBitmapFree(numa->mem_nodes[i].nodeset); - if (numa->mem_nodes[i].ndistances > 0) - g_free(numa->mem_nodes[i].distances); + g_free(numa->mem_nodes[i].distances); g_free(numa->mem_nodes[i].caches); } @@ -685,9 +684,8 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, xmlXPathContextPtr ctxt, unsigned int cur_cell) { - int ret = -1; int sibling; - xmlNodePtr *nodes = NULL; + g_autofree xmlNodePtr *nodes = NULL; size_t i, ndistances = def->nmem_nodes; if (ndistances == 0) @@ -698,12 +696,12 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, return 0; if ((sibling = virXPathNodeSet("./distances[1]/sibling", ctxt, &nodes)) < 0) - goto cleanup; + return -1; if (sibling == 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("NUMA distances defined without siblings")); - goto cleanup; + return -1; } for (i = 0; i < sibling; i++) { @@ -713,19 +711,19 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, if (virXMLPropUInt(nodes[i], "id", 10, VIR_XML_PROP_REQUIRED, &sibling_id) < 0) - goto cleanup; + return -1; /* The "id" needs to be within numa/cell range */ if (sibling_id >= ndistances) { virReportError(VIR_ERR_XML_ERROR, _("'sibling_id %1$d' does not refer to a valid cell within NUMA 'cell id %2$d'"), sibling_id, cur_cell); - goto cleanup; + return -1; } if (virXMLPropUInt(nodes[i], "value", 10, VIR_XML_PROP_REQUIRED, &sibling_value) < 0) - goto cleanup; + return -1; /* Assure LOCAL_DISTANCE <= "value" <= UNREACHABLE * and correct LOCAL_DISTANCE setting if such applies. @@ -739,7 +737,7 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, virReportError(VIR_ERR_XML_ERROR, _("'value %1$d' is invalid for 'sibling id %2$d' under NUMA 'cell id %3$d'"), sibling_value, sibling_id, cur_cell); - goto cleanup; + return -1; } /* Apply the local / remote distance */ @@ -770,17 +768,7 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def, rdist[cur_cell].value = sibling_value; } - ret = 0; - - cleanup: - if (ret < 0) { - for (i = 0; i < ndistances; i++) - VIR_FREE(def->mem_nodes[i].distances); - def->mem_nodes[i].ndistances = 0; - } - VIR_FREE(nodes); - - return ret; + return 0; } diff --git a/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err new file mode 100644 index 0000000000..4866ff5e80 --- /dev/null +++ b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.x86_64-latest.err @@ -0,0 +1 @@ +XML error: 'sibling_id 2' does not refer to a valid cell within NUMA 'cell id 1' diff --git a/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml new file mode 100644 index 0000000000..62a6c32fbe --- /dev/null +++ b/tests/qemuxmlconfdata/cpu-numa-distance-nonexistent-sibling.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>16</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <cpu> + <topology sockets='2' dies='1' cores='4' threads='2'/> + <numa> + <cell id='1' cpus='8-15' memory='109550' unit='KiB'> + <distances> + <sibling id='2' value='10'/> + </distances> + </cell> + <cell id='0' cpus='0-7' memory='109550' unit='KiB'/> + </numa> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 4a711fceeb..af49934c33 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2160,6 +2160,7 @@ mymain(void) DO_TEST_CAPS_LATEST_PARSE_ERROR("cpu-numa3"); DO_TEST_CAPS_LATEST("cpu-numa-disjoint"); DO_TEST_CAPS_LATEST("cpu-numa-memshared"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("cpu-numa-distance-nonexistent-sibling"); /* host-model cpu expansion depends on the cpu reported by qemu and thus * we invoke it for all real capability dumps we have */ -- 2.34.1

1 1

[PATCH] test_driver: add testUpdateDeviceFlags implementation
by John Levon 04 Jul '24

04 Jul '24

Add basic coverage of device update; for now, only support disk updates until other types are needed or tested. Signed-off-by: John Levon <john.levon(a)nutanix.com> --- src/test/test_driver.c | 127 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 40fb8a467d..213fbdbccb 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -41,6 +41,7 @@ #include "domain_conf.h" #include "domain_driver.h" #include "domain_event.h" +#include "domain_postparse.h" #include "domain_validate.h" #include "network_event.h" #include "snapshot_conf.h" @@ -10237,6 +10238,131 @@ testDomainAttachDevice(virDomainPtr domain, const char *xml) } +static int +testDomainUpdateDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev, + unsigned int parse_flags, + virDomainXMLOption *xmlopt) +{ + virDomainDiskDef *newDisk; + virDomainDeviceDef oldDev = { .type = dev->type }; + int pos; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + newDisk = dev->data.disk; + if ((pos = virDomainDiskIndexByName(vmdef, newDisk->dst, false)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("target %1$s doesn't exist."), newDisk->dst); + return -1; + } + + oldDev.data.disk = vmdef->disks[pos]; + if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + false) < 0) + return -1; + + virDomainDiskDefFree(vmdef->disks[pos]); + vmdef->disks[pos] = newDisk; + dev->data.disk = NULL; + break; + + // TODO: support these here once tested. + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_MEMORY: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent update of device '%1$s' is not supported by test driver"), + virDomainDeviceTypeToString(dev->type)); + return -1; + + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_VSOCK: + case VIR_DOMAIN_DEVICE_AUDIO: + case VIR_DOMAIN_DEVICE_CRYPTO: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent update of device '%1$s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + + if (virDomainDefPostParse(vmdef, parse_flags, xmlopt, NULL) < 0) + return -1; + + return 0; +} + + +static int +testDomainUpdateDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) +{ + testDriver *driver = dom->conn->privateData; + virDomainObj *vm = NULL; + g_autoptr(virDomainDef) vmdef = NULL; + g_autoptr(virDomainDeviceDef) dev = NULL; + int ret = -1; + unsigned int parse_flags = 0; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); + + if (!(vm = testDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + goto endjob; + + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !(flags & VIR_DOMAIN_AFFECT_LIVE)) + parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; + + if (!(dev = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + NULL, parse_flags))) + goto endjob; + + /* virDomainDefCompatibleDevice call is delayed until we know the + * device we're going to update. */ + if ((ret = testDomainUpdateDeviceConfig(vm->def, dev, + parse_flags, + driver->xmlopt)) < 0) + goto endjob; + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + /* search for a hostdev matching dev and detach it */ static int testDomainDetachPrepHostdev(virDomainObj *vm, @@ -10456,6 +10582,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainAttachDevice = testDomainAttachDevice, /* 10.0.0 */ .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 10.0.0 */ .domainDetachDeviceAlias = testDomainDetachDeviceAlias, /* 10.0.0 */ + .domainUpdateDeviceFlags = testDomainUpdateDeviceFlags, /* 10.6.0 */ .domainCreateXML = testDomainCreateXML, /* 0.1.4 */ .domainCreateXMLWithFiles = testDomainCreateXMLWithFiles, /* 5.7.0 */ .domainLookupByID = testDomainLookupByID, /* 0.1.1 */ -- 2.34.1

2 6

[PATCH v2] tests: Move domainEventState initialization to qemuTestDriverInit
by Rayhan Faizel 04 Jul '24

04 Jul '24

Under the test environment, driver->domainEventState is uninitialized. If a disk gets dropped, it will attempt to queue an event which will cause a segmentation fault. This crash does not occur during normal use. This patch moves driver->domainEventState initialization from qemuhotplugtest to qemuTestDriverInit in testutilsqemu (Credit goes to Michal Privoznik as he had already provided the diff). An additional test case is added to test dropping of disks with startupPolicy set as optional. Signed-off-by: Rayhan Faizel <rayhan.faizel(a)gmail.com> --- As the patch title has completely changed, v1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/4476… [Changes in v2] - Move domainEventState initialization instead of adding a NULL check tests/qemuhotplugtest.c | 3 -- ...tuppolicy-optional-drop.x86_64-latest.args | 33 ++++++++++++++++ ...rtuppolicy-optional-drop.x86_64-latest.xml | 38 +++++++++++++++++++ .../disk-startuppolicy-optional-drop.xml | 23 +++++++++++ tests/qemuxmlconftest.c | 1 + tests/testutilsqemu.c | 4 ++ 6 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d935ad58ed..f707121c47 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -501,9 +501,6 @@ mymain(void) virEventRegisterDefaultImpl(); - if (!(driver.domainEventState = virObjectEventStateNew())) - return EXIT_FAILURE; - driver.lockManager = virLockManagerPluginNew("nop", "qemu", driver.config->configBaseDir, 0); diff --git a/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args new file mode 100644 index 0000000000..13ddbc1a5d --- /dev/null +++ b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args @@ -0,0 +1,33 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-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"}' \ +-device '{"driver":"lsi","id":"scsi0","bus":"pci.0","addr":"0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml new file mode 100644 index 0000000000..27d0639109 --- /dev/null +++ b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml @@ -0,0 +1,38 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='volume' device='disk'> + <driver name='qemu'/> + <source pool='inactive' volume='inactive' startupPolicy='optional'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <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'/> + <controller type='scsi' index='0' model='lsilogic'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml new file mode 100644 index 0000000000..c6c59978c6 --- /dev/null +++ b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='volume' device='disk'> + <source pool='inactive' volume='inactive' startupPolicy='optional'/> + <target dev='sda'/> + </disk> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 4a711fceeb..2a495cc892 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2947,6 +2947,7 @@ mymain(void) DO_TEST_CAPS_LATEST_FAILURE("disk-network-iscsi-zero-hosts-invalid") DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-scsi-vhost-rawio-invalid") DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-scsi-vhost-sgio-invalid") + DO_TEST_CAPS_LATEST("disk-startuppolicy-optional-drop") /* check that all input files were actually used here */ if (testConfXMLCheck(existingTestCases) < 0) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index d70850cb5d..ee6cae218a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -236,6 +236,7 @@ void qemuTestDriverFree(virQEMUDriver *driver) virObjectUnref(driver->caps); virObjectUnref(driver->config); virObjectUnref(driver->securityManager); + virObjectUnref(driver->domainEventState); g_clear_object(&driver->nbdkitCapsCache); virCPUDefFree(cpuDefault); @@ -369,6 +370,9 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (!(driver->securityManager = virSecurityManagerNewStack(mgr))) goto error; + if (!(driver->domainEventState = virObjectEventStateNew())) + goto error; + qemuTestSetHostCPU(driver, driver->hostarch, NULL); VIR_FREE(cfg->vncTLSx509certdir); -- 2.34.1

2 1