March 2023 - Devel - libvirt List Archives

[PATCH] virt-host-validate: Detect SMMU support on ARMs
by Michal Privoznik 24 Mar '23

24 Mar '23

In vir-host-validate we do two checks related to IOMMU: 1) hardware support, and 2) kernel support. While users are usually interested in the latter, the former also makes sense. And for the former (hardware support) we have this huge if-else block for nearly every architecture, except ARM. Now, IOMMU is called SMMU in ARM world, and while there's certainly a definitive way of detecting SMMU support (e.g. via dumping some registers in asm), we can work around this - just like we do for Intel and AMD - and check for an ACPI table presence. In ARM world, there's I/O Remapping Table (IORT) which describes SMMU capabilities on given host and is exposed in sysfs (regardless of arm_smmu module). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2178885 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virt-host-validate-common.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index a41bb346d2..49d3c4083b 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -388,6 +388,15 @@ int virHostValidateIOMMU(const char *hvname, return VIR_HOST_VALIDATE_FAILURE(VIR_HOST_VALIDATE_NOTE); } virHostMsgPass(); + } else if (ARCH_IS_ARM(arch)) { + if (access("/sys/firmware/acpi/tables/IORT", F_OK) == 0) { + virHostMsgPass(); + } else { + virHostMsgFail(level, + "No ACPI IORT table found, IOMMU not " + "supported by this hardware platform"); + return VIR_HOST_VALIDATE_FAILURE(level); + } } else { virHostMsgFail(level, "Unknown if this platform has IOMMU support"); -- 2.39.2

2 1

[PATCH] ci: Disable optimization on macos-12
by Martin Kletzander 24 Mar '23

24 Mar '23

This enables our CI to run tests on on macos-12 after commit 7944700b4037. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- The pipeline can be seen here: https://gitlab.com/nertpinx/libvirt/-/pipelines/816876889 ci/gitlab/builds.yml | 1 + ci/manifest.yml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml index 545478d3516d..f070db637c80 100644 --- a/ci/gitlab/builds.yml +++ b/ci/gitlab/builds.yml @@ -865,6 +865,7 @@ aarch64-macos-12: needs: [] allow_failure: false variables: + CFLAGS: -O0 CIRRUS_VM_IMAGE_NAME: ghcr.io/cirruslabs/macos-monterey-base:latest CIRRUS_VM_IMAGE_SELECTOR: image CIRRUS_VM_INSTANCE_TYPE: macos_instance diff --git a/ci/manifest.yml b/ci/manifest.yml index b68c7361abb4..2f631376b24b 100644 --- a/ci/manifest.yml +++ b/ci/manifest.yml @@ -179,6 +179,8 @@ targets: variables: PATH_EXTRA: /usr/local/opt/ccache/libexec:/usr/local/opt/gettext/bin:/usr/local/opt/libpcap/bin:/usr/local/opt/libxslt/bin:/usr/local/opt/rpcgen/bin PKG_CONFIG_PATH: /usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/libpcap/lib/pkgconfig:/usr/local/opt/libxml2/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig + # Remove once macos has clang with -fsemantic-interposition + CFLAGS: -O0 ubuntu-2004: jobs: -- 2.40.0

2 1

[PATCH] esx: Change the default username when connecting to VCenter
by Michal Privoznik 24 Mar '23

24 Mar '23

While it's true that the default username is: administrator@${SSO-Domain} in majority of cases the ${SSO-Domain} is "vsphere.local". But our code (and what virsh displays then) says it's just "administrator". This is wrong also from a different POV: the username must contain the suffix no matter what and our default suggests otherwise. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2181234 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/esx/esx_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 9dc5489411..2443e6e88a 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -701,7 +701,8 @@ esxConnectToVCenter(esxPrivate *priv, if (conn->uri->user) { username = g_strdup(conn->uri->user); } else { - if (!(username = virAuthGetUsername(conn, auth, "esx", "administrator", + if (!(username = virAuthGetUsername(conn, auth, "esx", + "administrator(a)vsphere.local", hostname))) return -1; } -- 2.39.2

2 1

Re: [PATCH] vfio/pci: Propagate ACPI notifications to the user-space
by Alex Williamson 24 Mar '23

24 Mar '23

[Cc +libvir-list] On Wed, 8 Mar 2023 12:41:24 +0100 Grzegorz Jaszczyk <jaz(a)semihalf.com> wrote: > śr., 8 mar 2023 o 00:42 Alex Williamson <alex.williamson(a)redhat.com> napisał(a): > > > > On Tue, 7 Mar 2023 22:05:53 +0000 > > Grzegorz Jaszczyk <jaz(a)semihalf.com> wrote: > > > > > From: Dominik Behr <dbehr(a)chromium.org> > > > > > > Hitherto there was no support for propagating ACPI notifications to the > > > guest drivers. In order to provide such support, install a handler for > > > notifications on an ACPI device during vfio-pci device registration. The > > > handler role is to propagate such ACPI notifications to the user-space > > > via acpi netlink events, which allows VMM to receive and propagate them > > > further to the VMs. > > > > > > Thanks to the above, the actual driver for the pass-through device, > > > which belongs to the guest, can receive and react to device specific > > > notifications. > > > What consumes these events? > > Those events are consumed by the VMM, which can have a built-in ACPI > event listener. > > > Has this been proposed to any VM management tools like libvirt? > > This patch was evaluated and tested with crosvm VMM (but since the > kernel part is not in the tree the implementation is marked as WIP). Adding libvirt folks. This intentionally designs the interface in a way that requires a privileged intermediary to monitor netlink on the host, associate messages to VMs based on an attached device, and re-inject the event to the VMM. Why wouldn't we use a channel associated with the device for such events, such that the VMM has direct access? The netlink path seems like it has more moving pieces, possibly scalability issues, and maybe security issues? > > What sort of ACPI events are we expecting to see here and what does user space do with them? > > With this patch we are expecting to see and propagate any device > specific notifications, which are aimed to notify the proper device > (driver) which belongs to the guest. > > Here is the description how propagating such notification could be > implemented by VMM: > > 1) VMM could upfront generate proper virtual ACPI description for > guest per vfio-pci device (more precisely it could be e.g. ACPI GPE > handler, which aim is only to notify relevant device): The proposed interface really has no introspection, how does the VMM know which devices need ACPI tables added "upfront"? How do these events factor into hotplug device support, where we may not be able to dynamically inject ACPI code into the VM? > > Scope (_GPE) > { > Method (_E00, 0, NotSerialized) // _Exx: Edge-Triggered > GPE, xx=0x00-0xFF > { > Local0 = \_SB.PC00.PE08.NOTY > Notify (\_SB.PC00.PE08, Local0) > } > } > > 2) Now, when the VMM receives ACPI netlink event (thanks to VMM > builtin ACPI event listener, which is able to receive any event > generated through acpi_bus_generate_netlink_event) VMM classifies it > based on device_class ("vfio_pci" in this case) and parses it further > to get device name and the notification value for it. This > notification value is stored in a virtual register and VMM triggers > GPE associated with the pci-vfio device. Each VMM is listening for netlink events and sees all the netlink traffic from the host, including events destined for other VMMs? This doesn't seem terribly acceptable from a security perspective. > 3) Guest kernel upon handling GPE, thanks to generated AML (ad 1.), > triggers Notify on required pass-through device and therefore > replicates the ACPI Notification on the guest side (Accessing > \_SB.PC00.PE08.NOTY from above example, result with trap to VMM, which > returns previously stored notify value). The acpi_bus_generate_netlink_event() below really only seems to form a u8 event type from the u32 event. Is this something that could be provided directly from the vfio device uAPI with an ioeventfd, thus providing introspection that a device supports ACPI event notifications and the ability for the VMM to exclusively monitor those events, and only those events for the device, without additional privileges? Thanks, Alex > With above the ACPI notifications are actually replicated on the guest > side and from a guest driver perspective they don't differ from native > ones. > > > > > > Signed-off-by: Dominik Behr <dbehr(a)chromium.org> > > > Co-developed-by: Grzegorz Jaszczyk <jaz(a)semihalf.com> > > > Signed-off-by: Grzegorz Jaszczyk <jaz(a)semihalf.com> > > > --- > > > drivers/vfio/pci/vfio_pci_core.c | 33 ++++++++++++++++++++++++++++++++ > > > 1 file changed, 33 insertions(+) > > > > > > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c > > > index a5ab416cf476..92b8ed8d087c 100644 > > > --- a/drivers/vfio/pci/vfio_pci_core.c > > > +++ b/drivers/vfio/pci/vfio_pci_core.c > > > @@ -10,6 +10,7 @@ > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > > +#include <linux/acpi.h> > > > #include <linux/aperture.h> > > > #include <linux/device.h> > > > #include <linux/eventfd.h> > > > @@ -2120,10 +2121,20 @@ void vfio_pci_core_release_dev(struct vfio_device *core_vdev) > > > } > > > EXPORT_SYMBOL_GPL(vfio_pci_core_release_dev); > > > > > > +static void vfio_pci_core_acpi_notify(acpi_handle handle, u32 event, void *data) > > > +{ > > > + struct vfio_pci_core_device *vdev = (struct vfio_pci_core_device *)data; > > > + struct device *dev = &vdev->pdev->dev; > > > + > > > + acpi_bus_generate_netlink_event("vfio_pci", dev_name(dev), event, 0); > > > > Who listens to this? Should there be an in-band means to provide > > notifies related to the device? How does a userspace driver know to > > look for netlink events for a particular device? > > VMM which has implemented logic responsible for listening on acpi > netlink events. This netlink message already passes the device name so > VMM will associate it with a particular device. I've elaborated a bit > more in my previous answer. > > > > > > +} > > > + > > > int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev) > > > { > > > + acpi_status status; > > > struct pci_dev *pdev = vdev->pdev; > > > struct device *dev = &pdev->dev; > > > + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev); > > > int ret; > > > > > > /* Drivers must set the vfio_pci_core_device to their drvdata */ > > > @@ -2201,8 +2212,24 @@ int vfio_pci_core_register_device(struct vfio_pci_core_device *vdev) > > > ret = vfio_register_group_dev(&vdev->vdev); > > > if (ret) > > > goto out_power; > > > + > > > + if (!adev) { > > > + pci_info(pdev, "No ACPI companion"); > > > > This would be a log message generated for 99.99% of devices. > > Sure - I will remove that. > > > > > > + return 0; > > > + } > > > + > > > + status = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY, > > > + vfio_pci_core_acpi_notify, (void *)vdev); > > > > vfio-pci supports non-ACPI platforms, I don't see any !CONFIG_ACPI > > prototypes for this function. Thanks, > > Good point, I will address this in the next version. > > Thank you, > Grzegorz > > > > > Alex > > > > > + > > > + if (ACPI_FAILURE(status)) { > > > + pci_err(pdev, "Failed to install notify handler"); > > > + goto out_group_register; > > > + } > > > + > > > return 0; > > > > > > +out_group_register: > > > + vfio_unregister_group_dev(&vdev->vdev); > > > out_power: > > > if (!disable_idle_d3) > > > pm_runtime_get_noresume(dev); > > > @@ -2216,6 +2243,12 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_register_device); > > > > > > void vfio_pci_core_unregister_device(struct vfio_pci_core_device *vdev) > > > { > > > + struct acpi_device *adev = ACPI_COMPANION(&vdev->pdev->dev); > > > + > > > + if (adev) > > > + acpi_remove_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY, > > > + vfio_pci_core_acpi_notify); > > > + > > > vfio_pci_core_sriov_configure(vdev, 0); > > > > > > vfio_unregister_group_dev(&vdev->vdev); > > >

5 10

[PATCH] ci: Refresh CI files with current lcitool
by Martin Kletzander 24 Mar '23

24 Mar '23

This updates to FreeBSD 12.4 which has clang that supports -fsemantic-interposition, plus of course updates the system. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- ci/gitlab/builds.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml index d982824b65ad..545478d3516d 100644 --- a/ci/gitlab/builds.yml +++ b/ci/gitlab/builds.yml @@ -837,7 +837,7 @@ x86_64-freebsd-12: needs: [] allow_failure: false variables: - CIRRUS_VM_IMAGE_NAME: freebsd-12-3 + CIRRUS_VM_IMAGE_NAME: freebsd-12-4 CIRRUS_VM_IMAGE_SELECTOR: image_family CIRRUS_VM_INSTANCE_TYPE: freebsd_instance INSTALL_COMMAND: pkg install -y -- 2.40.0

2 1

[PATCH] meson: Don't build tests when CLang lacks -fsemantic-interposition
by Michal Privoznik 24 Mar '23

24 Mar '23

There are some CLang versions that do not support -fsemantic-interposition. If that's the case, the code is optimized so much that our mocking no longer works. Therefore, disable tests and produce a warning. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Technically, this is a v2 of: https://listman.redhat.com/archives/libvir-list/2023-March/238943.html but a different approach is implemented, so I'm sending it anew. meson.build | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/meson.build b/meson.build index a0682e8d0b..c15003ce02 100644 --- a/meson.build +++ b/meson.build @@ -2035,8 +2035,18 @@ subdir('src') subdir('tools') -build_tests = not get_option('tests').disabled() -if build_tests +build_tests = [ not get_option('tests').disabled() ] +if build_tests[0] and \ + cc.get_id() == 'clang' and \ + not supported_cc_flags.contains('-fsemantic-interposition') \ + and get_option('optimization') != '0' + # If CLang doesn't support -fsemantic-interposition then our + # mocking doesn't work. The best we can do is to not run the + # test suite. + build_tests = [ false, '!!! Forcibly disabling tests because CLang lacks -fsemantic-interposition. Update CLang or disable optimization !!!' ] +endif + +if build_tests[0] subdir('tests') endif -- 2.39.2

3 3

[PATCH] ci: Add OpenSUSE leap 15.4 job
by Peter Krempa 23 Mar '23

23 Mar '23

As a precursor to dropping the EOL OpenSUSE 15.3 job add first the definitions for the replacement version. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- Posting this one separately so that we validate that the container images can be built successfully before converting any jobs. I've witnessed some failures in the libvirt-ci repo regarding building the 15.4 container. ci/buildenv/opensuse-leap-154.sh | 96 +++++++++++++++++++++ ci/containers/opensuse-leap-154.Dockerfile | 97 ++++++++++++++++++++++ ci/gitlab/builds.yml | 20 +++++ ci/gitlab/containers.yml | 7 ++ ci/manifest.yml | 6 ++ 5 files changed, 226 insertions(+) create mode 100644 ci/buildenv/opensuse-leap-154.sh create mode 100644 ci/containers/opensuse-leap-154.Dockerfile diff --git a/ci/buildenv/opensuse-leap-154.sh b/ci/buildenv/opensuse-leap-154.sh new file mode 100644 index 0000000000..93cb99c83b --- /dev/null +++ b/ci/buildenv/opensuse-leap-154.sh @@ -0,0 +1,96 @@ +# THIS FILE WAS AUTO-GENERATED +# +# $ lcitool manifest ci/manifest.yml +# +# https://gitlab.com/libvirt/libvirt-ci + +function install_buildenv() { + zypper update -y + zypper install -y \ + audit-devel \ + augeas \ + augeas-lenses \ + bash-completion \ + ca-certificates \ + ccache \ + clang \ + codespell \ + cpp \ + cppi \ + cyrus-sasl-devel \ + device-mapper-devel \ + diffutils \ + dwarves \ + ebtables \ + fuse-devel \ + gcc \ + gettext-runtime \ + git \ + glib2-devel \ + glibc-devel \ + glibc-locale \ + glusterfs-devel \ + grep \ + iproute2 \ + iptables \ + kmod \ + libacl-devel \ + libapparmor-devel \ + libattr-devel \ + libblkid-devel \ + libcap-ng-devel \ + libcurl-devel \ + libgnutls-devel \ + libiscsi-devel \ + libnl3-devel \ + libnuma-devel \ + libpcap-devel \ + libpciaccess-devel \ + librbd-devel \ + libselinux-devel \ + libssh-devel \ + libssh2-devel \ + libtirpc-devel \ + libudev-devel \ + libwsman-devel \ + libxml2 \ + libxml2-devel \ + libxslt \ + libyajl-devel \ + lvm2 \ + make \ + meson \ + nfs-utils \ + ninja \ + numad \ + open-iscsi \ + parted-devel \ + perl-base \ + pkgconfig \ + polkit \ + python3-base \ + python3-docutils \ + python3-flake8 \ + qemu-tools \ + readline-devel \ + rpcgen \ + rpm-build \ + sanlock-devel \ + scrub \ + sed \ + systemd-rpm-macros \ + systemtap-sdt-devel \ + wireshark-devel \ + xen-devel + rpm -qa | sort > /packages.txt + mkdir -p /usr/libexec/ccache-wrappers + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc +} + +export CCACHE_WRAPPERSDIR="/usr/libexec/ccache-wrappers" +export LANG="en_US.UTF-8" +export MAKE="/usr/bin/make" +export NINJA="/usr/bin/ninja" +export PYTHON="/usr/bin/python3" diff --git a/ci/containers/opensuse-leap-154.Dockerfile b/ci/containers/opensuse-leap-154.Dockerfile new file mode 100644 index 0000000000..ca11edf808 --- /dev/null +++ b/ci/containers/opensuse-leap-154.Dockerfile @@ -0,0 +1,97 @@ +# THIS FILE WAS AUTO-GENERATED +# +# $ lcitool manifest ci/manifest.yml +# +# https://gitlab.com/libvirt/libvirt-ci + +FROM registry.opensuse.org/opensuse/leap:15.4 + +RUN zypper update -y && \ + zypper install -y \ + audit-devel \ + augeas \ + augeas-lenses \ + bash-completion \ + ca-certificates \ + ccache \ + clang \ + codespell \ + cpp \ + cppi \ + cyrus-sasl-devel \ + device-mapper-devel \ + diffutils \ + dwarves \ + ebtables \ + fuse-devel \ + gcc \ + gettext-runtime \ + git \ + glib2-devel \ + glibc-devel \ + glibc-locale \ + glusterfs-devel \ + grep \ + iproute2 \ + iptables \ + kmod \ + libacl-devel \ + libapparmor-devel \ + libattr-devel \ + libblkid-devel \ + libcap-ng-devel \ + libcurl-devel \ + libgnutls-devel \ + libiscsi-devel \ + libnl3-devel \ + libnuma-devel \ + libpcap-devel \ + libpciaccess-devel \ + librbd-devel \ + libselinux-devel \ + libssh-devel \ + libssh2-devel \ + libtirpc-devel \ + libudev-devel \ + libwsman-devel \ + libxml2 \ + libxml2-devel \ + libxslt \ + libyajl-devel \ + lvm2 \ + make \ + meson \ + nfs-utils \ + ninja \ + numad \ + open-iscsi \ + parted-devel \ + perl-base \ + pkgconfig \ + polkit \ + python3-base \ + python3-docutils \ + python3-flake8 \ + qemu-tools \ + readline-devel \ + rpcgen \ + rpm-build \ + sanlock-devel \ + scrub \ + sed \ + systemd-rpm-macros \ + systemtap-sdt-devel \ + wireshark-devel \ + xen-devel && \ + zypper clean --all && \ + rpm -qa | sort > /packages.txt && \ + mkdir -p /usr/libexec/ccache-wrappers && \ + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \ + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang && \ + ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc + +ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" +ENV LANG "en_US.UTF-8" +ENV MAKE "/usr/bin/make" +ENV NINJA "/usr/bin/ninja" +ENV PYTHON "/usr/bin/python3" diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml index 89c15fa28d..0f60ba89ce 100644 --- a/ci/gitlab/builds.yml +++ b/ci/gitlab/builds.yml @@ -305,6 +305,26 @@ x86_64-opensuse-leap-153-local-env: RPM: skip +x86_64-opensuse-leap-154-prebuilt-env: + extends: .native_build_job_prebuilt_env + needs: + - job: x86_64-opensuse-leap-154-container + optional: true + allow_failure: false + variables: + NAME: opensuse-leap-154 + RPM: skip + +x86_64-opensuse-leap-154-local-env: + extends: .native_build_job_local_env + needs: [] + allow_failure: false + variables: + IMAGE: registry.opensuse.org/opensuse/leap:15.4 + NAME: opensuse-leap-154 + RPM: skip + + x86_64-opensuse-tumbleweed-prebuilt-env: extends: .native_build_job_prebuilt_env needs: diff --git a/ci/gitlab/containers.yml b/ci/gitlab/containers.yml index fa426bd3a6..241475b5e3 100644 --- a/ci/gitlab/containers.yml +++ b/ci/gitlab/containers.yml @@ -84,6 +84,13 @@ x86_64-opensuse-leap-153-container: NAME: opensuse-leap-153 +x86_64-opensuse-leap-154-container: + extends: .container_job + allow_failure: false + variables: + NAME: opensuse-leap-154 + + x86_64-opensuse-tumbleweed-container: extends: .container_job allow_failure: true diff --git a/ci/manifest.yml b/ci/manifest.yml index 2aa9ce9952..3270be85fe 100644 --- a/ci/manifest.yml +++ b/ci/manifest.yml @@ -166,6 +166,12 @@ targets: variables: RPM: skip + opensuse-leap-154: + jobs: + - arch: x86_64 + variables: + RPM: skip + opensuse-tumbleweed: jobs: - arch: x86_64 -- 2.39.2

2 1

[PATCH] ci: Switch to OpenSUSE leap 15.4
by Peter Krempa 23 Mar '23

23 Mar '23

The '15.3' version is EOL now: https://get.opensuse.org/leap/15.3/ Also switch the 'codestyle' job to the appropriate container image. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- .gitlab-ci.yml | 8 ++++---- .../{opensuse-leap-153.sh => opensuse-leap-154.sh} | 5 +---- ...p-153.Dockerfile => opensuse-leap-154.Dockerfile} | 8 ++------ ci/gitlab/builds.yml | 12 ++++++------ ci/gitlab/containers.yml | 4 ++-- ci/manifest.yml | 2 +- 6 files changed, 16 insertions(+), 23 deletions(-) rename ci/buildenv/{opensuse-leap-153.sh => opensuse-leap-154.sh} (95%) rename ci/containers/{opensuse-leap-153.Dockerfile => opensuse-leap-154.Dockerfile} (93%) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1b72ebc493..8bf6e5c6b0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -118,18 +118,18 @@ codestyle_prebuilt_env: - .codestyle_job - .gitlab_native_build_job_prebuilt_env needs: - - job: x86_64-opensuse-leap-153-container + - job: x86_64-opensuse-leap-154-container optional: true variables: - NAME: opensuse-leap-153 + NAME: opensuse-leap-154 codestyle_local_env: extends: - .codestyle_job - .gitlab_native_build_job_local_env variables: - IMAGE: registry.opensuse.org/opensuse/leap:15.3 - NAME: opensuse-leap-153 + IMAGE: registry.opensuse.org/opensuse/leap:15.4 + NAME: opensuse-leap-154 # This artifact published by this job is downloaded to push to Weblate diff --git a/ci/buildenv/opensuse-leap-153.sh b/ci/buildenv/opensuse-leap-154.sh similarity index 95% rename from ci/buildenv/opensuse-leap-153.sh rename to ci/buildenv/opensuse-leap-154.sh index b78382470c..93cb99c83b 100644 --- a/ci/buildenv/opensuse-leap-153.sh +++ b/ci/buildenv/opensuse-leap-154.sh @@ -59,6 +59,7 @@ function install_buildenv() { libyajl-devel \ lvm2 \ make \ + meson \ nfs-utils \ ninja \ numad \ @@ -70,9 +71,6 @@ function install_buildenv() { python3-base \ python3-docutils \ python3-flake8 \ - python3-pip \ - python3-setuptools \ - python3-wheel \ qemu-tools \ readline-devel \ rpcgen \ @@ -89,7 +87,6 @@ function install_buildenv() { ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc - /usr/bin/pip3 install meson==0.56.0 } export CCACHE_WRAPPERSDIR="/usr/libexec/ccache-wrappers" diff --git a/ci/containers/opensuse-leap-153.Dockerfile b/ci/containers/opensuse-leap-154.Dockerfile similarity index 93% rename from ci/containers/opensuse-leap-153.Dockerfile rename to ci/containers/opensuse-leap-154.Dockerfile index 49e0c98c8a..ca11edf808 100644 --- a/ci/containers/opensuse-leap-153.Dockerfile +++ b/ci/containers/opensuse-leap-154.Dockerfile @@ -4,7 +4,7 @@ # # https://gitlab.com/libvirt/libvirt-ci -FROM registry.opensuse.org/opensuse/leap:15.3 +FROM registry.opensuse.org/opensuse/leap:15.4 RUN zypper update -y && \ zypper install -y \ @@ -60,6 +60,7 @@ RUN zypper update -y && \ libyajl-devel \ lvm2 \ make \ + meson \ nfs-utils \ ninja \ numad \ @@ -71,9 +72,6 @@ RUN zypper update -y && \ python3-base \ python3-docutils \ python3-flake8 \ - python3-pip \ - python3-setuptools \ - python3-wheel \ qemu-tools \ readline-devel \ rpcgen \ @@ -92,8 +90,6 @@ RUN zypper update -y && \ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang && \ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc -RUN /usr/bin/pip3 install meson==0.56.0 - ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" ENV LANG "en_US.UTF-8" ENV MAKE "/usr/bin/make" diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml index 89c15fa28d..d982824b65 100644 --- a/ci/gitlab/builds.yml +++ b/ci/gitlab/builds.yml @@ -285,23 +285,23 @@ x86_64-fedora-rawhide-clang-local-env: RPM: skip -x86_64-opensuse-leap-153-prebuilt-env: +x86_64-opensuse-leap-154-prebuilt-env: extends: .native_build_job_prebuilt_env needs: - - job: x86_64-opensuse-leap-153-container + - job: x86_64-opensuse-leap-154-container optional: true allow_failure: false variables: - NAME: opensuse-leap-153 + NAME: opensuse-leap-154 RPM: skip -x86_64-opensuse-leap-153-local-env: +x86_64-opensuse-leap-154-local-env: extends: .native_build_job_local_env needs: [] allow_failure: false variables: - IMAGE: registry.opensuse.org/opensuse/leap:15.3 - NAME: opensuse-leap-153 + IMAGE: registry.opensuse.org/opensuse/leap:15.4 + NAME: opensuse-leap-154 RPM: skip diff --git a/ci/gitlab/containers.yml b/ci/gitlab/containers.yml index fa426bd3a6..150df26519 100644 --- a/ci/gitlab/containers.yml +++ b/ci/gitlab/containers.yml @@ -77,11 +77,11 @@ x86_64-fedora-rawhide-container: NAME: fedora-rawhide -x86_64-opensuse-leap-153-container: +x86_64-opensuse-leap-154-container: extends: .container_job allow_failure: false variables: - NAME: opensuse-leap-153 + NAME: opensuse-leap-154 x86_64-opensuse-tumbleweed-container: diff --git a/ci/manifest.yml b/ci/manifest.yml index 2aa9ce9952..b68c7361ab 100644 --- a/ci/manifest.yml +++ b/ci/manifest.yml @@ -160,7 +160,7 @@ targets: freebsd-13: x86_64 - opensuse-leap-153: + opensuse-leap-154: jobs: - arch: x86_64 variables: -- 2.39.2

3 2

[PATCH] virnettlscontext: allow client/server cert chains
by matoro_mailinglist_libvirt＠matoro.tk 22 Mar '23

22 Mar '23

From: matoro <11910244-matoro3(a)users.noreply.gitlab.com> The existing implementation assumes that client/server certificates are single individual certificates. If using publicly-issued certificates, or internal CAs that use an intermediate issuer, this is unlikely to be the case, and they will instead be certificate chains. While this can be worked around by moving the intermediate certificates to the CA certificate, which DOES currently support multiple certificates, this instead allows the issued certificate chains to be used as-is, without requiring the overhead of shuffling certificates around. See: https://gitlab.com/libvirt/libvirt/-/merge_requests/222 Signed-off-by: matoro <matoro_github(a)matoro.tk> --- src/rpc/virnettlscontext.c | 97 +++++++++++++----------------------- tests/virnettlscontexttest.c | 72 +++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 65 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index cfd26f0701..78b4b0f187 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -424,7 +424,8 @@ static int virNetTLSContextCheckCert(gnutls_x509_crt_t cert, } -static int virNetTLSContextCheckCertPair(gnutls_x509_crt_t cert, +static int virNetTLSContextCheckCertPair(gnutls_x509_crt_t *certs, + size_t ncerts, const char *certFile, gnutls_x509_crt_t *cacerts, size_t ncacerts, @@ -433,7 +434,7 @@ static int virNetTLSContextCheckCertPair(gnutls_x509_crt_t cert, { unsigned int status; - if (gnutls_x509_crt_list_verify(&cert, 1, + if (gnutls_x509_crt_list_verify(certs, ncerts, cacerts, ncacerts, NULL, 0, 0, &status) < 0) { @@ -469,57 +470,18 @@ static int virNetTLSContextCheckCertPair(gnutls_x509_crt_t cert, } -static gnutls_x509_crt_t virNetTLSContextLoadCertFromFile(const char *certFile, - bool isServer) -{ - gnutls_datum_t data; - gnutls_x509_crt_t cert = NULL; - g_autofree char *buf = NULL; - int ret = -1; - - VIR_DEBUG("isServer %d certFile %s", - isServer, certFile); - - if (gnutls_x509_crt_init(&cert) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, "%s", - _("Unable to initialize certificate")); - goto cleanup; - } - - if (virFileReadAll(certFile, (1<<16), &buf) < 0) - goto cleanup; - - data.data = (unsigned char *)buf; - data.size = strlen(buf); - - if (gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, isServer ? - _("Unable to import server certificate %s") : - _("Unable to import client certificate %s"), - certFile); - goto cleanup; - } - - ret = 0; - - cleanup: - if (ret != 0) { - g_clear_pointer(&cert, gnutls_x509_crt_deinit); - } - return cert; -} - - -static int virNetTLSContextLoadCACertListFromFile(const char *certFile, - gnutls_x509_crt_t *certs, - unsigned int certMax, - size_t *ncerts) +static int virNetTLSContextLoadCertListFromFile(const char *certFile, + gnutls_x509_crt_t *certs, + unsigned int certMax, + size_t *ncerts, + bool isServer, + bool isCA) { gnutls_datum_t data; g_autofree char *buf = NULL; *ncerts = 0; - VIR_DEBUG("certFile %s", certFile); + VIR_DEBUG("isCA %d isServer %d certFile %s", isCA, isServer, certFile); if (virFileReadAll(certFile, (1<<16), &buf) < 0) return -1; @@ -527,9 +489,13 @@ static int virNetTLSContextLoadCACertListFromFile(const char *certFile, data.data = (unsigned char *)buf; data.size = strlen(buf); - if (gnutls_x509_crt_list_import(certs, &certMax, &data, GNUTLS_X509_FMT_PEM, 0) < 0) { + if (gnutls_x509_crt_list_import(certs, &certMax, &data, GNUTLS_X509_FMT_PEM, + isCA ? GNUTLS_X509_CRT_LIST_IMPORT_FAIL_IF_EXCEED : + GNUTLS_X509_CRT_LIST_IMPORT_FAIL_IF_EXCEED | GNUTLS_X509_CRT_LIST_FAIL_IF_UNSORTED) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, - _("Unable to import CA certificate list %s"), + isCA ? _("Unable to import CA certificate list %s") : + (isServer ? _("Unable to import server certificate %s") : + _("Unable to import client certificate %s")), certFile); return -1; } @@ -539,44 +505,49 @@ static int virNetTLSContextLoadCACertListFromFile(const char *certFile, } -#define MAX_CERTS 16 +// Limited by frame size of 4096 bytes. +// Typical system CA bundle contains 140-ish CAs. +#define MAX_CERTS 200 static int virNetTLSContextSanityCheckCredentials(bool isServer, const char *cacertFile, const char *certFile) { - gnutls_x509_crt_t cert = NULL; + gnutls_x509_crt_t certs[MAX_CERTS]; gnutls_x509_crt_t cacerts[MAX_CERTS]; - size_t ncacerts = 0; + size_t ncerts = 0, ncacerts = 0; size_t i; int ret = -1; + memset(certs, 0, sizeof(certs)); memset(cacerts, 0, sizeof(cacerts)); if ((access(certFile, R_OK) == 0) && - !(cert = virNetTLSContextLoadCertFromFile(certFile, isServer))) + virNetTLSContextLoadCertListFromFile(certFile, certs, + MAX_CERTS, &ncerts, isServer, false) < 0) goto cleanup; if ((access(cacertFile, R_OK) == 0) && - virNetTLSContextLoadCACertListFromFile(cacertFile, cacerts, - MAX_CERTS, &ncacerts) < 0) + virNetTLSContextLoadCertListFromFile(cacertFile, cacerts, + MAX_CERTS, &ncacerts, isServer, true) < 0) goto cleanup; - if (cert && - virNetTLSContextCheckCert(cert, certFile, isServer, false) < 0) - goto cleanup; + for (i = 0; i < ncerts; i++) { + if (virNetTLSContextCheckCert(certs[i], certFile, isServer, (i != 0)) < 0) + goto cleanup; + } for (i = 0; i < ncacerts; i++) { if (virNetTLSContextCheckCert(cacerts[i], cacertFile, isServer, true) < 0) goto cleanup; } - if (cert && ncacerts && - virNetTLSContextCheckCertPair(cert, certFile, cacerts, ncacerts, cacertFile, isServer) < 0) + if (ncerts && ncacerts && + virNetTLSContextCheckCertPair(certs, ncerts, certFile, cacerts, ncacerts, cacertFile, isServer) < 0) goto cleanup; ret = 0; cleanup: - if (cert) - gnutls_x509_crt_deinit(cert); + for (i = 0; i < ncerts; i++) + gnutls_x509_crt_deinit(certs[i]); for (i = 0; i < ncacerts; i++) gnutls_x509_crt_deinit(cacerts[i]); return ret; diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 2311524db8..918ff04134 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -517,6 +517,12 @@ mymain(void) true, true, GNUTLS_KEY_KEY_CERT_SIGN, false, false, NULL, NULL, 0, 0); + TLS_ROOT_REQ(someotherrootreq, + "UK", "some other random CA", NULL, NULL, NULL, NULL, + true, true, true, + true, true, GNUTLS_KEY_KEY_CERT_SIGN, + false, false, NULL, NULL, + 0, 0); TLS_CERT_REQ(cacertlevel1areq, cacertrootreq, "UK", "libvirt level 1a", NULL, NULL, NULL, NULL, true, true, true, @@ -555,15 +561,72 @@ mymain(void) cacertlevel2areq.crt, }; + gnutls_x509_crt_t cabundle[] = { + someotherrootreq.crt, + cacertrootreq.crt, + }; + + gnutls_x509_crt_t servercertchain[] = { + servercertlevel3areq.crt, + cacertlevel2areq.crt, + cacertlevel1areq.crt, + }; + + gnutls_x509_crt_t servercertchain_incomplete[] = { + servercertlevel3areq.crt, + cacertlevel2areq.crt, + }; + + gnutls_x509_crt_t servercertchain_unsorted[] = { + servercertlevel3areq.crt, + cacertlevel1areq.crt, + cacertlevel2areq.crt, + }; + + gnutls_x509_crt_t clientcertchain[] = { + clientcertlevel2breq.crt, + cacertlevel1breq.crt, + }; + testTLSWriteCertChain("cacertchain-ctx.pem", certchain, G_N_ELEMENTS(certchain)); - VIR_WARNINGS_RESET - DO_CTX_TEST(true, "cacertchain-ctx.pem", servercertlevel3areq.filename, false); DO_CTX_TEST(false, "cacertchain-ctx.pem", clientcertlevel2breq.filename, false); + testTLSWriteCertChain("servercertchain-ctx.pem", + servercertchain, + G_N_ELEMENTS(servercertchain)); + + DO_CTX_TEST(true, cacertrootreq.filename, "servercertchain-ctx.pem", false); + + testTLSWriteCertChain("cabundle-ctx.pem", + cabundle, + G_N_ELEMENTS(cabundle)); + + DO_CTX_TEST(true, "cabundle-ctx.pem", "servercertchain-ctx.pem", false); + + testTLSWriteCertChain("servercertchain_incomplete-ctx.pem", + servercertchain_incomplete, + G_N_ELEMENTS(servercertchain_incomplete)); + + DO_CTX_TEST(true, cacertrootreq.filename, "servercertchain_incomplete-ctx.pem", true); + + testTLSWriteCertChain("servercertchain_unsorted-ctx.pem", + servercertchain_unsorted, + G_N_ELEMENTS(servercertchain_unsorted)); + + DO_CTX_TEST(true, cacertrootreq.filename, "servercertchain_unsorted-ctx.pem", true); + + testTLSWriteCertChain("clientcertchain-ctx.pem", + clientcertchain, + G_N_ELEMENTS(clientcertchain)); + + VIR_WARNINGS_RESET + + DO_CTX_TEST(false, cacertrootreq.filename, "clientcertchain-ctx.pem", false); + DO_CTX_TEST(false, "cacertdoesnotexist.pem", "servercertdoesnotexist.pem", true); testTLSDiscardCert(&cacertreq); @@ -620,7 +683,12 @@ mymain(void) testTLSDiscardCert(&cacertlevel2areq); testTLSDiscardCert(&servercertlevel3areq); testTLSDiscardCert(&clientcertlevel2breq); + testTLSDiscardCert(&someotherrootreq); unlink("cacertchain-ctx.pem"); + unlink("servercertchain-ctx.pem"); + unlink("servercertchain_incomplete-ctx.pem"); + unlink("servercertchain_unsorted-ctx.pem"); + unlink("clientcertchain.pem"); testTLSCleanup(KEYFILE); -- 2.39.1

2 1

[PATCH] qemu_domain: Drop ATTRIBUTE_NONNULL() for non-existent arguments
by Michal Privoznik 22 Mar '23

22 Mar '23

After cleanup done in v8.2.0-rc1~47 the qemuDomainObjExitMonitor() and after v8.7.0-rc1~176 the qemuDomainObjEnterMonitor() lost the @driver argument. But corresponding ATTRIBUTE_NONNULL() annotation was not removed and both functions are still annotated as ATTRIBUTE_NONNULL(2) even though they accept just one argument (@obj). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_domain.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c716e64ccc..eaa75de3e5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -579,9 +579,9 @@ void qemuDomainEventFlush(int timer, void *opaque); qemuMonitor *qemuDomainGetMonitor(virDomainObj *vm) ATTRIBUTE_NONNULL(1); void qemuDomainObjEnterMonitor(virDomainObj *obj) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(1); void qemuDomainObjExitMonitor(virDomainObj *obj) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(1); int qemuDomainObjEnterMonitorAsync(virDomainObj *obj, virDomainAsyncJob asyncJob) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; -- 2.39.2

2 1

[PATCH] qemu_shim: Require absolute path for root directory
by Michal Privoznik 22 Mar '23

22 Mar '23

The virConnectOpen(), well virConnectOpenInternal() reports an error if embed root is not an absolute path. This is a fair requirement, but our qemu_shim doesn't check this requirement and instead mkdir()-s passed path only to fail later on, leaving the empty directory behind: $ ls -d asd ls: cannot access 'asd': No such file or directory $ virt-qemu-run -r asd whatever.xml virt-qemu-run: cannot open qemu:///embed?root=asd: unsupported configuration: root path must be absolute $ ls -d asd asd Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_shim.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c index 7b58583074..33a6aa7d94 100644 --- a/src/qemu/qemu_shim.c +++ b/src/qemu/qemu_shim.c @@ -209,10 +209,18 @@ int main(int argc, char **argv) } tmproot = true; - } else if (g_mkdir_with_parents(root, 0755) < 0) { - g_printerr("%s: cannot create dir: %s\n", - argv[0], g_strerror(errno)); - goto cleanup; + } else { + if (!g_path_is_absolute(root)) { + g_printerr("%s: the root directory must be an absolute path\n", + argv[0]); + goto cleanup; + } + + if (g_mkdir_with_parents(root, 0755) < 0) { + g_printerr("%s: cannot create dir: %s\n", + argv[0], g_strerror(errno)); + goto cleanup; + } } if (chmod(root, 0755) < 0) { -- 2.39.2

2 1

[libvirt PATCH] qemu_shim: set system identity
by Ján Tomko 22 Mar '23

22 Mar '23

Otherwise looking up a secret fails when we try to elevate the identity in qemuDomainSecretInfoSetupFromSecret. https://bugzilla.redhat.com/show_bug.cgi?id=2000410 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/qemu/qemu_shim.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c index 7b58583074..b642f6175a 100644 --- a/src/qemu/qemu_shim.c +++ b/src/qemu/qemu_shim.c @@ -26,6 +26,7 @@ #include "virfile.h" #include "virgettext.h" +#include "viridentity.h" #include "virthread.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -133,6 +134,7 @@ qemuShimQuench(void *userData G_GNUC_UNUSED, int main(int argc, char **argv) { + g_autoptr(virIdentity) sysident = NULL; GThread *eventLoopThread = NULL; virConnectPtr conn = NULL; virConnectPtr sconn = NULL; @@ -191,6 +193,9 @@ int main(int argc, char **argv) virSetErrorFunc(NULL, qemuShimQuench); + sysident = virIdentityGetSystem(); + virIdentitySetCurrent(sysident); + if (verbose) g_printerr("%s: %lld: initializing signal handlers\n", argv[0], deltams()); -- 2.39.2

2 1

[PATCH v2 0/6] Introduce active RBD disk snapshot support
by Or Ozeri 22 Mar '23

22 Mar '23

Internal disk snapshots are currently only supported on non-active VMs. For RBD disks only, this patch series extends this support for active VMs running with qemu. We also add the option to set a name for each RBD snapshot, and allow taking them alongside other external disk snapshots (mixing). Deletion and reverting to snapshots containing RBD snapshots is not allowed, and is validated. Note that taking RBD snapshots on a non-active VM is still unsupported. Changes in v2: - reduce patch to RBD use-case only (e.g. not including qcow2 internal snapshots) - add validation to disallow RBD snapshots deletion / reverting Or Ozeri (6): conf: Add snapshotName attribute for internal disk snapshot qemu: Block deletion and reverting on non-full internal snapshots qemu: Add internal support for active disk internal snapshots qemu: Allow active disk snapshots for RBD disks qemu: Allow setting per-disk snapshot name for RBD disks qemu: Allow mixing active internal and external active disk snapshots docs/formatsnapshot.rst | 5 + src/conf/schemas/domainsnapshot.rng | 4 + src/conf/snapshot_conf.c | 56 ++++++++++ src/conf/snapshot_conf.h | 5 + src/libvirt_private.syms | 1 + src/qemu/qemu_monitor.c | 9 ++ src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_monitor_json.c | 14 +++ src/qemu/qemu_monitor_json.h | 5 + src/qemu/qemu_snapshot.c | 105 ++++++++++++------ .../disk_snapshot.xml | 2 +- .../disk_snapshot.xml | 2 +- .../disk_snapshot_redefine.xml | 2 +- tests/qemumonitorjsontest.c | 1 + 14 files changed, 181 insertions(+), 35 deletions(-) -- 2.25.1

3 14

[libvirt PATCH 0/1] qemu: Fix memory leaks in firmware selection code
by Andrea Bolognani 22 Mar '23

22 Mar '23

Reported when building with gcc and clang both. ==10909==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 4 object(s) allocated from: #0 0x7f191d4a2808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144 #1 0x7f191987ae98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98) SUMMARY: AddressSanitizer: 128 byte(s) leaked in 4 allocation(s). This patch is a requirement for pushing my recent improvements to firmware selection[1] without further degrading the CI pipeline. [1] https://listman.redhat.com/archives/libvir-list/2023-March/238890.html Andrea Bolognani (1): qemu: Fix memory leaks in firmware selection code src/qemu/qemu_firmware.c | 4 ++++ 1 file changed, 4 insertions(+) -- 2.39.2

2 2

[libvirt PATCH 00/15] qemu: Further improvements to firmware selection
by Andrea Bolognani 22 Mar '23

22 Mar '23

More information in the commit message for 11/15. Ideally this would make it into 9.2.0 along with the other changes to firmware selection I've made in this cycle[1]. [1] https://listman.redhat.com/archives/libvir-list/2023-February/237806.html Andrea Bolognani (15): tests: Rename a few firmware tests tests: Fix firmware-auto-efi-loader-path-nonstandard tests: Add firmware-auto-efi-loader-path tests: Add more tests for manual Secure Boot configuration tests: Fix firmware descriptor masking test qemu: Introduce qemuFirmwareMatchesPaths() qemu: Discard requires-smm firmware when loader.secure=no qemu: Always go through firmware autoselection conf: Remove some firmware validation checks conf: Don't format firmware type/features when migrating qemu: Don't drop firmware type/features information qemu: Automatically add firmware type/features information conf: Don't explicitly set the secure-boot feature conf: Move validation check out of postparse qemu: Move validation check out of postparse src/conf/domain_conf.c | 5 +- src/conf/domain_postparse.c | 16 -- src/conf/domain_validate.c | 42 +--- src/qemu/qemu_domain.c | 6 - src/qemu/qemu_firmware.c | 186 +++++++++++++----- src/qemu/qemu_validate.c | 6 + .../{60-ovmf-sb.json => 42-masked.json} | 0 .../usr/share/qemu/firmware/42-masked.json | 37 ++++ tests/qemufirmwaretest.c | 2 + ...to-efi-loader-insecure.x86_64-latest.args} | 0 ...auto-efi-loader-insecure.x86_64-latest.err | 1 - ...-loader-path-nonstandard.x86_64-latest.err | 1 + ...ware-auto-efi-loader-path-nonstandard.xml} | 2 +- ...e-auto-efi-loader-path.x86_64-latest.args} | 0 ...are-auto-efi-loader-path.x86_64-latest.err | 1 - .../firmware-auto-efi-loader-path.xml | 2 +- ...to-efi-no-enrolled-keys.x86_64-latest.args | 5 +- ...nual-efi-loader-no-path.x86_64-latest.err} | 0 ...=> firmware-manual-efi-loader-no-path.xml} | 0 ...nual-efi-loader-secure.x86_64-latest.args} | 0 ... => firmware-manual-efi-loader-secure.xml} | 0 ...l-efi-no-enrolled-keys.x86_64-latest.args} | 0 ... firmware-manual-efi-no-enrolled-keys.xml} | 2 +- ...-manual-efi-no-secboot.x86_64-latest.args} | 4 +- ...xml => firmware-manual-efi-no-secboot.xml} | 5 +- ...are-manual-efi-secboot.x86_64-latest.args} | 0 ...re.xml => firmware-manual-efi-secboot.xml} | 2 +- tests/qemuxml2argvtest.c | 12 +- ...ware-auto-bios-stateless.x86_64-latest.xml | 6 +- .../firmware-auto-bios.x86_64-latest.xml | 6 +- ...rmware-auto-efi-aarch64.aarch64-latest.xml | 6 +- ...e-auto-efi-enrolled-keys.x86_64-latest.xml | 6 +- ...-efi-format-loader-qcow2.x86_64-latest.xml | 6 +- ...o-efi-format-loader-raw.aarch64-latest.xml | 6 +- ...-nvram-qcow2-network-nbd.x86_64-latest.xml | 5 +- ...-format-nvram-qcow2-path.x86_64-latest.xml | 6 +- ...o-efi-format-nvram-qcow2.x86_64-latest.xml | 6 +- ...uto-efi-loader-insecure.x86_64-latest.xml} | 8 +- ...loader-path-nonstandard.x86_64-latest.xml} | 4 +- ...re-auto-efi-loader-path.x86_64-latest.xml} | 6 +- ...e-auto-efi-loader-secure.x86_64-latest.xml | 6 +- ...uto-efi-no-enrolled-keys.x86_64-latest.xml | 9 +- ...ware-auto-efi-no-secboot.x86_64-latest.xml | 6 +- ...ware-auto-efi-nvram-file.x86_64-latest.xml | 6 +- ...-efi-nvram-network-iscsi.x86_64-latest.xml | 5 +- ...to-efi-nvram-network-nbd.x86_64-latest.xml | 5 +- .../firmware-auto-efi-nvram.x86_64-latest.xml | 6 +- ...irmware-auto-efi-secboot.x86_64-latest.xml | 6 +- ...irmware-auto-efi-smm-off.x86_64-latest.xml | 6 +- ...mware-auto-efi-stateless.x86_64-latest.xml | 6 +- .../firmware-auto-efi.x86_64-latest.xml | 6 +- ...manual-efi-acpi-aarch64.aarch64-latest.xml | 6 +- ...ware-manual-efi-acpi-q35.x86_64-latest.xml | 6 +- ...anual-efi-loader-secure.x86_64-latest.xml} | 6 +- ...al-efi-no-enrolled-keys.x86_64-latest.xml} | 8 +- ...e-manual-efi-no-secboot.x86_64-latest.xml} | 6 +- ...nual-efi-noacpi-aarch64.aarch64-latest.xml | 6 +- ...re-manual-efi-nvram-file.x86_64-latest.xml | 6 +- ...-efi-nvram-network-iscsi.x86_64-latest.xml | 5 +- ...al-efi-nvram-network-nbd.x86_64-latest.xml | 5 +- ...anual-efi-nvram-template.x86_64-latest.xml | 6 +- ...ware-manual-efi-secboot.x86_64-latest.xml} | 6 +- .../firmware-manual-efi.x86_64-latest.xml | 6 +- .../pvpanic-pci-aarch64.aarch64-latest.xml | 6 +- ...-pci-no-address-aarch64.aarch64-latest.xml | 6 +- .../virtio-iommu-aarch64.aarch64-latest.xml | 6 +- tests/qemuxml2xmltest.c | 8 +- 67 files changed, 409 insertions(+), 164 deletions(-) rename tests/qemufirmwaredata/etc/qemu/firmware/{60-ovmf-sb.json => 42-masked.json} (100%) create mode 100644 tests/qemufirmwaredata/usr/share/qemu/firmware/42-masked.json copy tests/qemuxml2argvdata/{firmware-auto-efi-no-enrolled-keys.x86_64-latest.args => firmware-auto-efi-loader-insecure.x86_64-latest.args} (100%) delete mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_64-latest.err copy tests/qemuxml2argvdata/{firmware-auto-efi-loader-path.xml => firmware-auto-efi-loader-path-nonstandard.xml} (86%) copy tests/qemuxml2argvdata/{firmware-auto-efi-no-enrolled-keys.x86_64-latest.args => firmware-auto-efi-loader-path.x86_64-latest.args} (100%) delete mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err rename tests/qemuxml2argvdata/{firmware-manual-efi-no-path.x86_64-latest.err => firmware-manual-efi-loader-no-path.x86_64-latest.err} (100%) rename tests/qemuxml2argvdata/{firmware-manual-efi-no-path.xml => firmware-manual-efi-loader-no-path.xml} (100%) copy tests/qemuxml2argvdata/{firmware-manual-efi-secure.x86_64-latest.args => firmware-manual-efi-loader-secure.x86_64-latest.args} (100%) copy tests/qemuxml2argvdata/{firmware-manual-efi-secure.xml => firmware-manual-efi-loader-secure.xml} (100%) copy tests/qemuxml2argvdata/{firmware-manual-efi-secure.x86_64-latest.args => firmware-manual-efi-no-enrolled-keys.x86_64-latest.args} (100%) copy tests/qemuxml2argvdata/{firmware-manual-efi-secure.xml => firmware-manual-efi-no-enrolled-keys.xml} (87%) copy tests/qemuxml2argvdata/{firmware-auto-efi-no-enrolled-keys.x86_64-latest.args => firmware-manual-efi-no-secboot.x86_64-latest.args} (90%) copy tests/qemuxml2argvdata/{firmware-manual-efi-secure.xml => firmware-manual-efi-no-secboot.xml} (72%) rename tests/qemuxml2argvdata/{firmware-manual-efi-secure.x86_64-latest.args => firmware-manual-efi-secboot.x86_64-latest.args} (100%) rename tests/qemuxml2argvdata/{firmware-manual-efi-secure.xml => firmware-manual-efi-secboot.xml} (86%) copy tests/qemuxml2xmloutdata/{firmware-auto-efi-no-enrolled-keys.x86_64-latest.xml => firmware-auto-efi-loader-insecure.x86_64-latest.xml} (82%) copy tests/qemuxml2xmloutdata/{firmware-auto-bios-stateless.x86_64-latest.xml => firmware-auto-efi-loader-path-nonstandard.x86_64-latest.xml} (92%) copy tests/qemuxml2xmloutdata/{firmware-auto-efi-no-enrolled-keys.x86_64-latest.xml => firmware-auto-efi-loader-path.x86_64-latest.xml} (89%) copy tests/qemuxml2xmloutdata/{firmware-manual-efi-secure.x86_64-latest.xml => firmware-manual-efi-loader-secure.x86_64-latest.xml} (89%) copy tests/qemuxml2xmloutdata/{firmware-manual-efi-secure.x86_64-latest.xml => firmware-manual-efi-no-enrolled-keys.x86_64-latest.xml} (83%) copy tests/qemuxml2xmloutdata/{firmware-manual-efi-acpi-q35.x86_64-latest.xml => firmware-manual-efi-no-secboot.x86_64-latest.xml} (89%) rename tests/qemuxml2xmloutdata/{firmware-manual-efi-secure.x86_64-latest.xml => firmware-manual-efi-secboot.x86_64-latest.xml} (89%) -- 2.39.2

2 17

[PATCH] meson: Disable optimizations if CLang doesn't support -fsemantic-interposition
by Michal Privoznik 21 Mar '23

21 Mar '23

For some older CLang (or in combination with !intel arch) the -fsemantic-interposition might be not present. But in that case, we still want our mocking in test suite to work properly. Disable optimizations as that seems to be the only way. Also, don't forget to drop this when FreeBSD 11 and macOS 12 are dropped. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- This still doesn't work when somebody overrides CFLAGS as those are placed AFTER our supported_cc_flags: $ export CFLAGS="-O2 -ggdb" $ meson setup _build $ ninja -v -C _build/ [1/1202] clang ... -O2 -g ... -O0 -O2 -ggdb -fPIC ... -c ../src/util/glibcompat.c meson.build | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/meson.build b/meson.build index a0682e8d0b..249505d474 100644 --- a/meson.build +++ b/meson.build @@ -492,6 +492,13 @@ if get_option('warning_level') == '2' endif endif + +if cc.get_id() == 'clang' and not supported_cc_flags.contains('-fsemantic-interposition') + # If CLang is old enough to not support -fsemantic-interposition + # then this is our best bet to make mocking in tests working properly. + supported_cc_flags += [ '-O0' ] +endif + add_project_arguments(supported_cc_flags, language: 'c') if cc.has_argument('-Wsuggest-attribute=format') -- 2.39.2

4 7

[PATCH 0/2] Work around broken clang. Again.
by Michal Privoznik 21 Mar '23

21 Mar '23

We ran into so many clang bugs recently, that I'm strongly in favor of making it optional and aim on gcc only. Debugging these issues burns our time needlessly. Just consider what's happening here. I've pushed patches recently that call some virNuma*() APIs during QEMU cmd line generation. And while at it, I removed virNuma*() mocks from qemuxml2argvmock.c (v9.1.0-213-g95ae91fdd4). Andrea told me in review to make sure our CI works. So I've fired up my freebsd VM and compiled my code there. It worked. Except, I'm compiling with -O0 and our CI compiles with the default (-O2). And of course this is the source of the current build breakage. Long story short, clang is so eager to produce the fastest code that it simply ignores written code and generates what it *assumes* behaves the same. Well, it doesn't and this breaks our mocking and thus our tests. And of course it's happening only with optimizations. In this particular case, I've noticed that while virNumaNodesetIsAvailable() calls virNumaNodeIsAvailable(), it disregards the return value and virReportError()-s immediately. This is because the !WITH_NUMACTL variant of virNumaNodeIsAvailable() calls virNumaGetMaxNode() which fails and thus false is returned. WORSE, then I stepped in gdb into the mock, I've seen random numbers as integers. This is because the function from virnumamock.c wasn't optimized as much and followed standard calling convention (it grabbed the integer argument from the stack). But the caller (virNumaNodesetIsAvailable()) was so optimized that it did not even bothered to push anything onto stack. After these patches, there is still one qemuxml2argvtest case failing, and it's simply because no matter how hard I try, I can't stop clang from optimizing the function. Consider the following code: virNumaCPUSetToNodeset(virBitmap *cpuset, virBitmap **nodeset) { g_autoptr(virBitmap) nodes = virBitmapNew(0); ssize_t pos = -1; while ((pos = virBitmapNextSetBit(cpuset, pos)) >= 0) { int node = virNumaGetNodeOfCPU(pos); if (node < 0) { virReportSystemError(errno, _("Unable to get NUMA node of cpu %zd"), pos); return -1; } ... } ... } And this is the assembly code that clang generates (even after VIR_OPTNONE treatment): 116f5f: e8 0c b7 f9 ff call b2670 <virBitmapNew@plt> 116f64: 48 89 44 24 40 mov %rax,0x40(%rsp) 116f69: 48 c7 44 24 18 ff ff movq $0xffffffffffffffff,0x18(%rsp) 116f70: ff ff 116f72: 48 8b 7c 24 38 mov 0x38(%rsp),%rdi 116f77: 48 8b 74 24 18 mov 0x18(%rsp),%rsi 116f7c: e8 0f 74 f9 ff call ae390 <virBitmapNextSetBit@plt> 116f81: eb 00 jmp 116f83 <virNumaCPUSetToNodeset+0x43> 116f83: 48 89 44 24 18 mov %rax,0x18(%rsp) 116f88: 48 83 f8 00 cmp $0x0,%rax 116f8c: 0f 8c b2 00 00 00 jl 117044 <virNumaCPUSetToNodeset+0x104> 116f92: 48 8b 7c 24 18 mov 0x18(%rsp),%rdi 116f97: e8 d4 a1 f9 ff call b1170 <virNumaGetNodeOfCPU@plt> 116f9c: c7 44 24 10 ff ff ff movl $0xffffffff,0x10(%rsp) 116fa3: ff 116fa4: 83 7c 24 10 00 cmpl $0x0,0x10(%rsp) 116fa9: 7d 74 jge 11701f <virNumaCPUSetToNodeset+0xdf> 116fab: e8 80 72 f9 ff call ae230 <__errno_location@plt> 116fb0: 8b 18 mov (%rax),%ebx 116fb2: 48 8d 3d 27 35 2b 00 lea 0x2b3527(%rip),%rdi # 3ca4e0 <vmdk4GetBackingStore.prefix+0xf260> 116fb9: 48 8d 35 8c e3 25 00 lea 0x25e38c(%rip),%rsi # 37534c <modeMap+0x215c> 116fc0: ba 05 00 00 00 mov $0x5,%edx 116fc5: e8 e6 a7 f9 ff call b17b0 <dcgettext@plt> 116fca: 48 8b 4c 24 18 mov 0x18(%rsp),%rcx 116fcf: 48 89 0c 24 mov %rcx,(%rsp) 116fd3: 48 8d 15 ea e0 25 00 lea 0x25e0ea(%rip),%rdx # 3750c4 <modeMap+0x1ed4> 116fda: 48 8d 0d 54 e3 25 00 lea 0x25e354(%rip),%rcx # 375335 <modeMap+0x2145> 116fe1: 31 ff xor %edi,%edi 116fe3: 89 de mov %ebx,%esi 116fe5: 41 b8 10 04 00 00 mov $0x410,%r8d 116feb: 49 89 c1 mov %rax,%r9 116fee: 31 c0 xor %eax,%eax 116ff0: e8 8b c1 f9 ff call b3180 <virReportSystemErrorFull@plt> My assembler is a bit rusty, but at virNumaGetNodeOfCPU@plt is called 116f97, after which, -1 is stored on a stack (which corresponds to @pos variable), after which comparison against 0 is made and the jump happens iff the value (-1) is greater or equal than zero. Otherwise virReportSystemError() is called. To reproduce this issue even on Linux, configure with: export CC=clang meson setup -Dsystem=true -Dexpensive_tests=enabled -Dnumactl=disabled _build At this point, I think we can call clang broken and focus our time on developing libvirt. Not coming up with hacks around broken compilers. Michal Prívozník (2): internal.h: Invent VIR_OPTNONE virnuma: Annotate some functions as VIR_OPTNONE src/internal.h | 20 ++++++++++++++++++++ src/util/virnuma.c | 20 ++++++++++---------- 2 files changed, 30 insertions(+), 10 deletions(-) -- 2.39.2

4 15

[PATCH v2] meson: Stop detecting -Wl,--version-script=
by Michal Privoznik 21 Mar '23

21 Mar '23

With its version 16.0, the LLVM's linker turned on --no-undefined-version by default [1]. This breaks how we detect --version-script= detection, because at the compile time there's no library built yet that we can use to make --version-script= happy. Unfortunately, meson does not provide a way to detect this either [2]. But there's not much sense in detecting the argument either. We already special case some systems (windows, darwin) and do the check for others, which are expected to support versioned symbols, because of ELF. Worst case scenario - the error is reported during compile time rather than configure time. 1: https://reviews.llvm.org/D135402 2: https://github.com/mesonbuild/meson/issues/3047 Resolves: https://bugs.gentoo.org/902211 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- v2 of: https://listman.redhat.com/archives/libvir-list/2023-March/238923.html diff to v1: - Instead of fixing the detection, drop it completely. meson.build | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/meson.build b/meson.build index c35823a79a..a0682e8d0b 100644 --- a/meson.build +++ b/meson.build @@ -534,12 +534,7 @@ elif host_machine.system() == 'darwin' # macOS libraries don't support symbol versioning version_script_flags = '' else - test_file = '@0@/src/libvirt_qemu.syms'.format(meson.project_source_root()) - if cc.has_link_argument('-Wl,--version-script=@0@'.format(test_file)) - version_script_flags = '-Wl,--version-script=' - else - error('No supported version script link argument found.') - endif + version_script_flags = '-Wl,--version-script=' endif libvirt_flat_namespace = [] -- 2.39.2

2 1

[PATCH] meson: Work around too strict lld
by Michal Privoznik 21 Mar '23

21 Mar '23

With its version 16.0, the LLVM's linker turned on --no-undefined-version by default [1]. This breaks how we detect --version-script= detection, because at the compile time there's no library built yet that we can use to make --version-script= happy. To cancel their choice of defaults, pass --undefined-version. Unfortunately, this flag is LLVM's invention and is not supported by GNU's ld. This all could be avoided if meson provided proper detection of supported linker arguments, but we are far away from that [2]. 1: https://reviews.llvm.org/D135402 2: https://github.com/mesonbuild/meson/issues/3047 Resolves: https://bugs.gentoo.org/902211 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Compile Libvirt? Absolutely Not Gonna. The choice of defaults for LLVM and related subprojects continues to surprise me. meson.build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index c35823a79a..df0d75449a 100644 --- a/meson.build +++ b/meson.build @@ -535,9 +535,13 @@ elif host_machine.system() == 'darwin' version_script_flags = '' else test_file = '@0@/src/libvirt_qemu.syms'.format(meson.project_source_root()) + version_script_flags = '' if cc.has_link_argument('-Wl,--version-script=@0@'.format(test_file)) version_script_flags = '-Wl,--version-script=' - else + elif cc.has_multi_link_arguments('-Wl,--undefined-version', '-Wl,--version-script=@0@'.format(test_file)) + version_script_flags = '-Wl,--version-script=' + endif + if version_script_flags == '' error('No supported version script link argument found.') endif endif -- 2.39.2

3 2

[PATCH 0/2] Fix clang build without numactl and without any code/optimization changes
by Martin Kletzander 20 Mar '23

20 Mar '23

See 1/2 for details, pipeline for this is here: https://gitlab.com/nertpinx/libvirt/-/pipelines/811802916 Martin Kletzander (2): util: Separate numactl and stubs into different source files Revert "meson: stop CLang doing inter-procedural analysis" meson.build | 20 -- po/POTFILES | 2 + src/util/meson.build | 10 + src/util/virnuma.c | 427 +------------------------------------ src/util/virnuma_common.h | 57 +++++ src/util/virnuma_numactl.c | 357 +++++++++++++++++++++++++++++++ src/util/virnuma_stubs.c | 124 +++++++++++ 7 files changed, 551 insertions(+), 446 deletions(-) create mode 100644 src/util/virnuma_common.h create mode 100644 src/util/virnuma_numactl.c create mode 100644 src/util/virnuma_stubs.c -- 2.40.0

2 4

[PATCH 1/1] capabilities: reduce scope in virCapabilitiesInitCaches()
by Shaleen Bathla 20 Mar '23

20 Mar '23

over-writing a variable in inner while-loop without freeing previous memory leaks it over time. To fix this, we can just change scope of bank variable to the inner loop. Signed-off-by: Shaleen Bathla <shaleen.bathla(a)oracle.com> --- src/conf/capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 34770fc4165f..9144ced4984f 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -2178,7 +2178,6 @@ virCapabilitiesInitCaches(virCaps *caps) int rv = -1; g_autoptr(DIR) dirp = NULL; g_autofree char *path = g_strdup_printf("%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos); - g_autoptr(virCapsHostCacheBank) bank = NULL; rv = virDirOpenIfExists(&dirp, path); if (rv < 0) @@ -2189,6 +2188,7 @@ virCapabilitiesInitCaches(virCaps *caps) while ((rv = virDirRead(dirp, &ent, path)) > 0) { g_autofree char *type = NULL; + g_autoptr(virCapsHostCacheBank) bank = NULL; int kernel_type; unsigned int level; int ret; -- 2.31.1

2 1

[libvirt PATCH 0/8] Slightly reduce the number of translatable strings
by Ján Tomko 19 Mar '23

19 Mar '23

*** BLURB HERE *** Ján Tomko (8): bhyve: fix typo in error message src: unify symlink creation error message bhyve: unify error message for UUID generation vbox: remove unreachable error in virVBoxSnapshotConfLoadVboxFile vbox: remove unreachable error in virVBoxSnapshotConfSaveVboxFile util: xml: remove function names from error messages Unify error message when namespaces are unsupported vbox: unify "unable to get hard disk id" message src/bhyve/bhyve_parse_command.c | 2 +- src/bhyve/bhyve_process.c | 2 +- src/libxl/libxl_driver.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_namespace.c | 2 +- src/util/virprocess.c | 6 +++--- src/util/virxml.c | 6 +++--- src/vbox/vbox_common.c | 4 ++-- src/vbox/vbox_snapshot_conf.c | 11 ----------- 10 files changed, 15 insertions(+), 26 deletions(-) -- 2.39.2

2 10

[libvirt PATCH] meson: stop CLang doing inter-procedural analysis
by Daniel P. Berrangé 17 Mar '23

17 Mar '23

The virNumaNodeIsAvailable function is stubbed out when building without libnuma, such that it just returns a constant value. When CLang is optimizing, it does inter-procedural analysis across function calls. When it sees that the call to virNumaNodeIsAvailable returns a fixed constant, it elides the conditional check for errors in the callers such as virNumaNodesetIsAvailable. This is a valid optimization as the C standard declares that there must only be one implementation of each function in a binary. This is normally the case, but ELF allows for function overrides when linking or at runtime with LD_PRELOAD, which is technically outside the mandated C language behaviour. So while CLang's optimization works fine at runtime, it breaks in our test suite which aims to mock the virNumaNodeIsAvailable function so that it has specific semantics regardless of whether libnuma is built or not. The return value check optimization though means our mock override won't have the right effect. The mock will be invoked, but its return value is not used. Potentially the same problem could be exhibited with GCC if certain combinations of optimizations are enabled, though thus far we've not seen it. To be robust on both CLang and GCC we need to make it more explicit that we want to be able to replace functions and thus optimization of calls must be limited. Currently we rely on 'noinline' which does succesfully prevent inlining of the function, but it cannot stop the eliding of checks based on the constant return value. Thus we need a bigger hammer. There are a couple of options to disable this optimization: * Annotate a symbol as 'weak'. This is tells the compiler that the symbol is intended to be overridable at linktime or runtime, and thus it will avoid doing inter-procedural analysis for optimizations. This was tried previously but have to be reverted as it had unintended consequences when linking .a files into our final .so, resulting in all the weak symbol impls being lost. See commit 407a281a8e2b6c5078ba1148535663ea64fd9314 * Annotate a symbol with 'noipa'. This tells the compiler to avoid inter-procedural analysis for calls to just this function. This wold be ideal match for our scenario, but unfortunately it is only implemented for GCC currently: https://reviews.llvm.org/D101011 * The '-fsemantic-interposition' argument tells the optimizer that any functions may be replaced with alternative implementations that have different semantics. It thus blocks any optimizations across function calls. This is quite a harsh block on the optimizer, but it appears to be the only one that is viable with CLang. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- meson.build | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/meson.build b/meson.build index 319ed790f9..c35823a79a 100644 --- a/meson.build +++ b/meson.build @@ -404,6 +404,26 @@ cc_flags += [ '-Wwrite-strings', ] +if cc.get_id() == 'clang' + # Stop CLang from doing inter-procedural analysis of calls + # between functions in the same compilation unit. Such an + # optimization has been know to break the test suite by + # making assumptions that a return value is a constant. + # This makes it impossible to mock certain functions with + # replacement definitions via LD_PRELOAD that have different + # semantics. + # + # This is a bit of a big hammer, but alternatives don't work: + # + # - 'weak' attribute - weak symbols get dropped from + # when the .a libs are combined into the .so + # see commit 407a281a8e2b6c5078ba1148535663ea64fd9314 + # + # - 'noipa' attribute - only available with GCC currently + # https://reviews.llvm.org/D101011 + cc_flags += [ '-fsemantic-interposition' ] +endif + supported_cc_flags = [] if get_option('warning_level') == '2' supported_cc_flags = cc.get_supported_arguments(cc_flags) -- 2.39.2

3 2

[libvirt PATCH] manpages: fix secret injection example for SEV
by Daniel P. Berrangé 17 Mar '23

17 Mar '23

The --disk-password argument was present in early impls of the patch but replaced by the more generic --inject-secret argument. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- docs/manpages/virt-qemu-sev-validate.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/manpages/virt-qemu-sev-validate.rst b/docs/manpages/virt-qemu-sev-validate.rst index b1392e0a50..617ef0a57e 100644 --- a/docs/manpages/virt-qemu-sev-validate.rst +++ b/docs/manpages/virt-qemu-sev-validate.rst @@ -316,7 +316,7 @@ inject a disk password on success: --api-minor 24 \ --build-id 13 \ --policy 3 \ - --disk-password passwd.txt \ + --inject-secret 736869e5-84f0-4973-92ec-06879ce3da0b:passwd.txt \ --secret-header secret-header.b64 \ --secret-payload secret-payload.b64 @@ -393,7 +393,7 @@ inject a disk password on success: --firmware OVMF.sev.fd \ --tk this-guest-tk.bin \ --domain fedora34x86_64 \ - --disk-password passwd.txt + --inject-secret 736869e5-84f0-4973-92ec-06879ce3da0b:passwd.txt Fetch from local libvirt ------------------------ @@ -454,7 +454,7 @@ inject a disk password on success: --insecure \ --tk this-guest-tk.bin \ --domain fedora34x86_64 \ - --disk-password passwd.txt + --inject-secret 736869e5-84f0-4973-92ec-06879ce3da0b:passwd.txt COMMON MISTAKES CHECKLIST ========================= -- 2.39.2

3 2

[libvirt PATCH] rpc: fix typo in admin code generation
by Ján Tomko 17 Mar '23

17 Mar '23

An extra '&' introduced a crash. https://bugzilla.redhat.com/show_bug.cgi?id=2178866 Fixes: 778c3004609ede0a9df4cf3e01c031047530efb7 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/rpc/gendispatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index b186849606..704c133215 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1812,7 +1812,7 @@ elsif ($mode eq "client") { } if ($structprefix eq "admin") { - print " VIR_LOCK_GUARD lock = virObjectLockGuard(&priv);\n"; + print " VIR_LOCK_GUARD lock = virObjectLockGuard(priv);\n"; } else { print " VIR_LOCK_GUARD lock = remoteDriverLock(priv);\n"; } -- 2.39.2

2 1