[PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
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@redhat.com> Signed-off-by: Timothée Ravier <tim@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
On Fri, Feb 02, 2024 at 18:59:44 -0000, tim@siosm.fr wrote:
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/
IMO we should also mention: https://bugzilla.redhat.com/show_bug.cgi?id=2095429
Based on previous work by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Timothée Ravier <tim@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
I've tested that 'rpmbuild -tb libvirt*.tar.xz' works properly after this patch. Note that in the way you've posted this patch the authorship looks like: commit 746e7b69bf57631ceb2be93e8a0b9db4b4b50e5f Author: tim@siosm.fr <tim@siosm.fr> Date: Fri Feb 2 18:59:44 2024 +0000 qemu: Add sysusers config file for qemu & kvm user/groups Is this what you've wanted? Or should I update it using the spelling from the sign off?
diff --git a/libvirt.spec.in b/libvirt.spec.in index 8413e3c19a..a411ac6515 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in
[...]
@@ -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
The quoting changes are okay and result in identical commands, but I'm not that much sold on the discarding of errors (' || : ') which we didn't do before. Why would we want to ignore errors here? Other than that: Reviewed-by: Peter Krempa <pkrempa@redhat.com> You don't need to re-send but I want to know the reason for dropping the errors first. I can update the patch before pushing.
IMO we should also mention: https://bugzilla.redhat.com/show_bug.cgi?id=2095429
Sure, feel free to add it to the list of references I added.
Note that in the way you've posted this patch the authorship looks like:
commit 746e7b69bf57631ceb2be93e8a0b9db4b4b50e5f Author: tim(a)siosm.fr <tim(a)siosm.fr> Date: Fri Feb 2 18:59:44 2024 +0000
qemu: Add sysusers config file for qemu & kvm user/groups
Is this what you've wanted? Or should I update it using the spelling from the sign off?
It should be "Timothée Ravier <tim@siosm.fr>", not sure why this got altered when I tried sending here.
The quoting changes are okay and result in identical commands, but I'm not that much sold on the discarding of errors (' || : ') which we didn't do before. Why would we want to ignore errors here?
This is taken from the output of the macro that I re-ran on the sysusers config file and trimmed to match what we have already. It's generally not a good idea to fail in a scriptlet as there isn't any option for the user to recover at that point. It's also unlikely to fail but if it does, we should try setting up as much as possible and move on. The user / group creation failures will likely be surfaced at runtime.
Other than that:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
You don't need to re-send but I want to know the reason for dropping the errors first. I can update the patch before pushing.
Thanks. Feel free to edit and apply as you wish.
On Mon, Feb 05, 2024 at 03:10:41PM +0100, Peter Krempa wrote:
On Fri, Feb 02, 2024 at 18:59:44 -0000, tim@siosm.fr wrote:
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@redhat.com> Signed-off-by: Timothée Ravier <tim@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
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Unfortunately I failed to notice this before it had already made it into a release...
+++ 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
These values are fine for Fedora/RHEL, but not elsewhere. For example, Debian would need something like g libvirt-qemu 64055 u libvirt-qemu 64055:libvirt-qemu instead. If you look at meson.build, you will see that we detect a number of operating systems/distributions in order to integrate as smoothly as possible with them. This can potentially break that, or at the very least make things quite confusing by virtue of more than one "QEMU user" existing on the system. Additionally, it completely fails to account for the qemu_user and qemu_group meson options, which have been around forever and can take arbitrary values. At the very least, installing this file needs to be gated behind a meson option that is off by default. A more complete solution that integrates properly with the existing facilities will require further work. -- Andrea Bolognani / Red Hat / Virtualization
Good catch. I've tried to look at fixing this but this requires meson knowledge that I don't have so I haven't been able to focus on it yet. Pointers appreciated.
participants (3)
-
Andrea Bolognani -
Peter Krempa -
tim＠siosm.fr