Hi Mauricio, On Mon, Nov 14, 2016 at 05:10:01PM -0200, Mauricio Faria de Oliveira wrote: > Hi Guido, > > Thanks for the review. > > On 11/14/2016 03:54 PM, Guido Günther wrote: > > > +# Allocated UID and GID for libvirt-qemu > > > > +libvirt_qemu_uid=64055 > > > > +libvirt_qemu_gid=64055 > > Please use all caps for the variable names. > > Okay; including the "parameter_(u|g)id" variables below. > (like another function that uses all caps for variables > in the function-scope that are not for-loops variables) > > > > > + # set uid if available (expected); don't fail otherwise. > > > > + parameter_uid='' > > > > + if ! getent passwd $libvirt_qemu_uid >/dev/null; then > > > > + parameter_uid="--uid $libvirt_qemu_uid" > > > > + fi > > > i wonder if _silently_ ignoring uid because it's already taken is the > > right action. Did you check what other packages with reserved uids/gids > > do in this case? > > Nice catch. > > Looking at the list, the ones which still use the allocated uid/gid > (grep'ing for adduser, addgroup, uid, gid) > - netqmail and plan: abort the installation > - linux-grsec-base: silently proceeds w/out groups (addgroup || true) > > > We should at least put out a warning or fail (which might not be > > nice since the problem might not be easily solvable by the person > > installing the package e.g. if users come from LDAP). > > Sure. What do you think of a debconf warning message/prompt, which > asks the user to confirm that it's OK not to use the uid/gid values, > and explains about the potential problem w/ guest migration over NFS?
Although it adds some complexity it makes the most sense. Are you going to look into this? Cheers, -- Guido
