September 2023 - Devel - libvirt List Archives

[PATCH 0/8] Finish effort to decrease maximum stack frame to 2048
by Peter Krempa 05 Sep '23

05 Sep '23

Few outstanding patches of code compiled only on FreeBSD Pipeline: https://gitlab.com/pipo.sk/libvirt/-/pipelines/986470469 Peter Krempa (8): bhyve: Don't stack-allocate huge error buffers virHostValidateBhyve: Declare one variable per line virHostValidateBhyve: Heap allocate massive 'struct kld_file_stat' nss: aiforaf: Format one argument/variable per line nss: aiforaf: Remove unused 'ret' variable nss: aiforaf: Drop unused buffer 'port' nss: aiforaf: Decrease stack size by scoping off large buffers. build: Decrease maximum stack frame size to 2048 meson.build | 2 +- src/bhyve/bhyve_process.c | 4 +- tools/nss/libvirt_nss.c | 105 ++++++++++++++++++------------- tools/virt-host-validate-bhyve.c | 20 +++--- 4 files changed, 74 insertions(+), 57 deletions(-) -- 2.41.0

4 24

Pure Python binding?
by Philipp Hahn 05 Sep '23

05 Sep '23

Hello, "pip install libvirt-python" does not work in several cases as that requires the low-level C library, compilers, tools, and Python header files to be installed. This prevents the use of "libvirt" in scenarios, where "root" privileges are not available to install these dependencies, e.g. running "libvirt" inside a JupyterHub notebook for gathering statistics. This also breaks the adoption on Windows, see <https://gitlab.com/libvirt/libvirt-python/-/issues/7> and <https://gitlab.com/libvirt/libvirt-python/-/issues/6> Looking at <https://libvirt.org/bindings.html> and <https://gitlab.com/libvirt> I only see "bindings" for the low-level C library. Does someone know of a "pure Python binding" for talking to a remote "libvirtd", either via "+ssh" or "+tls"? Or is there another "trick" to talk to libvirtd? And no, parsing the output of subprocess.run("ssh ... virsh ...") is not an option ;-) Philipp -- Philipp Hahn Open Source Software Engineer Univention GmbH Mary-Somerville-Str. 1 28359 Bremen Germany | Deutschland Phone: +49 (0)421 22232-0 | E-Mail: info(a)univention.de https://www.univention.de | https://www.univention.com Managing Directors: Peter H. Ganten, Stefan Gohmann Local court: Amtsgericht Bremen HRB 20755 | Ust-ID: DE220051310

2 1

[PATCH] conf: Generate MAC address instead of keeping all zeroes
by Martin Kletzander 04 Sep '23

04 Sep '23

When we parse <mac address="00:00:00:00:00:00"/> we keep that in memory and pass it down to the hypervisor. However, that MAC address is not strictly valid as it is not marked as locally administered (bit 0x02) but it is not even globally unique. It is also used for loopback device on Linux, for example. And QEMU sees such MAC address just as "not specified" and generates a new one that libvirt does not even know about. So to make the overall experience better we now generate it if the supplied one is all clear. Resolves: https://issues.redhat.com/browse/RHEL-974 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/conf/domain_conf.c | 2 +- src/util/virmacaddr.c | 5 +++++ src/util/virmacaddr.h | 1 + .../network-interface-mac-clear.xml | 21 +++++++++++++++++++ .../network-interface-mac-clear.xml | 21 +++++++++++++++++++ tests/genericxml2xmltest.c | 4 +++- 6 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 tests/genericxml2xmlindata/network-interface-mac-clear.xml create mode 100644 tests/genericxml2xmloutdata/network-interface-mac-clear.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bb4f1fdb948d..652bd09b21b8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9675,7 +9675,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - if (!macaddr) { + if (!macaddr || virMacAddrIsAllClear(&def->mac)) { virDomainNetGenerateMAC(xmlopt, &def->mac); def->mac_generated = true; } diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c index 073f298b5b66..e06bb200fc68 100644 --- a/src/util/virmacaddr.c +++ b/src/util/virmacaddr.c @@ -246,6 +246,11 @@ virMacAddrIsBroadcastRaw(const unsigned char s[VIR_MAC_BUFLEN]) return memcmp(virMacAddrBroadcastAddrRaw, s, sizeof(*s)) == 0; } +bool virMacAddrIsAllClear(const virMacAddr *addr) +{ + return !virMacAddrCmpRaw(addr, (const unsigned char[VIR_MAC_BUFLEN]){0}); +} + void virMacAddrFree(virMacAddr *addr) { diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h index f32b58805a61..7b9eb7443bd1 100644 --- a/src/util/virmacaddr.h +++ b/src/util/virmacaddr.h @@ -58,6 +58,7 @@ int virMacAddrParseHex(const char* str, bool virMacAddrIsUnicast(const virMacAddr *addr); bool virMacAddrIsMulticast(const virMacAddr *addr); bool virMacAddrIsBroadcastRaw(const unsigned char s[VIR_MAC_BUFLEN]); +bool virMacAddrIsAllClear(const virMacAddr *addr); void virMacAddrFree(virMacAddr *addr); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMacAddr, virMacAddrFree); diff --git a/tests/genericxml2xmlindata/network-interface-mac-clear.xml b/tests/genericxml2xmlindata/network-interface-mac-clear.xml new file mode 100644 index 000000000000..41beda8a79bb --- /dev/null +++ b/tests/genericxml2xmlindata/network-interface-mac-clear.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <interface type='bridge'> + <mac address='00:00:00:00:00:00'/> + <source bridge='br0'/> + </interface> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/network-interface-mac-clear.xml b/tests/genericxml2xmloutdata/network-interface-mac-clear.xml new file mode 100644 index 000000000000..a7935fa9f4de --- /dev/null +++ b/tests/genericxml2xmloutdata/network-interface-mac-clear.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <interface type='bridge'> + <mac address='52:54:00:00:00:00'/> + <source bridge='br0'/> + </interface> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 3501eadf5597..bf160b7e0bef 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -187,6 +187,7 @@ mymain(void) DO_TEST("cpu-cache-disable"); DO_TEST("network-interface-mac-check"); + DO_TEST_DIFFERENT("network-interface-mac-clear"); DO_TEST_DIFFERENT("chardev-tcp"); DO_TEST_FAIL_ACTIVE("chardev-tcp-missing-host"); @@ -255,4 +256,5 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN(mymain) +VIR_TEST_MAIN_PRELOAD(mymain, + VIR_TEST_MOCK("virrandom")) -- 2.42.0

2 2

[PATCH (pushed)] build: Fix logic bug determining whether running with optimization
by Peter Krempa 04 Sep '23

04 Sep '23

The conversion from ternary to a 'if' clause was wrong and thus didn't properly increase the stack size where needed but only where not actually needed. Fixes: b68faa99d9f16c2f504b23737040d25d072ee85d Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index e0ee4f2f21..3bf15d93a4 100644 --- a/meson.build +++ b/meson.build @@ -255,7 +255,7 @@ if cc.get_id() == 'clang' and get_option('optimization') == '0' endif # sanitizer instrumentation may enlarge stack frames -if get_option('b_sanitize') == 'none' +if get_option('b_sanitize') != 'none' stack_frame_size = 32768 endif -- 2.41.0

1 0

[PATCH (pushed)] build: Fix assignment into 'stack_frame_size' when sanitizer is enabled
by Peter Krempa 04 Sep '23

04 Sep '23

Instead of an assignment into the 'stack_frame_size' variable when sanitizers are enabled I've accidentally compared the value against the requested size. Fix the typo. Fixes: b68faa99d9f16c2f504b23737040d25d072ee85d Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index d29761bc71..e0ee4f2f21 100644 --- a/meson.build +++ b/meson.build @@ -256,7 +256,7 @@ endif # sanitizer instrumentation may enlarge stack frames if get_option('b_sanitize') == 'none' - stack_frame_size == 32768 + stack_frame_size = 32768 endif # array_bounds=2 check triggers false positive on some GCC -- 2.41.0

1 0

[PATCH 0/2] build: Fix build with clang without optimization
by Peter Krempa 04 Sep '23

04 Sep '23

Add a note for users how optimization is supposed to be controled with meson and fix the build by using 4k stack flame on clang without optimization. Peter Krempa (2): docs: compiling: Add a note about use of CFLAGS for optimization build: Work around clang's stack size calculation without optimization docs/compiling.rst | 5 +++++ meson.build | 11 ++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) -- 2.41.0

2 4

[PATCH] Revert "build: Decrease maximum stack frame size to 2048"
by Peter Krempa 04 Sep '23

04 Sep '23

Build fails with this patch with 'clang' when optimizations are disabled in long functions which have many helper variables declared in nested blocks such as for-loops. As there is no clean solution for now, let's keep the stack frame size at 4k. This reverts commit 42bc76cdb8486ef502200f3bce9e3faebdd78103. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 6b6f7ccb7c..dd7a4e8d34 100644 --- a/meson.build +++ b/meson.build @@ -248,7 +248,7 @@ alloc_max = run_command( ) # sanitizer instrumentation may enlarge stack frames -stack_frame_size = get_option('b_sanitize') == 'none' ? 2048 : 32768 +stack_frame_size = get_option('b_sanitize') == 'none' ? 4096 : 32768 # array_bounds=2 check triggers false positive on some GCC # versions when using sanitizers. Seen on Fedora 34 with -- 2.41.0

2 1

[PATCH 0/2] syntax-check: Introduce a rule for one line error messages
by Michal Privoznik 04 Sep '23

04 Sep '23

This is a follow up of: https://listman.redhat.com/archives/libvir-list/2023-August/241416.html And while the regular expression in patch 2/2 is very trivial it mostly works. But I'm open for suggestions. Michal Prívozník (2): tools: Reformat --help output of virsh and virt-admin syntax-check: Introduce a rule for one line error messages build-aux/syntax-check.mk | 8 ++++++++ tools/virsh.c | 6 ++++-- tools/virt-admin.c | 6 ++++-- 3 files changed, 16 insertions(+), 4 deletions(-) -- 2.41.0

2 5

[PATCH] docs, passt: Clarify some niche passt usage
by Martin Kletzander 04 Sep '23

04 Sep '23

Change example logfile path and clarify how complicated all things passt are. I chose not to create the non-existing directory because it could open a whole new can of worms. Also explain missing `dev` attribute of `<portForward/>` Resolves: https://issues.redhat.com/browse/RHEL-1833 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- docs/formatdomain.rst | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 68f54ab3ed62..bc469e5f9f94 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -4902,14 +4902,23 @@ When the passt backend is used, the ``<backend>`` attribute ``logFile`` can be used to tell the passt process for this interface where to write its message log, and the ``<source>`` attribute ``dev`` can tell it to use a particular host interface to derive the routes -given to the guest for forwarding traffic upstream. +given to the guest for forwarding traffic upstream. Due to the design +decisions of passt, if using SELinux, the log file is recommended to +reside in the runtime directory of a user under which the passt +process will run, most probably ``/run/user/$UID`` where ``$UID`` is +the UID of the user, e.g. ``qemu``. Beware that libvirt does not +create this directory if it does not already exist to avoid possible, +however unlikely, issues, especially since this logfile attribute is +meant mostly for debugging. Additionally, when passt is used, multiple ``<portForward>`` elements can be added to forward incoming network traffic for the host to this guest interface. Each ``<portForward>`` must have a ``proto`` -attribute (set to ``tcp`` or ``udp``) and optional original -``address`` (if not specified, then all incoming sessions to any host -IP for the given proto/port(s) will be forwarded to the guest). +attribute (set to ``tcp`` or ``udp``), optional original ``address`` +(if not specified, then all incoming sessions to any host IP for the +given proto/port(s) will be forwarded to the guest), and an optional +``dev`` attribute to limit the forwarded traffic to a specific host +interface. The decision of which ports to forward is described with zero or more ``<range>`` subelements of ``<portForward>`` (if there is no @@ -4934,7 +4943,7 @@ ports **with the exception of some subset**. <devices> ... <interface type='user'> - <backend type='passt' logFile='/tmp/passt.log'/> + <backend type='passt' logFile='/run/user/$UID/passt-domain.log'/> <mac address="00:11:22:33:44:55"/> <source dev='eth0'/> <ip family='ipv4' address='172.17.2.4' prefix='24'/> @@ -4946,7 +4955,7 @@ ports **with the exception of some subset**. <range start='5000' end='5020' to='6000'/> <range start='5010' end='5015' exclude='yes'/> </portForward> - <portForward proto='tcp' address='2001:db8:ac10:fd01::1:10'> + <portForward proto='tcp' address='2001:db8:ac10:fd01::1:10' dev='eth0'> <range start='80'/> <range start='443' to='344'/> </portForward> -- 2.41.0

2 1

[PATCH] conf, schema: Switch iothread/poll values to unsignedLong
by Martin Kletzander 04 Sep '23

04 Sep '23

They represent nanoseconds, and we accept such values already. Not that anyone would use such values in the wild, but even one person testing QEMU could put in a bigger value and will be bothered with validation errors after every `virsh edit`. Also add a test for it. Resolves: https://issues.redhat.com/browse/RHEL-1717 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/conf/schemas/domaincommon.rng | 6 +++--- tests/genericxml2xmlindata/iothreadids.xml | 23 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 2 ++ 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 tests/genericxml2xmlindata/iothreadids.xml diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index de3bd1c35c55..2f9ba31c0aec 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -876,17 +876,17 @@ <element name="poll"> <optional> <attribute name="max"> - <ref name="unsignedInt"/> + <ref name="unsignedLong"/> </attribute> </optional> <optional> <attribute name="grow"> - <ref name="unsignedInt"/> + <ref name="unsignedLong"/> </attribute> </optional> <optional> <attribute name="shrink"> - <ref name="unsignedInt"/> + <ref name="unsignedLong"/> </attribute> </optional> </element> diff --git a/tests/genericxml2xmlindata/iothreadids.xml b/tests/genericxml2xmlindata/iothreadids.xml new file mode 100644 index 000000000000..671a4672958d --- /dev/null +++ b/tests/genericxml2xmlindata/iothreadids.xml @@ -0,0 +1,23 @@ +<domain type='kvm'> + <name>foo</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <iothreads>1</iothreads> + <iothreadids> + <iothread id='8' thread_pool_min='2147483647' thread_pool_max='2147483647'> + <poll max='9223372036854775807' grow='456' shrink='789'/> + </iothread> + </iothreadids> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 3501eadf5597..ce8073e85a30 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -249,6 +249,8 @@ mymain(void) DO_TEST("cpu-phys-bits-emulate"); DO_TEST("cpu-phys-bits-passthrough"); + DO_TEST("iothreadids"); + virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.41.0

2 2