August 2023 - Devel - libvirt List Archives

[PATCH] conf: fix error message
by Anastasia Belova 08 Aug '23

08 Aug '23

From: Анастасия Белова <abelova(a)astralinux.ru> In case of invalid placement its value should be passed as a parameter of virReportError instead of mode. Fixes: 93e82727ec ("numatune: Encapsulate numatune configuration in order to unify results") Signed-off-by: Anastasia Belova <abelova(a)astralinux.ru> --- src/conf/numa_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index a616521763..31b5ea58f6 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -508,7 +508,7 @@ virDomainNumatuneSet(virDomainNuma *numa, (placement < 0 || placement >= VIR_DOMAIN_NUMATUNE_PLACEMENT_LAST)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported numatune placement '%1$d'"), - mode); + placement); return -1; } -- 2.30.2

2 1

[PATCH v2] lxc_container: Increase stack size for lxcContainerChild()
by Michal Privoznik 07 Aug '23

07 Aug '23

When spawning a new container (via clone()) we allocate stack for lxcContainerChild(). So far, we allocate 4 pages for the stack and this used to be enough until we started rewriting everything to glib. With glib we switched to g_strerror() which localizes errno strings and thus increases stack usage, while the previously used strerror_r() was more compact. Fortunately, the solution is easy - just increase how much stack the child can use (16 pages ought to be enough for anybody). And while at it, lets use mmap() for allocation which offer some nice features: MAP_STACK - align allocation to be suitable for stack (even though, currently ignored on Linux), MAP_GROWSDOWN - kernel guards out of bounds access from child Resolves: https://gitlab.com/libvirt/libvirt/-/issues/511 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- This is a v2 of: https://listman.redhat.com/archives/libvir-list/2023-August/241127.html diff to v1: - switched from g_new0() to mmap() for additional security src/lxc/lxc_container.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 63cf283285..c215b83848 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -26,6 +26,7 @@ #include <sys/ioctl.h> #include <sys/mount.h> #include <sys/stat.h> +#include <sys/mman.h> #include <unistd.h> #include <mntent.h> #include <sys/reboot.h> @@ -2132,9 +2133,10 @@ int lxcContainerStart(virDomainDef *def, { pid_t pid; int cflags; - int stacksize = getpagesize() * 4; - g_autofree char *stack = NULL; + int stacksize = getpagesize() * 16; + char *stack = NULL; char *stacktop; + int ret = -1; lxc_child_argv_t args = { .config = def, .securityDriver = securityDriver, @@ -2150,7 +2152,14 @@ int lxcContainerStart(virDomainDef *def, }; /* allocate a stack for the container */ - stack = g_new0(char, stacksize); + stack = mmap(NULL, stacksize, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_GROWSDOWN | MAP_STACK, + -1, 0); + if (stack == MAP_FAILED) { + virReportSystemError(errno, "%s", + _("Unable to allocate stack")); + return -1; + } stacktop = stack + stacksize; @@ -2160,7 +2169,7 @@ int lxcContainerStart(virDomainDef *def, if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_USER) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); - return -1; + goto cleanup; } VIR_DEBUG("Enable user namespace"); cflags |= CLONE_NEWUSER; @@ -2175,7 +2184,7 @@ int lxcContainerStart(virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Config asks for inherit net namespace " "as well as private network interfaces")); - return -1; + goto cleanup; } VIR_DEBUG("Inheriting a net namespace"); } @@ -2199,10 +2208,16 @@ int lxcContainerStart(virDomainDef *def, if (pid < 0) { virReportSystemError(errno, "%s", _("Failed to run clone container")); - return -1; + goto cleanup; } - return pid; + ret = pid; + cleanup: + if (munmap(stack, stacksize) < 0) { + VIR_WARN("Unable to munmap() stack: %s", g_strerror(errno)); + } + + return ret; } int lxcContainerChown(virDomainDef *def, const char *path) -- 2.41.0

2 1

[PATCH 0/2] libvirt-guests: small improvments
by Jim Fehlig 05 Aug '23

05 Aug '23

The first patch is trivial. I suppose the second is debatable. If I build libvirt with -Dremote_default_mode=legacy but deploy modular daemons, /run/libvirt/libvirt-sock is provided by virtproxyd, which may or may not be running when libvirt-guests starts/stops. I added an 'After=virtproxyd.socket' ordering dependency to libvirt-guests, but it hasn't fixed an issue I'm seeing when using libvirt-guests+virtproxyd libvirt-guests.sh[2607]: Can't connect to default. Skipping I'm still investigating that issue but think the dependency issue is worth discussing independently. Jim Fehlig (2): libvirt-guests: Remove unused variable 'libvirtd' libvirt-guests: Add systemd odering dependency to virtproxyd tools/libvirt-guests.service.in | 1 + tools/libvirt-guests.sh.in | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) -- 2.41.0

3 8

[PATCH] lxc_container: Increase stack size for lxcContainerChild()
by Michal Privoznik 04 Aug '23

04 Aug '23

When spawning a new container (via clone()) we allocate stack for lxcContainerChild(). So far, we allocate 4 pages for the stack and this used to be enough until we started rewriting everything to glib. With glib we switched to g_strerror() which localizes errno strings and thus increases stack usage, while the previously used strerror_r() was more compact. Fortunately, the solution is easy - just increase how much stack the child can use (16 pages ought to be enough for anybody). Resolves: https://gitlab.com/libvirt/libvirt/-/issues/511 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 63cf283285..f741a754ce 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2132,7 +2132,7 @@ int lxcContainerStart(virDomainDef *def, { pid_t pid; int cflags; - int stacksize = getpagesize() * 4; + int stacksize = getpagesize() * 16; g_autofree char *stack = NULL; char *stacktop; lxc_child_argv_t args = { -- 2.41.0

2 1

[PATCH 00/13] Use struct zero initializer instead of memset
by Michal Privoznik 04 Aug '23

04 Aug '23

This was inspired by Martin's comment here: https://listman.redhat.com/archives/libvir-list/2023-July/241007.html It has sent me down a rabbit hole. But hey, it helped me to identify some needless memset()-s, unused variables, problematic code patterns. Michal Prívozník (13): tools: Fix vshControl declaration and initialization virt-aa-helper: Use struct zero initializer instead of memset Decrease scope of some variables qemu: Don't reuse variable in processSerialChangedEvent() remote_driver: Drop explicit memset(&sargs) in remoteAuthSASL() virfirewalld: Drop useless memset() in virFirewallDApplyRule() virnetlink: Drop unused variable from virNetlinkCommand() securityselinuxhelper: Use g_new0() instead of malloc()+memset() combo virnetclient: Update comment about memset() virnetdaemon.c: Use struct zero initializer instead of memset lib: use struct zero initializer instead of memset lib: Finish using struct zero initializer manually lib: Prefer sizeof(variable) instead of sizeof(type) in memset src/ch/ch_monitor.c | 3 +- src/conf/domain_conf.c | 8 +- src/conf/network_conf.c | 28 ++---- src/cpu/cpu_x86.c | 4 +- src/esx/esx_driver.c | 20 ++--- src/esx/esx_interface_driver.c | 13 +-- src/esx/esx_storage_backend_iscsi.c | 11 +-- src/esx/esx_storage_backend_vmfs.c | 19 ++-- src/esx/esx_util.c | 4 +- src/libxl/libxl_capabilities.c | 4 +- src/libxl/libxl_driver.c | 9 +- src/locking/lock_driver_lockd.c | 19 ++-- src/logging/log_manager.c | 25 ++---- src/lxc/lxc_controller.c | 6 +- src/lxc/lxc_domain.c | 4 +- src/lxc/lxc_driver.c | 4 +- src/nwfilter/nwfilter_dhcpsnoop.c | 4 +- src/nwfilter/nwfilter_gentech_driver.c | 4 +- src/qemu/qemu_agent.c | 10 +-- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_driver.c | 28 +++--- src/qemu/qemu_monitor.c | 10 +-- src/qemu/qemu_monitor_json.c | 11 +-- src/qemu/qemu_process.c | 5 +- src/remote/remote_daemon_dispatch.c | 115 ++++++++----------------- src/remote/remote_daemon_stream.c | 23 ++--- src/remote/remote_driver.c | 23 ++--- src/rpc/virnetclient.c | 4 +- src/rpc/virnetclientprogram.c | 4 +- src/rpc/virnetclientstream.c | 9 +- src/rpc/virnetdaemon.c | 10 +-- src/rpc/virnetsaslcontext.c | 3 +- src/rpc/virnetserverprogram.c | 14 +-- src/rpc/virnetsocket.c | 49 +++-------- src/rpc/virnetsshsession.c | 8 +- src/rpc/virnettlscontext.c | 3 +- src/security/virt-aa-helper.c | 5 +- src/storage/storage_backend_logical.c | 10 +-- src/storage/storage_driver.c | 2 +- src/test/test_driver.c | 4 +- src/util/virarptable.c | 3 +- src/util/virauth.c | 4 +- src/util/virbpf.c | 52 +++-------- src/util/virdevmapper.c | 8 +- src/util/virfdstream.c | 3 +- src/util/virfile.c | 3 +- src/util/virfirewalld.c | 2 - src/util/virinitctl.c | 4 +- src/util/viriscsi.c | 4 +- src/util/virlog.c | 9 +- src/util/virnetdev.c | 8 +- src/util/virnetdevbridge.c | 10 +-- src/util/virnetdevip.c | 8 +- src/util/virnetdevmacvlan.c | 3 +- src/util/virnetdevtap.c | 9 +- src/util/virnetlink.c | 7 +- src/util/virperf.c | 3 +- src/util/virprocess.c | 3 +- src/util/virsocket.c | 6 +- src/util/virsocketaddr.c | 7 +- src/util/viruri.c | 4 +- src/util/virutil.c | 4 +- src/util/viruuid.c | 4 +- src/vbox/vbox_storage.c | 10 +-- src/vmx/vmx.c | 8 +- tests/libxlmock.c | 4 +- tests/nsstest.c | 8 +- tests/nwfilterxml2firewalltest.c | 4 +- tests/qemumonitorjsontest.c | 11 +-- tests/qemumonitortestutils.c | 8 +- tests/qemuxml2argvtest.c | 4 +- tests/securityselinuxhelper.c | 5 +- tests/sockettest.c | 6 +- tests/virhostcputest.c | 3 +- tests/virnetmessagetest.c | 8 +- tests/virnetsockettest.c | 7 +- tools/nss/libvirt_nss.c | 10 +-- tools/virsh-domain-monitor.c | 10 +-- tools/virsh-domain.c | 15 ++-- tools/virsh.c | 7 +- tools/virt-admin.c | 7 +- tools/vsh-table.c | 4 +- 82 files changed, 258 insertions(+), 591 deletions(-) -- 2.41.0

4 31

[PATCH] Fix some typos in documentation and comments
by Stefan Weil 02 Aug '23

02 Aug '23

Signed-off-by: Stefan Weil <sw(a)weilnetz.de> --- This patch was triggered by a spelling check for the generated QEMU documentation using codespell. It does not try to fix all typos which still exist in the QEMU code, but has a focus on those required to fix the documentation. Nevertheless some code comments with the same typos were fixed, too. I think the patch is trivial, so maybe it can still be included in the upcoming release, but that's not strictly necessary. Stefan docs/about/deprecated.rst | 2 +- docs/devel/qom.rst | 2 +- docs/system/devices/nvme.rst | 2 +- hw/core/loader.c | 4 ++-- include/exec/memory.h | 2 +- ui/vnc-enc-tight.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 1c35f55666..92a2bafd2b 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -369,7 +369,7 @@ mapping permissions et al by using its 'mapped' security model option. Nowadays it would make sense to reimplement the ``proxy`` backend by using QEMU's ``vhost`` feature, which would eliminate the high latency costs under which the 9p ``proxy`` backend currently suffers. However as of to date nobody -has indicated plans for such kind of reimplemention unfortunately. +has indicated plans for such kind of reimplementation unfortunately. Block device options diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index 0b506426d7..9918fac7f2 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -30,7 +30,7 @@ user configuration. Creating a QOM class ==================== -A simple minimal device implementation may look something like bellow: +A simple minimal device implementation may look something like below: .. code-block:: c :caption: Creating a minimal type diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst index a8bb8d729c..2a3af268f7 100644 --- a/docs/system/devices/nvme.rst +++ b/docs/system/devices/nvme.rst @@ -232,7 +232,7 @@ parameters: Set the number of Reclaim Groups. ``fdp.nruh`` (default: ``0``) - Set the number of Reclaim Unit Handles. This is a mandatory paramater and + Set the number of Reclaim Unit Handles. This is a mandatory parameter and must be non-zero. ``fdp.runs`` (default: ``96M``) diff --git a/hw/core/loader.c b/hw/core/loader.c index 8b7fd9e9e5..4dd5a71fb7 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -863,7 +863,7 @@ ssize_t load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz) /* * The Linux header magic number for a EFI PE/COFF - * image targetting an unspecified architecture. + * image targeting an unspecified architecture. */ #define EFI_PE_LINUX_MAGIC "\xcd\x23\x82\x81" @@ -1492,7 +1492,7 @@ RomGap rom_find_largest_gap_between(hwaddr base, size_t size) if (rom->mr || rom->fw_file) { continue; } - /* ignore anything finishing bellow base */ + /* ignore anything finishing below base */ if (rom->addr + rom->romsize <= base) { continue; } diff --git a/include/exec/memory.h b/include/exec/memory.h index 7f5c11a0cc..68284428f8 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -942,7 +942,7 @@ struct MemoryListener { * * @listener: The #MemoryListener. * @last_stage: The last stage to synchronize the log during migration. - * The caller should gurantee that the synchronization with true for + * The caller should guarantee that the synchronization with true for * @last_stage is triggered for once after all VCPUs have been stopped. */ void (*log_sync_global)(MemoryListener *listener, bool last_stage); diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index 09200d71b8..ee853dcfcb 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -77,7 +77,7 @@ static int tight_send_framebuffer_update(VncState *vs, int x, int y, #ifdef CONFIG_VNC_JPEG static const struct { - double jpeg_freq_min; /* Don't send JPEG if the freq is bellow */ + double jpeg_freq_min; /* Don't send JPEG if the freq is below */ double jpeg_freq_threshold; /* Always send JPEG if the freq is above */ int jpeg_idx; /* Allow indexed JPEG */ int jpeg_full; /* Allow full color JPEG */ -- 2.39.2

4 4

[libvirt PATCH] src: fix max file limits in systemd services
by Daniel P. Berrangé 02 Aug '23

02 Aug '23

This fixes commit 38abf9c34dc481b0dc923bdab446ee623bdc5ab6 Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Wed Jun 21 13:22:40 2023 +0100 src: set max open file limit to match systemd >= 240 defaults The bug referenced in that commit had suggested to set LimitNOFile=512000:1024 on the basis that matches current systemd default behaviour and is compatible with old systemd. That was good except * The setting is LimitNOFILE and these are case sensitive * The hard and soft limits were inverted - soft must come first and so it would have been ignored even if the setting name was correct. * The default hard limit is 524288 not 512000 Reported-by: Olaf Hering <olaf(a)aepfle.de> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/ch/virtchd.service.in | 2 +- src/locking/virtlockd.service.in | 2 +- src/logging/virtlogd.service.in | 2 +- src/lxc/virtlxcd.service.in | 2 +- src/qemu/virtqemud.service.in | 2 +- src/remote/libvirtd.service.in | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ch/virtchd.service.in b/src/ch/virtchd.service.in index be242fea78..351eee312b 100644 --- a/src/ch/virtchd.service.in +++ b/src/ch/virtchd.service.in @@ -24,7 +24,7 @@ Restart=on-failure # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations -LimitNOFile=512000:1024 +LimitNOFILE=1024:524288 # The cgroups pids controller can limit the number of tasks started by # the daemon, which can limit the number of domains for some hypervisors. # A conservative default of 8 tasks per guest results in a TasksMax of diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index f1792dcb43..dd0bbab083 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -18,7 +18,7 @@ OOMScoreAdjust=-900 # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations -LimitNOFile=512000:1024 +LimitNOFILE=1024:524288 [Install] Also=virtlockd.socket diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index cef4053f59..8e245ddb43 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -18,7 +18,7 @@ OOMScoreAdjust=-900 # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations -LimitNOFile=512000:1024 +LimitNOFILE=1024:524288 [Install] Also=virtlogd.socket diff --git a/src/lxc/virtlxcd.service.in b/src/lxc/virtlxcd.service.in index b615a3f32d..ee3a7f1083 100644 --- a/src/lxc/virtlxcd.service.in +++ b/src/lxc/virtlxcd.service.in @@ -24,7 +24,7 @@ Restart=on-failure # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations -LimitNOFile=512000:1024 +LimitNOFILE=1024:524288 # The cgroups pids controller can limit the number of tasks started by # the daemon, which can limit the number of domains for some hypervisors. # A conservative default of 8 tasks per guest results in a TasksMax of diff --git a/src/qemu/virtqemud.service.in b/src/qemu/virtqemud.service.in index e3dc738b8f..e79670ca95 100644 --- a/src/qemu/virtqemud.service.in +++ b/src/qemu/virtqemud.service.in @@ -26,7 +26,7 @@ Restart=on-failure # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations -LimitNOFile=512000:1024 +LimitNOFILE=1024:524288 # The cgroups pids controller can limit the number of tasks started by # the daemon, which can limit the number of domains for some hypervisors. # A conservative default of 8 tasks per guest results in a TasksMax of diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index abac58cb2c..84f1613adc 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -31,7 +31,7 @@ Restart=on-failure # Raise hard limits to match behaviour of systemd >= 240. # During startup, daemon will set soft limit to match hard limit # per systemd recommendations -LimitNOFile=512000:1024 +LimitNOFILE=1024:524288 # The cgroups pids controller can limit the number of tasks started by # the daemon, which can limit the number of domains for some hypervisors. # A conservative default of 8 tasks per guest results in a TasksMax of -- 2.41.0

2 1

[PATCH] daemon: Treat logging of VIR_ERR_MULTIPLE_INTERFACES same as VIR_ERR_NO_INTERFACE
by Peter Krempa 02 Aug '23

02 Aug '23

When a query for an interface via virInterfaceLookupByMACString finds multiple interfaces an error is returned. Treat such error with the same 'debug' priority as we treat when the interface was not found to avoid spamming logs with such configurations. Closes: https://gitlab.com/libvirt/libvirt/-/issues/514 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/remote/remote_daemon.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index d880711c91..d4d999e53a 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -97,6 +97,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) case VIR_ERR_NO_STORAGE_VOL: case VIR_ERR_NO_NODE_DEVICE: case VIR_ERR_NO_INTERFACE: + case VIR_ERR_MULTIPLE_INTERFACES: case VIR_ERR_NO_NWFILTER: case VIR_ERR_NO_NWFILTER_BINDING: case VIR_ERR_NO_SECRET: -- 2.41.0

2 1

[PATCH 0/3] lxc: Fix reporting of startup errors with debug logging enabled
by Peter Krempa 02 Aug '23

02 Aug '23

Few issues with propagating error to the user when debug logging is enabled, Peter Krempa (3): virLXCControllerSetupUsernsMap: Modify debug logging for clean startup errors virLXCProcessReadLogOutputData: Refill buffer after filtering out noise virLXCProcessReportStartupLogError: Strip trailing newline from error src/lxc/lxc_controller.c | 8 +++++--- src/lxc/lxc_process.c | 12 ++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) -- 2.41.0

2 4

[PATCH 0/2] qemu_passt: Stop pre-creating passt logfile
by Michal Privoznik 02 Aug '23

02 Aug '23

See reasoning in 2/2. Michal Prívozník (2): Revert "qemu_passt: Actually use @logfd" Revert "qemu_passt: Precreate passt logfile" src/qemu/qemu_passt.c | 40 +++++----------------------------------- 1 file changed, 5 insertions(+), 35 deletions(-) -- 2.41.0

2 3