[PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
by tim＠siosm.fr
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
8 months, 2 weeks
[RFC PATCH 0/2] One memory leak fix and one question
by Marc Hartmayer
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
8 months, 2 weeks
[PATCH] qemusecuritytest: Call real virFileExists in mock
by Michal Privoznik
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
8 months, 2 weeks
[PATCH V2] security: Ensure file exists before attempting to restore label
by Jim Fehlig
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
8 months, 2 weeks
[PATCH] qemusecuritytest: Don't call real virFileExists in mock
by Jim Fehlig
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
8 months, 2 weeks
[PATCH] rpcgen: tests: Include stdint.h in test_demo.c
by Michal Privoznik
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
8 months, 2 weeks
[RFC] virsysinfo: Try reading DMI table
by brett.holman＠canonical.com
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
8 months, 2 weeks
[PATCH v1] node_device_conf: virNodeDeviceGetSCSITargetCaps: fix memory leak
by Marc Hartmayer
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
8 months, 2 weeks
[PATCH 0/2] Network device docs/iterator fixes
by Peter Krempa
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
8 months, 2 weeks
[PATCH] Extend libvirt-guests to shutdown only persistent VMs
by Benjamin Taubmann
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
8 months, 3 weeks
