February 2025 - Devel - libvirt List Archives

Re: Support for: VIR_DOMAIN_EVENT_ID_DBGTOOLS.
by Daniel P. Berrangé 11 Feb '25

11 Feb '25

On Fri, Feb 07, 2025 at 02:34:34PM +0000, Martin Harvey wrote: > Probably a FAQ. > > I have implemented a prototype: > > * > QEMU event for an in-guest issue (tools installation / crash etc), via fwcfg / vmcore (similar). (QEMU_PROCESS_EVENT_DBGTOOLS_CHANGED). > * > Qemu Process / QemuMonitor handling for same event. > * > Mapped this through to VIR_DOMAIN_EVENT_ID_DBG_TOOLS, > * > Corresponding changes to top level event handling. > > We would find this *very useful* in cases where vm migration, > debugging and the like require co-operation with guest user-mode. > > Are you folks happy to countenance some simple, but top-level, > event handling changes triggered by guest software, or is this > considered a no no? Should I be burying the details somewhere > in EVENT_TUNABLE instead? This is all very abstract, making it hard to give a useful answer. > Patchsets also available ... IMHO it would be better to make a concrete proposal to QEMU so we can understand the whole problem space better. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

1 0

Re: Support for: VIR_DOMAIN_EVENT_ID_DBGTOOLS.
by Michal Prívozník 11 Feb '25

11 Feb '25

On 2/7/25 15:34, Martin Harvey wrote: > Probably a FAQ. > > I have implemented a prototype: > > * > QEMU event for an in-guest issue (tools installation / crash etc), > via fwcfg / vmcore (similar). (QEMU_PROCESS_EVENT_DBGTOOLS_CHANGED). > * > Qemu Process / QemuMonitor handling for same event. > * > Mapped this through to VIR_DOMAIN_EVENT_ID_DBG_TOOLS, > * > Corresponding changes to top level event handling. > > We would find this *very useful* in cases where vm migration, debugging > and the like require co-operation with guest user-mode. > > Are you folks happy to countenance some simple, but top-level, event > handling changes triggered by guest software, or is this considered a no > no? Should I be burying the details somewhere in EVENT_TUNABLE instead? I don't think this is achievable via EVENT_TUNABLE. I mean, EVENT_TUNABLE is completely detached from QEMU process, it's a libvirt originated event that's fired whenever some knob changes (alright, there are only a few specific cases where this event is emitted, but still). If you need to detect changes made by guest OS in fwcfg then we need QEMU to send an event to libvirt so that it can be relayed to users. Once it's merged in QEMU then libvirt can start supporting it too (we've had bad experience with merging patches in libvirt before they landed in QEMU). But hey, patches can be sent at the same time, it's only merging them that has to be serialized. Michal

2 1

[PATCH] cpu_map: Add missing feature "bhi-no"
by Tim Wiederhake 11 Feb '25

11 Feb '25

Introduced in qemu commit b611931d4f70b9a3e49e39c405c63b3b5e9c0df1. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- src/cpu_map/x86_features.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml index d06d60e230..08519a37bf 100644 --- a/src/cpu_map/x86_features.xml +++ b/src/cpu_map/x86_features.xml @@ -873,6 +873,9 @@ <feature name='fb-clear'> <msr index='0x0000010a' edx='0x00000000' eax='0x00020000'/> </feature> + <feature name='bhi-no'> + <msr index='0x0000010a' edx='0x00000000' eax='0x00100000'/> + </feature> <feature name='pbrsb-no'> <msr index='0x0000010a' edx='0x00000000' eax='0x01000000'/> </feature> -- 2.48.1

2 1

[PATCH] tests: Record negative tests as pass instead of expected-fail
by Tim Wiederhake 10 Feb '25

10 Feb '25

meson's "test()" function provides a "should_fail: bool" argument that checks for a command to exit with a non-zero exit code instead of the usual zero exit code to signal success. If the program under test does so, it is recorded as "EXPECTEDFAIL" instead of "OK". While there is an argument to be made that the program under test failed as expected, the test in itself is successful and should be recorded as such. Before: $ meson test ... 151/300 libvirt:bin / libvirtd fail with missing config EXPECTEDFAIL 0.03s exit status 1 ... Ok: 299 Expected Fail: 1 Fail: 0 Unexpected Pass: 0 Skipped: 0 Timeout: 0 After: $ meson test ... 151/300 libvirt:bin / libvirtd fail with missing config OK 0.03s ... Ok: 300 Expected Fail: 0 Fail: 0 Unexpected Pass: 0 Skipped: 0 Timeout: 0 Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- tests/expected-fail | 13 +++++++++++++ tests/meson.build | 5 ++--- 2 files changed, 15 insertions(+), 3 deletions(-) create mode 100755 tests/expected-fail diff --git a/tests/expected-fail b/tests/expected-fail new file mode 100755 index 0000000000..85738c95dc --- /dev/null +++ b/tests/expected-fail @@ -0,0 +1,13 @@ +#!/bin/sh + +# meson's "test()" function provides a "should_fail: bool" argument that +# checks for a command to exit with a non-zero exit code instead of the usual +# zero exit code to signal success. If the program under test does so, it is +# recorded as "EXPECTEDFAIL" instead of "OK". While there is an argument to be +# made that the program under test failed as expected, the test in itself is +# successful and should be recorded as such. + +if "$@" +then + exit 1 +fi diff --git a/tests/meson.build b/tests/meson.build index 0d76d37959..0ebcdc1496 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -725,9 +725,8 @@ endif if conf.has('WITH_LIBVIRTD') test('libvirtd fail with missing config', - libvirtd_prog, - args: [ '--config=no-such-conf', '--timeout=5' ], - should_fail: true, + find_program('expected-fail'), + args: [ libvirtd_prog, '--config=no-such-conf', '--timeout=5' ], suite: 'bin', ) -- 2.48.1

3 5

[PATCH V3 00/19] qemu: support mapped-ram+directio+mulitfd
by Jim Fehlig 07 Feb '25

07 Feb '25

V3 series adding support for QEMU's mapped-ram stream format [1] and migration capability. The use of mapped-ram is controlled by extending save_image_format setting in qemu.conf with a new 'sparse' option. The 'raw' format continues to be the default. Also included are patches that leverage mapped-ram to add support for parallel save/restore. In one of the previous threads on this topic we discussed per-VM control of save image format by adding a 'format' parameter to virDomainSaveParams. I can do that as a followup series if folks agree its useful. Changes in V3: * Drop patch adding qemuFDPassGetId * Drop now upstream patch decomposing qemuSaveImageOpen * Add patch to get an FDPass from fdset * If qemu is still running after save, remove fdset V1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/MNB… V2: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/GNJ… [1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/map… Claudio Fontana (3): include: Define constants for parallel save/restore tools: add parallel parameter to virsh save command tools: add parallel parameter to virsh restore command Jim Fehlig (16): lib: virDomain{Save,Restore}Params: Ensure absolute path qemu: Add function to get FDPass object from monitor qemu: Add function to check capability in migration params qemu: Add function to get bool value from migration params qemu: Add mapped-ram migration capability qemu: Add function to get migration params for save qemu_saveimage: add "sparse" to supported save image formats qemu: Add helper function for creating save image fd qemu: Move declaration of virQEMUSaveFormat to header file qemu: Add support for mapped-ram on save qemu: Move creation of qemuProcessIncomingDef struct qemu: Apply migration parameters in qemuMigrationDstRun qemu: Add support for mapped-ram on restore qemu: Support O_DIRECT with mapped-ram on save qemu: Support O_DIRECT with mapped-ram on restore qemu: Add support for parallel save and restore docs/manpages/virsh.rst | 21 +++- include/libvirt/libvirt-domain.h | 11 ++ src/libvirt-domain.c | 95 +++++++++++--- src/qemu/qemu.conf.in | 9 +- src/qemu/qemu_driver.c | 78 +++++++++--- src/qemu/qemu_fd.c | 46 +++++++ src/qemu/qemu_fd.h | 4 + src/qemu/qemu_migration.c | 204 ++++++++++++++++++++++--------- src/qemu/qemu_migration.h | 10 +- src/qemu/qemu_migration_params.c | 92 ++++++++++++++ src/qemu/qemu_migration_params.h | 17 +++ src/qemu/qemu_monitor.c | 37 ++++++ src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_process.c | 100 ++++++++++----- src/qemu/qemu_process.h | 19 ++- src/qemu/qemu_saveimage.c | 141 ++++++++++++--------- src/qemu/qemu_saveimage.h | 23 ++++ src/qemu/qemu_snapshot.c | 22 +++- tools/virsh-domain.c | 81 ++++++++++-- 19 files changed, 805 insertions(+), 210 deletions(-) -- 2.43.0

1 19

[PATCH] qemu: snapshot: Limit scope of checkpoint-snapshot interlock
by Peter Krempa 07 Feb '25

07 Feb '25

'qemuDomainSupportsCheckpointsBlockjobs()' should really be used only with active VMs based on the scope of interlocking it does. This means that the inactive snapshot code path needs to do the interlocking based on what's supported: - external snapshot support was not implemented yet (bitmaps need to be propagated to the new overlay image) - internal snapshot support can be deferred to qemu Move the check inside qemuSnapshotPrepare() which has knowledge about the snapshot type and implement an explicit check for the inactive case. See: https://gitlab.com/libvirt/libvirt/-/issues/739 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_snapshot.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 7ce018b026..ecf8eb3cca 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -39,6 +39,7 @@ #include "domain_audit.h" #include "locking/domain_lock.h" #include "virdomainsnapshotobjlist.h" +#include "virdomaincheckpointobjlist.h" #include "virqemu.h" #include "storage_source.h" @@ -1067,6 +1068,23 @@ qemuSnapshotPrepare(virDomainObj *vm, } + /* Handle interlocking with 'checkpoints': + * - if the VM is online use qemuDomainSupportsCheckpointsBlockjobs + * - if the VM is offline disallow external snapshots as the support for + * propagating bitmaps into the would-be-created overlay is not yet implemented + */ + if (!active) { + if (external && + virDomainListCheckpoints(vm->checkpoints, NULL, NULL, NULL, 0) > 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("support for offline external snapshots while checkpoint exists was not yet implemented")); + return -1; + } + } else { + if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) + return -1; + } + /* Alter flags to let later users know what we learned. */ if (external && !active) *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; @@ -2134,9 +2152,6 @@ qemuSnapshotCreateXML(virDomainPtr domain, VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, NULL); - if (qemuDomainSupportsCheckpointsBlockjobs(vm) < 0) - return NULL; - if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot halt after transient domain snapshot")); -- 2.48.1

2 1

Support for: VIR_DOMAIN_EVENT_ID_DBGTOOLS.
by Martin Harvey 07 Feb '25

07 Feb '25

Probably a FAQ. I have implemented a prototype: * QEMU event for an in-guest issue (tools installation / crash etc), via fwcfg / vmcore (similar). (QEMU_PROCESS_EVENT_DBGTOOLS_CHANGED). * Qemu Process / QemuMonitor handling for same event. * Mapped this through to VIR_DOMAIN_EVENT_ID_DBG_TOOLS, * Corresponding changes to top level event handling. We would find this *very useful* in cases where vm migration, debugging and the like require co-operation with guest user-mode. Are you folks happy to countenance some simple, but top-level, event handling changes triggered by guest software, or is this considered a no no? Should I be burying the details somewhere in EVENT_TUNABLE instead? Patchsets also available ... MH.

1 0

[PATCH] node_device_udev: add error reporting to udevProcessCCWGroup
by Boris Fiuczynski 07 Feb '25

07 Feb '25

Add reporting an internal error when the string to type conversion of devtype fails as this indicates a serious problem since devtype was used to get into this method during the udev event handling. Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- src/node_device/node_device_udev.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 7b4ff80f8f..344b39e97a 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1415,8 +1415,12 @@ udevProcessCCWGroup(struct udev_device *device, udevGenerateDeviceName(device, def, NULL); - if ((tmp = virNodeDevCCWGroupCapTypeFromString(devtype)) < 0) + if ((tmp = virNodeDevCCWGroupCapTypeFromString(devtype)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to find a supported CCW group capability type '%1$s'"), + devtype); return -1; + } data->ccwgroup_dev.type = tmp; -- 2.47.0

2 1

[PATCH 0/3] qemu: fix setup of FD passed disks
by Peter Krempa 07 Feb '25

07 Feb '25

See 3/3 Peter Krempa (3): qemuxmlconftest: Allow testing of the 'writable' flag for passed FDs for disks qemuxmlconftest: Add testing of FDs with 'writable' flag in 'disk-source-fd' qemu: domain: Initialize FD passthrough for a virStorageSource before using it src/qemu/qemu_domain.c | 6 ++-- .../disk-source-fd.x86_64-latest.args | 36 +++++++++++-------- .../disk-source-fd.x86_64-latest.xml | 21 +++++++++-- tests/qemuxmlconfdata/disk-source-fd.xml | 16 +++++++-- tests/qemuxmlconftest.c | 9 +++-- tests/testutilsqemu.c | 2 ++ tests/testutilsqemu.h | 2 +- 7 files changed, 67 insertions(+), 25 deletions(-) -- 2.48.1

2 4

[PATCH v2] ch: Enable SEV SNP support
by Praveen K Paladugu 06 Feb '25

06 Feb '25

Enable SEV-SNP support for ch guests. Co-Authored-by: Smit Gardhariya <sgardhariya(a)microsoft.com> Signed-off-by: Praveen K Paladugu <praveenkpaladugu(a)gmail.com> --- src/ch/ch_monitor.c | 70 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index bedcde2dde..55f0353fa9 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -130,29 +130,56 @@ static int virCHMonitorBuildPayloadJson(virJSONValue *content, virDomainDef *vmdef) { g_autoptr(virJSONValue) payload = virJSONValueNewObject(); - + g_autofree unsigned char *tmp = NULL; + size_t len; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_autofree char *host_data = NULL; + const size_t host_data_len = 32; if (vmdef->os.kernel == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Kernel image path in this domain is not defined")); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Kernel image path is not defined. With sev_snp=on, pass an igvm path")); return -1; - } else { - if (virJSONValueObjectAppendString(payload, "kernel", vmdef->os.kernel) < 0) - return -1; } - if (vmdef->os.cmdline) { - if (virJSONValueObjectAppendString(payload, "cmdline", vmdef->os.cmdline) < 0) + if (vmdef->sec && + vmdef->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP) { + if (virJSONValueObjectAppendString(payload, "igvm", vmdef->os.kernel) < 0) return -1; - } - if (vmdef->os.initrd != NULL) { - if (virJSONValueObjectAppendString(payload, "initramfs", vmdef->os.initrd) < 0) + if (vmdef->sec->data.sev_snp.host_data) { + /* Libvirt provided host_data is base64 encoded and cloud-hypervisor + requires host_data as hex encoded. Base64 decode and hex encode + before sending to cloud-hypervisor.*/ + tmp = g_base64_decode(vmdef->sec->data.sev_snp.host_data, &len); + if (len != host_data_len) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid host_data provdied. Expected '%1$ld' bytes"), + host_data_len); + return -1; + } + + while (len > 0) { + virBufferAsprintf(&buf, "%02x", tmp[host_data_len-len]); + len--; + } + + host_data = virBufferContentAndReset(&buf); + if (virJSONValueObjectAppendString(payload, "host_data", + host_data) < 0) + return -1; + } + } else { + if (virJSONValueObjectAdd(&payload, + "s:kernel", vmdef->os.kernel, + "S:cmdline", vmdef->os.cmdline, + "S:initramfs", vmdef->os.initrd, + NULL) < 0) return -1; } if (virJSONValueObjectAppend(content, "payload", &payload) < 0) - return -1; + return -1; return 0; } @@ -426,6 +453,23 @@ virCHMonitorBuildDevicesJson(virJSONValue *content, return 0; } +static int +virCHMonitorBuildPlatformJson(virJSONValue *content, virDomainDef *vmdef) +{ + g_autoptr(virJSONValue) platform = virJSONValueNewObject(); + + if (vmdef->sec && + vmdef->sec->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP) { + if (virJSONValueObjectAppendBoolean(platform, "sev_snp", 1) < 0) + return -1; + + if (virJSONValueObjectAppend(content, "platform", &platform) < 0) + return -1; + } + + return 0; +} + static int virCHMonitorBuildVMJson(virCHDriver *driver, virDomainDef *vmdef, char **jsonstr) @@ -454,6 +498,8 @@ virCHMonitorBuildVMJson(virCHDriver *driver, virDomainDef *vmdef, return -1; } + if (virCHMonitorBuildPlatformJson(content, vmdef) < 0) + return -1; if (virCHMonitorBuildDisksJson(content, vmdef) < 0) return -1; -- 2.47.0

2 1