severity 548 important
thanks
Meeuwissen Olaf wrote:
> I just upgraded a number of packages on a machine where I had
> manually added a `kvm` group. This caused the postinst script
> to fail with
>
> The group `kvm' already exists and is not a system group. Exiting.
>
> leaving `eudev` unconfigured.
I noticed this on Sunday and also investigated. And then today
another user DeepDive on #devuan IRC reported the same problem.
Therefore I am raising the severity to important due to the effect
this has for many users.
> I don't know if the `kvm` *must* be a system group but things
> have been working fine for me with a non-system group. As long as
> a non-system `kvm` group exists, configuring `eudev` will fail.
> This is easily confirmed with
>
> $ sudo dpkg-reconfigure eudev
> The group `kvm' already exists and is not a system group. Exiting.
>
> Changing the group to a system group fixes this behaviour.
>
> I worked around the issue by appending `|| true` in the postscript
> to the `adduser` command that tries to create the system group.
>
> If the `kvm` group does not have to be a system group, I would expect
> configuration to succeed if a non-system `kvm` group is present.
But the code used in the postinst is problematic. The code is this.
(I know the web version does not show indentation. Imagine the code
being indented. Check the original files.)
#!/bin/sh
set -e
...
case "$1" in
configure)
...
# Add new system group used by udev rules
addgroup --quiet --system input
# Make /dev/kvm accessible to kvm group
addgroup --quiet --system kvm
# Make /dev/dri/renderD* accessible to render group
addgroup --quiet --system render
Those are unconditional additions. Which means that if the addgroup
returns a non-zero exit code then due to the set -e the configure
fails and the package is left unconfigured.
It is true that the addgroup would return 0 (success) if it would not
need to do the action. But if the user has already locally configured
an account user or group then that previously existing configuration
should be maintained.
Therefore group addition should not be unconditional. It should be
conditional upon the group not already existing. I present two
alternative examples from existing packages that handle this in two
different ways.
The first from postfix. The "try it and see" method.
cd ${CHROOT}
# make sure that the postfix user exists. Simplest portable way to check is to
# chown something, so we'll create the directories that we need here.
makedir private root:root 700
chgrp postfix private 2>/dev/null ||
addgroup --system postfix
chown postfix private 2>/dev/null ||
adduser --system --home ${CHROOT} --no-create-home --disabled-password --ingroup postfix postfix
The second from libvirt-daemon-system. The "check it and see" method.
if ! getent group libvirt >/dev/null; then
addgroup --quiet --system libvirt
fi
if ! getent group kvm >/dev/null; then
addgroup --quiet --system kvm
fi
Again for the web display please imagine the above having indentation.
And so either way seems good and acceptable. I would probably do the
same thing libvirt-daemon-system is doing as that is simple enough.
Here is a suggested fix for this.
# Add new system group used by udev rules
if ! getent group input >/dev/null; then
addgroup --quiet --system input
fi
# Make /dev/kvm accessible to kvm group
if ! getent group kvm >/dev/null; then
addgroup --quiet --system kvm
fi
# Make /dev/dri/renderD* accessible to render group
if ! getent group render >/dev/null; then
addgroup --quiet --system render
fi
Thank you for maintaining eudev in Devuan! :-)
Bob