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