[Pkg-libvirt-maintainers] Bug#844339: [PATCH v2 2/4] libvirt-daemon-system.{config, templates, postinst}: warn if allocated uid/gid cannot be used

[Guido Günther]

Hi Mauricio,

On Thu, Nov 17, 2016 at 07:53:55PM -0200, Mauricio Faria de Oliveira wrote:
> On 11/17/2016 05:37 PM, Guido Günther wrote:
> > I'm basically fine with all of this (but did not du any actual testing)
> 
> Cool.
> 
> > but we should not warn if the user/group already exists (even with the
> > wrong uid). Otherwise we'd warn over and over again, we don't want to
> > force users to change existing installations.
> 
> Actually the warning is not repeated; the answer is saved by debconf.

Debconf is not a registry that is you must not rely on the debconf
database saving values for you:

    https://lintian.debian.org/tags/debconf-is-not-a-registry.html

> If you just install/upgrade after having answered the question once,
> it doesn't show up.

And it means all users of existing installation see it although they
can't do much about it - you really don't want to change uid/gid of the
user on an installed system since this requires cleanup of disk images,
etc. - leaving the user with a broken installation.

> You only get it again in intentional cases:
> 1) dpkg-reconfigure
> 2) apt-get purge && re-install
> 3) remove its 'Flags: seen' line from /var/cache/debconf/config.dat
> 
> > If we want to notify users of existing installations we have a
> > Debian.NEWS for this that can explain that switching to uid/git 64055 is
> > recommended.
> 
> Good point. I can write a snippet for that too if you want, but not
> sure it's enough depending on your decision about how/when to notify
> users (paragraph below).

I think it's sufficient for existing installations. People will see it
and can plan for the change to happen (they obviosly didn't run into the
problem for the last 10 years).

> 
> > IMHO the only important case to warn about is the case where user or
> > group does not yet exist _and_ the uid/gid is already taken by another
> > user or group. What do you think?
> 
> Well, I still think it's also important to warn when the user/group
> already exists (with a different uid/gid).
> 
> The main reason is to help users not to hit a known problem, and help
> maintainers not to get unnecessary bug reports, or having to debug it
> again (it was hard to debug/trace this, the root cause is very subtle,
> and the stack components pile up -- I ended up strace'ing qemu, found
> EACCES in read/write syscalls, and had to understand how NFS had come
> up with this type of error -- not that I'm proud, I imagine it's easy
> for people more experienced w/ the virtualization stack; just a story).
> 
> If existing installations are never told about this, they didn't
> even have a chance to try to fix their environment not to hit it,
> and I guess it's a valid expectation from the packages to set up
> things up correctly so not to hit problems (of course this case
> is not easy/automatic to resolve..)

And that's exactly what NEWS.Debian is for. We use your code for the
error case. That is we want to do the right thing but can't since the
uid/gid is already taken.

> (some systems will never hit it because users were just created
> in the same order on source/destination hosts; but that's luck)
> 
> On the other hand, I'm not sure how disruptive such a warning is
> for automated deployments/upgrades, specially because the default
> is to abort the install (perhaps we should change it).

Exactly. That's why NEWS.Debian is nicer iff the user libvirt-qemu
exists but with "wrong" (legacy) uid/gid on existing installations and
Debconf is better for new installations where the uid/gid is already
taken.

> Anyway, just trying to provide some pondering and other thoughts/
> views on the matter, with the intention it may help :- )  Let me
> know what/how you'd like it in the patches and I'll spin v3 ;- )

Please spin a v3 ;)
Cheers,
 -- Guido