February 2025 - Devel - libvirt List Archives

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

27 Feb '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> --- src/qemu/qemu_snapshot.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f7d6272907..c5f70f5b10 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2205,6 +2205,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 && @@ -2232,6 +2234,17 @@ qemuSnapshotRevertValidate(virDomainObj *vm, } } + 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 regural file, reverting to snapshot is not supported"), + disk->dst); + return -1; + } + } + return 0; } -- 2.48.1

2 3

Entering freeze for libvirt-11.1.0
by Jiri Denemark 27 Feb '25

27 Feb '25

I have just tagged v11.1.0-rc1 in the repository and pushed signed tarballs to https://download.libvirt.org/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka

1 1

[PATCH v3 0/1] add RAPL feature in libvirt
by Anthony Harivel 27 Feb '25

27 Feb '25

Hi, Since Fedora 41, RAPL feature in QEMU is available. The tested version with this patch is: qemu-9.1.2-2.fc41. Incorporating this feature into libvirt would significantly streamline the process of testing, adopting, and further developing it, thereby enhancing overall productivity and user experience. Thank you for your consideration. Anthony Anthony Harivel (1): qemu: Add support for RAPL MSRs feature docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 10 ++++++++++ src/qemu/qemu_command.c | 12 ++++++++++++ tests/qemuxmlconfdata/kvm-features-off.xml | 1 + .../kvm-features.x86_64-latest.args | 2 +- tests/qemuxmlconfdata/kvm-features.xml | 1 + 8 files changed, 47 insertions(+), 1 deletion(-) -- 2.48.1

2 5

[RFC 0/4] meson: Enable -Wundef
by Andrea Bolognani 27 Feb '25

27 Feb '25

A few days ago I have posted a patch[1] that addresses an issue introduced when a meson check was dropped but some uses of the corresponding WITH_ macro were not removed at the same time. That got me thinking about what we can do to prevent such scenarios from happening again in the future. I have come up with something that I think would be effective, but since applying the approach throughout the entire codebase would require a non-trivial amount of work, I figured I'd ask for feedback before embarking on it. The idea is that there are two types of macros we can use for conditional compilation: external ones, coming from the OS or other libraries, and internal ones, which are the result of meson tests. The external ones (e.g. SIOCSIFFLAGS, __APPLE__) are usually only defined if they apply, so it is correct to check for their presence with #ifdef. Using #if will also work, as undefined macros evaluate to zero, but it's not good practice to use them that way. If -Wundef has been passed to the compiler, those incorrect uses will be reported (only on platforms where they are not defined, of course). The internal ones (e.g. WITH_QEMU, WITH_STRUCT_IFREQ) are similar, but in this case we control their definition. This means that using means that the feature is not available on the machine we're building on, but it could also mean that we've removed the meson check and forgot to update all users of the macro. In this case, -Wundef would work 100% reliably to detect the issue: if the meson check doesn't exist, neither will the macro, regardless of what platform we're building on. So the approach I'm suggesting is to use a syntax-check rule to ensure that internal macros are only ever checked with #if instead of Of course this requires a full sweep to fix all cases in which we're not already doing things according to the proposal. Should be fairly easy, if annoying. A couple of examples are included here for demonstration purposes. The bigger impact is going to be on the build system. Right now we generally only define WITH_ macros if the check passed, but that will have to change and the result is going to be quite a bit of additional meson code I'm afraid. Thoughts? [1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/SKJ… Andrea Bolognani (4): configmake: Check for WIN32 correctly meson: Always define WITH_*_DECL macros syntax-check: Ensure WITH_ macros are used correctly meson: Enable -Wundef build-aux/syntax-check.mk | 5 +++++ configmake.h.in | 2 +- meson.build | 3 +++ tests/virmockstathelpers.c | 28 ++++++++++++++-------------- 4 files changed, 23 insertions(+), 15 deletions(-) -- 2.43.2

4 8

[PATCH 00/10] qemu: Refactor typed parameter handling in qemuDomainGetGuestInfo
by Peter Krempa 26 Feb '25

26 Feb '25

Use virTypedParam list to construct the returned data. Note that this applies on top of two patch(sets) that were ACK'd upstream: [PATCH v2 0/4] Add support for getting load averages from guest agent [PATCH] qemu: Report disk bus as reported by agent in virDomainGetGuestInfo You can fetch the whole thing via: git fetch https://gitlab.com/pipo.sk/libvirt.git agent-data-refactor Peter Krempa (10): qemuDomainGetGuestInfo: Prepare for refactor to virTypedParamList qemuDomainGetGuestInfo: Convert load code to virTypedParamList virDomainInterfaceFormatParams: Convert interface code to virTypedParamList qemuAgentDiskInfoFormatParams: Convert interface code to virTypedParamList qemuAgentFSInfoFormatParams: Convert interface code to virTypedParamList qemuAgentGetTimezone: Convert to virTypedParamList qemuAgentGetOSInfo: Convert to virTypedParamList qemuAgentGetUsers: Convert to virTypedParamList qemuDomainGetGuestInfo: Convert hostname code to virTypedParamList qemuDomainGetGuestInfo: Remove temporary infrastructure src/qemu/qemu_agent.c | 52 ++------- src/qemu/qemu_agent.h | 12 +- src/qemu/qemu_driver.c | 249 ++++++++++++----------------------------- tests/qemuagenttest.c | 148 +++++++++++------------- 4 files changed, 152 insertions(+), 309 deletions(-) -- 2.48.1

2 11

[PATCH 00/17] conf/qemu/etc: change several functions to return void instead of 0/-1
by Laine Stump 26 Feb '25

26 Feb '25

This started with me noticing one function call that checked for failure on something that I knew couldn't fail. So I changed that function to return void. But then I noticed another similar function that also should return void, so I changed that one too. Then eliminating the check for the return from those functions caused another function to become infallible, so I changed that one too, which led to more and more until the evening was finished and I had this long list of tiny mechanical patches. And here it is - an easy way to improve your review stats :-) P.S. I know there are more of these, but forced myself to stop here. A related question - is it possible for virObjectNew() to fail? I did finally find (after some searching, documentation that says g_object_new() can't return null, but I don't know enough about g_object stuff to know if the vir*Initialize functions could fail (for example). If virObjectNew() can't fail then that open a whole new can of worms... Laine Stump (17): conf: change virDomainHostdevInsert() to return void conf: change virDomainNetInsert() to return void conf: change virDomainFSInsert() to return void conf: change virDomainShmemDefInsert() to return void conf: change virDomainDefMaybeAddInput() to return void libxl: change xenDomainDefAddImplicitInputDevice() to return void conf: change qemuDomainDefAddImplicitInputDevice() to return void conf: stop checking for NULL return from virDomainControllerDefNew() conf: stop checking for NULL return from virDomainDefAddController() conf: change virDomainDefAddUSBController() to return void hyperv: change hypervDomainDefAppendController() to return void conf: change virDomainDefMaybeAddController() to return true/false conf: change virDomainDefMaybeAddHostdevSCSIcontroller() to return void conf: change virDomainDefAddDiskControllersForType() to return void conf: change virDomainDefMaybeAddVirtioSerialController() to return void conf: change virDomainDefMaybeAddSmartcardController() to return void conf: change virDomainDefAddImplicitControllers() to return void src/bhyve/bhyve_domain.c | 13 ++- src/conf/domain_addr.c | 6 +- src/conf/domain_conf.c | 174 ++++++++++----------------------- src/conf/domain_conf.h | 16 +-- src/hyperv/hyperv_driver.c | 28 ++---- src/libxl/libxl_conf.c | 4 +- src/libxl/libxl_domain.c | 11 +-- src/libxl/libxl_driver.c | 11 +-- src/libxl/xen_common.c | 15 +-- src/libxl/xen_common.h | 2 +- src/libxl/xen_xl.c | 4 +- src/lxc/lxc_driver.c | 6 +- src/qemu/qemu_domain_address.c | 8 +- src/qemu/qemu_driver.c | 14 +-- src/qemu/qemu_postparse.c | 58 ++++------- src/qemu/qemu_process.c | 3 +- src/vbox/vbox_common.c | 13 +-- src/vmx/vmx.c | 12 +-- src/vz/vz_driver.c | 11 +-- src/vz/vz_sdk.c | 14 +-- 20 files changed, 126 insertions(+), 297 deletions(-) -- 2.47.1

3 35

[PATCH 0/2] cpu_map: arm: Add AmpereOne CPU models
by Jiri Denemark 26 Feb '25

26 Feb '25

Jiri Denemark (2): cpu_arm: Report vendor ID for unknown PVRs cpu_map: arm: Add AmpereOne CPU models src/cpu/cpu_arm.c | 6 ++++-- src/cpu_map/arm_Ampere-1.xml | 6 ++++++ src/cpu_map/arm_Ampere-1a.xml | 6 ++++++ src/cpu_map/arm_vendors.xml | 1 + src/cpu_map/index.xml | 5 +++++ 5 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 src/cpu_map/arm_Ampere-1.xml create mode 100644 src/cpu_map/arm_Ampere-1a.xml -- 2.48.1

2 3

[PATCH 1/2] virt-aa-helper: Avoid using RUNSTATEDIR
by Michal Privoznik 26 Feb '25

26 Feb '25

On some systems /run is mounted as: tmpfs on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,size=348508k,mode=755,inode64) and /var/run is then just a symlink: # ls -ld /var/run lrwxrwxrwx 1 root root 4 Apr 23 2024 /var/run -> /run But because we still think it's 2004 and FHS 2.3 is active we have a rule in our meson.build which constructs RUNSTATEDIR as the following: runstatedir = get_option('runstatedir') if runstatedir == '' runstatedir = localstatedir / 'run' endif which (if unspecified on meson setup line) results in "/var/run". This in turn means, when when we're generating an AppArmor profile for a domain with allowed paths it contains stuff like: /var/run/libvirt/qemu/swtpm/2-guest-swtpm.sock But because of the aforementioned symlink the real path is: /run/libvirt/qemu/swtpm/2-guest-swtpm.sock and thus AppArmor denies access: audit: type=1400 audit(1740480419.348:415): apparmor="DENIED" operation="connect" class="file" profile="libvirt-126f2720-6f8e-45ab-a886-ec9277079a67" name="/run/libvirt/qemu/swtpm/2-guest-swtpm.sock" pid=8080 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=64055 Fortunately, there's a nice trick: AppArmor profile variables. We already use some of them (@{PROC}, @{HOME}, @{multiarch}) and instead of RUNSTATEDIR we can use @{run} which is declared as: # cat /etc/apparmor.d/tunables/run @{run}=/run/ /var/run/ and thus covers both scenarios. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/virt-aa-helper.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index c255b64f35..eabc29e9cf 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1238,8 +1238,8 @@ get_files(vahControl * ctl) /* Unix socket for QEMU and swtpm to use */ virBufferAsprintf(&buf, - " \"%s/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n", - RUNSTATEDIR, shortName); + " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n", + shortName); /* Paths for swtpm to use: give it access to its state * directory (state files and fsync on dir), log, and PID files. */ @@ -1253,8 +1253,8 @@ get_files(vahControl * ctl) " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n", LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, - " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n", - RUNSTATEDIR, shortName); + " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n", + shortName); VIR_FREE(shortName); } @@ -1528,10 +1528,10 @@ main(int argc, char **argv) LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"%s/lib/libvirt/qemu/domain-%d-%.*s/*\" rw,\n", LOCALSTATEDIR, ctl->def->id, 20, ctl->def->name); - virBufferAsprintf(&buf, " \"%s/libvirt/**/%s.pid\" rwk,\n", - RUNSTATEDIR, ctl->def->name); - virBufferAsprintf(&buf, " \"%s/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", - RUNSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"@{run}/libvirt/**/%s.pid\" rwk,\n", + ctl->def->name); + virBufferAsprintf(&buf, " \"@{run}/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + ctl->def->name); } if (ctl->files) virBufferAdd(&buf, ctl->files, -1); -- 2.45.3

2 3

[PATCH] virt-aa-helper: Avoid using RUNSTATEDIR
by Michal Privoznik 26 Feb '25

26 Feb '25

On some systems /run is mounted as: tmpfs on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,size=348508k,mode=755,inode64) and /var/run is then just a symlink: # ls -ld /var/run lrwxrwxrwx 1 root root 4 Apr 23 2024 /var/run -> /run But because we still think it's 2004 and FHS 2.3 is active we have a rule in our meson.build which constructs RUNSTATEDIR as the following: runstatedir = get_option('runstatedir') if runstatedir == '' runstatedir = localstatedir / 'run' endif which (if unspecified on meson setup line) results in "/var/run". This in turn means, when when we're generating an AppArmor profile for a domain with allowed paths it contains stuff like: /var/run/libvirt/qemu/swtpm/2-guest-swtpm.sock But because of the aforementioned symlink the real path is: /run/libvirt/qemu/swtpm/2-guest-swtpm.sock and thus AppArmor denies access: audit: type=1400 audit(1740480419.348:415): apparmor="DENIED" operation="connect" class="file" profile="libvirt-126f2720-6f8e-45ab-a886-ec9277079a67" name="/run/libvirt/qemu/swtpm/2-guest-swtpm.sock" pid=8080 comm="qemu-system-x86" requested_mask="wr" denied_mask="wr" fsuid=64055 ouid=64055 Fortunately, there's a nice trick: AppArmor profile variables. We already use some of them (@{PROC}, @{HOME}, @{multiarch}) and instead of RUNSTATEDIR we can use @{run} which is declared as: # cat /etc/apparmor.d/tunables/run @{run}=/run/ /var/run/ and thus covers both scenarios. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/security/virt-aa-helper.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index c255b64f35..eabc29e9cf 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1238,8 +1238,8 @@ get_files(vahControl * ctl) /* Unix socket for QEMU and swtpm to use */ virBufferAsprintf(&buf, - " \"%s/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n", - RUNSTATEDIR, shortName); + " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n", + shortName); /* Paths for swtpm to use: give it access to its state * directory (state files and fsync on dir), log, and PID files. */ @@ -1253,8 +1253,8 @@ get_files(vahControl * ctl) " \"%s/log/swtpm/libvirt/qemu/%s-swtpm.log\" w,\n", LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, - " \"%s/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n", - RUNSTATEDIR, shortName); + " \"@{run}/libvirt/qemu/swtpm/%s-swtpm.pid\" rw,\n", + shortName); VIR_FREE(shortName); } @@ -1528,10 +1528,10 @@ main(int argc, char **argv) LOCALSTATEDIR, ctl->def->name); virBufferAsprintf(&buf, " \"%s/lib/libvirt/qemu/domain-%d-%.*s/*\" rw,\n", LOCALSTATEDIR, ctl->def->id, 20, ctl->def->name); - virBufferAsprintf(&buf, " \"%s/libvirt/**/%s.pid\" rwk,\n", - RUNSTATEDIR, ctl->def->name); - virBufferAsprintf(&buf, " \"%s/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", - RUNSTATEDIR, ctl->def->name); + virBufferAsprintf(&buf, " \"@{run}/libvirt/**/%s.pid\" rwk,\n", + ctl->def->name); + virBufferAsprintf(&buf, " \"@{run}/libvirt/**/*.tunnelmigrate.dest.%s\" rw,\n", + ctl->def->name); } if (ctl->files) virBufferAdd(&buf, ctl->files, -1); -- 2.45.3

2 1

[PATCH] qemu: Report disk bus as reported by agent in virDomainGetGuestInfo
by Peter Krempa 26 Feb '25

26 Feb '25

KubeVirt decided to report this to the users. In order to allow them to use proper APIs expose the field as well. Resolves: https://issues.redhat.com/browse/RHEL-80688 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- docs/manpages/virsh.rst | 1 + src/libvirt-domain.c | 1 + src/qemu/qemu_driver.c | 9 +++++++++ 3 files changed, 11 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index f159c40631..baced15dec 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2999,6 +2999,7 @@ returned: * ``disk.<num>.serial`` - optional disk serial number * ``disk.<num>.alias`` - the device alias of the disk (e.g. sda) * ``disk.<num>.guest_alias`` - optional alias assigned to the disk +* ``disk.<num>.guest_bus`` - bus type as reported by the guest *--interface* returns: * ``if.count`` - the number of interfaces defined on this domain diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index ae15ab1e5a..05101756a1 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -13273,6 +13273,7 @@ virDomainSetVcpu(virDomainPtr domain, * "disk.<num>.alias" - the device alias of the disk (e.g. sda) * "disk.<num>.guest_alias" - optional alias assigned to the disk, on Linux * this is a name assigned by device mapper + * "disk.<num>.guest_bus" - disk bus as reported by the guest OS * * VIR_DOMAIN_GUEST_INFO_HOSTNAME: * Returns information about the hostname of the domain. The typed diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3b8b2c0a1e..86b945d9b9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19278,6 +19278,15 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, param_name, diskdef->dst) < 0) return; } + + if (address->bus_type) { + g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "disk.%zu.guest_bus", i); + + if (virTypedParamsAddString(params, nparams, maxparams, + param_name, address->bus_type) < 0) + return; + } } if (info[i]->alias) { -- 2.48.1

2 1