- Devel - libvirt List Archives

[PATCH] esx: Track VMs by instanceUuid instead of UUID
by Martin Kletzander 14 May '26

14 May '26

From: Martin Kletzander <mkletzan(a)redhat.com> The difference is that the usual UUID is supposed to be unique per host and instanceUuid should be unique across the whole cluster. One could think of them as HUID and CUID (as the first "U" does apparently mean something else in the Broadcom world). That _would_ be fine for our scenario. However, that piece of information turns out to be false as well and the UUID we were using (`config.uuid`, or in VMX the `uuid.bios`) can be the same in two machines on the same host. Fortunately the `FindByUuid()` function can also search for VMs based on their `instanceUuid`, dictated by the so far omitted third parameter. Unfortunately that parameter is not parsed (or at least properly) before vSphere API 4.0 (the documentation says 2.0, but we are not using that namespace and 4.0 is the lowest we can target), which we are not specifying in the server returns a 500 HTTP error if we use the `instanceUuid` parameter. So this patch adds the `SOAPAction: urn:vim25/4.0` header to the cURL requests which makes that `FindByUuid()` function work even with the `instanceUuid` set, but without any extra labor. After that this patch also changes all UUIDs to be parsed from the `config.instanceUuid` (or `vc.uuid` in the VMX, but there's a fallback to the old `uuid.bios`) and adjusts tests accordingly. Last, but not least it changes the parameter to aforementioned function to be always true and henceforth all searching ought to be done with the more unique ID. Resolves: https://redhat.atlassian.net/browse/RHEL-174300 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/esx/esx_driver.c | 8 ++++---- src/esx/esx_vi.c | 16 ++++++++++++++-- src/vmx/vmx.c | 10 +++++++--- tests/vmx2xmldata/esx-in-the-wild-10.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-11.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-12.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-13.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-14.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-15.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-16.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-17.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-5.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-6.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-7.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-8.xml | 2 +- tests/vmx2xmldata/esx-in-the-wild-9.xml | 2 +- 16 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 010c62b8e880..7cc54af69239 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1419,7 +1419,7 @@ esxDomainLookupByID(virConnectPtr conn, int id) "configStatus\0" "name\0" "runtime.powerState\0" - "config.uuid\0") < 0 || + "config.instanceUuid\0") < 0 || esxVI_LookupVirtualMachineList(priv->primary, propertyNameList, &virtualMachineList) < 0) { goto cleanup; @@ -1522,7 +1522,7 @@ esxDomainLookupByName(virConnectPtr conn, const char *name) if (esxVI_String_AppendValueListToList(&propertyNameList, "configStatus\0" "runtime.powerState\0" - "config.uuid\0") < 0 || + "config.instanceUuid\0") < 0 || esxVI_LookupVirtualMachineByName(priv->primary, name, propertyNameList, &virtualMachine, esxVI_Occurrence_RequiredItem) < 0) { @@ -4788,7 +4788,7 @@ esxConnectListAllDomains(virConnectPtr conn, if (esxVI_String_AppendValueListToList(&propertyNameList, "configStatus\0" "name\0" - "config.uuid\0") < 0) { + "config.instanceUuid\0") < 0) { goto cleanup; } } @@ -4970,7 +4970,7 @@ esxDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags) if (esxVI_FindByUuid(priv->primary, priv->primary->datacenter->_reference, uuid_string, esxVI_Boolean_True, - esxVI_Boolean_Undefined, + esxVI_Boolean_True, &managedObjectReference) < 0) { return -1; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index db006ed16f40..55d5d1478586 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -300,6 +300,17 @@ esxVI_CURL_Connect(esxVI_CURL *curl, esxUtil_ParsedUri *parsedUri) curl->headers = curl_slist_append(curl->headers, "Content-Type: text/xml; charset=UTF-8"); + /* + * Testing showed that the 4.0 version is most close to our current types in + * esx_vi_generator.input data. We could update the version if new method + * parameters or object properties are needed. + * + * The other option is to drop the "/x.y" suffix completely once + * https://gitlab.com/libvirt/libvirt/-/work_items/878 is implemented since + * that will not limit the version at all. + */ + curl->headers = curl_slist_append(curl->headers, "SOAPAction: urn:vim25/4.0"); + /* * Add an empty expect header to stop CURL from waiting for a response code * 100 (Continue) from the server before continuing the POST operation. @@ -2452,7 +2463,8 @@ esxVI_GetVirtualMachineIdentity(esxVI_ObjectContent *virtualMachine, for (dynamicProperty = virtualMachine->propSet; dynamicProperty; dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "config.uuid")) { + + if (STREQ(dynamicProperty->name, "config.instanceUuid")) { if (esxVI_AnyType_ExpectType(dynamicProperty->val, esxVI_Type_String) < 0) { goto failure; @@ -2682,7 +2694,7 @@ esxVI_LookupVirtualMachineByUuid(esxVI_Context *ctx, const unsigned char *uuid, virUUIDFormat(uuid, uuid_string); if (esxVI_FindByUuid(ctx, ctx->datacenter->_reference, uuid_string, - esxVI_Boolean_True, esxVI_Boolean_Undefined, + esxVI_Boolean_True, esxVI_Boolean_True, &managedObjectReference) < 0) { return -1; } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 084b4154427f..a2422c37355c 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1492,9 +1492,13 @@ virVMXParseConfig(virVMXContext *ctx, def->scsiBusMaxUnit = SCSI_SUPER_WIDE_BUS_MAX_CONT_UNIT; } - /* vmx:uuid.bios -> def:uuid */ - /* FIXME: Need to handle 'uuid.action = "create"' */ - if (virVMXGetConfigUUID(conf, "uuid.bios", def->uuid, true) < 0) + /* vmx:vc.uuid (fallback to uuid.bios) -> def:uuid */ + if (virVMXGetConfigUUID(conf, "vc.uuid", def->uuid, true) < 0) + goto cleanup; + + /* FIXME: Need to handle 'uuid.action = "create"' ? */ + if (!virUUIDIsValid(def->uuid) && + virVMXGetConfigUUID(conf, "uuid.bios", def->uuid, true) < 0) goto cleanup; /* vmx:displayName -> def:name */ diff --git a/tests/vmx2xmldata/esx-in-the-wild-10.xml b/tests/vmx2xmldata/esx-in-the-wild-10.xml index 1b1fdf06623f..3a8bb20140a1 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-10.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-10.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>w2019biosvmware</name> - <uuid>421a6177-5aa9-abb7-5924-fc376c18a1b4</uuid> + <uuid>501af9f2-6d29-1c76-19a9-b208ede5f374</uuid> <genid>13c67c91-9f47-526f-b0d6-e4dd2e4bb4f9</genid> <memory unit='KiB'>4194304</memory> <currentMemory unit='KiB'>4194304</currentMemory> diff --git a/tests/vmx2xmldata/esx-in-the-wild-11.xml b/tests/vmx2xmldata/esx-in-the-wild-11.xml index 0dd297af43f0..6398e50c5c6c 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-11.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-11.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>esx6.7-rhel7.7-x86_64</name> - <uuid>422c0152-63ab-cd03-9650-4301ae77aefd</uuid> + <uuid>502ca229-12eb-24f6-0c37-c025ff5da005</uuid> <memory unit='KiB'>2097152</memory> <currentMemory unit='KiB'>2097152</currentMemory> <vcpu placement='static'>1</vcpu> diff --git a/tests/vmx2xmldata/esx-in-the-wild-12.xml b/tests/vmx2xmldata/esx-in-the-wild-12.xml index ac83982b9b88..d505ff58d12d 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-12.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-12.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>Auto-esx8.0-rhell9.3-efi-with-empty-cdrom</name> - <uuid>4220fca7-11dd-d67e-19cc-fcad0a37c342</uuid> + <uuid>5020ca5b-7ac1-5c44-cfa1-9e762e2f933b</uuid> <memory unit='KiB'>2097152</memory> <currentMemory unit='KiB'>2097152</currentMemory> <vcpu placement='static'>1</vcpu> diff --git a/tests/vmx2xmldata/esx-in-the-wild-13.xml b/tests/vmx2xmldata/esx-in-the-wild-13.xml index cef9fd4e48c9..2bea51a087eb 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-13.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-13.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>Test-Mig-VM-1 (01ce57d0-4e20-41a5-8b6c-bcbf49a032ec)</name> - <uuid>421eb458-5448-fc12-2074-83d5e419e138</uuid> + <uuid>01ce57d0-4e20-41a5-8b6c-bcbf49a032ec</uuid> <description>name:Test-Mig-VM-1 userid:962314ba515c48388a0e95c0961709ff username:admin diff --git a/tests/vmx2xmldata/esx-in-the-wild-14.xml b/tests/vmx2xmldata/esx-in-the-wild-14.xml index f10707d1d412..574dd2097455 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-14.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-14.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>wild14</name> - <uuid>421b223a-f2c1-c7c9-a399-34d2d9fde26d</uuid> + <uuid>501b831e-75d8-15f8-36fa-b9e225f395aa</uuid> <description>execution env sandbox automation platform</description> <memory unit='KiB'>33554432</memory> <currentMemory unit='KiB'>33554432</currentMemory> diff --git a/tests/vmx2xmldata/esx-in-the-wild-15.xml b/tests/vmx2xmldata/esx-in-the-wild-15.xml index 78d15e1538e6..4c753f87e2ea 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-15.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-15.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>dokuwiki</name> - <uuid>420338bd-1c9e-ad50-99a2-59e92ddda8b6</uuid> + <uuid>500341d5-fe23-ac46-8cb3-77a8e5a1143d</uuid> <memory unit='KiB'>2097152</memory> <currentMemory unit='KiB'>2097152</currentMemory> <vcpu placement='static'>2</vcpu> diff --git a/tests/vmx2xmldata/esx-in-the-wild-16.xml b/tests/vmx2xmldata/esx-in-the-wild-16.xml index 51746dd77ef0..8c5bc7f43513 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-16.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-16.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>Auto-esx8.0-rhel9.4-efi-nvme-disk</name> - <uuid>4220df89-e3a8-8513-f611-ad252bfd7047</uuid> + <uuid>5020196e-c5c8-7310-fcdb-d2933d0c4b2f</uuid> <memory unit='KiB'>2097152</memory> <currentMemory unit='KiB'>2097152</currentMemory> <vcpu placement='static'>1</vcpu> diff --git a/tests/vmx2xmldata/esx-in-the-wild-17.xml b/tests/vmx2xmldata/esx-in-the-wild-17.xml index 725f21bdf601..506e08fc54ec 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-17.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-17.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>esx8.0-win11-with-second-disk-in-subfolder</name> - <uuid>42256ec1-e066-9364-3dd1-36a0b75263dd</uuid> + <uuid>50250f27-538b-5091-a43b-a50ce95f2382</uuid> <genid>3191ed70-eb21-9c71-2478-373fb27fed9b</genid> <memory unit='KiB'>4194304</memory> <currentMemory unit='KiB'>4194304</currentMemory> diff --git a/tests/vmx2xmldata/esx-in-the-wild-5.xml b/tests/vmx2xmldata/esx-in-the-wild-5.xml index c88e60bdc070..a86119508c70 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-5.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-5.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>vmtest.local</name> - <uuid>423e94a9-a1c7-b31d-7161-76c7586c830e</uuid> + <uuid>503e06db-f8d5-458e-a2fb-f9820253a7be</uuid> <description>Centos 5.5 64bit Server</description> <memory unit='KiB'>2097152</memory> <currentMemory unit='KiB'>2097152</currentMemory> diff --git a/tests/vmx2xmldata/esx-in-the-wild-6.xml b/tests/vmx2xmldata/esx-in-the-wild-6.xml index 805f03356129..f252304f9796 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-6.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-6.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>el6-test</name> - <uuid>564d15d4-d062-fe9a-80f5-eb8e1a2c3afc</uuid> + <uuid>5200b69b-8d88-7bdf-a14a-02705d653772</uuid> <memory unit='KiB'>1048576</memory> <currentMemory unit='KiB'>1048576</currentMemory> <vcpu placement='static'>1</vcpu> diff --git a/tests/vmx2xmldata/esx-in-the-wild-7.xml b/tests/vmx2xmldata/esx-in-the-wild-7.xml index b641574776b6..076655f37ac2 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-7.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-7.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>esx-rhel6-mini-with-scsi-device</name> - <uuid>564d9176-621f-0239-f5ad-3a002371953b</uuid> + <uuid>52409533-33a2-56c5-36ce-80d605f8ecf4</uuid> <memory unit='KiB'>2097152</memory> <currentMemory unit='KiB'>2097152</currentMemory> <vcpu placement='static'>1</vcpu> diff --git a/tests/vmx2xmldata/esx-in-the-wild-8.xml b/tests/vmx2xmldata/esx-in-the-wild-8.xml index f13e6f744880..2ef609e87bdb 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-8.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-8.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>RHEL7_10_NICs</name> - <uuid>42359420-99dc-4261-5264-ba58ddae20e4</uuid> + <uuid>50351de6-7d56-29ab-9d72-c7f9ea3fcfd0</uuid> <memory unit='KiB'>2097152</memory> <currentMemory unit='KiB'>2097152</currentMemory> <vcpu placement='static'>8</vcpu> diff --git a/tests/vmx2xmldata/esx-in-the-wild-9.xml b/tests/vmx2xmldata/esx-in-the-wild-9.xml index 6b4d878ab18a..4fbe141f6c47 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-9.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-9.xml @@ -1,6 +1,6 @@ <domain type='vmware'> <name>v2v-windows-kkulkarn</name> - <uuid>42009372-17da-be73-779d-007ccf1bd228</uuid> + <uuid>5000d2b0-616e-6129-8c06-82b07440a871</uuid> <description>MIQ GUID=b55c806d-99b9-4fa5-bbcc-a4de04f822e5</description> <memory unit='KiB'>16777216</memory> <currentMemory unit='KiB'>16777216</currentMemory> -- 2.54.0

3 8

Limit USB 1.0 (UHCI/OCHI) and 2.0 (EHCI) to non-virt use cases
by Daniel P. Berrangé 13 May '26

13 May '26

QEMU has implemented four generic USB controllers * UHCI - USB 1.0 only * OHCI - USB 1.0 only * EHCI - USB 2.0 only (must have UHCI companions for 1.0 compat) * XHCI - All of USB 3.0, 2.0, 1.0 in one controller We have two variants of XHCI, the generic one (hcd-xhci.c) and the NEC one (hcd-xhci-nec.c) Historically for security reports we have considered all of them in scope of virtualization, since initially with KVM the XHCI impl was not present in QEMU & thus KVM guests used UHCI/EHCI with USB tablet for a period of time All of the USB subsystem is currently orphaned, so we have no dedicated maintainer available to deal with bug reports (volunteers welcome to step up here...) While the need for USB is reduced given the availability of virtio-input, not all guests have drivers out of the box, so at least USB tablet is still interesting for KVM use cases with some modern OS. It is also not that unusual for people to need USB host device assignment with KVM virt to make various pieces of specialized hardware (security tokens, smart cards, custom dongles, etc) available directly to guests. IOW, we can't entirely exclude USB from virtualization use cases IMHO. In terms of virtualization, XHCI is the only impl that is sensible to use. UHCI/OHCI/EHCI all impose an unreasonable CPU load on any guest usage of USB. XHCI should be supported by any guest OS approx the last 15 years, which should be sufficient for virtualization use cases with OS that are not EOL by their vendor. Thus to reduce our maint burden around security bug handling, it is proposed henceforth to classify UHCI, OHCI and EHCI under the non- virtualization use case and thus be excluded from security bug triage processes. No CVEs would be assigned, bugs would be reported publically in gitlab: https://www.qemu.org/docs/master/system/security.html#non-virtualization-us… The XHCI controller (specifically the hcd-xhci.c variant) would remain as our only option for the virtualization use case, with security process applied to bugs & eligible for CVE assignment: https://www.qemu.org/docs/master/system/security.html#virtualization-use-ca… The other NEC XHCI variant (hcd-xhci-nec.c) would also be treated as non-virtualization use case, since it needlessly duplicates hcd-xhci.c impl and thus is only interesting for emulation of this specific HW variant. NB, there are no functional limitations / restrictions from this policy, it is largely just a semantic exercise. From a management application POV, however, it would strongly suggest that guests be configured with XHCI as the default choice if the user asks for USB, and in turn also imply that '-usb' not be used since that is UHCI/OHCI typically. CC'ing libvirt, though it is really a matter for virt-install/virt-manager/ openstack/kubevirt etc above libvirt to pick their defaults for USB controllers. None the less XHCI would still be in search of a maintainer to step forward and handle ongoing development and maint work. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|

4 6

[PATCH 0/2] tests: Stabilize tests when environment leaks FDs into them
by Peter Krempa 13 May '26

13 May '26

Jirka reported to me that one of the test cases I've introduced started to fail for him. It turned out that flatpak "leaks" a FD of the wayland socket to run programs in his env, which broke a test case wanting to use fd '205'. (we use stable FD numbers via dup2 to ensure output stays identical) To prevent this from happening refactor virCommandMassClose and use it in all tests to close everything except stdio. Peter Krempa (2): util: command: Extract non-virCommand related bits from virCommandMassClose testutils: Close FDs leaked into test programs to avoid random breakage src/libvirt_private.syms | 1 + src/util/vircommand.c | 101 +++++++++++++++++++++------------------ src/util/vircommand.h | 3 ++ tests/testutils.c | 10 ++++ 4 files changed, 69 insertions(+), 46 deletions(-) -- 2.54.0

2 7

[RFC PATCH] resctrl: add energy monitoring via resctrl's PERF_PKG_MON
by Jedrzej Wasiukiewicz 13 May '26

13 May '26

Linux kernel 7.0 introduced PERF_PKG_MON support in resctrl filesystem which exposes per-workload energy and performance monitoring. This patch enables per-VM energy monitoring via core_energy (Joules) and activity (Farads) counters. Energy monitors can be configured through new <energytune> element under <cputune> following earlier cachetune and memorytune patterns. Design notes: Energy values from resctrl are floating-point. I added separate dvals/ndvals pair to virResctrlMonitorStats to handle them. I kept them in a single struct for easier integration with performance counters (integers and floats within same monitor) that might be integrated in another patch. The new XML element is <energytune> under <cputune> following earlier pattern for resctrl features (cachetune, memorytune). Energytune doesn't currently support the "tuning" part, only monitoring. I added it as energytune for consistency with cache and memory features, keeping all resctrl handling under cputune. This also makes sense with current resctrl architecture - all monitoring groups are part of an allocation group. This approach allows for easy opt-in/out on the monitoring features. Signed-off-by: Jedrzej Wasiukiewicz <jedrzej.wasiukiewicz(a)intel.com> Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo(a)intel.com> --- Please let me know if there's a better approach for this. I'm happy to rework the design based on feedback. I can also split this patch if necessary. NEWS.rst | 6 + docs/formatdomain.rst | 20 ++ include/libvirt/libvirt-domain.h | 65 +++++++ src/conf/capabilities.c | 42 ++++ src/conf/capabilities.h | 6 + src/conf/domain_conf.c | 99 ++++++++++ src/conf/schemas/capability.rng | 26 +++ src/conf/schemas/domaincommon.rng | 19 ++ src/conf/virconftypes.h | 2 + src/qemu/qemu_driver.c | 70 ++++++- src/util/virresctrl.c | 180 +++++++++++++++--- src/util/virresctrl.h | 12 +- .../genericxml2xmlindata/energytune-basic.xml | 32 ++++ tests/genericxml2xmltest.c | 1 + 14 files changed, 543 insertions(+), 37 deletions(-) create mode 100644 tests/genericxml2xmlindata/energytune-basic.xml diff --git a/NEWS.rst b/NEWS.rst index 105a398ca8..02dda023ed 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,12 @@ v12.4.0 (unreleased) * **New features** + * resctrl: Add energy monitoring via resctrl's PERF_PKG_MON + + Add support for Linux kernel 7.0 feature - energy monitoring via resctrl. + This allows to monitor per-VM energy consumption on supported platforms. + Implemented via ``energytune`` element in ``cputune`` . + * **Improvements** * **Bug fixes** diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 078cd7aa84..db1ca5637a 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -900,6 +900,9 @@ CPU Tuning <memorytune vcpus='0-3'> <node id='0' bandwidth='60'/> </memorytune> + <energytune vcpus='0-3'> + <monitor vcpus='0-3'/> + </energytune> </cputune> ... @@ -1084,6 +1087,23 @@ CPU Tuning responsible for making sure the value makes sense on their system and configuration. +``energytune`` :since:`Since 12.4.0` + Optional ``energytune`` element allows to monitor energy consumption using the + resctrl filesystem on the host. Whether or not is this supported can be + gathered from capabilities where number of monitors and available features are + reported. The required attribute ``vcpus`` specifies to which allocation group + this monitor belongs. A vCPU can only be member of one allocation group and monitor + group. The ``vcpus`` specified by ``energytune`` can be identical to those + specified by ``cachetune`` or ``memorytune``. However they are not allowed to + overlap each other. Supported subelements are: + + ``monitor`` + The optional element ``monitor`` creates the energy monitor for + this allocation group and has the following required attribute: + + ``vcpus`` + vCPU list the monitor applies to. + Memory Allocation ----------------- diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4a8e3114b3..3ba1b27743 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4401,6 +4401,71 @@ struct _virDomainStatsRecord { # define VIR_DOMAIN_STATS_MEMORY_BANDWIDTH_MONITOR_SUFFIX_NODE_SUFFIX_BYTES_TOTAL ".bytes.total" +/** + * VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_COUNT: + * + * The number of energy monitors for this domain, as an unsigned int. + * + * Since: 12.4.0 + */ +# define VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_COUNT "cpu.energy.monitor.count" + +/** + * VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_PREFIX: + * + * Prefix for an individual energy monitor group. Concatenate + * with the monitor index and one of the "cpu.energy.monitor.<i>." suffix + * macros below to form a full parameter name. + * + * Since: 12.4.0 + */ +# define VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_PREFIX "cpu.energy.monitor." + +/** + * VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_NAME: + * + * Name of the monitor group as a string. + * + * Since: 12.4.0 + */ +# define VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_NAME ".name" + +/** + * VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_VCPUS: + * + * vCPU set covered by the monitor group as a string. + * + * Since: 12.4.0 + */ +# define VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_VCPUS ".vcpus" + +/** + * VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_COUNT: + * + * Number of PERF_PKG nodes the monitor group exposes, as an unsigned int. + * + * Since: 12.4.0 + */ +# define VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_COUNT ".pkg.count" + +/** + * VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_PREFIX: + * + * Prefix for a single mon_PERF_PKG node inside a monitor group. + * + * Since: 12.4.0 + */ +# define VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_PREFIX ".pkg." + +/** + * VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_SUFFIX_ID: + * + * Kernel-assigned mon_PERF_PKG node id, as an unsigned int. + * + * Since: 12.4.0 + */ +# define VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_SUFFIX_ID ".id" + /** * VIR_DOMAIN_STATS_DIRTYRATE_CALC_STATUS: * diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1821e36e61..e83de6d1bc 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -263,6 +263,8 @@ virCapsDispose(void *object) virResctrlInfoMonFree(caps->host.memBW.monitor); g_free(caps->host.memBW.nodes); + virResctrlInfoMonFree(caps->host.energy.monitor); + g_free(caps->host.netprefix); g_free(caps->host.pagesSize); virCPUDefFree(caps->host.cpu); @@ -1057,6 +1059,26 @@ virCapabilitiesFormatMemoryBandwidth(virBuffer *buf, } +static int +virCapabilitiesFormatEnergy(virBuffer *buf, + virCapsHostEnergy *energy) +{ + if (!energy->monitor) + return 0; + + virBufferAddLit(buf, "<energy>\n"); + virBufferAdjustIndent(buf, 2); + + if (virCapabilitiesFormatResctrlMonitor(buf, energy->monitor) < 0) + return -1; + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</energy>\n"); + + return 0; +} + + static int virCapabilitiesFormatHostXML(virCapsHost *host, virBuffer *buf) @@ -1156,6 +1178,9 @@ virCapabilitiesFormatHostXML(virCapsHost *host, if (virCapabilitiesFormatMemoryBandwidth(buf, &host->memBW) < 0) return -1; + if (virCapabilitiesFormatEnergy(buf, &host->energy) < 0) + return -1; + for (i = 0; i < host->nsecModels; i++) { virBufferAddLit(buf, "<secmodel>\n"); virBufferAdjustIndent(buf, 2); @@ -2143,6 +2168,20 @@ virCapabilitiesInitResctrlMemory(virCaps *caps) } +static int +virCapabilitiesInitEnergy(virCaps *caps) +{ + const char *prefix = virResctrlMonitorPrefixTypeToString( + VIR_RESCTRL_MONITOR_TYPE_ENERGY); + + if (virResctrlInfoGetMonitorPrefix(caps->host.resctrl, prefix, + &caps->host.energy.monitor) < 0) + return -1; + + return 0; +} + + int virCapabilitiesInitCaches(virCaps *caps) { @@ -2294,6 +2333,9 @@ virCapabilitiesInitCaches(virCaps *caps) &caps->host.cache.monitor) < 0) return -1; + if (virCapabilitiesInitEnergy(caps) < 0) + return -1; + return 0; } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index daea835817..0482e4297a 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -162,6 +162,10 @@ struct _virCapsHostMemBW { virResctrlInfoMon *monitor; }; +struct _virCapsHostEnergy { + virResctrlInfoMon *monitor; +}; + struct _virCapsHost { virArch arch; size_t nfeatures; @@ -184,6 +188,8 @@ struct _virCapsHost { virCapsHostMemBW memBW; + virCapsHostEnergy energy; + size_t nsecModels; virCapsHostSecModel *secModels; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3497e84bf5..946b34abf3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19467,6 +19467,57 @@ virDomainMemorytuneDefParse(virDomainDef *def, } +static int +virDomainEnergytuneDefParse(virDomainDef *def, + xmlXPathContextPtr ctxt, + xmlNodePtr node, + unsigned int flags) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + virDomainResctrlDef *resctrl = NULL; + virDomainResctrlDef *newresctrl = NULL; + g_autoptr(virBitmap) vcpus = NULL; + g_autoptr(virResctrlAlloc) alloc = NULL; + size_t nmons; + int ret = -1; + + ctxt->node = node; + + if (virDomainResctrlParseVcpus(def, node, &vcpus) < 0) + return -1; + + if (virBitmapIsAllClear(vcpus)) + return 0; + + if (virDomainResctrlVcpuMatch(def, vcpus, &resctrl) < 0) + return -1; + + if (resctrl) { + alloc = virObjectRef(resctrl->alloc); + } else { + if (!(alloc = virResctrlAllocNew())) + return -1; + if (!(newresctrl = virDomainResctrlNew(node, alloc, vcpus, flags))) + return -1; + resctrl = newresctrl; + } + + nmons = resctrl->nmonitors; + if (virDomainResctrlMonDefParse(def, ctxt, node, + VIR_RESCTRL_MONITOR_TYPE_ENERGY, + resctrl) < 0) + goto cleanup; + + if (newresctrl && resctrl->nmonitors > nmons) + VIR_APPEND_ELEMENT(def->resctrls, def->nresctrls, newresctrl); + + ret = 0; + cleanup: + virDomainResctrlDefFree(newresctrl); + return ret; +} + + static int virDomainDefTunablesParse(virDomainDef *def, xmlXPathContextPtr ctxt, @@ -19671,6 +19722,15 @@ virDomainDefTunablesParse(virDomainDef *def, } VIR_FREE(nodes); + if ((n = virXPathNodeSet("./cputune/energytune", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + if (virDomainEnergytuneDefParse(def, ctxt, nodes[i], flags) < 0) + return -1; + } + VIR_FREE(nodes); + return 0; } @@ -28721,6 +28781,42 @@ virDomainMemorytuneDefFormat(virBuffer *buf, return 0; } + +static int +virDomainEnergytuneDefFormat(virBuffer *buf, + virDomainResctrlDef *resctrl, + unsigned int flags) +{ + g_auto(virBuffer) childrenBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_autofree char *vcpus = NULL; + size_t i; + + for (i = 0; i < resctrl->nmonitors; i++) { + if (virDomainResctrlMonDefFormatHelper(resctrl->monitors[i], + VIR_RESCTRL_MONITOR_TYPE_ENERGY, + &childrenBuf) < 0) + return -1; + } + + if (!virBufferUse(&childrenBuf)) + return 0; + + vcpus = virBitmapFormat(resctrl->vcpus); + virBufferAsprintf(&attrBuf, " vcpus='%s'", vcpus); + + if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) { + const char *alloc_id = virResctrlAllocGetID(resctrl->alloc); + if (!alloc_id) + return -1; + + virBufferAsprintf(&attrBuf, " id='%s'", alloc_id); + } + + virXMLFormatElement(buf, "energytune", &attrBuf, &childrenBuf); + return 0; +} + static int virDomainCputuneDefFormat(virBuffer *buf, virDomainDef *def, @@ -28821,6 +28917,9 @@ virDomainCputuneDefFormat(virBuffer *buf, for (i = 0; i < def->nresctrls; i++) virDomainMemorytuneDefFormat(&childrenBuf, def->resctrls[i], flags); + for (i = 0; i < def->nresctrls; i++) + virDomainEnergytuneDefFormat(&childrenBuf, def->resctrls[i], flags); + virXMLFormatElement(buf, "cputune", NULL, &childrenBuf); return 0; diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng index 8ef6e9a282..2aa711178f 100644 --- a/src/conf/schemas/capability.rng +++ b/src/conf/schemas/capability.rng @@ -45,6 +45,9 @@ <optional> <ref name="memory_bandwidth"/> </optional> + <optional> + <ref name="energy"/> + </optional> <zeroOrMore> <ref name="secmodel"/> </zeroOrMore> @@ -333,6 +336,29 @@ </data> </define> + <define name="energyMonitorFeature"> + <data type="string"> + <param name="pattern">[a-zA-Z0-9\-_]+</param> + </data> + </define> + + <define name="energy"> + <element name="energy"> + <element name="monitor"> + <attribute name="maxMonitors"> + <ref name="unsignedInt"/> + </attribute> + <oneOrMore> + <element name="feature"> + <attribute name="name"> + <ref name="energyMonitorFeature"/> + </attribute> + </element> + </oneOrMore> + </element> + </element> + </define> + <define name="guestcaps"> <element name="guest"> <ref name="ostype"/> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 8c03e14d37..eb365a83b5 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -1293,6 +1293,25 @@ </oneOrMore> </element> </zeroOrMore> + <zeroOrMore> + <element name="energytune"> + <attribute name="vcpus"> + <ref name="cpuset"/> + </attribute> + <optional> + <attribute name="id"> + <data type="string"/> + </attribute> + </optional> + <oneOrMore> + <element name="monitor"> + <attribute name="vcpus"> + <ref name="cpuset"/> + </attribute> + </element> + </oneOrMore> + </element> + </zeroOrMore> </interleave> </element> </define> diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index 0596791a4d..f1a200bfe2 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -52,6 +52,8 @@ typedef struct _virCapsHostMemBW virCapsHostMemBW; typedef struct _virCapsHostMemBWNode virCapsHostMemBWNode; +typedef struct _virCapsHostEnergy virCapsHostEnergy; + typedef struct _virCapsHostNUMA virCapsHostNUMA; typedef struct _virCapsHostNUMACell virCapsHostNUMACell; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 529e9fe3be..3fafaaea19 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16917,11 +16917,11 @@ qemuDomainFreeResctrlMonData(virQEMUResctrlMonData *resdata) * returns an error, the caller is also required to call * qemuDomainFreeResctrlMonData to free each element in the * *@resdata array and then the array itself. - * @tag: Could be VIR_RESCTRL_MONITOR_TYPE_CACHE for getting cache statistics - * from @dom cache monitors. VIR_RESCTRL_MONITOR_TYPE_MEMBW for - * getting memory bandwidth statistics from memory bandwidth monitors. + * @tag: VIR_RESCTRL_MONITOR_TYPE_CACHE for getting cache statistics. + * VIR_RESCTRL_MONITOR_TYPE_MEMBW for getting memory bandwidth statistics. + * VIR_RESCTRL_MONITOR_TYPE_ENERGY for getting energy statistics. * - * Get cache or memory bandwidth statistics from @dom monitors. + * Get cache, memory bandwidth or energy statistics from @dom monitors. * * Returns -1 on failure, or 0 on success. */ @@ -16951,6 +16951,10 @@ qemuDomainGetResctrlMonData(virQEMUDriver *driver, if (caps->host.memBW.monitor) features = caps->host.memBW.monitor->features; break; + case VIR_RESCTRL_MONITOR_TYPE_ENERGY: + if (caps->host.energy.monitor) + features = caps->host.energy.monitor->features; + break; case VIR_RESCTRL_MONITOR_TYPE_UNSUPPORT: case VIR_RESCTRL_MONITOR_TYPE_LAST: virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -17066,6 +17070,62 @@ qemuDomainGetStatsMemoryBandwidth(virQEMUDriver *driver, } +static void +qemuDomainGetStatsEnergy(virQEMUDriver *driver, + virDomainObj *dom, + virTypedParamList *params) +{ + g_autofree virQEMUResctrlMonData **resdata = NULL; + size_t nresdata = 0; + size_t i = 0; + size_t j = 0; + size_t k = 0; + + if (!virDomainObjIsActive(dom)) + return; + + if (qemuDomainGetResctrlMonData(driver, dom, &resdata, &nresdata, + VIR_RESCTRL_MONITOR_TYPE_ENERGY) < 0) { + virResetLastError(); + return; + } + + if (nresdata == 0) + return; + + virTypedParamListAddUInt(params, nresdata, + VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_COUNT); + + for (i = 0; i < nresdata; i++) { + virTypedParamListAddString(params, resdata[i]->name, + VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_PREFIX "%zu" VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_NAME, i); + virTypedParamListAddString(params, resdata[i]->vcpus, + VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_PREFIX "%zu" VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_VCPUS, i); + virTypedParamListAddUInt(params, resdata[i]->nstats, + VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_PREFIX "%zu" VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_COUNT, i); + + for (j = 0; j < resdata[i]->nstats; j++) { + char **features = resdata[i]->stats[j]->features; + + virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, + VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_PREFIX "%zu" VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_PREFIX "%zu" VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_SUFFIX_ID, i, j); + + for (k = 0; features[k]; k++) { + if (k >= resdata[i]->stats[j]->ndvals) + break; + + virTypedParamListAddDouble(params, resdata[i]->stats[j]->dvals[k], + VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_PREFIX "%zu" VIR_DOMAIN_STATS_CPU_ENERGY_MONITOR_SUFFIX_PKG_PREFIX "%zu" ".%s", i, j, + features[k]); + } + } + } + + for (i = 0; i < nresdata; i++) + qemuDomainFreeResctrlMonData(resdata[i]); +} + + static void qemuDomainGetStatsCpuCache(virQEMUDriver *driver, virDomainObj *dom, @@ -17277,6 +17337,8 @@ qemuDomainGetStatsCpu(virQEMUDriver *driver, qemuDomainGetStatsCpuCache(driver, dom, params); + qemuDomainGetStatsEnergy(driver, dom, params); + qemuDomainGetStatsCpuHaltPollTime(dom, params, privflags); } diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 8f33a85a56..32cbfb6c2e 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -79,15 +79,24 @@ VIR_ENUM_IMPL(virResctrl, "DATA", ); -/* Monitor feature name prefix mapping for monitor naming */ +/* Monitor feature prefix/type mapping for monitor naming */ VIR_ENUM_IMPL(virResctrlMonitorPrefix, VIR_RESCTRL_MONITOR_TYPE_LAST, "__unsupported__", "llc_", "mbm_", + "energy", ); +/* PERF_PKG_MON features that report energy data (floating-point). */ +static const char *virResctrlEnergyFeatures[] = { + "core_energy", + "activity", + NULL, +}; + + /* All private typedefs so that they exist for all later definitions. This way * structs can be included in one or another without reorganizing the code every * time. */ @@ -183,6 +192,8 @@ struct _virResctrlInfo { virResctrlInfoMemBW *membw_info; virResctrlInfoMongrp *monitor_info; + + virResctrlInfoMongrp *perf_monitor_info; }; static void @@ -235,10 +246,14 @@ virResctrlInfoDispose(void *obj) if (resctrl->monitor_info) g_strfreev(resctrl->monitor_info->features); + if (resctrl->perf_monitor_info) + g_strfreev(resctrl->perf_monitor_info->features); + virResctrlInfoMemBWFree(resctrl->membw_info); g_free(resctrl->levels); g_free(resctrl->monitor_info); + g_free(resctrl->perf_monitor_info); } @@ -771,6 +786,52 @@ virResctrlGetMonitorInfo(virResctrlInfo *resctrl) } +static int +virResctrlGetPerfMonitorInfo(virResctrlInfo *resctrl) +{ + int rv = -1; + g_autofree char *featurestr = NULL; + g_autofree virResctrlInfoMongrp *info_monitor = NULL; + + info_monitor = g_new0(virResctrlInfoMongrp, 1); + + rv = virFileReadValueUint(&info_monitor->max_monitor, + SYSFS_RESCTRL_PATH + "/info/PERF_PKG_MON/num_rmids"); + if (rv == -2) { + VIR_INFO("The file '" SYSFS_RESCTRL_PATH "/info/PERF_PKG_MON/num_rmids' " + "does not exist"); + return 0; + } else if (rv < 0) { + return -1; + } + + rv = virFileReadValueString(&featurestr, + SYSFS_RESCTRL_PATH + "/info/PERF_PKG_MON/mon_features"); + if (rv == -2) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get mon_features from resctrl PERF_PKG_MON")); + if (rv < 0) + return -1; + + if (!*featurestr) { + VIR_WARN("Got empty feature list from PERF_PKG_MON; " + "AET energy monitoring will not be available"); + return 0; + } + + info_monitor->features = g_strsplit(featurestr, "\n", 0); + info_monitor->nfeatures = g_strv_length(info_monitor->features); + VIR_DEBUG("Resctrl supported %zd AET monitoring features", + info_monitor->nfeatures); + + resctrl->perf_monitor_info = g_steal_pointer(&info_monitor); + + return 0; +} + + static int virResctrlGetInfo(virResctrlInfo *resctrl) { @@ -790,6 +851,9 @@ virResctrlGetInfo(virResctrlInfo *resctrl) if ((ret = virResctrlGetMonitorInfo(resctrl)) < 0) return -1; + if ((ret = virResctrlGetPerfMonitorInfo(resctrl)) < 0) + return -1; + return 0; } @@ -830,6 +894,9 @@ virResctrlInfoIsEmpty(virResctrlInfo *resctrl) if (resctrl->monitor_info) return false; + if (resctrl->perf_monitor_info) + return false; + for (i = 0; i < resctrl->nlevels; i++) { virResctrlInfoPerLevel *i_level = resctrl->levels[i]; @@ -986,13 +1053,6 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfo *resctrl, if (virResctrlInfoIsEmpty(resctrl)) return 0; - mongrp_info = resctrl->monitor_info; - - if (!mongrp_info) { - VIR_INFO("Monitor is not supported in host"); - return 0; - } - for (i = 0; i < VIR_RESCTRL_MONITOR_TYPE_LAST; i++) { if (STREQ(prefix, virResctrlMonitorPrefixTypeToString(i))) { mon = g_new0(virResctrlInfoMon, 1); @@ -1008,6 +1068,19 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfo *resctrl, return -1; } + if (mon->type == VIR_RESCTRL_MONITOR_TYPE_ENERGY) + mongrp_info = resctrl->perf_monitor_info; + else + mongrp_info = resctrl->monitor_info; + + if (!mongrp_info) { + VIR_INFO("Monitor prefix '%s' is not supported in host", prefix); + virResctrlInfoMonFree(*monitor); + *monitor = NULL; + ret = 0; + goto cleanup; + } + mon->max_monitor = mongrp_info->max_monitor; if (mon->type == VIR_RESCTRL_MONITOR_TYPE_CACHE) { @@ -1018,8 +1091,12 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfo *resctrl, mon->features = g_new0(char *, mongrp_info->nfeatures + 1); for (i = 0; i < mongrp_info->nfeatures; i++) { - if (STRPREFIX(mongrp_info->features[i], prefix)) + if (mon->type == VIR_RESCTRL_MONITOR_TYPE_ENERGY) { + if (g_strv_contains(virResctrlEnergyFeatures, mongrp_info->features[i])) + mon->features[mon->nfeatures++] = g_strdup(mongrp_info->features[i]); + } else if (STRPREFIX(mongrp_info->features[i], prefix)) { mon->features[mon->nfeatures++] = g_strdup(mongrp_info->features[i]); + } } mon->features = g_renew(char *, mon->features, mon->nfeatures + 1); @@ -2558,7 +2635,7 @@ virResctrlMonitorStatsSorter(const void *a, * memory bandwidth usage data. * @nstats: A size_t pointer to hold the returned array length of @stats * - * Get cache or memory bandwidth utilization information. + * Get cache, memory bandwidth or energy utilization information. * * Returns 0 on success, -1 on error. */ @@ -2593,6 +2670,7 @@ virResctrlMonitorGetStats(virResctrlMonitor *monitor, while (virDirRead(dirp, &ent, datapath) > 0) { g_autofree char *filepath = NULL; char *node_id = NULL; + bool is_energy = false; /* Looking for directory that contains resource utilization * information file. The directory name is arranged in format @@ -2605,18 +2683,17 @@ virResctrlMonitorGetStats(virResctrlMonitor *monitor, if (!virFileIsDir(filepath)) continue; - /* Looking for directory has a prefix 'mon_L' */ - if (!(node_id = STRSKIP(ent->d_name, "mon_L"))) - continue; - - /* Looking for directory has another '_' */ - node_id = strchr(node_id, '_'); - if (!node_id) - continue; - - /* Skip the character '_' */ - if (!(node_id = STRSKIP(node_id, "_"))) + if ((node_id = STRSKIP(ent->d_name, "mon_PERF_PKG_"))) { + is_energy = true; + } else if ((node_id = STRSKIP(ent->d_name, "mon_L"))) { + node_id = strchr(node_id, '_'); + if (!node_id) + continue; + if (!(node_id = STRSKIP(node_id, "_"))) + continue; + } else { continue; + } stat = g_new0(virResctrlMonitorStats, 1); stat->features = g_new0(char *, nresources + 1); @@ -2626,21 +2703,61 @@ virResctrlMonitorGetStats(virResctrlMonitor *monitor, goto cleanup; for (i = 0; resources[i]; i++) { - rv = virFileReadValueUllong(&val, "%s/%s/%s", datapath, - ent->d_name, resources[i]); - if (rv == -2) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("File '%1$s/%2$s/%3$s' does not exist."), - datapath, ent->d_name, resources[i]); + if (is_energy) { + g_autofree char *valstr = NULL; + double dval = 0.0; + char *endp = NULL; + + rv = virFileReadValueString(&valstr, "%s/%s/%s", datapath, + ent->d_name, resources[i]); + if (rv == -2) { + if (i == 0) + break; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("File '%1$s/%2$s/%3$s' does not exist."), + datapath, ent->d_name, resources[i]); + goto cleanup; + } + if (rv < 0) + goto cleanup; + + g_strstrip(valstr); + errno = 0; + dval = g_ascii_strtod(valstr, &endp); + if (endp == valstr || *endp != '\0' || errno != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse resctrl monitor value '%1$s' from '%2$s/%3$s/%4$s'"), + valstr, datapath, ent->d_name, resources[i]); + goto cleanup; + } + + VIR_APPEND_ELEMENT(stat->dvals, stat->ndvals, dval); + } else { + rv = virFileReadValueUllong(&val, "%s/%s/%s", datapath, + ent->d_name, resources[i]); + if (rv == -2) { + if (i == 0) + break; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("File '%1$s/%2$s/%3$s' does not exist."), + datapath, ent->d_name, resources[i]); + goto cleanup; + } + if (rv < 0) + goto cleanup; + + VIR_APPEND_ELEMENT(stat->vals, stat->nvals, val); } - if (rv < 0) - goto cleanup; - - VIR_APPEND_ELEMENT(stat->vals, stat->nvals, val); stat->features[i] = g_strdup(resources[i]); } + if (resources[i]) { + virResctrlMonitorStatsFree(stat); + stat = NULL; + continue; + } + VIR_APPEND_ELEMENT(*stats, *nstats, stat); } @@ -2665,5 +2782,6 @@ virResctrlMonitorStatsFree(virResctrlMonitorStats *stat) g_strfreev(stat->features); g_free(stat->vals); + g_free(stat->dvals); g_free(stat); } diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index c70b112864..0f38db47cf 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -38,6 +38,7 @@ typedef enum { VIR_RESCTRL_MONITOR_TYPE_UNSUPPORT, VIR_RESCTRL_MONITOR_TYPE_CACHE, VIR_RESCTRL_MONITOR_TYPE_MEMBW, + VIR_RESCTRL_MONITOR_TYPE_ENERGY, VIR_RESCTRL_MONITOR_TYPE_LAST } virResctrlMonitorType; @@ -196,11 +197,18 @@ struct _virResctrlMonitorStats { /* @features is a NULL terminal string list tracking the statistical record * name.*/ char **features; - /* @vals store the statistical record values and @val[0] is the value for - * @features[0], @val[1] for@features[1] ... respectively */ + /* @vals store the statistical record values for integer-valued resources + * (cache occupancy, memory bandwidth). Entries correspond 1:1 with + * @features; empty when the resource reports floating-point data. */ unsigned long long *vals; /* The length of @vals array */ size_t nvals; + /* @dvals store double-precision values for floating-point resources + * (energy in Joules). Entries correspond 1:1 with + * @features; empty when the resource reports integer data. */ + double *dvals; + /* The length of @dvals array */ + size_t ndvals; }; virResctrlMonitor * diff --git a/tests/genericxml2xmlindata/energytune-basic.xml b/tests/genericxml2xmlindata/energytune-basic.xml new file mode 100644 index 0000000000..4ee4bedb68 --- /dev/null +++ b/tests/genericxml2xmlindata/energytune-basic.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>4</vcpu> + <cputune> + <energytune vcpus='0-1'> + <monitor vcpus='0-1'/> + </energytune> + <energytune vcpus='3'> + <monitor vcpus='3'/> + </energytune> + </cputune> + <os> + <type arch='i686' 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-i386</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 6be694cac5..bcc9b07a13 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -210,6 +210,7 @@ mymain(void) DO_TEST("cachetune-small"); DO_TEST("cachetune-cdp"); DO_TEST("cachetune"); + DO_TEST("energytune-basic"); DO_TEST_DIFFERENT("cachetune-extra-tunes"); DO_TEST_FAIL_INACTIVE("cachetune-colliding-allocs"); DO_TEST_FAIL_INACTIVE("cachetune-colliding-tunes"); -- 2.34.1 --------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN. Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

3 2

[RFC PATCH] bhyve: add agent support
by Roman Bogorodskiy 12 May '26

12 May '26

Implement QEMU Guest Agent support for bhyve. In bhyve it can configured using the virtio-console device. This change covers only two APIs using the agent: - DomainQemuAgentCommand -- the most generic one. - DomainGetHostname -- extended to support not only DHCP lease source, but an agent as well. There are two things that I'm not sure about this patch. - The protocol files were updated to make DomainQemuAgentCommand generic instead of Qemu specific. QEMU_PROC_DOMAIN_AGENT_COMMAND was removed in favor of REMOTE_PROC_DOMAIN_QEMU_AGENT_COMMAND. Even though the protocol is documented as private, I'm not sure how desired this change is. - src/qemu/bhyve_qemu_agent.c and src/qemu/qemu_agent.c should share the same implementation. Ideally this should live somewhere in src/util/virqemuagent.c, but as it is using things from conf/, such as virDomainObj, it cannot be moved as is. I considered extracting a simpler data structure that will not use the conf/ types, but it didn't work very well for me and I didn't like the way it looks in the driver code. Maybe I'm missing some better approach? Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- po/POTFILES | 1 + src/bhyve/bhyve_domain.c | 32 + src/bhyve/bhyve_domain.h | 14 + src/bhyve/bhyve_driver.c | 179 ++++- src/bhyve/bhyve_process.c | 117 ++++ src/bhyve/bhyve_process.h | 4 + src/bhyve/bhyve_qemu_agent.c | 1259 ++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_qemu_agent.h | 197 ++++++ src/bhyve/meson.build | 5 +- src/remote/qemu_protocol.x | 18 - src/remote/remote_protocol.x | 20 +- 11 files changed, 1822 insertions(+), 24 deletions(-) create mode 100644 src/bhyve/bhyve_qemu_agent.c create mode 100644 src/bhyve/bhyve_qemu_agent.h diff --git a/po/POTFILES b/po/POTFILES index a5f8705eb8..1d80d9a57a 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -19,6 +19,7 @@ src/bhyve/bhyve_firmware.c src/bhyve/bhyve_monitor.c src/bhyve/bhyve_parse_command.c src/bhyve/bhyve_process.c +src/bhyve/bhyve_qemu_agent.c src/ch/ch_alias.c src/ch/ch_conf.c src/ch/ch_domain.c diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 832a9b58d1..6367985efc 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -41,6 +41,7 @@ bhyveDomainObjPrivateAlloc(void *opaque) { bhyveDomainObjPrivate *priv = g_new0(bhyveDomainObjPrivate, 1); + priv->agentTimeout = 30; priv->driver = opaque; return priv; @@ -663,3 +664,34 @@ virXMLNamespace virBhyveDriverDomainXMLNamespace = { .uri = "http://libvirt.org/schemas/domain/bhyve/1.0", }; + + +int +virBhyveDomainObjStartWorker(virDomainObj *dom) +{ + bhyveDomainObjPrivate *priv = dom->privateData; + + if (!priv->eventThread) { + g_autofree char *threadName = g_strdup_printf("vm-%s", dom->def->name); + if (!(priv->eventThread = virEventThreadNew(threadName))) + return -1; + } + + return 0; +} + + +void +virBhyveDomainObjStopWorker(virDomainObj *dom) +{ + bhyveDomainObjPrivate *priv = dom->privateData; + virEventThread *eventThread; + + if (!priv->eventThread) + return; + + eventThread = g_steal_pointer(&priv->eventThread); + virObjectUnlock(dom); + g_object_unref(eventThread); + virObjectLock(dom); +} diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h index 5a539bc4c0..8e3663f4c0 100644 --- a/src/bhyve/bhyve_domain.h +++ b/src/bhyve/bhyve_domain.h @@ -22,8 +22,10 @@ #include "domain_addr.h" #include "domain_conf.h" +#include "vireventthread.h" #include "bhyve_monitor.h" +#include "bhyve_qemu_agent.h" typedef struct _bhyveDomainObjPrivate bhyveDomainObjPrivate; struct _bhyveDomainObjPrivate { @@ -33,10 +35,22 @@ struct _bhyveDomainObjPrivate { bool persistentAddrs; bhyveMonitor *mon; + + qemuAgent *agent; + bool agentError; + int agentTimeout; + + virEventThread *eventThread; }; +#define BHYVE_DOMAIN_PRIVATE(vm) \ + ((bhyveDomainObjPrivate *) (vm)->privateData) + virDomainXMLOption *virBhyveDriverCreateXMLConf(struct _bhyveConn *); extern virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks; extern virDomainDefParserConfig virBhyveDriverDomainDefParserConfig; extern virXMLNamespace virBhyveDriverDomainXMLNamespace; + +int virBhyveDomainObjStartWorker(virDomainObj *dom); +void virBhyveDomainObjStopWorker(virDomainObj *dom); diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index c8dd1a728a..8bffbbec43 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1895,6 +1895,165 @@ bhyveDomainInterfaceAddresses(virDomainPtr domain, } +static qemuAgent * +bhyveDomainObjEnterAgent(virDomainObj *obj) +{ + bhyveDomainObjPrivate *priv = obj->privateData; + qemuAgent *agent = priv->agent; + + VIR_DEBUG("Entering agent (agent=%p vm=%p name=%s)", + priv->agent, obj, obj->def->name); + + virObjectLock(agent); + virObjectRef(agent); + virObjectUnlock(obj); + + return agent; +} + + +static void +bhyveDomainObjExitAgent(virDomainObj *obj, qemuAgent *agent) +{ + virObjectUnlock(agent); + virObjectUnref(agent); + virObjectLock(obj); + + VIR_DEBUG("Exited agent (agent=%p vm=%p name=%s)", + agent, obj, obj->def->name); +} + + +static bool +bhyveDomainAgentAvailable(virDomainObj *vm, + bool reportError) +{ + bhyveDomainObjPrivate *priv = vm->privateData; + + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { + if (reportError) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + } + return false; + } + + if (!priv->agent) { + if (bhyveFindAgentConfig(vm->def)) { + if (reportError) { + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not connected")); + } + return false; + } else { + if (reportError) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + } + return false; + } + } + return true; +} + + +static int +bhyveDomainEnsureAgent(virDomainObj *vm, + bool reportError) +{ + bhyveDomainObjPrivate *priv = vm->privateData; + + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { + if (reportError) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + } + return -1; + } + + if (priv->agent) + return 0; + + if (!priv->eventThread && + virBhyveDomainObjStartWorker(vm) < 0) + return -1; + + if (bhyveConnectAgent(NULL, vm) < 0) + return -1; + + return 0; +} + + +static int +bhyveDomainGetHostnameAgent(virDomainObj *vm, + char **hostname) +{ + qemuAgent *agent; + int ret = -1; + + if (virDomainObjBeginAgentJob(vm, VIR_AGENT_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + if (bhyveDomainEnsureAgent(vm, true) < 0) + goto endjob; + + agent = bhyveDomainObjEnterAgent(vm); + ret = qemuAgentGetHostname(agent, hostname, true); + bhyveDomainObjExitAgent(vm, agent); + + endjob: + virDomainObjEndAgentJob(vm); + return ret; +} + + +static char * +bhyveDomainQemuAgentCommand(virDomainPtr domain, + const char *cmd, + int timeout, + unsigned int flags) +{ + virDomainObj *vm; + int ret = -1; + char *result = NULL; + qemuAgent *agent; + + virCheckFlags(0, NULL); + + if (!(vm = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainQemuAgentCommandEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (virDomainObjBeginAgentJob(vm, VIR_AGENT_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + if (!bhyveDomainAgentAvailable(vm, true)) + goto endjob; + + agent = bhyveDomainObjEnterAgent(vm); + ret = qemuAgentArbitraryCommand(agent, cmd, &result, timeout); + bhyveDomainObjExitAgent(vm, agent); + if (ret < 0) + VIR_FREE(result); + + endjob: + virDomainObjEndAgentJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return result; +} + + static int bhyveDomainGetHostnameLease(virDomainObj *vm, char **hostname) @@ -1961,7 +2120,15 @@ bhyveDomainGetHostname(virDomainPtr domain, virDomainObj *vm = NULL; char *hostname = NULL; - virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE, NULL); + virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE | + VIR_DOMAIN_GET_HOSTNAME_AGENT, NULL); + + VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_GET_HOSTNAME_LEASE, + VIR_DOMAIN_GET_HOSTNAME_AGENT, + NULL); + + if (!(flags & VIR_DOMAIN_GET_HOSTNAME_AGENT)) + flags |= VIR_DOMAIN_GET_HOSTNAME_LEASE; if (!(vm = bhyveDomObjFromDomain(domain))) return NULL; @@ -1969,8 +2136,13 @@ bhyveDomainGetHostname(virDomainPtr domain, if (virDomainGetHostnameEnsureACL(domain->conn, vm->def) < 0) goto cleanup; - if (bhyveDomainGetHostnameLease(vm, &hostname) < 0) - goto cleanup; + if (flags & VIR_DOMAIN_GET_HOSTNAME_LEASE) { + if (bhyveDomainGetHostnameLease(vm, &hostname) < 0) + goto cleanup; + } else if (flags & VIR_DOMAIN_GET_HOSTNAME_AGENT) { + if (bhyveDomainGetHostnameAgent(vm, &hostname) < 0) + goto cleanup; + } if (!hostname) { virReportError(VIR_ERR_NO_HOSTNAME, @@ -2052,6 +2224,7 @@ static virHypervisorDriver bhyveHypervisorDriver = { .domainGetVcpuPinInfo = bhyveDomainGetVcpuPinInfo, /* 12.1.0 */ .domainInterfaceAddresses = bhyveDomainInterfaceAddresses, /* 12.3.0 */ .domainGetHostname = bhyveDomainGetHostname, /* 12.3.0 */ + .domainQemuAgentCommand = bhyveDomainQemuAgentCommand, /* 12.4.0 */ }; diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 6078d995cd..fc97731510 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -171,6 +171,117 @@ bhyveSetResourceLimits(struct _bhyveConn *driver, virDomainObj *vm) return 0; } +virDomainChrDef * +bhyveFindAgentConfig(virDomainDef *def) +{ + size_t i; + + for (i = 0; i < def->nchannels; i++) { + virDomainChrDef *channel = def->channels[i]; + + if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) + continue; + + + if (STREQ_NULLABLE(channel->target.name, "org.qemu.guest_agent.0")) { + return channel; + } + } + + return NULL; +} + +static void +qemuProcessHandleAgentEOF(qemuAgent *agent, + virDomainObj *vm) +{ + bhyveDomainObjPrivate *priv; + + virObjectLock(vm); + VIR_INFO("Received EOF from agent on %p '%s'", vm, vm->def->name); + + priv = vm->privateData; + + if (!priv->agent) { + VIR_DEBUG("Agent freed already"); + goto unlock; + } + + qemuAgentClose(agent); + priv->agent = NULL; + priv->agentError = false; + + virObjectUnlock(vm); + return; + + unlock: + virObjectUnlock(vm); + return; +} + +/* + * This is invoked when there is some kind of error + * parsing data to/from the agent. The VM can continue + * to run, but no further agent commands will be + * allowed + */ +static void +qemuProcessHandleAgentError(qemuAgent *agent G_GNUC_UNUSED, + virDomainObj *vm) +{ + bhyveDomainObjPrivate *priv; + + virObjectLock(vm); + VIR_INFO("Received error from agent on %p '%s'", vm, vm->def->name); + + priv = vm->privateData; + + priv->agentError = true; + + virObjectUnlock(vm); +} + +static qemuAgentCallbacks agentCallbacks = { + .eofNotify = qemuProcessHandleAgentEOF, + .errorNotify = qemuProcessHandleAgentError, +}; + +int +bhyveConnectAgent(struct _bhyveConn *driver G_GNUC_UNUSED, virDomainObj *vm) +{ + bhyveDomainObjPrivate *priv = vm->privateData; + qemuAgent *agent = NULL; + virDomainChrDef *config = bhyveFindAgentConfig(vm->def); + + if (!config) + return 0; + + if (priv->agent) + return 0; + + agent = qemuAgentOpen(vm, + config->source, + virEventThreadGetContext(priv->eventThread), + &agentCallbacks); + + if (!virDomainObjIsActive(vm)) { + qemuAgentClose(agent); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest crashed while connecting to the guest agent")); + return -1; + } + + priv->agent = agent; + if (!priv->agent) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name); + priv->agentError = true; + virResetLastError(); + } + + return 0; +} + + static int virBhyveProcessStartImpl(struct _bhyveConn *driver, virDomainObj *vm, @@ -293,6 +404,9 @@ virBhyveProcessStartImpl(struct _bhyveConn *driver, virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); priv->mon = bhyveMonitorOpen(vm, driver); + if (virBhyveDomainObjStartWorker(vm) < 0) + goto cleanup; + if (virDomainObjSave(vm, driver->xmlopt, BHYVE_STATE_DIR) < 0) goto cleanup; @@ -714,6 +828,9 @@ virBhyveProcessReconnect(virDomainObj *vm, virDomainNetNotifyActualDevice(conn, vm->def, net); } + if (virBhyveDomainObjStartWorker(vm) < 0) + goto cleanup; + cleanup: if (ret < 0) { /* If VM is reported to be in active state, but we cannot find diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h index 5e0acc810c..bf82f748a6 100644 --- a/src/bhyve/bhyve_process.h +++ b/src/bhyve/bhyve_process.h @@ -56,6 +56,10 @@ int virBhyveGetDomainTotalCpuStats(virDomainObj *vm, void virBhyveProcessReconnectAll(struct _bhyveConn *driver); +int bhyveConnectAgent(struct _bhyveConn *driver, virDomainObj *vm); + +virDomainChrDef *bhyveFindAgentConfig(virDomainDef *def); + typedef enum { VIR_BHYVE_PROCESS_START_AUTODESTROY = 1 << 0, } bhyveProcessStartFlags; diff --git a/src/bhyve/bhyve_qemu_agent.c b/src/bhyve/bhyve_qemu_agent.c new file mode 100644 index 0000000000..619d8baf57 --- /dev/null +++ b/src/bhyve/bhyve_qemu_agent.c @@ -0,0 +1,1259 @@ +/* + * bhyve_qemu_agent.c: interaction with QEMU guest agent + * + * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include <poll.h> +#include <unistd.h> +#include <fcntl.h> +#include <sys/time.h> +#include <gio/gio.h> + +#include "bhyve_qemu_agent.h" +#include "bhyve_domain.h" +#include "viralloc.h" +#include "virlog.h" +#include "virerror.h" +#include "virjson.h" +#include "virfile.h" +#include "virtime.h" +#include "virobject.h" +#include "virstring.h" +#include "virenum.h" +#include "virutil.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("bhyve.bhyve_qemu_agent"); + +#define LINE_ENDING "\n" + +/* We read from QEMU until seeing a \r\n pair to indicate a + * completed reply or event. To avoid memory denial-of-service + * though, we must have a size limit on amount of data we + * buffer. 10 MB is large enough that it ought to cope with + * normal QEMU replies, and small enough that we're not + * consuming unreasonable mem. + */ +#define QEMU_AGENT_MAX_RESPONSE (10 * 1024 * 1024) + +typedef struct _qemuAgentMessage qemuAgentMessage; +struct _qemuAgentMessage { + char *txBuffer; + int txOffset; + int txLength; + + /* Used by the JSON agent to hold reply / error */ + char *rxBuffer; + int rxLength; + void *rxObject; + + /* True if rxBuffer / rxObject are ready, or a + * fatal error occurred on the agent channel + */ + bool finished; + /* true for sync command */ + bool sync; + /* id of the issued sync command */ + unsigned long long id; + bool first; +}; + + +struct _qemuAgent { + virObjectLockable parent; + + virCond notify; + + int fd; + + GMainContext *context; + GSocket *socket; + GSource *watch; + + bool running; + bool inSync; + + virDomainObj *vm; + + qemuAgentCallbacks *cb; + + /* If there's a command being processed this will be + * non-NULL */ + qemuAgentMessage *msg; + + /* Buffer incoming data ready for agent + * code to process & find message boundaries */ + size_t bufferOffset; + size_t bufferLength; + char *buffer; + + /* If anything went wrong, this will be fed back + * the next agent msg */ + virError lastError; + + /* Some guest agent commands don't return anything + * but fire up an event on qemu agent instead. + * Take that as indication of successful completion */ + qemuAgentEvent await_event; + int timeout; +}; + +static virClass *qemuAgentClass; +static void qemuAgentDispose(void *obj); + +static int qemuAgentOnceInit(void) +{ + if (!VIR_CLASS_NEW(qemuAgent, virClassForObjectLockable())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(qemuAgent); + + + +static void qemuAgentDispose(void *obj) +{ + qemuAgent *agent = obj; + + VIR_DEBUG("agent=%p", agent); + + if (agent->vm) + virObjectUnref(agent->vm); + virCondDestroy(&agent->notify); + g_free(agent->buffer); + g_main_context_unref(agent->context); + virResetError(&agent->lastError); +} + +static int +qemuAgentOpenUnix(const char *socketpath) +{ + struct sockaddr_un addr = { 0 }; + int agentfd; + + if ((agentfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + virReportSystemError(errno, + "%s", _("failed to create socket")); + return -1; + } + + if (virSetCloseExec(agentfd) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set agent close-on-exec flag")); + goto error; + } + + addr.sun_family = AF_UNIX; + if (virStrcpyStatic(addr.sun_path, socketpath) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Socket path %1$s too big for destination"), socketpath); + goto error; + } + + if (connect(agentfd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { + virReportSystemError(errno, "%s", + _("failed to connect to agent socket")); + goto error; + } + + return agentfd; + + error: + VIR_FORCE_CLOSE(agentfd); + return -1; +} + + +static int +qemuAgentIOProcessEvent(qemuAgent *agent, + virJSONValue *obj) +{ + const char *type; + VIR_DEBUG("agent=%p obj=%p", agent, obj); + + type = virJSONValueObjectGetString(obj, "event"); + if (!type) { + VIR_WARN("missing event type in message"); + errno = EINVAL; + return -1; + } + + return 0; +} + +static int +qemuAgentIOProcessLine(qemuAgent *agent, + const char *line, + qemuAgentMessage *msg) +{ + g_autoptr(virJSONValue) obj = NULL; + + VIR_DEBUG("Line [%s]", line); + + if (!(obj = virJSONValueFromString(line))) { + /* receiving garbage on first sync is regular situation */ + if (msg && msg->sync && msg->first) { + VIR_DEBUG("Received garbage on sync"); + msg->finished = true; + return 0; + } + + return -1; + } + + if (virJSONValueGetType(obj) != VIR_JSON_TYPE_OBJECT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Parsed JSON reply '%1$s' isn't an object"), line); + return -1; + } + + if (virJSONValueObjectHasKey(obj, "QMP")) { + return 0; + } else if (virJSONValueObjectHasKey(obj, "event")) { + return qemuAgentIOProcessEvent(agent, obj); + } else if (virJSONValueObjectHasKey(obj, "error") || + virJSONValueObjectHasKey(obj, "return")) { + if (msg) { + if (msg->sync) { + unsigned long long id; + + if (virJSONValueObjectGetNumberUlong(obj, "return", &id) < 0) { + VIR_DEBUG("Ignoring delayed reply on sync"); + return 0; + } + + VIR_DEBUG("Guest returned ID: %llu", id); + + if (msg->id != id) { + VIR_DEBUG("Guest agent returned ID: %llu instead of %llu", + id, msg->id); + return 0; + } + } + msg->rxObject = g_steal_pointer(&obj); + msg->finished = true; + } else { + /* we are out of sync */ + VIR_DEBUG("Ignoring delayed reply"); + } + + return 0; + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown JSON reply '%1$s'"), line); + return -1; +} + +static int qemuAgentIOProcessData(qemuAgent *agent, + char *data, + size_t len, + qemuAgentMessage *msg) +{ + int used = 0; + size_t i = 0; + + while (used < len) { + char *nl = strstr(data + used, LINE_ENDING); + + if (nl) { + int got = nl - (data + used); + for (i = 0; i < strlen(LINE_ENDING); i++) + data[used + got + i] = '\0'; + if (qemuAgentIOProcessLine(agent, data + used, msg) < 0) + return -1; + used += got + strlen(LINE_ENDING); + } else { + break; + } + } + + VIR_DEBUG("Total used %d bytes out of %zd available in buffer", used, len); + return used; +} + +/* This method processes data that has been received + * from the agent. Looking for async events and + * replies/errors. + */ +static int +qemuAgentIOProcess(qemuAgent *agent) +{ + int len; + qemuAgentMessage *msg = NULL; + + /* See if there's a message ready for reply; that is, + * one that has completed writing all its data. + */ + if (agent->msg && agent->msg->txOffset == agent->msg->txLength) + msg = agent->msg; + + len = qemuAgentIOProcessData(agent, + agent->buffer, agent->bufferOffset, + msg); + + if (len < 0) + return -1; + + if (len < agent->bufferOffset) { + memmove(agent->buffer, agent->buffer + len, agent->bufferOffset - len); + agent->bufferOffset -= len; + } else { + VIR_FREE(agent->buffer); + agent->bufferOffset = agent->bufferLength = 0; + } + if (msg && msg->finished) + virCondBroadcast(&agent->notify); + return len; +} + + +/* + * Called when the agent is able to write data + * Call this function while holding the agent lock. + */ +static int +qemuAgentIOWrite(qemuAgent *agent) +{ + int done; + + /* If no active message, or fully transmitted, then no-op */ + if (!agent->msg || agent->msg->txOffset == agent->msg->txLength) + return 0; + + done = safewrite(agent->fd, + agent->msg->txBuffer + agent->msg->txOffset, + agent->msg->txLength - agent->msg->txOffset); + + if (done < 0) { + if (errno == EAGAIN) + return 0; + + virReportSystemError(errno, "%s", + _("Unable to write to agent")); + return -1; + } + agent->msg->txOffset += done; + return done; +} + +/* + * Called when the agent has incoming data to read + * Call this function while holding the agent lock. + * + * Returns -1 on error, or number of bytes read + */ +static int +qemuAgentIORead(qemuAgent *agent) +{ + size_t avail = agent->bufferLength - agent->bufferOffset; + int ret = 0; + + if (avail < 1024) { + if (agent->bufferLength >= QEMU_AGENT_MAX_RESPONSE) { + virReportSystemError(ERANGE, + _("No complete agent response found in %1$d bytes"), + QEMU_AGENT_MAX_RESPONSE); + return -1; + } + VIR_REALLOC_N(agent->buffer, agent->bufferLength + 1024); + agent->bufferLength += 1024; + avail += 1024; + } + + /* Read as much as we can get into our buffer, + until we block on EAGAIN, or hit EOF */ + while (avail > 1) { + int got; + got = read(agent->fd, + agent->buffer + agent->bufferOffset, + avail - 1); + if (got < 0) { + if (errno == EAGAIN) + break; + virReportSystemError(errno, "%s", + _("Unable to read from agent")); + ret = -1; + break; + } + if (got == 0) + break; + + ret += got; + avail -= got; + agent->bufferOffset += got; + agent->buffer[agent->bufferOffset] = '\0'; + } + + return ret; +} + + +static gboolean +qemuAgentIO(GSocket *socket, + GIOCondition cond, + gpointer opaque); + + +static void +qemuAgentRegister(qemuAgent *agent) +{ + GIOCondition cond = 0; + + if (agent->lastError.code == VIR_ERR_OK) { + cond |= G_IO_IN; + + if (agent->msg && agent->msg->txOffset < agent->msg->txLength) + cond |= G_IO_OUT; + } + + agent->watch = g_socket_create_source(agent->socket, + cond, + NULL); + + virObjectRef(agent); + g_source_set_callback(agent->watch, + (GSourceFunc)qemuAgentIO, + agent, + (GDestroyNotify)virObjectUnref); + + g_source_attach(agent->watch, + agent->context); +} + + +static void +qemuAgentUnregister(qemuAgent *agent) +{ + if (agent->watch) { + g_source_destroy(agent->watch); + g_source_unref(agent->watch); + agent->watch = NULL; + } +} + + +static void qemuAgentUpdateWatch(qemuAgent *agent) +{ + qemuAgentUnregister(agent); + if (agent->socket) + qemuAgentRegister(agent); +} + + +static gboolean +qemuAgentIO(GSocket *socket G_GNUC_UNUSED, + GIOCondition cond, + gpointer opaque) +{ + qemuAgent *agent = opaque; + bool error = false; + bool eof = false; + + virObjectRef(agent); + /* lock access to the agent and protect fd */ + virObjectLock(agent); + + if (agent->fd == -1 || !agent->watch) { + virObjectUnlock(agent); + virObjectUnref(agent); + return G_SOURCE_REMOVE; + } + + if (agent->lastError.code != VIR_ERR_OK) { + if (cond & (G_IO_HUP | G_IO_ERR)) + eof = true; + error = true; + } else { + if (cond & G_IO_OUT) { + if (qemuAgentIOWrite(agent) < 0) + error = true; + } + + if (!error && + cond & G_IO_IN) { + int got = qemuAgentIORead(agent); + if (got < 0) { + error = true; + } else if (got == 0) { + eof = true; + } else { + /* Ignore hangup/error cond if we read some data, to + * give time for that data to be consumed */ + cond = 0; + + if (qemuAgentIOProcess(agent) < 0) + error = true; + } + } + + if (!error && + cond & G_IO_HUP) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("End of file from agent socket")); + eof = true; + } + + if (!error && !eof && + cond & G_IO_ERR) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid file descriptor while waiting for agent")); + eof = true; + } + } + + if (error || eof) { + if (agent->lastError.code != VIR_ERR_OK) { + /* Already have an error, so clear any new error */ + virResetLastError(); + } else { + if (virGetLastErrorCode() == VIR_ERR_OK) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error while processing agent IO")); + virCopyLastError(&agent->lastError); + virResetLastError(); + } + + VIR_DEBUG("Error on agent %s", NULLSTR(agent->lastError.message)); + /* If IO process resulted in an error & we have a message, + * then wakeup that waiter */ + if (agent->msg && !agent->msg->finished) { + agent->msg->finished = true; + virCondSignal(&agent->notify); + } + } + + qemuAgentUpdateWatch(agent); + + /* We have to unlock to avoid deadlock against command thread, + * but is this safe ? I think it is, because the callback + * will try to acquire the virDomainObj *mutex next */ + if (eof) { + void (*eofNotify)(qemuAgent *, virDomainObj *) + = agent->cb->eofNotify; + virDomainObj *vm = agent->vm; + + /* Make sure anyone waiting wakes up now */ + virCondSignal(&agent->notify); + virObjectUnlock(agent); + virObjectUnref(agent); + VIR_DEBUG("Triggering EOF callback"); + (eofNotify)(agent, vm); + } else if (error) { + void (*errorNotify)(qemuAgent *, virDomainObj *) + = agent->cb->errorNotify; + virDomainObj *vm = agent->vm; + + /* Make sure anyone waiting wakes up now */ + virCondSignal(&agent->notify); + virObjectUnlock(agent); + virObjectUnref(agent); + VIR_DEBUG("Triggering error callback"); + (errorNotify)(agent, vm); + } else { + virObjectUnlock(agent); + virObjectUnref(agent); + } + + return G_SOURCE_REMOVE; +} + + +qemuAgent * +qemuAgentOpen(virDomainObj *vm, + const virDomainChrSourceDef *config, + GMainContext *context, + qemuAgentCallbacks *cb) +{ + qemuAgent *agent; + g_autoptr(GError) gerr = NULL; + + if (!cb || !cb->eofNotify) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("EOF notify callback must be supplied")); + return NULL; + } + + if (qemuAgentInitialize() < 0) + return NULL; + + if (!(agent = virObjectLockableNew(qemuAgentClass))) + return NULL; + + agent->timeout = BHYVE_DOMAIN_PRIVATE(vm)->agentTimeout; + agent->fd = -1; + if (virCondInit(&agent->notify) < 0) { + virReportSystemError(errno, "%s", + _("cannot initialize agent condition")); + virObjectUnref(agent); + return NULL; + } + agent->vm = virObjectRef(vm); + agent->cb = cb; + + if (config->type != VIR_DOMAIN_CHR_TYPE_UNIX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to handle agent type: %1$s"), + virDomainChrTypeToString(config->type)); + goto cleanup; + } + + virObjectUnlock(vm); + agent->fd = qemuAgentOpenUnix(config->data.nix.path); + virObjectLock(vm); + + if (agent->fd == -1) + goto cleanup; + + agent->context = g_main_context_ref(context); + + agent->socket = g_socket_new_from_fd(agent->fd, &gerr); + if (!agent->socket) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to create socket object: %1$s"), + gerr->message); + goto cleanup; + } + + qemuAgentRegister(agent); + + agent->running = true; + VIR_DEBUG("New agent %p fd=%d", agent, agent->fd); + + return agent; + + cleanup: + qemuAgentClose(agent); + return NULL; +} + + +static void +qemuAgentNotifyCloseLocked(qemuAgent *agent) +{ + if (agent) { + agent->running = false; + + /* If there is somebody waiting for a message + * wake him up. No message will arrive anyway. */ + if (agent->msg && !agent->msg->finished) { + agent->msg->finished = true; + virCondSignal(&agent->notify); + } + } +} + + +void +qemuAgentNotifyClose(qemuAgent *agent) +{ + if (!agent) + return; + + VIR_DEBUG("agent=%p", agent); + + VIR_WITH_OBJECT_LOCK_GUARD(agent) { + qemuAgentNotifyCloseLocked(agent); + } +} + + +void qemuAgentClose(qemuAgent *agent) +{ + if (!agent) + return; + + VIR_DEBUG("agent=%p", agent); + + VIR_WITH_OBJECT_LOCK_GUARD(agent) { + if (agent->socket) { + qemuAgentUnregister(agent); + g_clear_pointer(&agent->socket, g_object_unref); + agent->fd = -1; + } + + qemuAgentNotifyCloseLocked(agent); + } + + virObjectUnref(agent); +} + +#define QEMU_AGENT_WAIT_TIME 5 + +/** + * qemuAgentSend: + * @agent: agent object + * @msg: Message + * @seconds: number of seconds to wait for the result, it can be either + * -2, -1, 0 or positive. + * @report_sync: On timeout; report synchronization error instead of the normal error + * + * Send @msg to agent @agent. If @seconds is equal to + * VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK(-2), this function will block forever + * waiting for the result. The value of + * VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT(-1) means use default timeout value + * and VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT(0) makes this function return + * immediately without waiting. Any positive value means the number of seconds + * to wait for the result. + * + * Returns: 0 on success, + * -2 on timeout, + * -1 otherwise + */ +static int +qemuAgentSend(qemuAgent *agent, + qemuAgentMessage *msg, + int seconds, + bool report_sync) +{ + int ret = -1; + unsigned long long then = 0; + + VIR_INFO("qemuAgentSend: seconds=%d", seconds); + + /* Check whether qemu quit unexpectedly */ + if (agent->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Attempt to send command while error is set %s", + NULLSTR(agent->lastError.message)); + virSetError(&agent->lastError); + return -1; + } + + if (seconds > VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) { + unsigned long long now; + if (virTimeMillisNow(&now) < 0) + return -1; + if (seconds == VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT) + seconds = QEMU_AGENT_WAIT_TIME; + then = now + seconds * 1000ull; + } + + agent->msg = msg; + qemuAgentUpdateWatch(agent); + + while (!agent->msg->finished) { + if ((then && virCondWaitUntil(&agent->notify, &agent->parent.lock, then) < 0) || + (!then && virCondWait(&agent->notify, &agent->parent.lock) < 0)) { + if (errno == ETIMEDOUT) { + if (report_sync) { + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, + _("guest agent didn't respond to synchronization within '%1$d' seconds"), + seconds); + } else { + virReportError(VIR_ERR_AGENT_COMMAND_TIMEOUT, + _("guest agent didn't respond to command within '%1$d' seconds"), + seconds); + } + ret = -2; + } else { + virReportSystemError(errno, "%s", + _("Unable to wait on agent socket condition")); + } + agent->inSync = false; + goto cleanup; + } + } + + if (agent->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Send command resulted in error %s", + NULLSTR(agent->lastError.message)); + virSetError(&agent->lastError); + goto cleanup; + } + + ret = 0; + + cleanup: + agent->msg = NULL; + qemuAgentUpdateWatch(agent); + + return ret; +} + + +/** + * qemuAgentGuestSyncSend: + * @agent: agent object + * @timeout: timeout for the command + * @first: true when this is the first invocation to drain possible leftovers + * from the pipe + * + * Sends a sync request to the guest agent. + * Returns: -1 on error + * 0 on successful send, but when no reply was received + * 1 when a reply was received + */ +static int +qemuAgentGuestSyncSend(qemuAgent *agent, + int timeout, + bool first) +{ + g_autofree char *txMsg = NULL; + g_autoptr(virJSONValue) rxObj = NULL; + unsigned long long id; + qemuAgentMessage sync_msg = { 0 }; + int rc; + + if (virTimeMillisNow(&id) < 0) + return -1; + + txMsg = g_strdup_printf("{\"execute\":\"guest-sync\", " + "\"arguments\":{\"id\":%llu}}\n", id); + + sync_msg.txBuffer = txMsg; + sync_msg.txLength = strlen(txMsg); + sync_msg.sync = true; + sync_msg.id = id; + sync_msg.first = first; + + VIR_DEBUG("Sending guest-sync command with ID: %llu", id); + + rc = qemuAgentSend(agent, &sync_msg, timeout, true); + rxObj = g_steal_pointer(&sync_msg.rxObject); + + VIR_DEBUG("qemuAgentSend returned: %d", rc); + + if (rc < 0) + return -1; + + if (rxObj) + return 1; + + return 0; +} + + +/** + * qemuAgentGuestSync: + * @agent: agent object + * @seconds: qemu agent command timeout value + * + * Send guest-sync with unique ID + * and wait for reply. If we get one, check if + * received ID is equal to given. + * + * Returns: 0 on success, + * -1 otherwise + */ +static int +qemuAgentGuestSync(qemuAgent *agent, + int seconds) +{ + int timeout = QEMU_AGENT_WAIT_TIME; + int rc; + + if (agent->inSync) + return 0; + + /* if user specified a custom agent timeout that is lower than the + * default timeout, use the shorter timeout instead */ + if ((agent->timeout >= 0) && (agent->timeout < timeout)) + timeout = agent->timeout; + + /* If user specified a timeout parameter smaller than both default + * value and agent->timeout in qga APIs(such as qemu-agent-command), + * use the parameter timeout value */ + if ((seconds >= 0) && (seconds < timeout)) + timeout = seconds; + + if ((rc = qemuAgentGuestSyncSend(agent, timeout, true)) < 0) + return -1; + + /* successfully sync'd */ + if (rc == 1) { + agent->inSync = true; + return 0; + } + + /* send another sync */ + if ((rc = qemuAgentGuestSyncSend(agent, timeout, false)) < 0) + return -1; + + /* successfully sync'd */ + if (rc == 1) { + agent->inSync = true; + return 0; + } + + if (agent->running) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing agent reply object")); + else + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("Guest agent disappeared while executing command")); + + return -1; +} + +static const char * +qemuAgentStringifyErrorClass(const char *klass) +{ + if (STREQ_NULLABLE(klass, "BufferOverrun")) + return "Buffer overrun"; + else if (STREQ_NULLABLE(klass, "CommandDisabled")) + return "The command has been disabled for this instance"; + else if (STREQ_NULLABLE(klass, "CommandNotFound")) + return "The command has not been found"; + else if (STREQ_NULLABLE(klass, "FdNotFound")) + return "File descriptor not found"; + else if (STREQ_NULLABLE(klass, "InvalidParameter")) + return "Invalid parameter"; + else if (STREQ_NULLABLE(klass, "InvalidParameterType")) + return "Invalid parameter type"; + else if (STREQ_NULLABLE(klass, "InvalidParameterValue")) + return "Invalid parameter value"; + else if (STREQ_NULLABLE(klass, "OpenFileFailed")) + return "Cannot open file"; + else if (STREQ_NULLABLE(klass, "QgaCommandFailed")) + return "Guest agent command failed"; + else if (STREQ_NULLABLE(klass, "QMPBadInputObjectMember")) + return "Bad QMP input object member"; + else if (STREQ_NULLABLE(klass, "QMPExtraInputObjectMember")) + return "Unexpected extra object member"; + else if (STREQ_NULLABLE(klass, "UndefinedError")) + return "An undefined error has occurred"; + else if (STREQ_NULLABLE(klass, "Unsupported")) + return "this feature or command is not currently supported"; + else if (klass) + return klass; + else + return "unknown QEMU command error"; +} + +/* Ignoring OOM in this method, since we're already reporting + * a more important error + * + * XXX see qerror.h for different klasses & fill out useful params + */ +static const char * +qemuAgentStringifyError(virJSONValue *error) +{ + const char *klass = virJSONValueObjectGetString(error, "class"); + const char *detail = virJSONValueObjectGetString(error, "desc"); + + /* The QMP 'desc' field is usually sufficient for our generic + * error reporting needs. However, if not present, translate + * the class into something readable. + */ + if (!detail) + detail = qemuAgentStringifyErrorClass(klass); + + return detail; +} + +static const char * +qemuAgentCommandName(virJSONValue *cmd) +{ + const char *name = virJSONValueObjectGetString(cmd, "execute"); + if (name) + return name; + return "<unknown>"; +} + +static int +qemuAgentCheckError(virJSONValue *cmd, + virJSONValue *reply, + bool report_unsupported) +{ + if (virJSONValueObjectHasKey(reply, "error")) { + virJSONValue *error = virJSONValueObjectGet(reply, "error"); + g_autofree char *cmdstr = virJSONValueToString(cmd, false); + g_autofree char *replystr = virJSONValueToString(reply, false); + + /* Log the full JSON formatted command & error */ + VIR_DEBUG("unable to execute QEMU agent command %s: %s", + NULLSTR(cmdstr), NULLSTR(replystr)); + + /* Only send the user the command name + friendly error */ + if (!error) { + virReportError(VIR_ERR_AGENT_COMMAND_FAILED, + _("unable to execute QEMU agent command '%1$s'"), + qemuAgentCommandName(cmd)); + return -1; + } + + if (!report_unsupported) { + const char *klass = virJSONValueObjectGetString(error, "class"); + + if (STREQ_NULLABLE(klass, "CommandNotFound") || + STREQ_NULLABLE(klass, "CommandDisabled")) + return -2; + } + + virReportError(VIR_ERR_AGENT_COMMAND_FAILED, + _("unable to execute QEMU agent command '%1$s': %2$s"), + qemuAgentCommandName(cmd), + qemuAgentStringifyError(error)); + + return -1; + } + if (!virJSONValueObjectHasKey(reply, "return")) { + g_autofree char *cmdstr = virJSONValueToString(cmd, false); + g_autofree char *replystr = virJSONValueToString(reply, false); + + VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s", + NULLSTR(cmdstr), NULLSTR(replystr)); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QEMU agent command '%1$s' returned neither error nor success"), + qemuAgentCommandName(cmd)); + return -1; + } + return 0; +} + +static int +qemuAgentCommandFull(qemuAgent *agent, + virJSONValue *cmd, + virJSONValue **reply, + int seconds, + bool report_unsupported) +{ + int ret = -1; + qemuAgentMessage msg = { 0 }; + g_autofree char *cmdstr = NULL; + int await_event = agent->await_event; + + *reply = NULL; + + if (!agent->running) { + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("Guest agent disappeared while executing command")); + goto cleanup; + } + + if (qemuAgentGuestSync(agent, seconds) < 0) + goto cleanup; + + if (!(cmdstr = virJSONValueToString(cmd, false))) + goto cleanup; + msg.txBuffer = g_strdup_printf("%s" LINE_ENDING, cmdstr); + msg.txLength = strlen(msg.txBuffer); + + VIR_DEBUG("Send command '%s' for write, seconds = %d", cmdstr, seconds); + + ret = qemuAgentSend(agent, &msg, seconds, false); + + VIR_DEBUG("Receive command reply ret=%d rxObject=%p", + ret, msg.rxObject); + + if (ret < 0) + goto cleanup; + + /* If we haven't obtained any reply but we wait for an + * event, then don't report this as error */ + if (!msg.rxObject) { + if (!await_event) { + if (agent->running) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing agent reply object")); + } else { + virReportError(VIR_ERR_AGENT_COMMAND_TIMEOUT, "%s", + _("Guest agent disappeared while executing command")); + } + ret = -1; + } + goto cleanup; + } + + *reply = msg.rxObject; + ret = qemuAgentCheckError(cmd, *reply, report_unsupported); + + cleanup: + VIR_FREE(msg.txBuffer); + agent->await_event = QEMU_AGENT_EVENT_NONE; + + return ret; +} + +static int +qemuAgentCommand(qemuAgent *agent, + virJSONValue *cmd, + virJSONValue **reply, + int seconds) +{ + return qemuAgentCommandFull(agent, cmd, reply, seconds, true); +} + +static virJSONValue *G_GNUC_NULL_TERMINATED +qemuAgentMakeCommand(const char *cmdname, + ...) +{ + g_autoptr(virJSONValue) obj = NULL; + g_autoptr(virJSONValue) jargs = NULL; + va_list args; + + va_start(args, cmdname); + + if (virJSONValueObjectAddVArgs(&jargs, args) < 0) { + va_end(args); + return NULL; + } + + va_end(args); + + if (virJSONValueObjectAdd(&obj, + "s:execute", cmdname, + "A:arguments", &jargs, + NULL) < 0) + return NULL; + + return g_steal_pointer(&obj); +} + +static virJSONValue * +qemuAgentMakeStringsArray(const char **strings, unsigned int len) +{ + size_t i; + g_autoptr(virJSONValue) ret = virJSONValueNewArray(); + + for (i = 0; i < len; i++) { + if (virJSONValueArrayAppendString(ret, strings[i]) < 0) + return NULL; + } + + return g_steal_pointer(&ret); +} + +void qemuAgentNotifyEvent(qemuAgent *agent, + qemuAgentEvent event) +{ + VIR_LOCK_GUARD lock = virObjectLockGuard(agent); + + VIR_DEBUG("agent=%p event=%d await_event=%d", agent, event, agent->await_event); + if (agent->await_event == event) { + agent->await_event = QEMU_AGENT_EVENT_NONE; + /* somebody waiting for this event, wake him up. */ + if (agent->msg && !agent->msg->finished) { + agent->msg->finished = true; + virCondSignal(&agent->notify); + } + } +} + + +int +qemuAgentArbitraryCommand(qemuAgent *agent, + const char *cmd_str, + char **result, + int timeout) +{ + int rc; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + *result = NULL; + if (timeout < VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN) { + virReportError(VIR_ERR_INVALID_ARG, + _("guest agent timeout '%1$d' is less than the minimum '%2$d'"), + timeout, VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN); + return -1; + } + + if (!(cmd = virJSONValueFromString(cmd_str))) + return -1; + + if ((rc = qemuAgentCommand(agent, cmd, &reply, timeout)) < 0) + return rc; + + if (!(*result = virJSONValueToString(reply, false))) + return -1; + + return rc; +} + + +/** + * qemuAgentGetHostname: + * + * Gets the guest hostname using the guest agent. + * + * Returns 0 on success and fills @hostname. On error -1 is returned with an + * error reported and if '@report_unsupported' is false -2 is returned if the + * guest agent does not support the command without reporting an error + */ +int +qemuAgentGetHostname(qemuAgent *agent, + char **hostname, + bool report_unsupported) +{ + g_autoptr(virJSONValue) cmd = qemuAgentMakeCommand("guest-get-host-name", NULL); + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data = NULL; + const char *result = NULL; + int rc; + + if (!cmd) + return -1; + + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, + report_unsupported)) < 0) + return rc; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + return -1; + } + + if (!(result = virJSONValueObjectGetString(data, "host-name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'host-name' missing in guest-get-host-name reply")); + return -1; + } + + *hostname = g_strdup(result); + + return 0; +} + + +int +qemuAgentGetTime(qemuAgent *agent, + long long *seconds, + unsigned int *nseconds) +{ + unsigned long long json_time; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + cmd = qemuAgentMakeCommand("guest-get-time", + NULL); + if (!cmd) + return -1; + + if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) + return -1; + + if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + return -1; + } + + /* guest agent returns time in nanoseconds, + * we need it in seconds here */ + *seconds = json_time / 1000000000LL; + *nseconds = json_time % 1000000000LL; + return 0; +} diff --git a/src/bhyve/bhyve_qemu_agent.h b/src/bhyve/bhyve_qemu_agent.h new file mode 100644 index 0000000000..0e1752a2cb --- /dev/null +++ b/src/bhyve/bhyve_qemu_agent.h @@ -0,0 +1,197 @@ +/* + * bhyve_qemu_agent.h: interaction with QEMU guest agent + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "internal.h" +#include "domain_conf.h" + +typedef struct _qemuAgent qemuAgent; + +typedef struct _qemuAgentCallbacks qemuAgentCallbacks; +struct _qemuAgentCallbacks { + void (*eofNotify)(qemuAgent *mon, + virDomainObj *vm); + void (*errorNotify)(qemuAgent *mon, + virDomainObj *vm); +}; + + +qemuAgent *qemuAgentOpen(virDomainObj *vm, + const virDomainChrSourceDef *config, + GMainContext *context, + qemuAgentCallbacks *cb); + +void qemuAgentClose(qemuAgent *mon); + +void qemuAgentNotifyClose(qemuAgent *mon); + +typedef enum { + QEMU_AGENT_EVENT_NONE = 0, + QEMU_AGENT_EVENT_SHUTDOWN, + QEMU_AGENT_EVENT_SUSPEND, + QEMU_AGENT_EVENT_RESET, +} qemuAgentEvent; + +void qemuAgentNotifyEvent(qemuAgent *mon, + qemuAgentEvent event); + +typedef enum { + QEMU_AGENT_SHUTDOWN_POWERDOWN, + QEMU_AGENT_SHUTDOWN_REBOOT, + QEMU_AGENT_SHUTDOWN_HALT, + + QEMU_AGENT_SHUTDOWN_LAST, +} qemuAgentShutdownMode; + +typedef struct _qemuAgentDiskAddress qemuAgentDiskAddress; +struct _qemuAgentDiskAddress { + char *serial; + virPCIDeviceAddress pci_controller; + char *bus_type; + unsigned int bus; + unsigned int target; + unsigned int unit; + char *devnode; + virCCWDeviceAddress *ccw_addr; +}; +void qemuAgentDiskAddressFree(qemuAgentDiskAddress *addr); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuAgentDiskAddress, qemuAgentDiskAddressFree); + +typedef struct _qemuAgentDiskInfo qemuAgentDiskInfo; +struct _qemuAgentDiskInfo { + char *name; + bool partition; + char **dependencies; + qemuAgentDiskAddress *address; + char *alias; +}; +void qemuAgentDiskInfoFree(qemuAgentDiskInfo *info); + +typedef struct _qemuAgentFSInfo qemuAgentFSInfo; +struct _qemuAgentFSInfo { + char *mountpoint; /* path to mount point */ + char *name; /* device name in the guest (e.g. "sda1") */ + char *fstype; /* filesystem type */ + long long total_bytes; + long long used_bytes; + size_t ndisks; + qemuAgentDiskAddress **disks; +}; +void qemuAgentFSInfoFree(qemuAgentFSInfo *info); + +int qemuAgentShutdown(qemuAgent *mon, + qemuAgentShutdownMode mode); + +int qemuAgentFSFreeze(qemuAgent *mon, + const char **mountpoints, unsigned int nmountpoints); +int qemuAgentFSThaw(qemuAgent *mon); +int qemuAgentGetFSInfo(qemuAgent *mon, + qemuAgentFSInfo ***info, + bool report_unsupported); + +int qemuAgentSuspend(qemuAgent *mon, + unsigned int target); + +int qemuAgentArbitraryCommand(qemuAgent *mon, + const char *cmd, + char **result, + int timeout); +int qemuAgentFSTrim(qemuAgent *mon, + unsigned long long minimum); + + +typedef struct _qemuAgentCPUInfo qemuAgentCPUInfo; +struct _qemuAgentCPUInfo { + unsigned int id; /* logical cpu ID */ + bool online; /* true if the CPU is activated */ + bool offlinable; /* true if the CPU can be offlined */ + + bool modified; /* set to true if the vcpu state needs to be changed */ +}; + +int qemuAgentGetVCPUs(qemuAgent *mon, qemuAgentCPUInfo **info); +int qemuAgentSetVCPUs(qemuAgent *mon, qemuAgentCPUInfo *cpus, size_t ncpus); +int qemuAgentUpdateCPUInfo(unsigned int nvcpus, + qemuAgentCPUInfo *cpuinfo, + int ncpuinfo); + +int +qemuAgentGetHostname(qemuAgent *mon, + char **hostname, + bool report_unsupported); + +int qemuAgentGetTime(qemuAgent *mon, + long long *seconds, + unsigned int *nseconds); +int qemuAgentSetTime(qemuAgent *mon, + long long seconds, + unsigned int nseconds, + bool sync); + +int qemuAgentGetInterfaces(qemuAgent *mon, + virDomainInterfacePtr **ifaces, + bool report_unsupported); + +int qemuAgentSetUserPassword(qemuAgent *mon, + const char *user, + const char *password, + bool crypted); + +int qemuAgentGetUsers(qemuAgent *mon, + virTypedParamList *list, + bool report_unsupported); + +int qemuAgentGetOSInfo(qemuAgent *mon, + virTypedParamList *list, + bool report_unsupported); + +int qemuAgentGetTimezone(qemuAgent *mon, + virTypedParamList *list, + bool report_unsupported); + +void qemuAgentSetResponseTimeout(qemuAgent *mon, + int timeout); + +int qemuAgentSSHGetAuthorizedKeys(qemuAgent *agent, + const char *user, + char ***keys); + +int qemuAgentSSHAddAuthorizedKeys(qemuAgent *agent, + const char *user, + const char **keys, + size_t nkeys, + bool reset); + +int qemuAgentSSHRemoveAuthorizedKeys(qemuAgent *agent, + const char *user, + const char **keys, + size_t nkeys); + +int qemuAgentGetDisks(qemuAgent *mon, + qemuAgentDiskInfo ***disks, + bool report_unsupported); + +int qemuAgentGetLoadAvg(qemuAgent *agent, + double *load1m, + double *load5m, + double *load15m, + bool report_unsupported); diff --git a/src/bhyve/meson.build b/src/bhyve/meson.build index 11920d9c3e..a1f0aa63b2 100644 --- a/src/bhyve/meson.build +++ b/src/bhyve/meson.build @@ -2,13 +2,14 @@ bhyve_sources = files( 'bhyve_capabilities.c', 'bhyve_command.c', 'bhyve_conf.c', - 'bhyve_firmware.c', - 'bhyve_parse_command.c', 'bhyve_device.c', 'bhyve_domain.c', 'bhyve_driver.c', + 'bhyve_firmware.c', 'bhyve_monitor.c', + 'bhyve_parse_command.c', 'bhyve_process.c', + 'bhyve_qemu_agent.c', ) driver_source_files += bhyve_sources diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x index c7f3abfcbf..757e54efcc 100644 --- a/src/remote/qemu_protocol.x +++ b/src/remote/qemu_protocol.x @@ -44,17 +44,6 @@ struct qemu_domain_attach_ret { remote_nonnull_domain dom; }; -struct qemu_domain_agent_command_args { - remote_nonnull_domain dom; - remote_nonnull_string cmd; - int timeout; - unsigned int flags; -}; - -struct qemu_domain_agent_command_ret { - remote_string result; -}; - struct qemu_connect_domain_monitor_event_register_args { remote_domain dom; @@ -135,13 +124,6 @@ enum qemu_procedure { */ QEMU_PROC_DOMAIN_ATTACH = 2, - /** - * @generate: both - * @priority: low - * @acl: domain:write - */ - QEMU_PROC_DOMAIN_AGENT_COMMAND = 3, - /** * @generate: none * @priority: high diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 38a83c64ea..a3a0920061 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -4009,6 +4009,17 @@ struct remote_domain_event_nic_mac_change_msg { remote_nonnull_string newMAC; }; +struct remote_domain_qemu_agent_command_args { + remote_nonnull_domain dom; + remote_nonnull_string cmd; + int timeout; + unsigned int flags; +}; + +struct remote_domain_qemu_agent_command_ret { + remote_string result; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -7120,5 +7131,12 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_NIC_MAC_CHANGE = 453 + REMOTE_PROC_DOMAIN_EVENT_NIC_MAC_CHANGE = 453, + + /** + * @generate: both + * @priority: low + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_QEMU_AGENT_COMMAND = 454 }; -- 2.52.0

2 3

[PATCH v3 0/5] qemu: support async live vCPU unplug
by Akash Kulhalli 12 May '26

12 May '26

This series adds support for asynchronous live vCPU unplug in the QEMU driver. For live vCPU unplug libvirt currently waits only for a short synchronous completion window. If the unplug completes later there is no dedicated event reporting successful removal of the vCPU. This series adds a new domain event for successful unplug completion and extends the public APIs to allow requesting asynchronous unplug. As recommended, this changeset is split into five parts- Patch 1 adds the new event and the required public/internal plumbing. Patch 2 wires the event into the existing QEMU unplug completion path. Patch 3 adds the internal QEMU plumbing needed to skip waiting for unplug completion, but leaves it unused for now. Patches 4 and 5 then expose the new behaviour through virDomainSetVcpusFlags() and virDomainSetVcpu() together with the corresponding virsh changes. The new public flags are limited to live unplug. Rejected unplug requests continue to be reported via device-removal-failed. Most impactful change from the previous iteration is that event emission moves from 1 vcpu-removed per qemu event model to one vcpu-removed per vcpu actually removed. This change is effectively a no-op for x86 (because there is a 1:1 mapping), but is relevant for architectures where more than 1 vcpu might be grouped under a single logical qemu vcpu object. Changes from v2: - s/virDomainSetVcpuBehaviour/virDomainSetVcpuFlags/g - Reduce redundant phrasing - emit `vcpu-removed` for every vcpu removed instead of per qemu event - bump version ton 12.4.0 All test runs are still passing cleanly. Akash Kulhalli (5): API/qemu: add async unplug flag to virDomainSetVcpu API/qemu: add async unplug flag to virDomainSetVcpusFlags qemu: thread async vcpu unplug through internal helpers qemu: emit vcpu-removed event on unplug completion conf,remote: add vcpu-removed domain event Related discussion: RFC: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/2J53… v2: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/YZID…

2 10

[PATCH 1/1] virStorageSource: note that capacity/allocation/physical are stale caches
by Denis V. Lunev 12 May '26

12 May '26

These three fields are cached values that do not reflect reality unless the caller refreshes them. 'allocation' is in addition ultra-unreliable: any guest write into a previously unallocated part of a sparse image invalidates it, even right after a refresh. Document this on the struct so new callers do not trust the values. Signed-off-by: Denis V. Lunev <den(a)openvz.org> CC: Peter Krempa <pkrempa(a)redhat.com> --- src/conf/storage_source_conf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 5ddcebb282..fa71a9ef39 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -326,6 +326,8 @@ struct _virStorageSource { virStoragePerms *perms; virStorageTimestamps *timestamps; + + /* Cached, unreliable. Refresh before use. */ unsigned long long capacity; /* in bytes, 0 if unknown */ unsigned long long allocation; /* in bytes, 0 if unknown */ unsigned long long physical; /* in bytes, 0 if unknown */ -- 2.51.0

1 0

[libvirt PATCH] util: virGetSubIDs: do not limit file size
by Ján Tomko 12 May '26

12 May '26

From: Ján Tomko <jtomko(a)redhat.com> https://redhat.atlassian.net/browse/RHEL-174491 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/util/virutil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 187b8202dd..2cc7edac50 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1223,7 +1223,7 @@ virGetSubIDs(virSubID **retval, const char *file) *retval = NULL; - if (virFileReadAll(file, BUFSIZ, &buf) < 0) + if (virFileReadAll(file, SIZE_MAX, &buf) < 0) return -1; lines = g_strsplit(buf, "\n", 0); -- 2.54.0

2 1

[libvirt PATCH v2] util: virGetSubIDs: do not limit file size
by Ján Tomko 12 May '26

12 May '26

From: Ján Tomko <jtomko(a)redhat.com> https://redhat.atlassian.net/browse/RHEL-174491 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- v2: it's `int maxlen`, not size_t src/util/virutil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 187b8202dd..24b0a10878 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1223,7 +1223,7 @@ virGetSubIDs(virSubID **retval, const char *file) *retval = NULL; - if (virFileReadAll(file, BUFSIZ, &buf) < 0) + if (virFileReadAll(file, INT_MAX, &buf) < 0) return -1; lines = g_strsplit(buf, "\n", 0); -- 2.54.0

1 0

[PATCH] test: qemu: Fix error message when populating fd groups
by Peter Krempa 12 May '26

12 May '26

From: Peter Krempa <pkrempa(a)redhat.com> The 'new->fds' array is not yet initialized at the point where the check if the FD is occupied happens so the error would always report that FD '0' is in use. Use 'new->testfds' instead. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/testutilsqemu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index e9bdbdbbe7..e7a61d0c6f 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -727,7 +727,7 @@ testQemuInfoSetArgs(testQemuInfo *info, new->testfds[i] = va_arg(argptr, unsigned int); if (fcntl(new->testfds[i], F_GETFD) != -1) { - fprintf(stderr, "fd '%d' is already in use\n", new->fds[i]); + fprintf(stderr, "fd '%d' is already in use\n", new->testfds[i]); abort(); } -- 2.54.0

2 1