March 2025 - Devel - libvirt List Archives

[libvirt PATCH v2] qemu: snapshot: error out early when reverting snapshot for VM with non-file disk
by Pavel Hrdina 11 Mar '25

11 Mar '25

Before this patch the code would start the revert process by destroying the VM and preparing to revert where it would fail with following error: error: unsupported configuration: source for disk 'sdb' is not a regular file; refusing to generate external snapshot name and leaving user with offline VM even if it was running. Make the check before we start the revert process to not destroy VMs. Resolves: https://issues.redhat.com/browse/RHEL-30971 Resolves: https://issues.redhat.com/browse/RHEL-79928 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- Changes in v2: - add comments - limit the check for external snapshots src/qemu/qemu_snapshot.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 891e67e98b..dab841ef5b 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2203,6 +2203,8 @@ qemuSnapshotRevertValidate(virDomainObj *vm, virDomainSnapshotDef *snapdef, unsigned int flags) { + size_t i; + if (!vm->persistent && snapdef->state != VIR_DOMAIN_SNAPSHOT_RUNNING && snapdef->state != VIR_DOMAIN_SNAPSHOT_PAUSED && @@ -2230,6 +2232,22 @@ qemuSnapshotRevertValidate(virDomainObj *vm, } } + /* Reverting to external snapshot creates overlay files for every disk and + * it would fail for non-file based disks. + * See qemuSnapshotRevertExternalPrepare for more details. */ + if (virDomainSnapshotIsExternal(snap)) { + for (i = 0; i < snap->def->dom->ndisks; i++) { + virDomainDiskDef *disk = snap->def->dom->disks[i]; + + if (disk->src->type != VIR_STORAGE_TYPE_FILE) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("source disk for '%1$s' is not a regular file, reverting to snapshot is not supported"), + disk->dst); + return -1; + } + } + } + return 0; } @@ -2381,6 +2399,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, if (virDomainMomentDefPostParse(&tmpsnapdef->parent) < 0) return -1; + /* Force default location to be external in order to create overlay files + * for every disk. In qemuSnapshotRevertValidate we make sure that each + * disk is regular file otherwise this would fail. */ if (virDomainSnapshotAlignDisks(tmpsnapdef, domdef, VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL, false, true) < 0) { -- 2.48.1

2 1

[PATCH RFC-v2 00/11] qemu: Add support for iothread to virtqueue mapping for 'virtio-scsi'
by Peter Krempa 11 Mar '25

11 Mar '25

Unfortunately still RFC as the qemu support is still not merged. Once again there are a few patches that are applicable: qemu: Use modern header formatting in 'qemu_command.h' virXMLNodeGetSubelementList: Document return value semantics virDomainIothreadMappingDefParse: Fix usage of virXMLNodeGetSubelementList docs: formatdomain: Clarify configuration of iothread <-> virtqueue mapping The patches below are RFC: qemucapabilitiestest: Update 'caps_10.0.0_x86_64' to XXXXXX qemu: capabilities: Introduce QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING virDomainIothreadMapping: Add support for naming certain virt queues conf: Add support for iothread to queue mapping config for 'virtio-scsi' qemu: Implement support for iothread <-> virtqueue mapping for 'virtio-scsi' controllers qemuxmlconftest: Add 'iothreads-virtio-scsi-mapping' case NEWS: Mention multiple iothread support for 'virtio-scsi' controller Changes to v1: - pushed the non-RFC refactors - few new cleanups added - new docs patch emphasizing the auto-assignment (in example XML and description) in qemu and improved wording of docs for 'virtio-blk' miltiqueue mapping - changed code to use names for 'ctrl' and 'event' virtqueues of 'virtio-scsi' - used the improved docs also for virtio scsi - added NEWS NEWS.rst | 6 + docs/formatdomain.rst | 66 ++++++++++- src/conf/domain_conf.c | 32 ++++-- src/conf/domain_conf.h | 4 +- src/conf/domain_validate.c | 10 +- src/conf/schemas/domaincommon.rng | 9 +- src/hypervisor/domain_driver.c | 3 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 86 ++++++++++++-- src/qemu/qemu_command.h | 95 ++++++++++------ src/qemu/qemu_domain.c | 10 +- src/qemu/qemu_validate.c | 105 ++++++++++++------ src/util/virxml.c | 2 + .../caps_10.0.0_x86_64.replies | 7 +- .../caps_10.0.0_x86_64.xml | 3 +- ...ads-virtio-scsi-mapping.x86_64-latest.args | 40 +++++++ ...eads-virtio-scsi-mapping.x86_64-latest.xml | 65 +++++++++++ .../iothreads-virtio-scsi-mapping.xml | 57 ++++++++++ tests/qemuxmlconftest.c | 1 + 20 files changed, 495 insertions(+), 109 deletions(-) create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/iothreads-virtio-scsi-mapping.xml -- 2.48.1

2 16

[PATCH 00/19] Add typed param constants for guest info and domain stats APIs
by Daniel P. Berrangé 11 Mar '25

11 Mar '25

Contrary to most APIs returning typed parameters, there are no constants defined for the domain stats data keys. This is was because many of the keys needs to be dynamically constructed using one or more array index values. It is possible to define constants while still supporting dynamic array indexes by simply defining the prefixes and suffixes as constants. The consuming code can then combine the constants with array index value. With this approach, it is practical to add constants for the domain stats API keys. The pattern for array entries is as follows * VIR_DOMAIN_GUEST_INFO_blah_COUNT - the number of items in the array * VIR_DOMAIN_GUEST_INFO_blah_PREFIX - the key prefix, including the trailing '.' - eg "disk." * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah - the key suffix, including the leading '.' - eg ".name" Where there are nested arrays, this repeats in the natural way * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah_COUNT - the number of items in the array * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah_PREFIX - the key prefix, including the leading and trailing '.' - eg ".addr." * VIR_DOMAIN_GUEST_INFO_blah_SUFFIX_blah_SUFFIX_blah - the key suffix, including the leading '.' - eg ".name" A usage of this would look like, relying on CPP string concatenation asprintf(&str, VIR_DOMAIN_STATS_VCPU_PREFIX "%d" VIR_DOMAIN_STATS_VCPU_SUFFIX_DELAY, idx); Or in language bindings such as Golang fmt.Sprintf(C.VIR_DOMAIN_STATS_VCPU_PREFIX+"%d"+C.VIR_DOMAIN_STATS_VCPU_SUFFIX_HALTED, idx) Adding these constants has a few benefits: * We can ensure that different drivers are all consistent in keys used for typed parameters * Language bindings can query the API XML to identify any gaps in their API coverage A note on "Since" tags. We use the "Since" tags to express when an API was added to libvirt. In this case we're adding constants long /after/ we first introduced these APIs. So the question is whether the Since tags reflect when the typed parameter key was first used or when the API constant was added. For convenience I chose the latter, because the Golang bnidings use the Since tags to auto generate code with #if !LIBVIRT_CHECK_VERSION() around its usage of the constants. This would break if we retroactively dated the Since tags to when the keys were first used. As proof of the value, adding these constants has lead to be find sooooooooooo many mistakes in the Golang bindings where we forgot to add new domain stats/guest info data items, and one place where we typod the name. Daniel P. Berrangé (19): src: add constants for guest info 'user.' parameters src: add constants for guest info 'os.' parameters src: add constants for guest info 'timezone.' parameters src: add constant for the guest info 'hostname' parameter src: add constants for guest info 'fs.' parameters src: add constants for guest info 'disk.' parameters src: add constants for guest info 'if.' parameters src: add constants for domain stats 'state.' parameters src: add constants for domain stats 'cpu.' parameters src: add constants for domain stats 'balloon.' parameters src: add constants for domain stats 'vcpu.' parameters src: add constants for domain stats 'net.' parameters src: add constants for domain stats 'block.' parameters src: add constants for domain stats 'perf.' parameters src: add constants for domain stats 'iothread.' parameters src: add constants for domain stats 'memory.' parameters src: add constants for domain stats 'dirtyrate.' parameters src: add constants for domain stats 'dirtyrate.' parameters src: document that no constants are provided for custom VM stats include/libvirt/libvirt-domain.h | 1614 ++++++++++++++++++++++++++++++ src/libvirt-domain.c | 424 ++------ src/qemu/qemu_agent.c | 36 +- src/qemu/qemu_driver.c | 425 +++++--- 4 files changed, 1996 insertions(+), 503 deletions(-) -- 2.48.1

2 41

[PATCH] security: Don't stop restoring labels too early
by Michal Privoznik 11 Mar '25

11 Mar '25

The point of virSecurityManagerRestoreAllLabel() function is to restore ALL labels and be tolerant to possible errors, i.e. continue restoring seclabels and NOT return early. Well, in two implementations of this internal API this type of problem was found: 1) virSecurityDACRestoreAllLabel() returned early if virSecurityDACRestoreGraphicsLabel() failed, or when def->sec->sectype equals to an impossible value. 2) virSecuritySELinuxRestoreAllLabel() returned early if virSecuritySELinuxRestoreMemoryLabel() failed. Fix all three places. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Inspired by this Peter's patch: https://marc.info/?l=libvir-list&m=174169408218982&w=2 src/security/security_dac.c | 4 ++-- src/security/security_selinux.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index e07977300f..3ecbc7277d 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1973,7 +1973,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, for (i = 0; i < def->ngraphics; i++) { if (virSecurityDACRestoreGraphicsLabel(mgr, def, def->graphics[i]) < 0) - return -1; + rc = -1; } for (i = 0; i < def->ninputs; i++) { @@ -2021,7 +2021,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, case VIR_DOMAIN_LAUNCH_SECURITY_NONE: case VIR_DOMAIN_LAUNCH_SECURITY_LAST: virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype); - return -1; + rc = -1; } } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 38e611f567..64e7f41ce0 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2969,7 +2969,7 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr, for (i = 0; i < def->nmems; i++) { if (virSecuritySELinuxRestoreMemoryLabel(mgr, def, def->mems[i]) < 0) - return -1; + rc = -1; } for (i = 0; i < def->ntpms; i++) { -- 2.48.1

2 1

[PATCH 0/3] qemu: Refactor typed parameter list construction in SEV apis
by Peter Krempa 11 Mar '25

11 Mar '25

Peter Krempa (3): qemuDomainGetLaunchSecurityInfo: Don't forget unlock VM object on (impossible) error qemuDomainGetLaunchSecurityInfo: Use virTypedParamList to construct return value qemuNodeGetSEVInfo: Use virTypedParamList to construct return value src/qemu/qemu_driver.c | 107 ++++++++++++----------------------------- 1 file changed, 31 insertions(+), 76 deletions(-) -- 2.48.1

2 5

[PATCH] docs: Correct dbus graphics' accepted p2p values
by Martin Kletzander 11 Mar '25

11 Mar '25

The attribute is used (and formatted) as virTristateBool() and even in schema defined as virYesNo, so the values are supposed to be `yes` and `no`. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- docs/formatdomain.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 4d03768c5f4e..3bbf94e192c9 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -6740,7 +6740,7 @@ interaction with the admin. <graphics type='dbus'/> - ``p2p`` (accepts ``on`` or ``off``) enables peer-to-peer connections, + ``p2p`` (accepts ``yes`` or ``no``) enables peer-to-peer connections, established through virDomainOpenGraphics() APIs. ``address`` (accepts a `D-Bus address -- 2.48.1

2 1

[PATCH 0/3] qemu: fix shared memory validation for vhost-user interfaces
by Peter Krempa 11 Mar '25

11 Mar '25

Use the common check which produces hard errors instead of the post-start check adding a warning which wasn't updated with the new logic. Peter Krempa (3): qemuxmlconftest: Include shared memory 'net-vhostuser' test cases qemuValidateDomainDeviceDefNetwork: Require shared memory for all vhost-user interfaces qemu: process: Remove un-updated 'qemuProcessStartWarnShmem' src/qemu/qemu_process.c | 54 ------------------- src/qemu/qemu_validate.c | 9 ++-- .../net-vhostuser-fail.x86_64-latest.xml | 3 ++ tests/qemuxmlconfdata/net-vhostuser-fail.xml | 3 ++ .../net-vhostuser-multiq.x86_64-latest.args | 2 +- .../net-vhostuser-multiq.x86_64-latest.xml | 3 ++ .../qemuxmlconfdata/net-vhostuser-multiq.xml | 3 ++ ...vhostuser-passt-no-shmem.x86_64-latest.err | 2 +- .../net-vhostuser.x86_64-latest.args | 2 +- .../net-vhostuser.x86_64-latest.xml | 3 ++ tests/qemuxmlconfdata/net-vhostuser.xml | 3 ++ 11 files changed, 24 insertions(+), 63 deletions(-) -- 2.48.1

2 4

[PATCH] esx: Refactor esxVI_LookupHostScsiTopologyLunListByTargetName
by Jiri Denemark 11 Mar '25

11 Mar '25

With a specific combination of compiler options gcc reported the following bogus warning (I added a context to it to make the issue visible): ../src/esx/esx_vi.c: In function ‘esxVI_LookupHostScsiTopologyLunListByTargetName’: ../src/esx/esx_vi.c:4674:32: error: potential null pointer dereference [-Werror=null-dereference] 4671 | if (!found || !hostScsiTopologyTarget) 4672 | goto cleanup; 4673 | 4674 | if (!hostScsiTopologyTarget->lun) { | ~~~~~~~~~~~~~~~~~~~~~~^~~~~ Most likely this is caused by found and hostScsiTopologyTarget doing essentially the same thing as found is true if and only if hostScsiTopologyTarget is non-NULL. The found variable is completely redundant. Removing it would be enough, but I decided to make the code a little bit easier to read by not using the iterator variable directly. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/esx/esx_vi.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index e2589aa69a..aee649b86e 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -4616,7 +4616,6 @@ esxVI_LookupHostScsiTopologyLunListByTargetName esxVI_HostScsiTopologyInterface *hostScsiInterfaceList = NULL; esxVI_HostScsiTopologyInterface *hostScsiInterface = NULL; esxVI_HostScsiTopologyTarget *hostScsiTopologyTarget = NULL; - bool found = false; esxVI_HostInternetScsiTargetTransport *candidate = NULL; ESX_VI_CHECK_ARG_LIST(hostScsiTopologyLunList); @@ -4653,22 +4652,21 @@ esxVI_LookupHostScsiTopologyLunListByTargetName /* See vSphere API documentation about HostScsiTopologyInterface */ for (hostScsiInterface = hostScsiInterfaceList; - hostScsiInterface && !found; + hostScsiInterface && !hostScsiTopologyTarget; hostScsiInterface = hostScsiInterface->_next) { - for (hostScsiTopologyTarget = hostScsiInterface->target; - hostScsiTopologyTarget; - hostScsiTopologyTarget = hostScsiTopologyTarget->_next) { + esxVI_HostScsiTopologyTarget *target; + for (target = hostScsiInterface->target; target; target = target->_next) { candidate = esxVI_HostInternetScsiTargetTransport_DynamicCast - (hostScsiTopologyTarget->transport); + (target->transport); if (candidate && STREQ(candidate->iScsiName, name)) { - found = true; + hostScsiTopologyTarget = target; break; } } } - if (!found || !hostScsiTopologyTarget) + if (!hostScsiTopologyTarget) goto cleanup; if (!hostScsiTopologyTarget->lun) { -- 2.48.1

2 1

[PATCH 0/3] Full boot order support on s390x
by Boris Fiuczynski 11 Mar '25

11 Mar '25

This series adds on s390x full boot order support which has been introduced recently in QEMU with the PR https://lore.kernel.org/qemu-devel/20241023131710.906748-1-thuth@redhat.com/ The replies and xml files are removed from the patch in this series and are available in https://gitlab.com/fiuczy/libvirt/-/commits/fullbootorder Boris Fiuczynski (3): qemu: capabilities: Add QEMU_CAPS_VIRTIO_CCW_DEVICE_LOADPARM tests: add capabilities for QEMU 9.2.0 on s390x qemu: command: add multi boot device support on s390x src/qemu/qemu_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 40 +- src/qemu/qemu_command.h | 6 +- src/qemu/qemu_hotplug.c | 6 +- tests/domaincapsdata/qemu_9.2.0.s390x.xml | 311 + .../caps_5.2.0_s390x.replies | 321 +- .../caps_6.0.0_s390x.replies | 321 +- .../caps_8.1.0_s390x.replies | 321 +- .../caps_8.2.0_s390x.replies | 321 +- .../caps_9.1.0_s390x.replies | 321 +- .../caps_9.2.0_s390x.replies | 36741 ++++++++++++++++ .../qemucapabilitiesdata/caps_9.2.0_s390x.xml | 3752 ++ .../machine-loadparm-hostdev.s390x-9.1.0.args | 33 + .../machine-loadparm-hostdev.s390x-9.1.0.xml | 33 + ...machine-loadparm-hostdev.s390x-latest.args | 4 +- ...-multiple-disks-nets-s390.s390x-9.1.0.args | 40 + ...m-multiple-disks-nets-s390.s390x-9.1.0.xml | 51 + ...multiple-disks-nets-s390.s390x-latest.args | 8 +- ...machine-loadparm-net-s390.s390x-9.1.0.args | 34 + .../machine-loadparm-net-s390.s390x-9.1.0.xml | 32 + ...achine-loadparm-net-s390.s390x-latest.args | 4 +- .../machine-loadparm-s390.s390x-9.1.0.args | 34 + .../machine-loadparm-s390.s390x-9.1.0.xml | 33 + .../machine-loadparm-s390.s390x-latest.args | 4 +- tests/qemuxmlconftest.c | 4 + 26 files changed, 42664 insertions(+), 120 deletions(-) create mode 100644 tests/domaincapsdata/qemu_9.2.0.s390x.xml create mode 100644 tests/qemucapabilitiesdata/caps_9.2.0_s390x.replies create mode 100644 tests/qemucapabilitiesdata/caps_9.2.0_s390x.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-hostdev.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-hostdev.s390x-9.1.0.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-multiple-disks-nets-s390.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-multiple-disks-nets-s390.s390x-9.1.0.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-net-s390.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-net-s390.s390x-9.1.0.xml create mode 100644 tests/qemuxmlconfdata/machine-loadparm-s390.s390x-9.1.0.args create mode 100644 tests/qemuxmlconfdata/machine-loadparm-s390.s390x-9.1.0.xml -- 2.45.0

2 11

[PATCH v1 0/3] Introduce qemuDomainSetVcpuTuneParameters API
by yong.huang＠smartx.com 10 Mar '25

10 Mar '25

From: Hyman Huang <yong.huang(a)smartx.com> This patchset is the prerequisite for the vCPU dirty-limit feature: https://patchew.org/Libvirt/cover.1703135535.git.yong.huang@smartx.com/ As suggested by Daniel: We've generally tried to avoid adding single purpose APIs for tunable parameters, instead using APIs with virTypedParameter arrays to allow bulk updates. I note that we don't appear to have any mechanism currently to set the VCPU scheduler tunables either Perhaps we should have a more general virDomainSetVCPUTuneParameters(virDomainPtr domain, int vcpu, virTypedParameterPtr params, unsigned int params, unsigned int flags); Refer the following link to see more details: https://patchew.org/Libvirt/169397083100.4628.15196043252714532301-0@git.sr… We present the qemuDomainSetVcpuTuneParameters API separately because the patchset is somewhat self-contained. Please review, Yong Hyman Huang (3): libvirt: Add virDomainSetVcpuTuneParameters API qemu_driver: Implement qemuDomainSetVcpuTuneParameters virsh: Use the new API to implement cmdSetvcpu include/libvirt/libvirt-domain.h | 25 ++++++++++++++ src/driver-hypervisor.h | 8 +++++ src/libvirt-domain.c | 56 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 29 +++++++++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 20 +++++++++++- tools/virsh-domain.c | 10 +++++- 8 files changed, 152 insertions(+), 2 deletions(-) -- 2.27.0

4 6