October 2022 - Devel - libvirt List Archives

need directions for filling a bug report
by alexandru.iancu＠gmail.com 25 Oct '22

25 Oct '22

Hi everybody, I'm running a KVM linux hypervisor and a Windows guest. I'm passing through the GPU. From time to time, the guest crashes. I'd like to file a bug report. a) I'm aware of the many gitlab projects but I'm not sure which one can address this issue or should have a first look at and dispatch it. b) Windows dump is big(2G). I'm probably going to need to put it somewhere for inspection. Best regards, Alex

2 1

[PATCH v3 0/5] network: firewalld: fix routed network
by Eric Garver 24 Oct '22

24 Oct '22

This series fixes routed networks when a newer firewalld (>= 1.0.0) is present [1]. Firewalld 1.0.0 included a change that disallows implicit forwarding between zones [2]. libvirt was relying on this behavior to allow routed networks to function. Firewalld policies are added. Policies have been supported since firewalld 0.9.0. If the running firewall does not support policies, then it will fallback to the current zone only behavior. My goal is to get libvirt to a fully native firewalld backend; no iptables rules. This series is phase 1 of that effort. The next steps are: 1. introduce a "libvirt-nat" zone and policies - the current "libvirt" zone will become obsolete 2. go full native firewalld, do not use iptables directly - currently a hybrid of iptables + firewalld is used v3: - rebase, retest, resend v2: - keep existing libvirt zone as is - remove "<forward />" in libvirt-routed zone because this feature requires firewalld >= 0.9.0. Has no impact since the added policies allow forwarding libvirt-routed <--> ANY zone (including itself). - add probe for policies: virFirewallDGetPolicies(), virFirewallDPolicyExists() [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2055706 [2]: https://github.com/firewalld/firewalld/issues/177 Eric Garver (5): util: add virFirewallDGetPolicies() util: add virFirewallDPolicyExists() network: firewalld: add zone for routed networks network: firewalld: add policies for routed networks network: firewalld: add support for routed networks src/libvirt_private.syms | 2 + src/network/bridge_driver_linux.c | 11 +++- src/network/libvirt-routed-in.policy | 11 ++++ src/network/libvirt-routed-out.policy | 12 +++++ src/network/libvirt-routed.zone | 10 ++++ src/network/libvirt-to-host.policy | 20 ++++++++ src/network/meson.build | 20 ++++++++ src/util/virfirewalld.c | 72 +++++++++++++++++++++++++++ src/util/virfirewalld.h | 2 + 9 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/network/libvirt-routed-in.policy create mode 100644 src/network/libvirt-routed-out.policy create mode 100644 src/network/libvirt-routed.zone create mode 100644 src/network/libvirt-to-host.policy -- 2.35.3

3 7

[libvirt PATCH 00/28] Synchronize x86 cpu features from qemu
by Tim Wiederhake 24 Oct '22

24 Oct '22

libvirt is missing support for some x86 cpu features supported by qemu. This series adds the missing cpu features to libvirt's feature map and adds a script to ease the detection of new features in the future. Support for amx-int8, amx-tile, and amx-bf16 is deliberately not included, as they were already proposed on the list[1]. [1] https://listman.redhat.com/archives/libvir-list/2022-September/234292.html Tim Wiederhake (28): cpu-data.py: Allow for more than child in feature nodes cpu_x86: Ignore alias names cpu: make x86 feature alias names machine readable cpu_map: Add script to sync from QEMU i386 cpu features cpu_map: Rename sync_qemu_i386.py cpu_map: Add missing x86 feature alias names cpu_map: Add missing x86 feature "sgx" cpu_map: Add missing x86 feature "sgxlc" cpu_map: Add missing x86 feature "sgx-exinfo" cpu_map: Add missing x86 feature "sgx1" cpu_map: Add missing x86 feature "sgx2" cpu_map: Add missing x86 feature "sgx-debug" cpu_map: Add missing x86 feature "sgx-mode64" cpu_map: Add missing x86 feature "sgx-provisionkey" cpu_map: Add missing x86 feature "sgx-tokenkey" cpu_map: Add missing x86 feature "sgx-kss" cpu_map: Add missing x86 feature "bus-lock-detect" cpu_map: Add missing x86 feature "pks" cpu_map: Add missing x86 feature "avx512-vp2intersect" cpu_map: Add missing x86 feature "avx512-fp16" cpu_map: Add missing x86 feature "serialize" cpu_map: Add missing x86 feature "tsx-ldtrk" cpu_map: Add missing x86 feature "arch-lbr" cpu_map: Add missing x86 feature "xfd" cpu_map: Add missing x86 feature "intel-pt-lip" cpu_map: Add missing x86 feature "avic" cpu_map: Add missing x86 feature "v-vmsave-vmload" cpu_map: Add missing x86 feature "vgif" src/cpu/cpu_x86.c | 10 +- src/cpu_map/sync_qemu_features_i386.py | 278 ++++++++++++++++++ ..._qemu_i386.py => sync_qemu_models_i386.py} | 0 src/cpu_map/x86_features.xml | 133 +++++++-- tests/cputestdata/cpu-data.py | 11 +- .../x86_64-cpuid-Atom-P5362-disabled.xml | 1 + .../x86_64-cpuid-Atom-P5362-guest.xml | 1 + .../x86_64-cpuid-Atom-P5362-host.xml | 1 + .../x86_64-cpuid-Core-i7-7600U-disabled.xml | 2 +- .../x86_64-cpuid-Core-i7-7600U-guest.xml | 1 + .../x86_64-cpuid-Core-i7-7600U-host.xml | 1 + .../x86_64-cpuid-Core-i7-7700-disabled.xml | 2 +- .../x86_64-cpuid-Core-i7-7700-guest.xml | 1 + .../x86_64-cpuid-Core-i7-7700-host.xml | 1 + .../x86_64-cpuid-Core-i7-8550U-disabled.xml | 2 +- .../x86_64-cpuid-Core-i7-8550U-guest.xml | 1 + .../x86_64-cpuid-Core-i7-8550U-host.xml | 1 + .../x86_64-cpuid-Core-i7-8700-disabled.xml | 2 +- .../x86_64-cpuid-Core-i7-8700-guest.xml | 2 + .../x86_64-cpuid-Core-i7-8700-host.xml | 2 + ...86_64-cpuid-EPYC-7502-32-Core-disabled.xml | 2 +- .../x86_64-cpuid-EPYC-7502-32-Core-guest.xml | 3 + .../x86_64-cpuid-EPYC-7502-32-Core-host.xml | 3 + ...86_64-cpuid-EPYC-7601-32-Core-disabled.xml | 2 +- .../x86_64-cpuid-EPYC-7601-32-Core-guest.xml | 3 + .../x86_64-cpuid-EPYC-7601-32-Core-host.xml | 3 + ...-cpuid-EPYC-7601-32-Core-ibpb-disabled.xml | 2 +- ..._64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml | 3 + ...6_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml | 3 + ...-cpuid-Hygon-C86-7185-32-core-disabled.xml | 2 +- ..._64-cpuid-Hygon-C86-7185-32-core-guest.xml | 3 + ...6_64-cpuid-Hygon-C86-7185-32-core-host.xml | 3 + .../x86_64-cpuid-Ice-Lake-Server-disabled.xml | 2 +- .../x86_64-cpuid-Ice-Lake-Server-guest.xml | 2 + .../x86_64-cpuid-Ice-Lake-Server-host.xml | 2 + ...puid-Ryzen-7-1800X-Eight-Core-disabled.xml | 2 +- ...4-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml | 3 + ...64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml | 3 + ...4-cpuid-Ryzen-9-3900X-12-Core-disabled.xml | 2 +- ...6_64-cpuid-Ryzen-9-3900X-12-Core-guest.xml | 3 + ...86_64-cpuid-Ryzen-9-3900X-12-Core-host.xml | 3 + .../x86_64-cpuid-Xeon-E3-1225-v5-disabled.xml | 2 +- .../x86_64-cpuid-Xeon-E3-1225-v5-guest.xml | 1 + .../x86_64-cpuid-Xeon-E3-1225-v5-host.xml | 1 + .../x86_64-cpuid-Xeon-E3-1245-v5-disabled.xml | 2 +- .../x86_64-cpuid-Xeon-E3-1245-v5-guest.xml | 1 + .../x86_64-cpuid-Xeon-E3-1245-v5-host.xml | 1 + .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml | 1 + .../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml | 1 + .../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 2 + .../domaincapsdata/qemu_7.0.0-tcg.x86_64.xml | 2 + .../domaincapsdata/qemu_7.1.0-tcg.x86_64.xml | 2 + 52 files changed, 487 insertions(+), 36 deletions(-) create mode 100755 src/cpu_map/sync_qemu_features_i386.py rename src/cpu_map/{sync_qemu_i386.py => sync_qemu_models_i386.py} (100%) -- 2.36.1

2 56

[PATCH] qemuAppendDomainMemoryMachineParams: Refactor formatting of 'dump-guest-core'
by Peter Krempa 24 Oct '22

24 Oct '22

Use virTristateSwitchFromBool to fill in the default if user didn't request it explicitly. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_command.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 150824f2e1..bb2a3ea82f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6625,15 +6625,13 @@ qemuAppendDomainMemoryMachineParams(virBuffer *buf, const virDomainDef *def, virQEMUCaps *qemuCaps) { + virTristateSwitch dump = def->mem.dump_core; size_t i; - if (def->mem.dump_core) { - virBufferAsprintf(buf, ",dump-guest-core=%s", - virTristateSwitchTypeToString(def->mem.dump_core)); - } else { - virBufferAsprintf(buf, ",dump-guest-core=%s", - cfg->dumpGuestCore ? "on" : "off"); - } + if (dump == VIR_TRISTATE_SWITCH_ABSENT) + dump = virTristateSwitchFromBool(cfg->dumpGuestCore); + + virBufferAsprintf(buf, ",dump-guest-core=%s", virTristateSwitchTypeToString(dump)); if (def->mem.nosharepages) virBufferAddLit(buf, ",mem-merge=off"); -- 2.37.3

2 1

[PATCH] docs: Update best practices wrt "Fixes:" and GitLab
by Michal Privoznik 24 Oct '22

24 Oct '22

We document that a commit fixing an issue tracked in GitLab should put just "Fixes: #NNN" into its commit message. But when viewing git log, having full URL which is directly clickable is more developer friendly and GitLab is capable of handling both. Therefore, document that users should put full URL, just like when fixing a bug tracked in other sites. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- docs/best-practices.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/best-practices.rst b/docs/best-practices.rst index ba8478ab05..a4651c19d0 100644 --- a/docs/best-practices.rst +++ b/docs/best-practices.rst @@ -15,11 +15,11 @@ with minimal back-and-forth. by any longer description of why your patch makes sense. If the patch fixes a regression, and you know what commit introduced the problem, mentioning that is useful. If the patch resolves a - upstream bug reported in GitLab, put "Fixes: #NNN" in the commit - message. For a downstream bug, mention the URL of the bug instead. - In both cases also summarize the issue rather than making all - readers follow the link. You can use 'git shortlog -30' to get - an idea of typical summary lines. + upstream bug reported in GitLab, or downstream bug, put + "Resolves: $fullURL" of the bug. In both cases also summarize + the issue rather than making all readers follow the link. You + can use 'git shortlog -30' to get an idea of typical summary + lines. - Split large changes into a series of smaller patches, self-contained if possible, with an explanation of each patch -- 2.37.4

2 1

Re: [libvirt PATCH v3] cgroup/LXC: Do not condition availability of v2 by controllers
by Pavel Hrdina 24 Oct '22

24 Oct '22

On Mon, Oct 24, 2022 at 12:51:03PM +0000, Eric van Blokland wrote: > ------- Original Message ------- > On Monday, October 24th, 2022 at 1:54 PM, Pavel Hrdina <phrdina(a)redhat.com> wrote: > > > > On Sun, Oct 23, 2022 at 02:08:28PM +0200, Eric van Blokland wrote: > > > > > systemd in hybrid mode uses v1 hierarchies for controllers and v2 for > > > process tracking. > > > > > > The LXC code uses virCgroupAddMachineProcess() to move processes into > > > appropriate cgroup by manipulating cgroupfs directly. (Note, despite > > > libvirt also supports talking to systemd directly via > > > org.freedesktop.machine1 API.) > > > > > > If this path is taken, libvirt/lxc must convince systemd that processes > > > really belong to new cgroup, i.e. also the tracking v2 hierarchy must > > > undergo migration too. > > > > > > The current check would evaluate v2 backend as unavailable with hybrid > > > mode (because there are no available controllers). Simplify the > > > condition and consider the mounted cgroup2 as sufficient to touch v2 > > > hierarchy. > > > > > > This consequently creates an issue with binding the V2 mount. In hybrid > > > mode the V2 filesystem may be mounted upon the V1 filesystem. By reversing > > > the order in which backends are mounted in virCgroupBindMount this problem > > > is circumvented. > > > > > > Fixes: #182 > > > Signed-off-by: Eric van Blokland mail(a)ericvanblokland.nl > > > --- > > > src/util/vircgroup.c | 8 +++++--- > > > src/util/vircgroupv2.c | 12 ------------ > > > 2 files changed, 5 insertions(+), 15 deletions(-) > > > > > > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c > > > index a6a409af3d..48fbcf625a 100644 > > > --- a/src/util/vircgroup.c > > > +++ b/src/util/vircgroup.c > > > @@ -2924,9 +2924,11 @@ virCgroupBindMount(virCgroup *group, const char *oldroot, > > > size_t i; > > > virCgroup *parent = virCgroupGetNested(group); > > > > > > - for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { > > > - if (parent->backends[i] && > > > - parent->backends[i]->bindMount(parent, oldroot, mountopts) < 0) { > > > + /* In hybrid environments, V2 may be mounted over V1. > > > + * Mount the backends in reverse order. */ > > > > > > I don't understand what you mean by mounted over? > > > > In the hybrid environments I've seen V2 is mounted upon "/unified" in the V1 cgroup filesystem. What I've seen is that hybrid systemd environments have: /sys/fs/cgroup/unified /sys/fs/cgroup/memory /sys/fs/cgroup/blkio ... but it doesn't mean that it is mounted in V1 cgroup filesystem. In this case the /sys/fs/cgroup is tmpfs filesystem and there is nothing mounted over V1. In case you've managed to run into environment where unified is actually mounted inside cgroup filesystem then there is something else seriously broken. > > > + for (i = 1; i <= VIR_CGROUP_BACKEND_TYPE_LAST; i++) { > > > + if (parent->backends[VIR_CGROUP_BACKEND_TYPE_LAST - i] && > > > + parent->backends[VIR_CGROUP_BACKEND_TYPE_LAST - i]->bindMount(parent, oldroot, mountopts) < 0) { > > > return -1; > > > } > > > } > > > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c > > > index 4c110940cf..0e0c61d466 100644 > > > --- a/src/util/vircgroupv2.c > > > +++ b/src/util/vircgroupv2.c > > > @@ -75,22 +75,10 @@ virCgroupV2Available(void) > > > if (STRNEQ(entry.mnt_type, "cgroup2")) > > > continue; > > > > > > - /* Systemd uses cgroup v2 for process tracking but no controller is > > > - * available. We should consider this configuration as cgroup v2 is > > > - * not available. */ > > > - contFile = g_strdup_printf("%s/cgroup.controllers", entry.mnt_dir); > > > - > > > - if (virFileReadAll(contFile, 1024 * 1024, &contStr) < 0) > > > - goto cleanup; > > > - > > > - if (STREQ(contStr, "")) > > > - continue; > > > - > > > > > > I don't like this at all and IMO this is incorrect fix of the issue you > > are trying to address. In hybrid mode with systemd the cgroup v2 > > controller is not a real controller. It's something systemd uses for > > process tracking and some other features. It is owned by systemd and we > > should not touch it directly at all. We need to use proper systemd APIs > > to make any changes to that directory or if needed ask systemd to create > > cgroup with Delegate=yes which in this case is probably also not the > > correct approach. > > > > I must admit I'm a little in over my head here, but if I understand correctly, > there isn't anything done in the v2 backend in hybrid mode that wouldn't be done > in the v2 backend in unified mode. Does systemd behave differently or is the v2 > implementation in error in both hybrid and unified modes? > > Also in theory there could be a controller bound to the v2 hierarchy which would > activate the v2 backend anyway. So org.freedesktop.systemd1.Manager has method called AttachProcessesToUnit which would be most likely perfect for us for this specific case but it is not documented and from the commit message it was intended for internal use only. I'll ask systemd developers what the state is and if we could use it otherwise we might need to drop the check like this patch does. Pavel > > I know it was already pushed but I'll most likely revert this patch and > > we should find better and proper solution. > > > > Pavel > > > > I'd love to get suggestions for a better solution. > > Eric > > > > ret = true; > > > break; > > > } > > > > > > - cleanup: > > > VIR_FORCE_FCLOSE(mounts); > > > return ret; > > > } > > > -- > > > 2.35.3 >

1 0

[libvirt PATCH 0/7] ci: Integration test suite template fixes
by Erik Skultety 24 Oct '22

24 Oct '22

Failed job to demonstrate fixed log collecting: https://gitlab.com/eskultety/libvirt/-/pipelines/666789669 Successfull job with all patches applied: https://gitlab.com/eskultety/libvirt/-/pipelines/666906197 Erik Skultety (7): ci: integration: Add virtnetworkd to the list of restarted services ci: integration: Add an extra level of quoting for augeas set options ci: integration: Fix the test on avocado directory existence ci: integration: Only create a coredump log when a core exists ci: integration: Use 'set +e' in the log collecting job ci: integration: Create the avocado logs artifact dir conditionally ci: integration: Bundle only failed Avocado test results ci/integration-template.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) -- 2.37.3

2 9

[libvirt PATCH 0/3] qemu: Fix canceling migration
by Jiri Denemark 24 Oct '22

24 Oct '22

This series fixes commit v8.7.0-57-g2d7b22b561 "qemu: Make qemuMigrationSrcCancel optionally synchronous" which was broken in several ways (although the overall idea was correct). Jiri Denemark (3): qemu_migration: Properly wait for migration to be canceled qemu: Do not crash when canceling migration on reconnect NEWS: Document daemon crash on reconnect NEWS.rst | 5 ++++ src/qemu/qemu_migration.c | 61 ++++++++++++++++++++++++++++----------- src/qemu/qemu_migration.h | 3 +- src/qemu/qemu_process.c | 4 +-- 4 files changed, 53 insertions(+), 20 deletions(-) -- 2.38.0

2 4

[PATCH] vircgroup: Remove unused variables in virCgroupV2Available
by Peter Krempa 24 Oct '22

24 Oct '22

After recent commit 'contFile' and 'contStr' became unused breaking build with clang: ../../../libvirt/src/util/vircgroupv2.c:72:26: error: unused variable 'contFile' [-Werror,-Wunused-variable] g_autofree char *contFile = NULL; ^ ../../../libvirt/src/util/vircgroupv2.c:73:26: error: unused variable 'contStr' [-Werror,-Wunused-variable] g_autofree char *contStr = NULL; ^ Fixes: a0f37232b9c4296ca16955cc625f75eb848ace39 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/util/vircgroupv2.c | 3 --- 1 file changed, 3 deletions(-) Pushed as a build fix. diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 0e0c61d466..bf6bd11fef 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -69,9 +69,6 @@ virCgroupV2Available(void) return false; while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { - g_autofree char *contFile = NULL; - g_autofree char *contStr = NULL; - if (STRNEQ(entry.mnt_type, "cgroup2")) continue; -- 2.37.3

1 0

[PATCH v3 0/6] qemu: tpm: Add support for migration across shared storage
by Stefan Berger 24 Oct '22

24 Oct '22

This series of patches adds support for migrating vTPMs across hosts whose storage has been set up to share the directory structure holding the state of the TPM (swtpm). The existence of share storage influences the management of the directory structure holding the TPM state, which for example is only removed when a domain is undefined and not when a VM is removed on the migration source host. Further, when shared storage is used then security labeling on the destination side is skipped assuming that the labeling was already done on the source side. I have tested this with an NFS setup where I had to turn SELinux off on the hosts since the SELinux MLS range labeling is not supported by NFS. For shared storage support to work properly both sides of the migration need to be clients of the shared storage setup, meaning that they both have to have /var/lib/libvirt/swtpm mounted as shared storage, because other- wise the virFileIsSharedFS() may not detect shared storage and in the worst case may cause the TPM emulator (swtpm) to malfunction if for example the source side removed the TPM state directory structure. Shared storage migration requires (upcoming) swtpm v0.8. Stefan v3: - Relying entirely on virFileIsSharedFS() on migration source and destination sides to detect whether shared storage is set up between hosts; no more hint about shared storage from user via flag - Added support for virDomainTPMPrivate structure to store and persist TPM-related private data Stefan Berger (6): util: Add parsing support for swtpm's cmdarg-migration capability qemu: tpm: Conditionally create storage on incoming migration qemu: tpm: Add support for storing private TPM-related data qemu: tpm: Pass --migration option to swtpm if supported and needed qemu: tpm: Avoid security labels on incoming migration with shared storage qemu: tpm: Never remove state on outgoing migration and shared storage src/conf/domain_conf.c | 63 ++++++++++++++++++++-- src/conf/domain_conf.h | 9 ++++ src/qemu/qemu_domain.c | 85 +++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 17 +++++- src/qemu/qemu_driver.c | 20 +++---- src/qemu/qemu_extdevice.c | 10 ++-- src/qemu/qemu_extdevice.h | 6 ++- src/qemu/qemu_migration.c | 20 ++++--- src/qemu/qemu_process.c | 9 ++-- src/qemu/qemu_snapshot.c | 4 +- src/qemu/qemu_tpm.c | 111 ++++++++++++++++++++++++++++++++++---- src/qemu/qemu_tpm.h | 12 ++++- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + 14 files changed, 318 insertions(+), 50 deletions(-) -- 2.37.3

2 16