- Devel - libvirt List Archives

[PATCH 00/12] qemu: migration: Fix crashes when VM shutdowns itself during migration in active state
by Peter Krempa 19 Jun '24

19 Jun '24

The daemon crashes due to unexpected cleanup happening due to bad assumptions about locking and state. See patch 5. Peter Krempa (12): qemuBlockJobProcessEventConcludedBackup: Handle potentially NULL 'job->disk' qemuDomainDiskPrivateDispose: Prevent dangling 'disk' pointer in blockjob data qemuDomainDeviceBackendChardevForeach: Fix typo in comment qemuDomainObjWait: Add documentation qemuProcessStop: Prevent crash when qemuDomainObjStopWorker() unlocks the VM qemuProcessStop: Move code not depending on 'vm->def->id' after reset of the ID qemu: process: Ensure that 'beingDestroyed' gets cleared only after VM id is reset qemu: domain: Introduce qemuDomainObjIsActive helper qemu: migration: Properly check for live VM after qemuDomainObjWait() qemu: migration: Inline 'qemuMigrationDstFinishResume()' qemuMigrationSrcRun: Re-check whether VM is active before accessing job data qemu: migration: Preserve error across qemuDomainSetMaxMemLock() on error paths src/qemu/qemu_backup.c | 6 +-- src/qemu/qemu_backup.h | 2 +- src/qemu/qemu_blockjob.c | 9 +++- src/qemu/qemu_domain.c | 40 +++++++++++++- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_migration.c | 43 +++++++--------- src/qemu/qemu_process.c | 106 ++++++++++++++++++++++---------------- 7 files changed, 131 insertions(+), 77 deletions(-) -- 2.45.2

2 15

[PATCH] qemu: Don't specify vfio-pci.ramfb when ramfb is false
by Jonathon Jongsma 19 Jun '24

19 Jun '24

Commit 7c8e606b64c73ca56d7134cb16d01257f39c53ef attempted to fix the specification of the ramfb property for vfio-pci devices, but it failed when ramfb is explicitly set to 'off'. This is because only the 'vfio-pci-nohotplug' device supports the 'ramfb' property. Since we use the base 'vfio-pci' device unless ramfb is enabled, attempting to set the 'ramfb' parameter to 'off' this will result in an error like the following: error: internal error: QEMU unexpectedly closed the monitor (vm='rhel'): 2024-06-06T04:43:22.896795Z qemu-kvm: -device {"driver":"vfio-pci","host":"0000:b1:00.4","id":"hostdev0","display":"on ","ramfb":false,"bus":"pci.7","addr":"0x0"}: Property 'vfio-pci.ramfb' not found. This also more closely matches what is done for mdev devices. Resolves: https://issues.redhat.com/browse/RHEL-28808 Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- .../hostdev-pci-display-ramfb.x86_64-latest.args | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2d0eddc79e..1e9de6460e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4760,12 +4760,14 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, g_autofree char *host = virPCIDeviceAddressAsString(&pcisrc->addr); const char *failover_pair_id = NULL; const char *driver = NULL; + /* 'ramfb' property must be omitted unless it's to be enabled */ + bool ramfb = pcisrc->ramfb == VIR_TRISTATE_SWITCH_ON; /* caller has to assign proper passthrough driver name */ switch (pcisrc->driver.name) { case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO: /* ramfb support requires the nohotplug variant */ - if (pcisrc->ramfb == VIR_TRISTATE_SWITCH_ON) + if (ramfb) driver = "vfio-pci-nohotplug"; else driver = "vfio-pci"; @@ -4798,7 +4800,7 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, "p:bootindex", dev->info->effectiveBootIndex, "S:failover_pair_id", failover_pair_id, "S:display", qemuOnOffAuto(pcisrc->display), - "T:ramfb", pcisrc->ramfb, + "B:ramfb", ramfb, NULL) < 0) return NULL; diff --git a/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args index e6e538ef1c..a681504c3d 100644 --- a/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args +++ b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args @@ -29,6 +29,6 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2/.config \ -audiodev '{"id":"audio1","driver":"none"}' \ -vnc 127.0.0.1:0,audiodev=audio1 \ -device '{"driver":"vfio-pci-nohotplug","host":"0000:06:12.5","id":"hostdev0","display":"on","ramfb":true,"bus":"pci.0","addr":"0x2"}' \ --device '{"driver":"vfio-pci","host":"0000:06:13.6","id":"hostdev1","display":"off","ramfb":false,"bus":"pci.0","addr":"0x3"}' \ +-device '{"driver":"vfio-pci","host":"0000:06:13.6","id":"hostdev1","display":"off","bus":"pci.0","addr":"0x3"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on -- 2.45.1

2 2

[PATCH v2] Pass shutoff reason to release hook
by Swapnil Ingle 19 Jun '24

19 Jun '24

Sometimes in release hook it is useful to know if the VM shutdown was graceful or not. This is especially useful to do cleanup based on the VM shutdown failure reason in release hook. This patch proposes to use the last argument 'extra' to pass VM shutoff reason in the call to release hook. Making this change for Qemu and LXC. Signed-off-by: Swapnil Ingle <swapnil.ingle(a)nutanix.com> --- docs/hooks.rst | 49 +++++++++++++++++++++++++++++++++++++++-- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_process.c | 2 +- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/docs/hooks.rst b/docs/hooks.rst index 1dbc492bd4..cdcb161c7e 100644 --- a/docs/hooks.rst +++ b/docs/hooks.rst @@ -215,7 +215,30 @@ operation. There is no specific operation to indicate a "restart" is occurring. :: - /etc/libvirt/hooks/qemu guest_name release end - + /etc/libvirt/hooks/qemu guest_name release end <shutoff-reason> + + +-------------------+------------------------------------------------------------------+ + | Shutoff reason | Description | + +===================+==================================================================+ + | unknown | the reason is unknown | + +-------------------+------------------------------------------------------------------+ + | shutdown | normal shutdown | + +-------------------+------------------------------------------------------------------+ + | destroyed | forced poweroff | + +-------------------+------------------------------------------------------------------+ + | crashed | domain crashed | + +-------------------+------------------------------------------------------------------+ + | migrated | migrated to another host | + +-------------------+------------------------------------------------------------------+ + | saved | saved to a file | + +-------------------+------------------------------------------------------------------+ + | failed | domain failed to start | + +-------------------+------------------------------------------------------------------+ + | from snapshot | restored from a snapshot which was taken while domain was shutoff| + +-------------------+------------------------------------------------------------------+ + | daemon | daemon decides to kill domain during reconnection processing | + +-------------------+------------------------------------------------------------------+ + - :since:`Since 0.9.11`, the qemu hook script is also called at the beginning of incoming migration. It is called as: @@ -312,7 +335,29 @@ operation. There is no specific operation to indicate a "restart" is occurring. :: - /etc/libvirt/hooks/lxc guest_name release end - + /etc/libvirt/hooks/lxc guest_name release end <shutoff-reason> + + +-------------------+------------------------------------------------------------------+ + | Shutoff reason | Description | + +===================+==================================================================+ + | unknown | the reason is unknown | + +-------------------+------------------------------------------------------------------+ + | shutdown | normal shutdown | + +-------------------+------------------------------------------------------------------+ + | destroyed | forced poweroff | + +-------------------+------------------------------------------------------------------+ + | crashed | domain crashed | + +-------------------+------------------------------------------------------------------+ + | migrated | migrated to another host | + +-------------------+------------------------------------------------------------------+ + | saved | saved to a file | + +-------------------+------------------------------------------------------------------+ + | failed | domain failed to start | + +-------------------+------------------------------------------------------------------+ + | from snapshot | restored from a snapshot which was taken while domain was shutoff| + +-------------------+------------------------------------------------------------------+ + | daemon | daemon decides to kill domain during reconnection processing | + +-------------------+------------------------------------------------------------------+ - :since:`Since 0.9.13`, the lxc hook script is also called when the libvirtd daemon restarts and reconnects to previously running LXC processes. If the diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 30ff4eb3d0..f5eb5383ec 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -244,7 +244,7 @@ static void virLXCProcessCleanup(virLXCDriver *driver, /* we can't stop the operation even if the script raised an error */ virHookCall(VIR_HOOK_DRIVER_LXC, vm->def->name, VIR_HOOK_LXC_OP_RELEASE, VIR_HOOK_SUBOP_END, - NULL, xml, NULL); + virDomainShutoffReasonTypeToString(reason), xml, NULL); } if (flags & VIR_LXC_PROCESS_CLEANUP_REMOVE_TRANSIENT) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2c33ac3bf7..9de01b1a0d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8722,7 +8722,7 @@ void qemuProcessStop(virQEMUDriver *driver, /* we can't stop the operation even if the script raised an error */ virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, VIR_HOOK_QEMU_OP_RELEASE, VIR_HOOK_SUBOP_END, - NULL, xml, NULL); + virDomainShutoffReasonTypeToString(reason), xml, NULL); } virDomainObjRemoveTransientDef(vm); -- 2.45.2

2 1

[PATCH] Pass shutoff reason to release hook
by Swapnil Ingle 18 Jun '24

18 Jun '24

Sometimes in release hook it is useful to know if the VM shutdown was graceful or not. This is especially useful to do cleanup based on the VM shutdown failure reason in release hook. This patch proposes to use the last argument 'extra' to pass VM shutoff reason in the call to release hook. Signed-off-by: Swapnil Ingle <swapnil.ingle(a)nutanix.com> --- docs/hooks.rst | 24 +++++++++++++++++++++++- src/qemu/qemu_process.c | 2 +- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/docs/hooks.rst b/docs/hooks.rst index 1dbc492bd4..e76208021a 100644 --- a/docs/hooks.rst +++ b/docs/hooks.rst @@ -312,7 +312,29 @@ operation. There is no specific operation to indicate a "restart" is occurring. :: - /etc/libvirt/hooks/lxc guest_name release end - + /etc/libvirt/hooks/lxc guest_name release end <shutoff-reason> + + +-------------------+------------------------------------------------------------------+ + | Shutoff reason | Description | + +===================+==================================================================+ + | unknown | the reason is unknown | + +-------------------+------------------------------------------------------------------+ + | shutdown | normal shutdown | + +-------------------+------------------------------------------------------------------+ + | destroyed | forced poweroff | + +-------------------+------------------------------------------------------------------+ + | crashed | domain crashed | + +-------------------+------------------------------------------------------------------+ + | migrated | migrated to another host | + +-------------------+------------------------------------------------------------------+ + | saved | saved to a file | + +-------------------+------------------------------------------------------------------+ + | failed | domain failed to start | + +-------------------+------------------------------------------------------------------+ + | from snapshot | restored from a snapshot which was taken while domain was shutoff| + +-------------------+------------------------------------------------------------------+ + | daemon | daemon decides to kill domain during reconnection processing | + +-------------------+------------------------------------------------------------------+ - :since:`Since 0.9.13`, the lxc hook script is also called when the libvirtd daemon restarts and reconnects to previously running LXC processes. If the diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7ef7040a85..0a03685ca7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8716,7 +8716,7 @@ void qemuProcessStop(virQEMUDriver *driver, /* we can't stop the operation even if the script raised an error */ virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name, VIR_HOOK_QEMU_OP_RELEASE, VIR_HOOK_SUBOP_END, - NULL, xml, NULL); + virDomainShutoffReasonTypeToString(reason), xml, NULL); } virDomainObjRemoveTransientDef(vm); -- 2.45.2

2 3

Plans for 10.5.0 release (freeze on Tuesday 25 Jun)
by Jiri Denemark 18 Jun '24

18 Jun '24

We are getting close to 10.5.0 release of libvirt. To aim for the release on Monday 01 Jul I suggest entering the freeze on Tuesday 25 Jun and tagging RC2 on Thursday 27 Jun. I hope this works for everyone. Jirka

1 0

[PATCH 0/2] qemu: implement iommu coldplug/unplug
by Adam Julis 18 Jun '24

18 Jun '24

Adam Julis (2): syms: Properly export virDomainIOMMUDefFree() qemu: implement iommu coldplug/unplug src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 20 ++++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) -- 2.45.0

2 4

[PATCH v2] ci: fix CI package list and refresh with 'lcitool manifest'
by Daniel P. Berrangé 18 Jun '24

18 Jun '24

The ci/manifest.yml file references a package 'libclang-rt-dev' that does not exist in libvirt-ci mappings.yml. The latest refresh in commit 0759cf3fa6ed8d12bd327c5752785c53e35c8483 Author: Michal Prívozník <mprivozn(a)redhat.com> Date: Fri May 3 15:58:20 2024 +0200 ci: Introduce Ubuntu 24.04 was presumably done against a local change to libvirt-ci.git that had not yet been merged, as the clang packages now appear on many more build envs. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- ci/buildenv/almalinux-9.sh | 2 +- ci/buildenv/debian-11-cross-aarch64.sh | 1 + ci/buildenv/debian-11-cross-armv6l.sh | 1 + ci/buildenv/debian-11-cross-armv7l.sh | 1 + ci/buildenv/debian-11-cross-i686.sh | 1 + ci/buildenv/debian-11-cross-mips64el.sh | 1 + ci/buildenv/debian-11-cross-mipsel.sh | 1 + ci/buildenv/debian-11-cross-ppc64le.sh | 1 + ci/buildenv/debian-11-cross-s390x.sh | 1 + ci/buildenv/debian-11.sh | 1 + ci/buildenv/opensuse-leap-15.sh | 1 + ci/buildenv/opensuse-tumbleweed.sh | 1 + ci/buildenv/ubuntu-2204.sh | 1 + ci/containers/almalinux-9.Dockerfile | 2 +- ci/containers/debian-11-cross-aarch64.Dockerfile | 1 + ci/containers/debian-11-cross-armv6l.Dockerfile | 1 + ci/containers/debian-11-cross-armv7l.Dockerfile | 1 + ci/containers/debian-11-cross-i686.Dockerfile | 1 + ci/containers/debian-11-cross-mips64el.Dockerfile | 1 + ci/containers/debian-11-cross-mipsel.Dockerfile | 1 + ci/containers/debian-11-cross-ppc64le.Dockerfile | 1 + ci/containers/debian-11-cross-s390x.Dockerfile | 1 + ci/containers/debian-11.Dockerfile | 1 + ci/containers/opensuse-leap-15.Dockerfile | 1 + ci/containers/opensuse-tumbleweed.Dockerfile | 1 + ci/containers/ubuntu-2204.Dockerfile | 1 + ci/lcitool/projects/libvirt.yml | 2 +- 27 files changed, 27 insertions(+), 3 deletions(-) diff --git a/ci/buildenv/almalinux-9.sh b/ci/buildenv/almalinux-9.sh index f0826e1313..5791a73d23 100644 --- a/ci/buildenv/almalinux-9.sh +++ b/ci/buildenv/almalinux-9.sh @@ -16,7 +16,7 @@ function install_buildenv() { ca-certificates \ ccache \ clang \ - clang-devel \ + compiler-rt \ cpp \ cyrus-sasl-devel \ device-mapper-devel \ diff --git a/ci/buildenv/debian-11-cross-aarch64.sh b/ci/buildenv/debian-11-cross-aarch64.sh index 3afb09aee5..8540fb8d74 100644 --- a/ci/buildenv/debian-11-cross-aarch64.sh +++ b/ci/buildenv/debian-11-cross-aarch64.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11-cross-armv6l.sh b/ci/buildenv/debian-11-cross-armv6l.sh index ff78ec0b86..131a7019c0 100644 --- a/ci/buildenv/debian-11-cross-armv6l.sh +++ b/ci/buildenv/debian-11-cross-armv6l.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11-cross-armv7l.sh b/ci/buildenv/debian-11-cross-armv7l.sh index ff3ef03463..ba78ffcfac 100644 --- a/ci/buildenv/debian-11-cross-armv7l.sh +++ b/ci/buildenv/debian-11-cross-armv7l.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11-cross-i686.sh b/ci/buildenv/debian-11-cross-i686.sh index e68e2ffcbe..104eb20805 100644 --- a/ci/buildenv/debian-11-cross-i686.sh +++ b/ci/buildenv/debian-11-cross-i686.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11-cross-mips64el.sh b/ci/buildenv/debian-11-cross-mips64el.sh index 0653223a3d..7b1830453c 100644 --- a/ci/buildenv/debian-11-cross-mips64el.sh +++ b/ci/buildenv/debian-11-cross-mips64el.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11-cross-mipsel.sh b/ci/buildenv/debian-11-cross-mipsel.sh index cee2feff59..eef5cdbfab 100644 --- a/ci/buildenv/debian-11-cross-mipsel.sh +++ b/ci/buildenv/debian-11-cross-mipsel.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11-cross-ppc64le.sh b/ci/buildenv/debian-11-cross-ppc64le.sh index 7193d4acd0..f2c2f60623 100644 --- a/ci/buildenv/debian-11-cross-ppc64le.sh +++ b/ci/buildenv/debian-11-cross-ppc64le.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11-cross-s390x.sh b/ci/buildenv/debian-11-cross-s390x.sh index ca0fb54839..519d9c8b31 100644 --- a/ci/buildenv/debian-11-cross-s390x.sh +++ b/ci/buildenv/debian-11-cross-s390x.sh @@ -27,6 +27,7 @@ function install_buildenv() { iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/buildenv/debian-11.sh b/ci/buildenv/debian-11.sh index 18350321b0..5986682af8 100644 --- a/ci/buildenv/debian-11.sh +++ b/ci/buildenv/debian-11.sh @@ -36,6 +36,7 @@ function install_buildenv() { libblkid-dev \ libc6-dev \ libcap-ng-dev \ + libclang-dev \ libcurl4-gnutls-dev \ libdevmapper-dev \ libfuse-dev \ diff --git a/ci/buildenv/opensuse-leap-15.sh b/ci/buildenv/opensuse-leap-15.sh index bc7394839b..a59af136ca 100644 --- a/ci/buildenv/opensuse-leap-15.sh +++ b/ci/buildenv/opensuse-leap-15.sh @@ -14,6 +14,7 @@ function install_buildenv() { ca-certificates \ ccache \ clang \ + clang-devel \ codespell \ cpp \ cppi \ diff --git a/ci/buildenv/opensuse-tumbleweed.sh b/ci/buildenv/opensuse-tumbleweed.sh index 88ccff99c6..ac566d349f 100644 --- a/ci/buildenv/opensuse-tumbleweed.sh +++ b/ci/buildenv/opensuse-tumbleweed.sh @@ -14,6 +14,7 @@ function install_buildenv() { ca-certificates \ ccache \ clang \ + clang-devel \ codespell \ cpp \ cppi \ diff --git a/ci/buildenv/ubuntu-2204.sh b/ci/buildenv/ubuntu-2204.sh index 6bd67ba777..c71a0b5f47 100644 --- a/ci/buildenv/ubuntu-2204.sh +++ b/ci/buildenv/ubuntu-2204.sh @@ -36,6 +36,7 @@ function install_buildenv() { libblkid-dev \ libc6-dev \ libcap-ng-dev \ + libclang-dev \ libcurl4-gnutls-dev \ libdevmapper-dev \ libfuse-dev \ diff --git a/ci/containers/almalinux-9.Dockerfile b/ci/containers/almalinux-9.Dockerfile index 68608b12a9..27ac990b22 100644 --- a/ci/containers/almalinux-9.Dockerfile +++ b/ci/containers/almalinux-9.Dockerfile @@ -17,7 +17,7 @@ RUN dnf update -y && \ ca-certificates \ ccache \ clang \ - clang-devel \ + compiler-rt \ cpp \ cyrus-sasl-devel \ device-mapper-devel \ diff --git a/ci/containers/debian-11-cross-aarch64.Dockerfile b/ci/containers/debian-11-cross-aarch64.Dockerfile index 0f971ff9cb..1cb573821f 100644 --- a/ci/containers/debian-11-cross-aarch64.Dockerfile +++ b/ci/containers/debian-11-cross-aarch64.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11-cross-armv6l.Dockerfile b/ci/containers/debian-11-cross-armv6l.Dockerfile index cfed7a7fc9..6989546ebf 100644 --- a/ci/containers/debian-11-cross-armv6l.Dockerfile +++ b/ci/containers/debian-11-cross-armv6l.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11-cross-armv7l.Dockerfile b/ci/containers/debian-11-cross-armv7l.Dockerfile index f703be3423..fcd6a6383b 100644 --- a/ci/containers/debian-11-cross-armv7l.Dockerfile +++ b/ci/containers/debian-11-cross-armv7l.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11-cross-i686.Dockerfile b/ci/containers/debian-11-cross-i686.Dockerfile index 58be733459..8d79934a52 100644 --- a/ci/containers/debian-11-cross-i686.Dockerfile +++ b/ci/containers/debian-11-cross-i686.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11-cross-mips64el.Dockerfile b/ci/containers/debian-11-cross-mips64el.Dockerfile index c3198e0470..d80f741311 100644 --- a/ci/containers/debian-11-cross-mips64el.Dockerfile +++ b/ci/containers/debian-11-cross-mips64el.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11-cross-mipsel.Dockerfile b/ci/containers/debian-11-cross-mipsel.Dockerfile index 21e9b0c7f9..dc674150f5 100644 --- a/ci/containers/debian-11-cross-mipsel.Dockerfile +++ b/ci/containers/debian-11-cross-mipsel.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11-cross-ppc64le.Dockerfile b/ci/containers/debian-11-cross-ppc64le.Dockerfile index 29be7997f8..fc3a9ee157 100644 --- a/ci/containers/debian-11-cross-ppc64le.Dockerfile +++ b/ci/containers/debian-11-cross-ppc64le.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11-cross-s390x.Dockerfile b/ci/containers/debian-11-cross-s390x.Dockerfile index fd1507b294..336694b2d3 100644 --- a/ci/containers/debian-11-cross-s390x.Dockerfile +++ b/ci/containers/debian-11-cross-s390x.Dockerfile @@ -29,6 +29,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ iproute2 \ iptables \ kmod \ + libclang-dev \ libxml2-utils \ locales \ lvm2 \ diff --git a/ci/containers/debian-11.Dockerfile b/ci/containers/debian-11.Dockerfile index c16c43d407..6f08eb7448 100644 --- a/ci/containers/debian-11.Dockerfile +++ b/ci/containers/debian-11.Dockerfile @@ -38,6 +38,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ libblkid-dev \ libc6-dev \ libcap-ng-dev \ + libclang-dev \ libcurl4-gnutls-dev \ libdevmapper-dev \ libfuse-dev \ diff --git a/ci/containers/opensuse-leap-15.Dockerfile b/ci/containers/opensuse-leap-15.Dockerfile index 6deaea0904..bf794d6929 100644 --- a/ci/containers/opensuse-leap-15.Dockerfile +++ b/ci/containers/opensuse-leap-15.Dockerfile @@ -15,6 +15,7 @@ RUN zypper update -y && \ ca-certificates \ ccache \ clang \ + clang-devel \ codespell \ cpp \ cppi \ diff --git a/ci/containers/opensuse-tumbleweed.Dockerfile b/ci/containers/opensuse-tumbleweed.Dockerfile index d4ebcd7176..2b7cdb4af5 100644 --- a/ci/containers/opensuse-tumbleweed.Dockerfile +++ b/ci/containers/opensuse-tumbleweed.Dockerfile @@ -15,6 +15,7 @@ RUN zypper dist-upgrade -y && \ ca-certificates \ ccache \ clang \ + clang-devel \ codespell \ cpp \ cppi \ diff --git a/ci/containers/ubuntu-2204.Dockerfile b/ci/containers/ubuntu-2204.Dockerfile index 8e32d992f3..5e8829bc2b 100644 --- a/ci/containers/ubuntu-2204.Dockerfile +++ b/ci/containers/ubuntu-2204.Dockerfile @@ -38,6 +38,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ libblkid-dev \ libc6-dev \ libcap-ng-dev \ + libclang-dev \ libcurl4-gnutls-dev \ libdevmapper-dev \ libfuse-dev \ diff --git a/ci/lcitool/projects/libvirt.yml b/ci/lcitool/projects/libvirt.yml index a5d2248437..5e0bd66958 100644 --- a/ci/lcitool/projects/libvirt.yml +++ b/ci/lcitool/projects/libvirt.yml @@ -36,7 +36,7 @@ packages: - libblkid - libc - libcap-ng - - libclang-rt-dev + - libclang-rt - libcurl - libiscsi - libnbd -- 2.45.1

2 1

[PATCH v2 0/4] qemu: Use TPM 2.0 in most scenarios
by Andrea Bolognani 18 Jun '24

18 Jun '24

Changes from [v1] * use TPM 2.0 more; * reject TPM 1.2 more; * add better comments to loongarch64 and s390x test cases. [v1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/YZMV… Andrea Bolognani (4): tests: Add TPM coverage to default-models tests tests: Delete some redundant test cases qemu: Default to TPM 2.0 in most scenarios qemu: Reject TPM 1.2 in most scenarios src/qemu/qemu_domain.c | 13 ++-- src/qemu/qemu_validate.c | 22 +++--- ...aarch64-tpm-wrong-model.aarch64-latest.err | 2 +- .../aarch64-tpm.aarch64-latest.args | 34 --------- .../aarch64-tpm.aarch64-latest.xml | 29 -------- tests/qemuxmlconfdata/aarch64-tpm.xml | 15 ---- ...ault-models.aarch64-latest.abi-update.args | 3 + ...fault-models.aarch64-latest.abi-update.xml | 3 + ...64-virt-default-models.aarch64-latest.args | 3 + ...h64-virt-default-models.aarch64-latest.xml | 3 + .../aarch64-virt-default-models.xml | 3 + .../loongarch64-virt-default-models.xml | 3 + ...efault-models.ppc64-latest.abi-update.args | 3 + ...default-models.ppc64-latest.abi-update.xml | 4 ++ ...4-pseries-default-models.ppc64-latest.args | 3 + ...64-pseries-default-models.ppc64-latest.xml | 4 ++ .../ppc64-pseries-default-models.xml | 3 + ...ault-models.riscv64-latest.abi-update.args | 3 + ...fault-models.riscv64-latest.abi-update.xml | 3 + ...64-virt-default-models.riscv64-latest.args | 3 + ...v64-virt-default-models.riscv64-latest.xml | 3 + .../riscv64-virt-default-models.xml | 3 + .../s390x-ccw-default-models.xml | 2 + .../tpm-emulator-spapr.ppc64-latest.args | 45 ------------ .../tpm-emulator-spapr.ppc64-latest.xml | 1 - tests/qemuxmlconfdata/tpm-emulator-spapr.xml | 70 ------------------- ...fault-models.x86_64-latest.abi-update.args | 3 + ...efault-models.x86_64-latest.abi-update.xml | 3 + ...86_64-pc-default-models.x86_64-latest.args | 3 + ...x86_64-pc-default-models.x86_64-latest.xml | 3 + .../x86_64-pc-default-models.xml | 3 + ...fault-models.x86_64-latest.abi-update.args | 3 + ...efault-models.x86_64-latest.abi-update.xml | 3 + ...6_64-q35-default-models.x86_64-latest.args | 3 + ...86_64-q35-default-models.x86_64-latest.xml | 3 + .../x86_64-q35-default-models.xml | 3 + tests/qemuxmlconftest.c | 2 - 37 files changed, 100 insertions(+), 215 deletions(-) delete mode 100644 tests/qemuxmlconfdata/aarch64-tpm.aarch64-latest.args delete mode 100644 tests/qemuxmlconfdata/aarch64-tpm.aarch64-latest.xml delete mode 100644 tests/qemuxmlconfdata/aarch64-tpm.xml delete mode 100644 tests/qemuxmlconfdata/tpm-emulator-spapr.ppc64-latest.args delete mode 120000 tests/qemuxmlconfdata/tpm-emulator-spapr.ppc64-latest.xml delete mode 100644 tests/qemuxmlconfdata/tpm-emulator-spapr.xml -- 2.45.1

3 9

[PATCH 2/2] qemu: implement iommu coldplug/unplug
by Adam Julis 18 Jun '24

18 Jun '24

Resolves: https://issues.redhat.com/browse/RHEL-23833 Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/qemu/qemu_driver.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 67b9778c67..74d5e3bb86 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6847,6 +6847,15 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, vmdef->vsock = g_steal_pointer(&dev->data.vsock); break; + case VIR_DOMAIN_DEVICE_IOMMU: + if (vmdef->iommu) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain already has an iommu configuration")); + return -1; + } + vmdef->iommu = g_steal_pointer(&dev->data.iommu); + break; + case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: @@ -6856,7 +6865,6 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_IOMMU: case VIR_DOMAIN_DEVICE_AUDIO: case VIR_DOMAIN_DEVICE_CRYPTO: case VIR_DOMAIN_DEVICE_LAST: @@ -7057,6 +7065,15 @@ qemuDomainDetachDeviceConfig(virDomainDef *vmdef, g_clear_pointer(&vmdef->vsock, virDomainVsockDefFree); break; + case VIR_DOMAIN_DEVICE_IOMMU: + if (!vmdef->iommu) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching iommu config not found")); + return -1; + } + g_clear_pointer(&vmdef->iommu, virDomainIOMMUDefFree); + break; + case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: @@ -7066,7 +7083,6 @@ qemuDomainDetachDeviceConfig(virDomainDef *vmdef, case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_IOMMU: case VIR_DOMAIN_DEVICE_AUDIO: case VIR_DOMAIN_DEVICE_CRYPTO: case VIR_DOMAIN_DEVICE_LAST: -- 2.44.0

2 1

[PATCH] conf: add validation of potential dependencies
by Adam Julis 18 Jun '24

18 Jun '24

Although existing virDomainDefPostParse is called after modifying the XML and it contains validating process for changed device, the virDomainDefValidate function performs a more comprehensive check. It should detect errors resulting from dependencies between devices. Therefore, the virDomainDefValidate is added at the end. Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e2698c7924..67b9778c67 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6869,6 +6869,9 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, if (virDomainDefPostParse(vmdef, parse_flags, xmlopt, qemuCaps) < 0) return -1; + if (virDomainDefValidate(vmdef, parse_flags, xmlopt, qemuCaps) < 0) + return -1; + return 0; } -- 2.44.0

2 1

[PATCH v3] network: introduce a "none" firewall backend type
by Daniel P. Berrangé 17 Jun '24

17 Jun '24

There are two scenarios identified after the recent firewall backend selection was introduced, which result in libvirtd failing to startup due to an inability to find either iptables/nftables - On Linux if running unprivileged with $PATH lacking the dir containing iptables/nftables - On non-Linux where iptables/nftables never existed In the former case, it is preferrable to restore the behaviour whereby the driver starts successfully. Users will get an error reported when attempting to start any virtual network, due to the lack of permissions needed to create bridge devices. This makes the missing firewall backend irrelevant. In the latter case, the network driver calls the 'nop' platform implementation which does not attempt to implement any firewall logic, just allowing the network to start without firewall rules. To solve this are number of changes are required * Introduce VIR_FIREWALL_BACKEND_NONE, which does nothing except report a fatal error from virFirewallApply(). This code path is unreachable, since we'll never create a virFirewall object with with VIR_FIREWALL_BACKEND_NONE, so the error reporting is just a sanity check. * Ignore the compile time backend defaults and assume use of the 'none' backend if running unprivileged. This fixes the first regression, avoiding the failure to start libvirtd on Linux in unprivileged context, instead allowing use of the driver and expecting a permission denied when creating a bridge. * Reject the use of compile time backend defaults no non-Linux and hardcode the 'none' backend. The non-Linux platforms have no firewall implementation at all currently, so there's no reason to permit the use of 'firewall_backend_priority' meson option. This fixes the second regression, avoiding the failure to start libvirtd on non-Linux hosts due to non-existant Linux binaries. * Change the Linux platform backend to raise an error if the firewall backend is 'none'. Again this code path is unreachable by default since we'll fail to create the bridge before getting here, but if someone modified network.conf to request the 'none' backend, this will stop further progress. * Change the nop platform backend to raise an error if the firewall backend is 'iptables' or 'nftables'. Again this code path is unreachable, since we should already have failed to find the iptables/nftables binaries on non-Linux hosts, so this is just a sanity check. * 'none' is not permited as a value in 'firewall_backend_priority' meson option, since it is conceptually meaningless to ask for that on Linux. NB, 'firewall_backend_priority' allows repeated options temporarily, which we don't want. Meson intends to turn this into a hard error DEPRECATION: Duplicated values in array option is deprecated. This will become a hard error in the future. and we can live with the reduced error checking until that happens. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Changed in v3: - Fix various syntax-check errors - Added note about meson tightening up validation of duplicated choices meson.build | 26 +++++++++++++++++++------- meson_options.txt | 2 +- po/POTFILES | 1 + src/network/bridge_driver_conf.c | 19 ++++++++++++++----- src/network/bridge_driver_linux.c | 10 ++++++++++ src/network/bridge_driver_nop.c | 15 ++++++++++++++- src/util/virfirewall.c | 6 ++++++ src/util/virfirewall.h | 1 + 8 files changed, 66 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index 5c7cd7ec2e..2e8b87280d 100644 --- a/meson.build +++ b/meson.build @@ -1647,15 +1647,27 @@ if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD') conf.set('WITH_NETWORK', 1) firewall_backend_priority = get_option('firewall_backend_priority') - if (not firewall_backend_priority.contains('nftables') or - not firewall_backend_priority.contains('iptables') or - firewall_backend_priority.length() != 2) - error('invalid value for firewall_backend_priority option') + if firewall_backend_priority.length() == 0 + if host_machine.system() == 'linux' + firewall_backend_priority = ['nftables', 'iptables'] + else + # No firewall impl on non-Linux so far, so force 'none' + # as placeholder + firewall_backend_priority = ['none'] + endif + else + if host_machine.system() != 'linux' + error('firewall backend priority only supported on linux hosts') + endif endif - conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[0].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[1].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length()) + backends = [] + foreach backend: firewall_backend_priority + backend = 'VIR_FIREWALL_BACKEND_' + backend.to_upper() + backends += backend + endforeach + + conf.set('FIREWALL_BACKENDS', ', '.join(backends)) elif get_option('driver_network').enabled() error('libvirtd must be enabled to build the network driver') endif diff --git a/meson_options.txt b/meson_options.txt index 50d71427cb..2d440c63d8 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -117,7 +117,7 @@ option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for st option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support') # dep:firewalld option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') -option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], description: 'order in which to try firewall backends') +option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], value: [], description: 'order in which to try firewall backends') option('host_validate', type: 'feature', value: 'auto', description: 'build virt-host-validate') option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check', 'none'], value: 'check', description: 'Style of init script to install') option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of <loader>:<nvram> paths. Both pairs and list items are separated by a colon.') diff --git a/po/POTFILES b/po/POTFILES index 4bfbb91164..1ed4086d2c 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -143,6 +143,7 @@ src/lxc/lxc_process.c src/network/bridge_driver.c src/network/bridge_driver_conf.c src/network/bridge_driver_linux.c +src/network/bridge_driver_nop.c src/network/leaseshelper.c src/network/network_iptables.c src/network/network_nftables.c diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c index e2f3613a41..9da5e790b7 100644 --- a/src/network/bridge_driver_conf.c +++ b/src/network/bridge_driver_conf.c @@ -61,6 +61,7 @@ networkGetDnsmasqCaps(virNetworkDriverState *driver) static int virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, + bool privileged, const char *filename) { g_autoptr(virConf) conf = NULL; @@ -68,13 +69,17 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, bool fwBackendSelected = false; size_t i; int fwBackends[] = { - FIREWALL_BACKEND_PRIORITY_0, - FIREWALL_BACKEND_PRIORITY_1, + FIREWALL_BACKENDS }; - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST); - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == FIREWALL_BACKEND_PRIORITY_NUM); + G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) > 0 && + G_N_ELEMENTS(fwBackends) <= VIR_FIREWALL_BACKEND_LAST); int nFwBackends = G_N_ELEMENTS(fwBackends); + if (!privileged) { + fwBackends[0] = VIR_FIREWALL_BACKEND_NONE; + nFwBackends = 1; + } + if (access(filename, R_OK) == 0) { conf = virConfReadFile(filename, 0); @@ -104,6 +109,10 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, for (i = 0; i < nFwBackends && !fwBackendSelected; i++) { switch ((virFirewallBackend)fwBackends[i]) { + case VIR_FIREWALL_BACKEND_NONE: + fwBackendSelected = true; + break; + case VIR_FIREWALL_BACKEND_IPTABLES: { g_autofree char *iptablesInPath = virFindFileInPath(IPTABLES); @@ -187,7 +196,7 @@ virNetworkDriverConfigNew(bool privileged) configfile = g_strconcat(configdir, "/network.conf", NULL); - if (virNetworkLoadDriverConfig(cfg, configfile) < 0) + if (virNetworkLoadDriverConfig(cfg, privileged, configfile) < 0) return NULL; if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) { diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 35e6bd1154..fe7c6e193c 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -47,6 +47,11 @@ networkFirewallSetupPrivateChains(virFirewallBackend backend, virFirewallLayer layer) { switch (backend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesSetupPrivateChains(layer); @@ -417,6 +422,11 @@ networkAddFirewallRules(virNetworkDef *def, } switch (firewallBackend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesAddFirewallRules(def, fwRemoval); diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c index 537b9234f8..8bf3367bff 100644 --- a/src/network/bridge_driver_nop.c +++ b/src/network/bridge_driver_nop.c @@ -19,6 +19,8 @@ #include <config.h> +#define VIR_FROM_THIS VIR_FROM_NETWORK + void networkPreReloadFirewallRules(virNetworkDriverState *driver G_GNUC_UNUSED, bool startup G_GNUC_UNUSED, bool force G_GNUC_UNUSED) @@ -37,9 +39,20 @@ int networkCheckRouteCollision(virNetworkDef *def G_GNUC_UNUSED) } int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED, - virFirewallBackend firewallBackend G_GNUC_UNUSED, + virFirewallBackend firewallBackend, virFirewall **fwRemoval G_GNUC_UNUSED) { + /* + * Shouldn't be possible, since virNetworkLoadDriverConfig + * ought to fail to find the required binaries when loading, + * so this is just a sanity check + */ + if (firewallBackend != VIR_FIREWALL_BACKEND_NONE) { + virReportError(VIR_ERR_NO_SUPPORT, + _("Firewall backend '%1$s' not available on this platform"), + virFirewallBackendTypeToString(firewallBackend)); + return -1; + } return 0; } diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 2219506b18..090dbcdbed 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -37,6 +37,7 @@ VIR_LOG_INIT("util.firewall"); VIR_ENUM_IMPL(virFirewallBackend, VIR_FIREWALL_BACKEND_LAST, + "none", "iptables", "nftables"); @@ -815,6 +816,11 @@ virFirewallApplyCmd(virFirewall *firewall, } switch (virFirewallGetBackend(firewall)) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Firewall backend is not implemented")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: if (virFirewallCmdIptablesApply(firewall, fwCmd, &output) < 0) return -1; diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 302a6a4e5b..bce51259d2 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -44,6 +44,7 @@ typedef enum { } virFirewallLayer; typedef enum { + VIR_FIREWALL_BACKEND_NONE, /* Always fails */ VIR_FIREWALL_BACKEND_IPTABLES, VIR_FIREWALL_BACKEND_NFTABLES, -- 2.45.1

2 1

[PATCH] conf: Drop needless NULL checks guarding virBufferEscapeString()
by Michal Privoznik 17 Jun '24

17 Jun '24

There's no need to guard virBufferEscapeString() with a call to NULL as the very first thing the function does is check all three arguments for NULL. This patch was generated using the following spatch: @@ expression X, Y, E; @@ - if (E) virBufferEscapeString(X, Y, E); Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/capabilities.c | 6 ++-- src/conf/domain_conf.c | 57 +++++++++++------------------- src/conf/network_conf.c | 6 ++-- src/conf/node_device_conf.c | 57 ++++++++++++------------------ src/conf/snapshot_conf.c | 5 ++- src/conf/storage_encryption_conf.c | 9 ++--- src/conf/storage_source_conf.c | 3 +- src/conf/virnwfilterbindingdef.c | 3 +- 8 files changed, 55 insertions(+), 91 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index fe5e42c167..74e6293766 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -693,10 +693,8 @@ virCapabilitiesDomainDataLookupInternal(virCaps *caps, if (domaintype > VIR_DOMAIN_VIRT_NONE) virBufferAsprintf(&buf, "domaintype=%s ", virDomainVirtTypeToString(domaintype)); - if (emulator) - virBufferEscapeString(&buf, "emulator=%s ", emulator); - if (machinetype) - virBufferEscapeString(&buf, "machine=%s ", machinetype); + virBufferEscapeString(&buf, "emulator=%s ", emulator); + virBufferEscapeString(&buf, "machine=%s ", machinetype); if (virBufferCurrentContent(&buf) && !virBufferCurrentContent(&buf)[0]) virBufferAsprintf(&buf, "%s", _("any configuration")); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fde594f811..2f1e99865b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5394,8 +5394,7 @@ virDomainDeviceInfoFormat(virBuffer *buf, if (rombar) virBufferAsprintf(buf, " bar='%s'", rombar); } - if (info->romfile) - virBufferEscapeString(buf, " file='%s'", info->romfile); + virBufferEscapeString(buf, " file='%s'", info->romfile); virBufferAddLit(buf, "/>\n"); } @@ -22175,8 +22174,7 @@ virSecurityDeviceLabelDefFormat(virBuffer *buf, virBufferAddLit(buf, "<seclabel"); - if (def->model) - virBufferEscapeString(buf, " model='%s'", def->model); + virBufferEscapeString(buf, " model='%s'", def->model); if (def->labelskip) virBufferAddLit(buf, " labelskip='yes'"); @@ -22371,8 +22369,7 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf, virBufferAsprintf(childBuf, "<timeout seconds='%llu'/>\n", src->timeout); if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) { - if (src->ssh_known_hosts_file) - virBufferEscapeString(childBuf, "<knownHosts path='%s'/>\n", src->ssh_known_hosts_file); + virBufferEscapeString(childBuf, "<knownHosts path='%s'/>\n", src->ssh_known_hosts_file); if (src->ssh_keyfile || src->ssh_agent) { virBufferAddLit(childBuf, "<identity"); @@ -23162,8 +23159,7 @@ virDomainControllerDefFormat(virBuffer *buf, " type='%s' index='%d'", type, def->idx); - if (model) - virBufferEscapeString(&attrBuf, " model='%s'", model); + virBufferEscapeString(&attrBuf, " model='%s'", model); switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: @@ -24581,8 +24577,7 @@ virDomainChrTargetDefFormat(virBuffer *buf, case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (def->target.name) - virBufferEscapeString(buf, " name='%s'", def->target.name); + virBufferEscapeString(buf, " name='%s'", def->target.name); if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && @@ -26004,9 +25999,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->data.vnc.keymap) - virBufferEscapeString(buf, " keymap='%s'", - def->data.vnc.keymap); + virBufferEscapeString(buf, " keymap='%s'", + def->data.vnc.keymap); if (def->data.vnc.sharePolicy) virBufferAsprintf(buf, " sharePolicy='%s'", @@ -26021,13 +26015,11 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - if (def->data.sdl.display) - virBufferEscapeString(buf, " display='%s'", - def->data.sdl.display); + virBufferEscapeString(buf, " display='%s'", + def->data.sdl.display); - if (def->data.sdl.xauth) - virBufferEscapeString(buf, " xauth='%s'", - def->data.sdl.xauth); + virBufferEscapeString(buf, " xauth='%s'", + def->data.sdl.xauth); if (def->data.sdl.fullscreen) virBufferAddLit(buf, " fullscreen='yes'"); @@ -26066,9 +26058,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - if (def->data.desktop.display) - virBufferEscapeString(buf, " display='%s'", - def->data.desktop.display); + virBufferEscapeString(buf, " display='%s'", + def->data.desktop.display); if (def->data.desktop.fullscreen) virBufferAddLit(buf, " fullscreen='yes'"); @@ -26121,9 +26112,8 @@ virDomainGraphicsDefFormat(virBuffer *buf, break; } - if (def->data.spice.keymap) - virBufferEscapeString(buf, " keymap='%s'", - def->data.spice.keymap); + virBufferEscapeString(buf, " keymap='%s'", + def->data.spice.keymap); if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) virBufferAsprintf(buf, " defaultMode='%s'", @@ -26503,11 +26493,9 @@ virDomainResourceDefFormat(virBuffer *buf, if (!def) return; - if (def->partition) - virBufferEscapeString(&childBuf, "<partition>%s</partition>\n", def->partition); + virBufferEscapeString(&childBuf, "<partition>%s</partition>\n", def->partition); - if (def->appid) - virBufferEscapeString(&childBuf, "<fibrechannel appid='%s'/>\n", def->appid); + virBufferEscapeString(&childBuf, "<fibrechannel appid='%s'/>\n", def->appid); virXMLFormatElement(buf, "resource", NULL, &childBuf); } @@ -26680,11 +26668,9 @@ virDomainSecDefFormat(virBuffer *buf, virDomainSecDef *sec) virBufferAsprintf(&childBuf, "<reducedPhysBits>%d</reducedPhysBits>\n", sev->reduced_phys_bits); virBufferAsprintf(&childBuf, "<policy>0x%04x</policy>\n", sev->policy); - if (sev->dh_cert) - virBufferEscapeString(&childBuf, "<dhCert>%s</dhCert>\n", sev->dh_cert); + virBufferEscapeString(&childBuf, "<dhCert>%s</dhCert>\n", sev->dh_cert); - if (sev->session) - virBufferEscapeString(&childBuf, "<session>%s</session>\n", sev->session); + virBufferEscapeString(&childBuf, "<session>%s</session>\n", sev->session); break; } @@ -27910,9 +27896,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, for (i = 0; def->os.initenv && def->os.initenv[i]; i++) virBufferAsprintf(buf, "<initenv name='%s'>%s</initenv>\n", def->os.initenv[i]->name, def->os.initenv[i]->value); - if (def->os.initdir) - virBufferEscapeString(buf, "<initdir>%s</initdir>\n", - def->os.initdir); + virBufferEscapeString(buf, "<initdir>%s</initdir>\n", + def->os.initdir); if (def->os.inituser) virBufferAsprintf(buf, "<inituser>%s</inituser>\n", def->os.inituser); if (def->os.initgroup) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index cc92ed0b03..f5ccf4bd12 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2004,10 +2004,8 @@ virNetworkDNSDefFormat(virBuffer *buf, def->srvs[i].service); virBufferEscapeString(buf, "protocol='%s'", def->srvs[i].protocol); - if (def->srvs[i].domain) - virBufferEscapeString(buf, " domain='%s'", def->srvs[i].domain); - if (def->srvs[i].target) - virBufferEscapeString(buf, " target='%s'", def->srvs[i].target); + virBufferEscapeString(buf, " domain='%s'", def->srvs[i].domain); + virBufferEscapeString(buf, " target='%s'", def->srvs[i].target); if (def->srvs[i].port) virBufferAsprintf(buf, " port='%d'", def->srvs[i].port); if (def->srvs[i].priority) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index fe6d9a36b2..d2b578178b 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -176,20 +176,16 @@ virNodeDeviceCapSystemDefFormat(virBuffer *buf, { char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (data->system.product_name) - virBufferEscapeString(buf, "<product>%s</product>\n", - data->system.product_name); + virBufferEscapeString(buf, "<product>%s</product>\n", + data->system.product_name); virBufferAddLit(buf, "<hardware>\n"); virBufferAdjustIndent(buf, 2); - if (data->system.hardware.vendor_name) - virBufferEscapeString(buf, "<vendor>%s</vendor>\n", - data->system.hardware.vendor_name); - if (data->system.hardware.version) - virBufferEscapeString(buf, "<version>%s</version>\n", - data->system.hardware.version); - if (data->system.hardware.serial) - virBufferEscapeString(buf, "<serial>%s</serial>\n", - data->system.hardware.serial); + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", + data->system.hardware.vendor_name); + virBufferEscapeString(buf, "<version>%s</version>\n", + data->system.hardware.version); + virBufferEscapeString(buf, "<serial>%s</serial>\n", + data->system.hardware.serial); virUUIDFormat(data->system.hardware.uuid, uuidstr); virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); virBufferAdjustIndent(buf, -2); @@ -197,15 +193,12 @@ virNodeDeviceCapSystemDefFormat(virBuffer *buf, virBufferAddLit(buf, "<firmware>\n"); virBufferAdjustIndent(buf, 2); - if (data->system.firmware.vendor_name) - virBufferEscapeString(buf, "<vendor>%s</vendor>\n", - data->system.firmware.vendor_name); - if (data->system.firmware.version) - virBufferEscapeString(buf, "<version>%s</version>\n", - data->system.firmware.version); - if (data->system.firmware.release_date) - virBufferEscapeString(buf, "<release_date>%s</release_date>\n", - data->system.firmware.release_date); + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", + data->system.firmware.vendor_name); + virBufferEscapeString(buf, "<version>%s</version>\n", + data->system.firmware.version); + virBufferEscapeString(buf, "<release_date>%s</release_date>\n", + data->system.firmware.release_date); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</firmware>\n"); } @@ -225,9 +218,8 @@ virNodeDeviceCapMdevTypesFormat(virBuffer *buf, virMediatedDeviceType *type = mdev_types[i]; virBufferEscapeString(buf, "<type id='%s'>\n", type->id); virBufferAdjustIndent(buf, 2); - if (type->name) - virBufferEscapeString(buf, "<name>%s</name>\n", - type->name); + virBufferEscapeString(buf, "<name>%s</name>\n", + type->name); virBufferEscapeString(buf, "<deviceAPI>%s</deviceAPI>\n", type->device_api); virBufferAsprintf(buf, @@ -454,10 +446,9 @@ virNodeDeviceCapUSBInterfaceDefFormat(virBuffer *buf, data->usb_if.subclass); virBufferAsprintf(buf, "<protocol>%d</protocol>\n", data->usb_if.protocol); - if (data->usb_if.description) - virBufferEscapeString(buf, - "<description>%s</description>\n", - data->usb_if.description); + virBufferEscapeString(buf, + "<description>%s</description>\n", + data->usb_if.description); } @@ -469,9 +460,8 @@ virNodeDeviceCapNetDefFormat(virBuffer *buf, virBufferEscapeString(buf, "<interface>%s</interface>\n", data->net.ifname); - if (data->net.address) - virBufferEscapeString(buf, "<address>%s</address>\n", - data->net.address); + virBufferEscapeString(buf, "<address>%s</address>\n", + data->net.address); virInterfaceLinkFormat(buf, &data->net.lnk); if (data->net.features) { for (i = 0; i < VIR_NET_DEV_FEAT_LAST; i++) { @@ -533,9 +523,8 @@ virNodeDeviceCapSCSIDefFormat(virBuffer *buf, virBufferAsprintf(buf, "<target>%d</target>\n", data->scsi.target); virBufferAsprintf(buf, "<lun>%d</lun>\n", data->scsi.lun); - if (data->scsi.type) - virBufferEscapeString(buf, "<type>%s</type>\n", - data->scsi.type); + virBufferEscapeString(buf, "<type>%s</type>\n", + data->scsi.type); } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index d7fcded302..039ed77b84 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -819,9 +819,8 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf, virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "<name>%s</name>\n", def->parent.name); - if (def->parent.description) - virBufferEscapeString(buf, "<description>%s</description>\n", - def->parent.description); + virBufferEscapeString(buf, "<description>%s</description>\n", + def->parent.description); if (def->state) virBufferAsprintf(buf, "<state>%s</state>\n", virDomainSnapshotStateTypeToString(def->state)); diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 1849df5c6c..b86001ec50 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -317,16 +317,13 @@ virStorageEncryptionInfoDefFormat(virBuffer *buf, { virBufferEscapeString(buf, "<cipher name='%s'", enc->cipher_name); virBufferAsprintf(buf, " size='%u'", enc->cipher_size); - if (enc->cipher_mode) - virBufferEscapeString(buf, " mode='%s'", enc->cipher_mode); - if (enc->cipher_hash) - virBufferEscapeString(buf, " hash='%s'", enc->cipher_hash); + virBufferEscapeString(buf, " mode='%s'", enc->cipher_mode); + virBufferEscapeString(buf, " hash='%s'", enc->cipher_hash); virBufferAddLit(buf, "/>\n"); if (enc->ivgen_name) { virBufferEscapeString(buf, "<ivgen name='%s'", enc->ivgen_name); - if (enc->ivgen_hash) - virBufferEscapeString(buf, " hash='%s'", enc->ivgen_hash); + virBufferEscapeString(buf, " hash='%s'", enc->ivgen_hash); virBufferAddLit(buf, "/>\n"); } } diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index 959ec5ed40..908bc5fab2 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -1347,8 +1347,7 @@ int virStorageSourcePrivateDataFormatRelPath(virStorageSource *src, virBuffer *buf) { - if (src->relPath) - virBufferEscapeString(buf, "<relPath>%s</relPath>\n", src->relPath); + virBufferEscapeString(buf, "<relPath>%s</relPath>\n", src->relPath); return 0; } diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 423ed7a392..fe45c84347 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -203,8 +203,7 @@ virNWFilterBindingDefFormatBuf(virBuffer *buf, virBufferAddLit(buf, "</owner>\n"); virBufferEscapeString(buf, "<portdev name='%s'/>\n", def->portdevname); - if (def->linkdevname) - virBufferEscapeString(buf, "<linkdev name='%s'/>\n", def->linkdevname); + virBufferEscapeString(buf, "<linkdev name='%s'/>\n", def->linkdevname); virMacAddrFormat(&def->mac, mac); virBufferAsprintf(buf, "<mac address='%s'/>\n", mac); -- 2.44.2

2 1

[PATCH v2] network: introduce a "none" firewall backend type
by Daniel P. Berrangé 17 Jun '24

17 Jun '24

There are two scenarios identified after the recent firewall backend selection was introduced, which result in libvirtd failing to startup due to an inability to find either iptables/nftables - On Linux if running unprivileged with $PATH lacking the dir containing iptables/nftables - On non-Linux where iptables/nftables never existed In the former case, it is preferrable to restore the behaviour whereby the driver starts successfully. Users will get an error reported when attempting to start any virtual network, due to the lack of permissions needed to create bridge devices. This makes the missing firewall backend irrelevant. In the latter case, the network driver calls the 'nop' platform implementation which does not attempt to implement any firewall logic, just allowing the network to start without firewall rules. To solve this are number of changes are required * Introduce VIR_FIREWALL_BACKEND_NONE, which does nothing except report a fatal error from virFirewallApply(). This code path is unreachable, since we'll never create a virFirewall object with with VIR_FIREWALL_BACKEND_NONE, so the error reporting is just a sanity check. * Ignore the compile time backend defaults and assume use of the 'none' backend if running unprivileged. This fixes the first regression, avoiding the failure to start libvirtd on Linux in unprivileged context, instead allowing use of the driver and expecting a permission denied when creating a bridge. * Reject the use of compile time backend defaults no non-Linux and hardcode the 'none' backend. The non-Linux platforms have no firewall implementation at all currently, so there's no reason to permit the use of 'firewall_backend_priority' meson option. This fixes the second regression, avoiding the failure to start libvirtd on non-Linux hosts due to non-existant Linux binaries. * Change the Linux platform backend to raise an error if the firewall backend is 'none'. Again this code path is unreachable by default since we'll fail to create the bridge before getting here, but if someone modified network.conf to request the 'none' backend, this will stop further progress. * Change the nop platform backend to raise an error if the firewall backend is 'iptables' or 'nftables'. Again this code path is unreachable, since we should already have failed to find the iptables/nftables binaries on non-Linux hosts, so this is just a sanity check. * 'none' is not permited as a value in 'firewall_backend_priority' meson option, since it is conceptually meaningless to ask for that on Linux. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Changed in v2: - Fix build problems on FreeBSD (changes proposed by Roman) meson.build | 26 +++++++++++++++++++------- meson_options.txt | 2 +- src/network/bridge_driver_conf.c | 19 ++++++++++++++----- src/network/bridge_driver_linux.c | 10 ++++++++++ src/network/bridge_driver_nop.c | 15 ++++++++++++++- src/util/virfirewall.c | 6 ++++++ src/util/virfirewall.h | 1 + 7 files changed, 65 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index 5c7cd7ec2e..2e8b87280d 100644 --- a/meson.build +++ b/meson.build @@ -1647,15 +1647,27 @@ if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD') conf.set('WITH_NETWORK', 1) firewall_backend_priority = get_option('firewall_backend_priority') - if (not firewall_backend_priority.contains('nftables') or - not firewall_backend_priority.contains('iptables') or - firewall_backend_priority.length() != 2) - error('invalid value for firewall_backend_priority option') + if firewall_backend_priority.length() == 0 + if host_machine.system() == 'linux' + firewall_backend_priority = ['nftables', 'iptables'] + else + # No firewall impl on non-Linux so far, so force 'none' + # as placeholder + firewall_backend_priority = ['none'] + endif + else + if host_machine.system() != 'linux' + error('firewall backend priority only supported on linux hosts') + endif endif - conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[0].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[1].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length()) + backends = [] + foreach backend: firewall_backend_priority + backend = 'VIR_FIREWALL_BACKEND_' + backend.to_upper() + backends += backend + endforeach + + conf.set('FIREWALL_BACKENDS', ', '.join(backends)) elif get_option('driver_network').enabled() error('libvirtd must be enabled to build the network driver') endif diff --git a/meson_options.txt b/meson_options.txt index 50d71427cb..2d440c63d8 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -117,7 +117,7 @@ option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for st option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support') # dep:firewalld option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') -option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], description: 'order in which to try firewall backends') +option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], value: [], description: 'order in which to try firewall backends') option('host_validate', type: 'feature', value: 'auto', description: 'build virt-host-validate') option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check', 'none'], value: 'check', description: 'Style of init script to install') option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of <loader>:<nvram> paths. Both pairs and list items are separated by a colon.') diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c index e2f3613a41..9da5e790b7 100644 --- a/src/network/bridge_driver_conf.c +++ b/src/network/bridge_driver_conf.c @@ -61,6 +61,7 @@ networkGetDnsmasqCaps(virNetworkDriverState *driver) static int virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, + bool privileged, const char *filename) { g_autoptr(virConf) conf = NULL; @@ -68,13 +69,17 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, bool fwBackendSelected = false; size_t i; int fwBackends[] = { - FIREWALL_BACKEND_PRIORITY_0, - FIREWALL_BACKEND_PRIORITY_1, + FIREWALL_BACKENDS }; - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST); - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == FIREWALL_BACKEND_PRIORITY_NUM); + G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) > 0 && + G_N_ELEMENTS(fwBackends) <= VIR_FIREWALL_BACKEND_LAST); int nFwBackends = G_N_ELEMENTS(fwBackends); + if (!privileged) { + fwBackends[0] = VIR_FIREWALL_BACKEND_NONE; + nFwBackends = 1; + } + if (access(filename, R_OK) == 0) { conf = virConfReadFile(filename, 0); @@ -104,6 +109,10 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, for (i = 0; i < nFwBackends && !fwBackendSelected; i++) { switch ((virFirewallBackend)fwBackends[i]) { + case VIR_FIREWALL_BACKEND_NONE: + fwBackendSelected = true; + break; + case VIR_FIREWALL_BACKEND_IPTABLES: { g_autofree char *iptablesInPath = virFindFileInPath(IPTABLES); @@ -187,7 +196,7 @@ virNetworkDriverConfigNew(bool privileged) configfile = g_strconcat(configdir, "/network.conf", NULL); - if (virNetworkLoadDriverConfig(cfg, configfile) < 0) + if (virNetworkLoadDriverConfig(cfg, privileged, configfile) < 0) return NULL; if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) { diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 35e6bd1154..fe7c6e193c 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -47,6 +47,11 @@ networkFirewallSetupPrivateChains(virFirewallBackend backend, virFirewallLayer layer) { switch (backend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesSetupPrivateChains(layer); @@ -417,6 +422,11 @@ networkAddFirewallRules(virNetworkDef *def, } switch (firewallBackend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesAddFirewallRules(def, fwRemoval); diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c index 537b9234f8..2114d521d1 100644 --- a/src/network/bridge_driver_nop.c +++ b/src/network/bridge_driver_nop.c @@ -19,6 +19,8 @@ #include <config.h> +#define VIR_FROM_THIS VIR_FROM_NETWORK + void networkPreReloadFirewallRules(virNetworkDriverState *driver G_GNUC_UNUSED, bool startup G_GNUC_UNUSED, bool force G_GNUC_UNUSED) @@ -37,9 +39,20 @@ int networkCheckRouteCollision(virNetworkDef *def G_GNUC_UNUSED) } int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED, - virFirewallBackend firewallBackend G_GNUC_UNUSED, + virFirewallBackend firewallBackend, virFirewall **fwRemoval G_GNUC_UNUSED) { + /* + * Shouldn't be possible, since virNetworkLoadDriverConfig + * ought to fail to find the required binaries when loading, + * so this is just a sanity check + */ + if (firewallBackend != VIR_FIREWALL_BACKEND_NONE) { + virReportError(VIR_ERR_NO_SUPPORT, + _("Firewall backend '%s' not available on this platform"), + virFirewallBackendTypeToString(firewallBackend)); + return -1; + } return 0; } diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 2219506b18..d374f54b64 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -37,6 +37,7 @@ VIR_LOG_INIT("util.firewall"); VIR_ENUM_IMPL(virFirewallBackend, VIR_FIREWALL_BACKEND_LAST, + "none", "iptables", "nftables"); @@ -815,6 +816,11 @@ virFirewallApplyCmd(virFirewall *firewall, } switch (virFirewallGetBackend(firewall)) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, + _("Firewall backend is not implemented")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: if (virFirewallCmdIptablesApply(firewall, fwCmd, &output) < 0) return -1; diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 302a6a4e5b..bce51259d2 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -44,6 +44,7 @@ typedef enum { } virFirewallLayer; typedef enum { + VIR_FIREWALL_BACKEND_NONE, /* Always fails */ VIR_FIREWALL_BACKEND_IPTABLES, VIR_FIREWALL_BACKEND_NFTABLES, -- 2.45.1

4 9

[PATCH 0/3] ci: update the CI dashboard
by Daniel P. Berrangé 17 Jun '24

17 Jun '24

Daniel P. Berrangé (3): docs: trim many projects from CI dashboard docs: fix link to virttools-web pipeline status docs: add libosinfo & virt-viewer to CI dashboard docs/ci-dashboard.rst | 78 ++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 46 deletions(-) -- 2.45.1

3 5

[PATCH] gitlab: add missing job inheritance for codestyle
by Daniel P. Berrangé 17 Jun '24

17 Jun '24

The previous fix: commit b069efe29c950d1a45e88ef7dc924d3ee223103a Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Fri Jun 14 19:57:06 2024 +0100 gitlab: fix codestyle CI job was incomplete, as the job inheritance was also broken. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- .gitlab-ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 635e731f82..6de867e1f2 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -106,6 +106,7 @@ pages: codestyle_job: stage: sanity_checks + extends: .gitlab_native_build_job needs: - job: x86_64-opensuse-leap-15-container optional: true -- 2.45.1

2 1

[PATCH 0/2] Make affinity setting a bit more debug friendly
by Michal Privoznik 17 Jun '24

17 Jun '24

*** BLURB HERE *** Michal Prívozník (2): qemu_process: Issue an info message when subtracting isolcpus virprocess: Debug affinity map in virProcessSetAffinity() src/qemu/qemu_process.c | 6 ++++++ src/util/virprocess.c | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) -- 2.43.2

3 4

[PATCH] gitlab: fix codestyle CI job
by Daniel P. Berrangé 15 Jun '24

15 Jun '24

Jobs whose names start with a '.' as treated as templates, so not actually run in a pipeline. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d9d8b1e3cd..635e731f82 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -104,7 +104,7 @@ pages: paths: - public -.codestyle_job: +codestyle_job: stage: sanity_checks needs: - job: x86_64-opensuse-leap-15-container -- 2.45.1

2 1

Re: [libvirt PATCH 00/28] native support for nftables in virtual network driver
by Roman Bogorodskiy 14 Jun '24

14 Jun '24

Laine Stump wrote: > On 6/10/24 2:54 PM, Roman Bogorodskiy wrote: > > Laine Stump wrote: > > > > > This patch series enables libvirt to use nftables rules rather than > > > iptables *when setting up virtual networks* (it does *not* add > > > nftables support to the nwfilter driver). It accomplishes this by > > > abstracting several iptables functions (from viriptables.[ch] called > > > by the virtual network driver into a rudimentary "virNetfilter API" > > > (in virnetfilter.[ch], having the virtual network driver call the > > > virNetFilter API rather than calling the existing iptables functions > > > directly, and then finally adding an equivalent virNftables backend > > > that can be used instead of iptables (selected manually via a > > > network.conf setting, or automatically if iptables isn't found on the > > > host). > > > > Hi, > > > > Apparently, I'm late to the discussion. > > > > I noticed that now I cannot use the bridge driver on FreeBSD as it's > > failing to initialize both iptables and nftables backends (which is > > expect). > > Yeah, previously we wouldn't check if iptables was available until someone > tried to start a network that would need to use it, and would then log an > error (and just fail starting that network, but the network driver would > remain running). But now we figure out which firewall backend to use > immediately when the driver is loaded, and if we fail to fin a workable > backend we fail the entire driver init.r > > How did you use the network driver before? With a <forward mode='open'/> > network? Truthfully I hadn't ever considered the case of someone using it > with only network types that didn't need firewall rules. I wonder if there > are other platforms we support that have a usable network driver for > <forward mode='open'/> (MacOS?) I'm using it with the following network configuration: virsh # net-dumpxml default <network> <name>default</name> <uuid>2a1415c9-325b-41e4-82c6-e805162d8934</uuid> <forward mode='nat'/> <bridge name='virbr0' stp='on' delay='0'/> <mac address='52:54:00:24:fa:43'/> <ip address='192.168.122.1' netmask='255.255.255.0'> <dhcp> <range start='192.168.122.2' end='192.168.122.254'/> </dhcp> </ip> </network> So basically all the mechanics like creating tap devices, bridges, serving dhcp, etc, all these work for me. On top of that I had a few iterations of manual firewall configurations (with both ipfw and pf) to implement NAT on guests. Unfortunately, I don't have access to that setup anymore and I haven't re-created it yet. IIRC, it could probably show some warnings about missing iptables, but it didn't affect anything for me. > > > > What would be a good way to address that? I see at least two options: > > > > 1. Add a Noop firewall driver > > 2. Implement a "real" FreeBSD driver based either on pf or ipfw (that's > > been on my TODO list forever, but I somehow got stuck on the very first > > step on choosing between pf and ipfw). > > Why not both? :-) > > > This obviously will take much > > more time. > > > > Maybe there are other options I'm missing. > > Obviously (2) would be nicest, but I guess in the short term some variation > of (1) would be quicker. > > Another possibility could be to restore the old behavior of saving the error > and only reporting it when a network requiring a firewall is loaded, but I > think I remember a discussion about this during review of an earlier > revision of the patches, and we agreed that it made the problem easier to > find if it was reported immediately and the driver load failed. > > I suppose in the long run the build-time option firewall_backend_priority > should be used to control which backends are included in the build (rather > than just which ones are checked at runtime), so that FreeBSD could > completely skip all the iptables and nftables code (and firewalld when > that's done), and Linux platforms could skip pf and ipfw. > > > > > What do you think? > > I'm about to be offline for 3 weeks, but in the meantime if you'd like to > try making a NULL backend that is only an option if it's listed in > firewall_backend_priority (you'll need to remove the compile-time check that > all possible backends are accounted for - I think that is the first of the > two G_STATIC_ASSERTS at the top of virNetworkLoadDriverConfig()), always > initializes successfully in bridge_driver_conf.c if it is listed in the > options, and then in networkAddFirewallRules add a check to log an error and > fail if backend == NULL (something about attempting to start a network type > that would require firewall rules, but the system not having any of the > supported types of firewallbackend or something - it's too late now and my > brain is too fried and sleepy to think of good wording :-)). As long as it > isn't a valid selection on Linux builds that are done with > firewall_backend_priority=nftables,iptables, but *is* a valid selection if > the setting is "firewall_backend_priority=null" that shouldn't be *too* > controversial. Ok, I think I can try making the NULL backend. > Later we can talk about pf and ipfw backends :-) Yeah, that sounds good. My main problem with the choice is that ipfw is the most actively supported firewall, but it relies quite heavily on the rule numbering, which makes it a little hard to integrate with user-specific rules (i.e. defined outside of libvirt). The "pf" seem to be better in this regard (at least to my taste), but it's not "native" FreeBSD firewall and is not as active (at least, to my impression). Roman

4 13

[PATCH 0/3] Couple of memleak fixes
by Michal Privoznik 14 Jun '24

14 Jun '24

The third patch MIGHT fix the following issue: https://issues.redhat.com/browse/RHEL-22574 but at this point it's still unclear. I'll append appropriate 'Resolves:' line when I learn more. Michal Prívozník (3): virfirewall: Fir a memleak in virFirewallParseXML() virnetworkobj: Free fwRemoval before setting another one in virNetworkObjSetFwRemoval() remote_daemon_dispatch: Unref sasl session when closing client connection src/conf/virnetworkobj.c | 1 + src/remote/remote_daemon_dispatch.c | 4 ++++ src/util/virfirewall.c | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) -- 2.44.2

2 6

[PATCH] network: introduce a "none" firewall backend type
by Daniel P. Berrangé 13 Jun '24

13 Jun '24

There are two scenarios identified after the recent firewall backend selection was introduced, which result in libvirtd failing to startup due to an inability to find either iptables/nftables - On Linux if running unprivileged with $PATH lacking the dir containing iptables/nftables - On non-Linux where iptables/nftables never existed In the former case, it is preferrable to restore the behaviour whereby the driver starts successfully. Users will get an error reported when attempting to start any virtual network, due to the lack of permissions needed to create bridge devices. This makes the missing firewall backend irrelevant. In the latter case, the network driver calls the 'nop' platform implementation which does not attempt to implement any firewall logic, just allowing the network to start without firewall rules. To solve this are number of changes are required * Introduce VIR_FIREWALL_BACKEND_NONE, which does nothing except report a fatal error from virFirewallApply(). This code path is unreachable, since we'll never create a virFirewall object with with VIR_FIREWALL_BACKEND_NONE, so the error reporting is just a sanity check. * Ignore the compile time backend defaults and assume use of the 'none' backend if running unprivileged. This fixes the first regression, avoiding the failure to start libvirtd on Linux in unprivileged context, instead allowing use of the driver and expecting a permission denied when creating a bridge. * Reject the use of compile time backend defaults no non-Linux and hardcode the 'none' backend. The non-Linux platforms have no firewall implementation at all currently, so there's no reason to permit the use of 'firewall_backend_priority' meson option. This fixes the second regression, avoiding the failure to start libvirtd on non-Linux hosts due to non-existant Linux binaries. * Change the Linux platform backend to raise an error if the firewall backend is 'none'. Again this code path is unreachable by default since we'll fail to create the bridge before getting here, but if someone modified network.conf to request the 'none' backend, this will stop further progress. * Change the nop platform backend to raise an error if the firewall backend is 'iptables' or 'nftables'. Again this code path is unreachable, since we should already have failed to find the iptables/nftables binaries on non-Linux hosts, so this is just a sanity check. * 'none' is not permited as a value in 'firewall_backend_priority' meson option, since it is conceptually meaningless to ask for that on Linux. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- meson.build | 26 +++++++++++++++++++------- meson_options.txt | 2 +- src/network/bridge_driver_conf.c | 19 ++++++++++++++----- src/network/bridge_driver_linux.c | 10 ++++++++++ src/network/bridge_driver_nop.c | 13 ++++++++++++- src/util/virfirewall.c | 6 ++++++ src/util/virfirewall.h | 1 + 7 files changed, 63 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index 5c7cd7ec2e..2e8b87280d 100644 --- a/meson.build +++ b/meson.build @@ -1647,15 +1647,27 @@ if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD') conf.set('WITH_NETWORK', 1) firewall_backend_priority = get_option('firewall_backend_priority') - if (not firewall_backend_priority.contains('nftables') or - not firewall_backend_priority.contains('iptables') or - firewall_backend_priority.length() != 2) - error('invalid value for firewall_backend_priority option') + if firewall_backend_priority.length() == 0 + if host_machine.system() == 'linux' + firewall_backend_priority = ['nftables', 'iptables'] + else + # No firewall impl on non-Linux so far, so force 'none' + # as placeholder + firewall_backend_priority = ['none'] + endif + else + if host_machine.system() != 'linux' + error('firewall backend priority only supported on linux hosts') + endif endif - conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[0].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[1].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length()) + backends = [] + foreach backend: firewall_backend_priority + backend = 'VIR_FIREWALL_BACKEND_' + backend.to_upper() + backends += backend + endforeach + + conf.set('FIREWALL_BACKENDS', ', '.join(backends)) elif get_option('driver_network').enabled() error('libvirtd must be enabled to build the network driver') endif diff --git a/meson_options.txt b/meson_options.txt index 50d71427cb..2d440c63d8 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -117,7 +117,7 @@ option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for st option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support') # dep:firewalld option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') -option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], description: 'order in which to try firewall backends') +option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], value: [], description: 'order in which to try firewall backends') option('host_validate', type: 'feature', value: 'auto', description: 'build virt-host-validate') option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check', 'none'], value: 'check', description: 'Style of init script to install') option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of <loader>:<nvram> paths. Both pairs and list items are separated by a colon.') diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c index e2f3613a41..9da5e790b7 100644 --- a/src/network/bridge_driver_conf.c +++ b/src/network/bridge_driver_conf.c @@ -61,6 +61,7 @@ networkGetDnsmasqCaps(virNetworkDriverState *driver) static int virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, + bool privileged, const char *filename) { g_autoptr(virConf) conf = NULL; @@ -68,13 +69,17 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, bool fwBackendSelected = false; size_t i; int fwBackends[] = { - FIREWALL_BACKEND_PRIORITY_0, - FIREWALL_BACKEND_PRIORITY_1, + FIREWALL_BACKENDS }; - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST); - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == FIREWALL_BACKEND_PRIORITY_NUM); + G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) > 0 && + G_N_ELEMENTS(fwBackends) <= VIR_FIREWALL_BACKEND_LAST); int nFwBackends = G_N_ELEMENTS(fwBackends); + if (!privileged) { + fwBackends[0] = VIR_FIREWALL_BACKEND_NONE; + nFwBackends = 1; + } + if (access(filename, R_OK) == 0) { conf = virConfReadFile(filename, 0); @@ -104,6 +109,10 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, for (i = 0; i < nFwBackends && !fwBackendSelected; i++) { switch ((virFirewallBackend)fwBackends[i]) { + case VIR_FIREWALL_BACKEND_NONE: + fwBackendSelected = true; + break; + case VIR_FIREWALL_BACKEND_IPTABLES: { g_autofree char *iptablesInPath = virFindFileInPath(IPTABLES); @@ -187,7 +196,7 @@ virNetworkDriverConfigNew(bool privileged) configfile = g_strconcat(configdir, "/network.conf", NULL); - if (virNetworkLoadDriverConfig(cfg, configfile) < 0) + if (virNetworkLoadDriverConfig(cfg, privileged, configfile) < 0) return NULL; if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) { diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 35e6bd1154..fe7c6e193c 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -47,6 +47,11 @@ networkFirewallSetupPrivateChains(virFirewallBackend backend, virFirewallLayer layer) { switch (backend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesSetupPrivateChains(layer); @@ -417,6 +422,11 @@ networkAddFirewallRules(virNetworkDef *def, } switch (firewallBackend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesAddFirewallRules(def, fwRemoval); diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c index 537b9234f8..7797be1ba8 100644 --- a/src/network/bridge_driver_nop.c +++ b/src/network/bridge_driver_nop.c @@ -37,9 +37,20 @@ int networkCheckRouteCollision(virNetworkDef *def G_GNUC_UNUSED) } int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED, - virFirewallBackend firewallBackend G_GNUC_UNUSED, + virFirewallBackend firewallBackend, virFirewall **fwRemoval G_GNUC_UNUSED) { + /* + * Shouldn't be possible, since virNetworkLoadDriverConfig + * ought to fail to find the required binaries when loading, + * so this is just a sanity check + */ + if (firewallBackend != VIR_FIREWALL_NONE) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Firewall backend '%s' not available on this platform"), + virFirewallBackendTypeToSTring(firewallBackend)); + return -1; + } return 0; } diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 2219506b18..d374f54b64 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -37,6 +37,7 @@ VIR_LOG_INIT("util.firewall"); VIR_ENUM_IMPL(virFirewallBackend, VIR_FIREWALL_BACKEND_LAST, + "none", "iptables", "nftables"); @@ -815,6 +816,11 @@ virFirewallApplyCmd(virFirewall *firewall, } switch (virFirewallGetBackend(firewall)) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, + _("Firewall backend is not implemented")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: if (virFirewallCmdIptablesApply(firewall, fwCmd, &output) < 0) return -1; diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 302a6a4e5b..bce51259d2 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -44,6 +44,7 @@ typedef enum { } virFirewallLayer; typedef enum { + VIR_FIREWALL_BACKEND_NONE, /* Always fails */ VIR_FIREWALL_BACKEND_IPTABLES, VIR_FIREWALL_BACKEND_NFTABLES, -- 2.45.1

2 1

[PATCH 0/2] network: fix network driver to gracefully skip startup
by Daniel P. Berrangé 13 Jun '24

13 Jun '24

We should gracefully skip startup when: * No network.conf firewall_backend is explicitly set, and neither iptables/nftables are present * Running unprivileged The former fixes libvirtd startup on non-Linux, or minimal linux installs without firewall tools. The latter skips pointless initialization that creates a driver that cannot do anything useful Daniel P. Berrangé (2): network: skip network driver init if no firewall backend is present network: don't attempt to initialize if non-privileged src/network/bridge_driver.c | 14 +++++++++++++- src/network/bridge_driver_conf.c | 8 ++++---- 2 files changed, 17 insertions(+), 5 deletions(-) -- 2.45.1

3 6

[PATCH] tools: fix paths in PKI validation error messages
by Daniel P. Berrangé 13 Jun '24

13 Jun '24

A couple of paths passed in the error messages, didnt match the paths that were actually being tested. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- tools/virt-pki-validate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virt-pki-validate.c b/tools/virt-pki-validate.c index 656f29fdc5..e693ffaed6 100644 --- a/tools/virt-pki-validate.c +++ b/tools/virt-pki-validate.c @@ -184,7 +184,7 @@ virPKIValidateIdentity(bool isServer, bool system, const char *path) _("Checking system cert dir access"), 0, 0, 0755, _("The system cert dir %1$s must be accessible to all users. As root, run: chown root.root; chmod 0755 %2$s"), - LIBVIRT_PKI_DIR, LIBVIRT_PKI_DIR); + LIBVIRT_CERT_DIR, LIBVIRT_CERT_DIR); FILE_REQUIRE_EXISTS(scope, LIBVIRT_KEY_DIR, @@ -197,7 +197,7 @@ virPKIValidateIdentity(bool isServer, bool system, const char *path) _("Checking system key dir access"), 0, 0, 0755, _("The system key dir %1$s must be accessible to all users. As root, run: chown root.root; chmod 0755 %2$s"), - LIBVIRT_KEY_DIR, LIBVIRT_PKI_DIR); + LIBVIRT_KEY_DIR, LIBVIRT_KEY_DIR); } else if (path) { virNetTLSConfigCustomTrust(path, &cacert, -- 2.45.1

2 1

[libvirt PATCH 00/28] native support for nftables in virtual network driver
by Laine Stump 12 Jun '24

12 Jun '24

This patch series enables libvirt to use nftables rules rather than iptables *when setting up virtual networks* (it does *not* add nftables support to the nwfilter driver). It accomplishes this by abstracting several iptables functions (from viriptables.[ch] called by the virtual network driver into a rudimentary "virNetfilter API" (in virnetfilter.[ch], having the virtual network driver call the virNetFilter API rather than calling the existing iptables functions directly, and then finally adding an equivalent virNftables backend that can be used instead of iptables (selected manually via a network.conf setting, or automatically if iptables isn't found on the host). A first look at the result may have you thinking that it's filled with a lot of bad decisions. While I would agree with that in many cases, I think that overall they are the "least bad" decisions, or at least "bad within acceptable limits / no worse than something else", and point out that it's been done in a way that minimizes (actually eliminates) the need for immediate changes to nwfilter (the other consumer of iptables, which *also* needs to be updated to use native nftables), and makes it much easier to change our mind about the details in the future. When I first started on this (long, protracted, repeatedly interrupted for extended periods - many of these patches are > a year old) task, I considered doing an all-at-once complete replacement of iptables with nftables, since all the Linux distros we support have had nftables for several years, and I'm pretty sure nobody has it disabled (not even sure if it's possible to disable nftables while still enabling iptables, since they both use xtables in the kernel). But due to libvirt's use of "-t mangle -j CHECKSUM --checksum-fill" (see commit fd5b15ff all the way back in July 2010 for details) which has no equivalent in nftables rules (and we don't *want* it to!!), and the desire to be able to easily switch back to iptables in case of an unforeseen regression, we decided that both iptables and nftables need to be supported (for now), with the default (for now) remaining as iptables. Just allowing for dual backends complicated matters, since it means that we have to have a config file, a setting, detection of which backends are available, and of course some sort of concept of an abstracted frontend that can use either backend based on the config setting (and/or auto-detection). Combining that with the fact that it would just be "too big" of a project to switch over nwfilter's iptables usage at the same time means that we have to keep around a lot of existing code for compatibility's sake rather than just wiping it all away and starting over. So, what I've ended up with is: 1) a network.conf file (didn't exist before) with a single setting "firewall_backend". If unset, the network driver tries to use iptables on the backend, and if that's missing, then tries to use nftables. 2) a new (internal-only, so transient!) virNetFilterXXX API that is used by the network driver in place of the iptablesXXX API, and calls either iptablesXXX or: 3) a virNftablesXXX API that exactly replicates the filtering rules of the existing iptablesXXX API (except in the custom "libvirt" base table rather than the system "filter" and "nat" tables). This means that: 4) when the nftables backend is used, the rules added are *exactly the same* (functionally speaking) as we currently add for iptables (except they are in the "libvirt" table). We had spent some time in IRC discussing different ways of using new functionality available in nftables to make a more efficient/performant implemention of the desired filtering, and there are some really great possibilities that need to be explored, but in the end there were too many details up in the air, and I decided that it would be more "accomplishable" (coined a new word there!) to first replicate existing behavior with nftables, but do it inside a framework that makes it easy to modify the details in the future (in particular making it painless to switch back and forth between builds with differing filter models at runtime) - this way we'll be able to separate the infrastructure work from the details of the rules (which we can then more easily work on and experiment with). (This implies that the main objective right now is "get rid of iptables dependencies", not "make the filtering faster and more efficient"). Notable features of this patchset: * allows switching between iptables/nftables backends without rebooting or restarting networks/guests. Because the commands required to remove a network's filter rules are now saved in the network status XML, each time libvirtd (or virtnetworkd) is restarted, it will execute exactly the commands needed to remove the filter rules that had been added by the previous libvirtd/virtnetworkd (rather than just making a guess, as we've always done up until now), and then add new rules using the current backend+binary's set of rules (while also saving the info needed for future removal of these new rules back into the network's status XML). * firewall_backend can be explicitly set in (new) /etc/libvirt/network.conf, but if it's not explicitly set, libvirt will default to the iptables backend if the iptables binary is found, and otherwise fall back to nftables as long as the nft binary is found; otherwise the first attempt to start a network will fail with an appropriate error. Things that seem ugly / that I would like to clean up / that I think are just fine as they are: * virFirewall does *not* provide a backend-agnostic interface [this is fine] * We need to maintain a backward-compatible API for virFirewall so that we don't have to touch nwfilter code. Trying to make its API backend-agnostic would require individually considering/changing every nwfilter use of virFirewall. * instead virFirewall objects are just a way to build a collection of commands to execute to build a firewall, then execute them while collecting info for and building a collection of commands that will tear down that firewall in the future. Do I want to "fix" this in the future by making virFirewall a higher level interface that accepts tokens describing the type of rule to add (rather than backend-specific arguments to a backend-specific command)? No. I think I like the way virFirewall works (as described in that previous bullet-point), instead I'm thinking that it is just slightly mis-named - I've lately been thinking of it as a "virNetFilterCmdList". Similarly, the virFirewallRules that it has a list of aren't really "rules", they are better described as commands or actions, so maybe they should be renamed to virNetfilterCmd or virNetfilterAction. But that is just cosmetic, so I didn't want to get into it in these patches (especially in case someone disagrees, or has a better idea for naming). * Speaking of renaming - I should probably rename all the "iptablesXXX" functions to "virIptablesXXX" to be consistent with so much of our other code. I lost the ambition to deal with it right now though, so I'm leaving that for later cleanup (or I could do it now if it really makes someone's day :-). * I could have chosen a higher place in the callchain to make the virNetfilter abstraction, e.g. at the level of "networkAddXXXFirewallRules()" rather than at the lower level of iptablesXXX(). That is actually probably what will happen in the future (since it will be necessary in order for an nftables-based firewall to be significantly different in structure from an iptables-based firewall). But that's the beauty of an API being private - we can freely add/remove things as needed. the important thing is that we now have the basic structure there. For now, the split is just above the existing iptablesXXX API (util/viriptables.[ch], which seems like a "narrow" enough place. Most iptablesXXX functions are written in terms of just 10 *other* iptablesXXX functions that add iptables-specific commands - I've just moved those functions into virnetfilter.[ch] (appropriately renamed), and changed them to call the 10 virNetfilterXXX functions that will in-turn call those 10 iptablesXXX (or equivalent virNftablesXXX) functions. * Some people may dislike that the 10 virNetfilterXXX functions are each written with a switch statement that has cases to directly call each backend, rather than each backend driver having a table of pointers to API functions, with the virNetfilter API function calling backends[fwBackend]->XXX() (ie the pattern for so many drivers in libvirt). But for just 2 backends, that really seemed like overkill and unnecessary obfuscation. * As implemented here, I am storing a "<fwRemoval>" element in the network status XML - it contains a serialized virFirewall object that directly contains the commands necessary to remove the firewall. I could instead just store "<firewall>", which would include all the commands that were used to *create* the firewall in addition to the commands needed to remove the firewall. The way it's done currently takes up less space; switching to storing the full firewall *might* be more informative to somebody, but on the other hand would make the network status XML *very* long. If anybody has an opinion about this, now is the time to bring it up - do you think it's worth having a separate list of all the commands that were used to create a network's firewall (keeping in mind that there is no public API to access it)? Or is it enough to just store what's needed to remove the firewall? * Several months ago Eric Garver posted patches for a pure firewalld backend, and I requested that they not be pushed because I wanted that to be integrated with my nftables backend support. Due to the fact that the firewalld backend is almost entirely implemented by putting the bridge into a new firewalld "zone", with no individual rules added, that won't happen as just another backend driver file in parallel to iptables and nftables; it will instead work by checking firewall_backend at a higher level in the network driver, thus avoiding the calls to virNetfilterXXX() entirely. I have locally merged Eric's patches over the top of these patches, and there are surprisingly few conflicts, but since his patches didn't account for a user-settable config (but instead just always used the firewalld backend if firewalld was active), some of the patches are going to require a bit of rework, which I'll take care of after getting these patches in. Laine Stump (28): util: add -w/--concurrent when applying the rule rather than when building it util: new virFirewallRuleGet*() APIs util: determine ignoreErrors value when creating rule, not when applying util: rename iptables helpers that will become the frontend for ip&nftables util: move backend-agnostic virNetfilter*() functions to their own file util: make netfilter action a proper typedefed (virFirewall) enum util: #define the names used for private packet filter chains util: move/rename virFirewallApplyRuleDirect to virIptablesApplyFirewallRule util/network: reintroduce virFirewallBackend, but different network: add (empty) network.conf file to distribution files network: allow setting firewallBackend from network.conf network: do not add DHCP checksum mangle rule unless using iptables network: call backend agnostic function to init private filter chains util: setup functions in virnetfilter which will call appropriate backend build: add nft to the list of binaries we attempt to locate util: add nftables backend to virnetfilter API used by network driver tests: test cases for nftables backend util: new functions to support adding individual rollback rules util: check for 0 args when applying iptables rule util: implement rollback rule autosave for iptables backend util: implement rollback rule autosave for nftables backend network: turn on auto-rollback for the rules added for virtual networks util: new function virFirewallNewFromRollback() util: new functions virFirewallParseXML() and virFirewallFormat() conf: add a virFirewall object to virNetworkObj network: use previously saved list of firewall rules when removing network: save network status when firewall rules are reloaded network: improve log message when reloading virtual network firewall rules libvirt.spec.in | 5 + meson.build | 1 + po/POTFILES | 2 + src/conf/virnetworkobj.c | 40 + src/conf/virnetworkobj.h | 11 + src/libvirt_private.syms | 68 +- src/network/bridge_driver.c | 40 +- src/network/bridge_driver_conf.c | 44 + src/network/bridge_driver_conf.h | 3 + src/network/bridge_driver_linux.c | 241 +++-- src/network/bridge_driver_nop.c | 6 +- src/network/bridge_driver_platform.h | 6 +- src/network/libvirtd_network.aug | 39 + src/network/meson.build | 11 + src/network/network.conf | 24 + src/network/test_libvirtd_network.aug.in | 5 + src/nwfilter/nwfilter_ebiptables_driver.c | 16 +- src/util/meson.build | 2 + src/util/virebtables.c | 4 +- src/util/virfirewall.c | 490 ++++++++-- src/util/virfirewall.h | 51 +- src/util/viriptables.c | 762 ++++----------- src/util/viriptables.h | 222 ++--- src/util/virnetfilter.c | 892 ++++++++++++++++++ src/util/virnetfilter.h | 159 ++++ src/util/virnftables.c | 698 ++++++++++++++ src/util/virnftables.h | 118 +++ .../{base.args => base.iptables} | 0 tests/networkxml2firewalldata/base.nftables | 256 +++++ ...-linux.args => nat-default-linux.iptables} | 0 .../nat-default-linux.nftables | 248 +++++ ...pv6-linux.args => nat-ipv6-linux.iptables} | 0 .../nat-ipv6-linux.nftables | 384 ++++++++ ...rgs => nat-ipv6-masquerade-linux.iptables} | 0 .../nat-ipv6-masquerade-linux.nftables | 456 +++++++++ ...linux.args => nat-many-ips-linux.iptables} | 0 .../nat-many-ips-linux.nftables | 472 +++++++++ ...-linux.args => nat-no-dhcp-linux.iptables} | 0 .../nat-no-dhcp-linux.nftables | 384 ++++++++ ...ftp-linux.args => nat-tftp-linux.iptables} | 0 .../nat-tftp-linux.nftables | 274 ++++++ ...inux.args => route-default-linux.iptables} | 0 .../route-default-linux.nftables | 162 ++++ tests/networkxml2firewalltest.c | 56 +- tests/virfirewalltest.c | 20 +- 45 files changed, 5718 insertions(+), 954 deletions(-) create mode 100644 src/network/libvirtd_network.aug create mode 100644 src/network/network.conf create mode 100644 src/network/test_libvirtd_network.aug.in create mode 100644 src/util/virnetfilter.c create mode 100644 src/util/virnetfilter.h create mode 100644 src/util/virnftables.c create mode 100644 src/util/virnftables.h rename tests/networkxml2firewalldata/{base.args => base.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/base.nftables rename tests/networkxml2firewalldata/{nat-default-linux.args => nat-default-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-default-linux.nftables rename tests/networkxml2firewalldata/{nat-ipv6-linux.args => nat-ipv6-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-ipv6-linux.nftables rename tests/networkxml2firewalldata/{nat-ipv6-masquerade-linux.args => nat-ipv6-masquerade-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables rename tests/networkxml2firewalldata/{nat-many-ips-linux.args => nat-many-ips-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-many-ips-linux.nftables rename tests/networkxml2firewalldata/{nat-no-dhcp-linux.args => nat-no-dhcp-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables rename tests/networkxml2firewalldata/{nat-tftp-linux.args => nat-tftp-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-tftp-linux.nftables rename tests/networkxml2firewalldata/{route-default-linux.args => route-default-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/route-default-linux.nftables -- 2.39.2

7 78

New libvirt-rust-xml project
by Daniel P. Berrangé 12 Jun '24

12 Jun '24

There has been interest in creating an equivalent to the libvirt-go-xml-module project, but for Rust. Thus I've created the following new project: https://gitlab.com/libvirt/libvirt-rust-xml Don't expect to see any actual code there yet. It is just the absolute bare minimum Rust boilerplate setup and CI integration to allow us to kick off the concept. Anyone interested in this should open issues/merge requests there in the usual way. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

1 0

[PATCH RFC v2 00/12] Support throttle block filters
by wucf＠linux.ibm.com 12 Jun '24

12 Jun '24

From: Chun Feng Wu <wucf(a)linux.ibm.com> Hi, I am thinking to leverage "throttle block filter" in QEMU to support more flexible I/O limits(e.g. tiered I/O groups), one sample provided by QEMU doc is: https://github.com/qemu/qemu/blob/master/docs/throttle.txt "For example, let's say that we have three different drives and we want to set I/O limits for each one of them and an additional set of limits for the combined I/O of all three drives." The implementation idea is to - Define throttle groups(limit) in domain - Define throttle filter to reference throttle group within disk - Within domain disk, throttle filters references multiple throttle groups to form filter chain to apply multiple limits in QEMU like above sample - Add new virsh cmds for throttle group management: throttlegroupset Add or update a throttling group. throttlegroupdel Delete a throttling group. throttlegroupinfo Get a throttling group. throttlegrouplist list all domain throttlegroups - Update "attach-disk" to add one more option "--throttle-groups" to apply throttle filters e.g. "virsh attach-disk $VM_ID ${DISK_PATH}/vm1_disk_2.qcow2 vdd --driver qemu --subdriver qcow2 --targetbus virtio --throttle-groups limit2,limit012" - I chose above semantics as I felt they're appropriate, if there are better ones please kindly suggest. This patchset includes: - Throttle group and throttle filter definition in patch 1 - New QMP processing to update and get throttle group in patch 2 - New API definition and implementation in patch 3 - QEMU driver implementation in patch 4 - Hotplug and qemuProcessLaunch flow implemenation in patch 5,6 - Domain XML schema and doc(formatdomain.rst) change in patch 7 - Tests in patch 8,9 - Virsh cmd implementation in patch 10 - Other verification implementation in patch 11, 12 v2 changes: - fix failure caused by "check-remote_protocol" between patch 3 and patch 9 in v1 - make sure coding style is consistent about function declaration - patch 3 in v1 is splitted into two commits: "remote: New APIs for ThrottleGroup lifecycle management" and "qemu: Implement qemu driver for throttle API" - replace "// comment" with "/* comment */" - avoid empty 'cleanup' labels - use "virJSONValueObjectAdd(&limits, "P:name", value)" to avoid introducing extra helper, and check return value - preserve spacing between blocks of code which are not related - remove "VSH_OT_ALIAS" section for new cmds in virsh_domain.c From QMP perspective, the sample flow works this way: - Throttle group creation: virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit0","limits":{"iops-total":200,"iops-read":0,"iops-total-max":200,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit1","limits":{"iops-total":250,"iops-read":0,"iops-total-max":250,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit2","limits":{"iops-total":300,"iops-read":0,"iops-total-max":300,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit012","limits":{"iops-total":400,"iops-read":0,"iops-total-max":400,"iops-total-max-length":1}}}' - Chain up filters during attaching disk to apply two filters(limit0 and limit012): virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments": {"driver":"file","filename":"/virt/disks/vm1_disk_1.qcow2","node-name":"test-3-storage","auto-read-only":true,"discard":"unmap"}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments":{"node-name":"test-3-format","read-only":false,"driver":"qcow2","file":"test-3-storage","backing":null}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments":{"driver":"throttle","node-name":"libvirt-5-filter","throttle-group": "limit0","file":"test-3-format"}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments": {"driver":"throttle","node-name":"libvirt-6-filter","throttle-group":"limit012","file":"libvirt-5-filter"}}' virsh qemu-monitor-command 1 '{"execute": "device_add", "arguments": {"driver":"virtio-blk-pci","scsi":false,"bus":"pci.0","addr":"0x5","drive":"libvirt-6-filter","id":"virtio-disk1"}}' Any comments/suggestions will be appriciated! Chun Feng Wu (10): config: Introduce ThrottleGroup and ThrottleFilter qemu: monitor: Add support for ThrottleGroup operations remote: New APIs for ThrottleGroup lifecycle management qemu: Implement qemu driver for throttle API qemu: hotplug: Support hot attach block disk along with throttle filters qemu: command: Support throttle groups and filters during qemuProcessLaunch schema: Add new domain elements to support multiple throttle filters test: Test throttle group lifecycle APIs tests: Test qemuMonitorJSONGetThrottleGroup and qemuMonitorJSONUpdateThrottleGroup virsh: Add support for throttle group operations Hao Ning Xin (1): config: validate: Verify throttle group fields Yan Xiu Wu (1): config: validate: Use "iotune" and "throttlefilters" exclusively for specific disk docs/formatdomain.rst | 48 ++ include/libvirt/libvirt-domain.h | 29 + src/conf/domain_conf.c | 386 +++++++++++++ src/conf/domain_conf.h | 54 ++ src/conf/domain_validate.c | 120 ++-- src/conf/schemas/domaincommon.rng | 164 +++++- src/conf/virconftypes.h | 4 + src/driver-hypervisor.h | 22 + src/libvirt-domain.c | 190 +++++++ src/libvirt_private.syms | 9 + src/libvirt_public.syms | 7 + src/qemu/qemu_block.c | 131 +++++ src/qemu/qemu_block.h | 53 ++ src/qemu/qemu_command.c | 158 ++++++ src/qemu/qemu_command.h | 9 + src/qemu/qemu_domain.c | 85 +++ src/qemu/qemu_domain.h | 15 + src/qemu/qemu_driver.c | 529 ++++++++++++++++++ src/qemu/qemu_hotplug.c | 24 + src/qemu/qemu_monitor.c | 34 ++ src/qemu/qemu_monitor.h | 14 + src/qemu/qemu_monitor_json.c | 151 +++++ src/qemu/qemu_monitor_json.h | 14 + src/remote/remote_daemon_dispatch.c | 61 ++ src/remote/remote_driver.c | 49 ++ src/remote/remote_protocol.x | 50 +- src/remote_protocol-structs | 30 + src/test/test_driver.c | 382 +++++++++++++ tests/qemumonitorjsontest.c | 88 +++ .../qemustatusxml2xmldata/backup-pull-in.xml | 1 + .../blockjob-blockdev-in.xml | 1 + .../blockjob-mirror-in.xml | 1 + .../migration-in-params-in.xml | 1 + .../migration-out-nbd-bitmaps-in.xml | 1 + .../migration-out-nbd-out.xml | 1 + .../migration-out-nbd-tls-out.xml | 1 + .../migration-out-params-in.xml | 1 + tests/qemustatusxml2xmldata/modern-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-out.xml | 1 + .../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 + tools/virsh-completer-domain.c | 64 +++ tools/virsh-completer-domain.h | 10 + tools/virsh-domain.c | 471 ++++++++++++++++ 43 files changed, 3430 insertions(+), 36 deletions(-) -- 2.34.1

3 39