- Devel - libvirt List Archives

[PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
by tim＠siosm.fr 10 Apr '24

10 Apr '24

Install a systemd sysusers config file for the qemu & kvm user/groups. We can not use the sysusers_create_compat macro in the RPM specfile to create those users as we want to keep the specfile standalone and not relying on additionnal files. Update the specfile to make the commands closer to what is generated by the current macro. See: https://src.fedoraproject.org/rpms/libvirt/pull-request/22 See: https://gitlab.com/libvirt/libvirt/-/merge_requests/319 See: https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ Based on previous work by: Peter Krempa <pkrempa(a)redhat.com> Signed-off-by: Timothée Ravier <tim(a)siosm.fr> --- libvirt.spec.in | 21 +++++++++++++-------- src/qemu/libvirt-qemu.sysusers.conf | 4 ++++ src/qemu/meson.build | 7 +++++++ 3 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 src/qemu/libvirt-qemu.sysusers.conf diff --git a/libvirt.spec.in b/libvirt.spec.in index 8413e3c19a..a411ac6515 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1473,6 +1473,7 @@ chmod 600 $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/nwfilter/*.xml %if ! %{with_qemu} rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/libvirtd_qemu.aug rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug +rm -f $RPM_BUILD_ROOT%{_sysusersdir}/libvirt-qemu.conf %endif %find_lang %{name} @@ -1834,16 +1835,19 @@ exit 0 %pre daemon-driver-qemu %libvirt_sysconfig_pre virtqemud %libvirt_systemd_unix_pre virtqemud + # We want soft static allocation of well-known ids, as disk images -# are commonly shared across NFS mounts by id rather than name; see -# https://fedoraproject.org/wiki/Packaging:UsersAndGroups -getent group kvm >/dev/null || groupadd -f -g 36 -r kvm -getent group qemu >/dev/null || groupadd -f -g 107 -r qemu -if ! getent passwd qemu >/dev/null; then - if ! getent passwd 107 >/dev/null; then - useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu +# are commonly shared across NFS mounts by id rather than name. +# See https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ +# We can not use the sysusers_create_compat macro here as we want to keep the +# specfile standalone and not relying on additionnal files. +getent group 'kvm' >/dev/null || groupadd -f -g '36' -r 'kvm' || : +getent group 'qemu' >/dev/null || groupadd -f -g '107' -r 'qemu' || : +if ! getent passwd 'qemu' >/dev/null; then + if ! getent passwd '107' >/dev/null; then + useradd -r -u '107' -g 'qemu' -G 'kvm' -d '/' -s '/sbin/nologin' -c 'qemu user' 'qemu' || : else - useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu + useradd -r -g 'qemu' -G 'kvm' -d '/' -s '/sbin/nologin' -c 'qemu user' 'qemu' || : fi fi exit 0 @@ -2246,6 +2250,7 @@ exit 0 %{_bindir}/virt-qemu-run %{_mandir}/man1/virt-qemu-run.1* %{_mandir}/man8/virtqemud.8* +%{_sysusersdir}/libvirt-qemu.conf %endif %if %{with_lxc} diff --git a/src/qemu/libvirt-qemu.sysusers.conf b/src/qemu/libvirt-qemu.sysusers.conf new file mode 100644 index 0000000000..3189191e73 --- /dev/null +++ b/src/qemu/libvirt-qemu.sysusers.conf @@ -0,0 +1,4 @@ +g kvm 36 +g qemu 107 +u qemu 107:qemu "qemu user" - - +m qemu kvm diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 4c3e1dee78..7a0e908a66 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -160,6 +160,13 @@ if conf.has('WITH_QEMU') configuration: qemu_user_group_hack_conf, ) + # Install the sysuser config for the qemu driver + install_data( + 'libvirt-qemu.sysusers.conf', + install_dir: prefix / 'lib' / 'sysusers.d', + rename: [ 'libvirt-qemu.conf' ], + ) + virt_conf_files += qemu_conf virt_aug_files += files('libvirtd_qemu.aug') virt_test_aug_files += { -- 2.43.0

3 4

[RFC PATCH 0/2] One memory leak fix and one question
by Marc Hartmayer 10 Apr '24

10 Apr '24

Marc Hartmayer (2): node_device_conf: virNodeDeviceGetSCSITargetCaps: fix memory leak TODO virNodeDeviceUpdateCaps: checks missing? src/conf/node_device_conf.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) -- 2.34.1

2 10

[PATCH] qemusecuritytest: Call real virFileExists in mock
by Michal Privoznik 09 Apr '24

09 Apr '24

When I suggested to Jim to call real virFileExists() I forgot to also suggest calling init_syms(). Without it, real_virFileExists pointer might be left unset. And indeed, that's what we were seeing on FreeBSD. This effectively reverts commit 4b5cc57ed35dc24d11673dd3f04bfb8073c0340d. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Green pipeline: https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/1245457856 Okay, it doesn't test FreeBSD, but it tests x86_64-ubuntu-2204-clang which was also experiencing the failure: https://gitlab.com/libvirt/libvirt/-/jobs/6574951734 tests/qemusecuritymock.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index dc8a893e9d..2dfd6c33a0 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -66,6 +66,7 @@ static int (*real_close)(int fd); static int (*real_setfilecon_raw)(const char *path, const char *context); static int (*real_getfilecon_raw)(const char *path, char **context); #endif +static bool (*real_virFileExists)(const char *file); /* Global mutex to avoid races */ @@ -123,6 +124,7 @@ init_syms(void) VIR_MOCK_REAL_INIT(setfilecon_raw); VIR_MOCK_REAL_INIT(getfilecon_raw); #endif + VIR_MOCK_REAL_INIT(virFileExists); /* Intentionally not calling init_hash() here */ } @@ -386,8 +388,11 @@ bool virFileExists(const char *path) { VIR_LOCK_GUARD lock = virLockGuardLock(&m); - if (getenv(ENVVAR) == NULL) - return access(path, F_OK) == 0; + if (getenv(ENVVAR) == NULL) { + init_syms(); + + return real_virFileExists(path); + } init_hash(); if (virHashHasEntry(chown_paths, path)) -- 2.43.2

3 2

[PATCH V2] security: Ensure file exists before attempting to restore label
by Jim Fehlig 09 Apr '24

09 Apr '24

When performing an install, it's common for tooling such as virt-install to remove the install kernel/initrd once they are successfully booted and the domain has been redefined to boot without them. After the installation is complete and the domain is rebooted/shutdown, the DAC and selinux security drivers attempt to restore labels on the now deleted files. It's harmles wrt functionality, but results in error messages such as Mar 08 12:40:37 virtqemud[5639]: internal error: child reported (status=125): unable to stat: /var/lib/libvirt/boot/vir> Mar 08 12:40:37 virtqemud[5639]: unable to stat: /var/lib/libvirt/boot/virtinst-yvp19moo-linux: No such file or directo> Mar 08 12:40:37 virtqemud[5639]: Unable to run security manager transaction Add a check for file existence to the virSecurity*RestoreFileLabel functions, and avoid relabeling if the file is no longer available. Skipping the restore caused failures in qemusecuritytest, which mocks stat, chown, etc as part of ensuring the security drivers properly restore labels. virFileExists is now mocked in qemusecuritymock.c to return true when passed a file previously seen by the mocked stat, chown, etc functions. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/security/security_dac.c | 3 +++ src/security/security_selinux.c | 2 ++ tests/qemusecuritymock.c | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 567be4bd23..4e850e219e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -825,6 +825,9 @@ virSecurityDACRestoreFileLabelInternal(virSecurityManager *mgr, virStorageSourceIsLocalStorage(src)) path = src->path; + if (!virFileExists(path)) + return 0; + /* Be aware that this function might run in a separate process. * Therefore, any driver state changes would be thrown away. */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b49af26e49..aaec34ff8b 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1488,6 +1488,8 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManager *mgr, */ if (!path) return 0; + if (!virFileExists(path)) + return 0; VIR_INFO("Restoring SELinux context on '%s'", path); diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index 80d59536b1..bb0a85544b 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -382,6 +382,24 @@ int virFileUnlock(int fd G_GNUC_UNUSED, } +bool virFileExists(const char *path G_GNUC_UNUSED) +{ + VIR_LOCK_GUARD lock = virLockGuardLock(&m); + + if (getenv(ENVVAR) == NULL) + return access(path, F_OK) == 0; + + init_hash(); + if (virHashHasEntry(chown_paths, path)) + return true; + + if (virHashHasEntry(selinux_paths, path)) + return true; + + return false; +} + + typedef struct _checkOwnerData checkOwnerData; struct _checkOwnerData { GHashTable *paths; -- 2.44.0

2 4

[PATCH] qemusecuritytest: Don't call real virFileExists in mock
by Jim Fehlig 08 Apr '24

08 Apr '24

Calling the real virFileExists in qemusecuritymock.c can cause a segfault in qemusecuritytest. No segfaults are noticed when calling access(2) instead of virFileExists. Fixes: 4ed5ade753d8f1136cdbf17ddfe1d9093bcd933d Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- Pushing as a build-breaker fix. tests/qemusecuritymock.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index a7bb1f8bea..dc8a893e9d 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -66,7 +66,6 @@ static int (*real_close)(int fd); static int (*real_setfilecon_raw)(const char *path, const char *context); static int (*real_getfilecon_raw)(const char *path, char **context); #endif -static bool (*real_virFileExists)(const char *file); /* Global mutex to avoid races */ @@ -124,7 +123,6 @@ init_syms(void) VIR_MOCK_REAL_INIT(setfilecon_raw); VIR_MOCK_REAL_INIT(getfilecon_raw); #endif - VIR_MOCK_REAL_INIT(virFileExists); /* Intentionally not calling init_hash() here */ } @@ -389,7 +387,7 @@ bool virFileExists(const char *path) VIR_LOCK_GUARD lock = virLockGuardLock(&m); if (getenv(ENVVAR) == NULL) - return real_virFileExists(path); + return access(path, F_OK) == 0; init_hash(); if (virHashHasEntry(chown_paths, path)) -- 2.44.0

1 0

[PATCH] rpcgen: tests: Include stdint.h in test_demo.c
by Michal Privoznik 08 Apr '24

08 Apr '24

Since header file structure is a bit different on MacOS, it doesn't get uint64_t type declaration and thus test_demo.c must include it explicitly. This is proper solution anyway, because on Linux we're apparently relying on the header file sneaking through some other include. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/619 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- scripts/rpcgen/tests/test_demo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/rpcgen/tests/test_demo.c b/scripts/rpcgen/tests/test_demo.c index ae19a674cf..e6ba7ddbc5 100644 --- a/scripts/rpcgen/tests/test_demo.c +++ b/scripts/rpcgen/tests/test_demo.c @@ -2,6 +2,7 @@ #include <rpc/types.h> #include <rpc/xdr.h> #include <stdbool.h> +#include <stdint.h> #ifdef __APPLE__ # define xdr_uint64_t xdr_u_int64_t -- 2.43.2

2 1

[RFC] virsysinfo: Try reading DMI table
by brett.holman＠canonical.com 08 Apr '24

08 Apr '24

This patch intends to add DMI support to libvirt for RISC-V and mips. This is based on commit ec6ce6363, which added ARM support. This is untested, as I've been unable to find hardware to test this on. src/util/virsysinfo.c | 2 ++ 1 file changed, 2 insertions(+) Cheers, Brett Holman P.S. This is my first post on this mailing list, I believe that I've followed

5 8

[PATCH v1] node_device_conf: virNodeDeviceGetSCSITargetCaps: fix memory leak
by Marc Hartmayer 05 Apr '24

05 Apr '24

Make sure the old value in `scsi_target->wwpn` is free'd before replacing it. ==9104== 38 bytes in 2 blocks are definitely lost in loss record 1,943 of 3,250 ==9104== at 0x483B8C0: malloc (vg_replace_malloc.c:442) ==9104== by 0x4DFB69B: g_malloc (gmem.c:130) ==9104== by 0x4E1921D: g_strdup (gstrfuncs.c:363) ==9104== by 0x495D60B: g_strdup_inline (gstrfuncs.h:321) ==9104== by 0x495D60B: virFCReadRportValue (virfcp.c:62) ==9104== by 0x4A5F5CB: virNodeDeviceGetSCSITargetCaps (node_device_conf.c:2914) ==9104== by 0xBF62529: udevProcessSCSITarget (node_device_udev.c:657) ==9104== by 0xBF62529: udevGetDeviceDetails (node_device_udev.c:1406) ==9104== by 0xBF62529: udevAddOneDevice (node_device_udev.c:1563) ==9104== by 0xBF639B5: udevProcessDeviceListEntry (node_device_udev.c:1637) ==9104== by 0xBF639B5: udevEnumerateDevices (node_device_udev.c:1691) ==9104== by 0xBF639B5: nodeStateInitializeEnumerate (node_device_udev.c:2009) ==9104== by 0x49BDBFD: virThreadHelper (virthread.c:256) ==9104== by 0x5242069: start_thread (in /usr/lib64/libc.so.6) Signed-off-by: Marc Hartmayer <mhartmay(a)linux.ibm.com> --- src/conf/node_device_conf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 32b69aae84f3..14504cbb9c1f 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2897,6 +2897,7 @@ virNodeDeviceGetSCSITargetCaps(const char *sysfsPath, int ret = -1; g_autofree char *dir = NULL; g_autofree char *rport = NULL; + g_autofree char *wwpn = NULL; VIR_DEBUG("Checking if '%s' is an FC remote port", scsi_target->name); @@ -2912,11 +2913,13 @@ virNodeDeviceGetSCSITargetCaps(const char *sysfsPath, scsi_target->rport = g_steal_pointer(&rport); if (virFCReadRportValue(scsi_target->rport, "port_name", - &scsi_target->wwpn) < 0) { + &wwpn) < 0) { VIR_WARN("Failed to read port_name for '%s'", scsi_target->rport); goto cleanup; } + VIR_FREE(scsi_target->wwpn); + scsi_target->wwpn = g_steal_pointer(&wwpn); scsi_target->flags |= VIR_NODE_DEV_CAP_FLAG_FC_RPORT; ret = 0; base-commit: b902cfece0db71c3421b0bfe0e05d1dbe7890c31 -- 2.34.1

2 1

[PATCH 0/2] Network device docs/iterator fixes
by Peter Krempa 05 Apr '24

05 Apr '24

Two things noticed when reviewing the series for 'usb-net' device. Peter Krempa (2): docs: Rewrite documentation for network device models virDomainDeviceIsUSB: Handle all USB devices and simplify the code docs/formatdomain.rst | 29 +++--- src/conf/domain_conf.c | 228 +++++++++++++++++++---------------------- 2 files changed, 122 insertions(+), 135 deletions(-) -- 2.44.0

2 4

[PATCH] Extend libvirt-guests to shutdown only persistent VMs
by Benjamin Taubmann 04 Apr '24

04 Apr '24

At the moment, there is no configuration option for the libvirt-guests service that allows users to define that only persistent virtual machines should be shutdown on host shutdown. Currently, the service config allows to choose between two ON_SHUTDOWN actions that are executed on running virtual machines when the host goes down: shutdown, suspend. The ON_SHUTDOWN action should be orthogonal to the type of the virtual machine. However, the existing implementation, does not suspend transient virtual machines. This is the matrix of actions that is executed on virtual machines based on the configured ON_SHUTDOWN action and the type of a virtual machine. | persistent | transient shutdown | shutdown | shutdown (what we want to change) suspend | suspend | nothing Add config option PERSISTENT_ONLY to libvirt-guests config that allows users to define if the ON_SHUTDOWN action should be applied only on persistent virtual machines. PERSISTENT_ONLY can be set to true, false, default. The default option will implement the already existing logic. Case 1: PERSISTENT_ONLY=default | persistent | transient shutdown | shutdown | shutdown suspend | suspend | nothing Case 2: PERSISTENT_ONLY=true | persistent | transient shutdown | shutdown | nothing suspend | suspend | nothing Case 3: PERSISTENT_ONLY=false | persistent | transient shutdown | shutdown | shutdown suspend | suspend | suspend Change-Id: Ib03013d00b3ec60716287dad4743a038cf000763 --- tools/libvirt-guests.sh.in | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index 344b54390a..c3c5954e17 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -38,6 +38,7 @@ PARALLEL_SHUTDOWN=0 START_DELAY=0 BYPASS_CACHE=0 SYNC_TIME=0 +PERSISTENT_ONLY="default" test -f "$initconfdir"/libvirt-guests && . "$initconfdir"/libvirt-guests @@ -438,14 +439,16 @@ shutdown_guests_parallel() # stop # Shutdown or save guests on the configured uris stop() { - local suspending="true" local uri= + local action="suspend" + local persistent_only="default" + # last stop was not followed by start [ -f "$LISTFILE" ] && return 0 if [ "x$ON_SHUTDOWN" = xshutdown ]; then - suspending="false" + action="shutdown" if [ $SHUTDOWN_TIMEOUT -lt 0 ]; then gettext "SHUTDOWN_TIMEOUT must be equal or greater than 0" echo @@ -454,6 +457,22 @@ stop() { fi fi + case "x$PERSISTENT_ONLY" in + xtrue) + persistent_only="true" + ;; + xfalse) + persistent_only="false" + ;; + *) + if [ "x$action" = xshutdown ]; then + persistent_only="false" + elif [ "x$action" = xsuspend ]; then + persistent_only="true" + fi + ;; + esac + : >"$LISTFILE" set -f for uri in $URIS; do @@ -478,7 +497,7 @@ stop() { echo fi - if "$suspending"; then + if "$persistent_only"; then local transient="$(list_guests "$uri" "--transient")" if [ $? -eq 0 ]; then local empty="true" @@ -486,7 +505,11 @@ stop() { for uuid in $transient; do if "$empty"; then - eval_gettext "Not suspending transient guests on URI: \$uri: " + if [ "x$action" = xsuspend ]; then + eval_gettext "Not suspending transient guests on URI: \$uri: " + else + eval_gettext "Not shutting down transient guests on URI: \$uri: " + fi empty="false" else printf ", " @@ -520,19 +543,19 @@ stop() { if [ -s "$LISTFILE" ]; then while read uri list; do - if "$suspending"; then + if [ "x$action" = xsuspend ]; then eval_gettext "Suspending guests on \$uri URI..."; echo else eval_gettext "Shutting down guests on \$uri URI..."; echo fi if [ "$PARALLEL_SHUTDOWN" -gt 1 ] && - ! "$suspending"; then + [ "x$action" = xshutdown ]; then shutdown_guests_parallel "$uri" "$list" else local guest= for guest in $list; do - if "$suspending"; then + if [ "x$action" = xsuspend ]; then suspend_guest "$uri" "$guest" else shutdown_guest "$uri" "$guest" -- 2.39.2

4 5