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?

cheers,

--
Mauricio Faria de Oliveira
IBM Linux Technology Center

Reply via email to