Re: [libvirt] [PATCH 2/3] new virSetUIDGID() utility function

On 12/23/2010 11:39 AM, Laine Stump wrote: > virSetUIDGID() sets both the real and effective group and user of the > process, and additionally calls initgroups() to assure that the > process joins all the auxiliary groups that the given uid is a member > of. > --- > src/libvirt_private.syms | 1 + > src/util/util.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++ > src/util/util.h | 2 + > 3 files changed, 66 insertions(+), 0 deletions(-) I'm guessing that the only code that called this previously was the qemu driver, in code compiled only for Linux (as the qemu driver is not compiled for mingw). Ultimately, it might be nicer to find portable ways to do the equivalent of initgroups on other platforms that lack the Linux interface, but do have a way to set supplementary groups (POSIX doesn't standardize setting supplementary groups on purpose); or even better, to have gnulib implement initgroups() for as many platforms as possible. > + if (initgroups(pwd.pw_name, pwd.pw_gid)< 0) { > + virReportSystemError(errno, > + _("cannot initgroups(\"%s\", %d)"), > + pwd.pw_name, pwd.pw_gid); > + VIR_FREE(buf); > + return -1; > + } My biggest worry is that checking this in will cause compilation failures on other platforms, so here's hoping we can get the word out that we need testing, or even modify this patch to add a configure.ac AC_CHECK_FUNCS_ONCE([initgroups]) and bracket the initgroups() call within #ifdef HAVE_INITGROUPS (non-Linux platforms won't set supplementary groups, just the primary gid, but that's better than failing to compile). Can you handle that, or would you like me to do that as a followup commit? Conditional ACK, based on that answer.