On 10/05/2012 06:53 AM, Marcelo Cerri wrote:
Just one question: do you think that makes sense for virGetUserID and virGetGroupID to check if a id exists when a numeric value is given?
Just because a uid hasn't been named in /etc/passwd doesn't mean it won't be added in the future. If the user requests a numeric value, I think we have to respect it as a uid_t value (assuming the name lookup failed or the leading '+' was present) regardless of whether we can do a lookup on that uid. One other thing - uid_t might be smaller than 'int', so we have to do overflow checking and make sure that a user requesting 65536 doesn't end up mapping to uid 0.
...out of security_dac.c and into src/util/util.c:virGetUserID(), so that we are consistently parsing in this manner for ALL places where we convert a string into a user id, and also so that virGetUserID will quit logging such a bogus error message when it fails to find a given id string that happens to be a valid number. Ok. I'll provide a patch for this.
I made a search in the code for virGetUserID and, other than in security_dac.c, it seems to be used just for parsing user name in qemu.conf.
I'm actually in favor of this change - it means qemu.conf also gains our consistent behavior (and consistent with coreutils' chown) for handling names vs. numbers. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org