[PATCH] qemu: Add sysusers config file for qemu & kvm user/groups

tim@siosm.fr posted 1 patch 3 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240202185944.11826.41061@lists.libvirt.org.85.43.8.in-addr.arpa
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
[PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
Posted by tim@siosm.fr 3 weeks, 2 days ago
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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
Posted by Peter Krempa 2 weeks, 6 days ago
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.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
Posted by tim@siosm.fr 1 week, 6 days ago
> 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&gt;
> 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&gt;
> 
> 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.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org