[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 2/3] util: add virGetGroupList



On 07/10/2013 12:23 PM, Eric Blake wrote:
> On 07/09/2013 11:20 PM, Laine Stump wrote:
>> On 07/09/2013 09:17 PM, Eric Blake wrote:
>>> Since neither getpwuid_r() nor initgroups() are safe to call in
>>> between fork and exec (they obtain a mutex, but if some other
>>> thread in the parent also held the mutex at the time of the fork,
>>> the child will deadlock), we have to split out the functionality
>>> that is unsafe.  At least glibc's initgroups() uses getgrouplist
>>> under the hood, so the ideal split is to expose getgrouplist for
>>> use before a fork.  Gnulib already gives us a nice wrapper via
>>> mgetgroups; we wrap it once more to look up by uid instead of name.
>>>
>>> * .gnulib: Update for mgetgroups.
>>> * bootstrap.conf (gnulib_modules): Add mgetgroups.
>>> * src/util/virutil.h (virGetGroupList): New declaration.
>>> * src/util/virutil.c (virGetGroupList): New function.
>>> * src/libvirt_private.syms (virutil.h): Export it.
>>>
>>> Signed-off-by: Eric Blake <eblake redhat com>
>>> ---
>>> +++ b/.gnulib
>>> @@ -1 +1 @@
>>> -Subproject commit b72ff2a45efde544c406804186d37a3254728571
>>> +Subproject commit da8d59ee79138b85daee1ad5b22ea12e4fb6ce47
>>
>> Are you sure you want to update the gnulib version in this same commit?
>> Although it is necessary for the rest of this patch to work, there are
>> probably other changes in gnulib that could potentially cause a
>> regression unrelated to the libvirt parts of the patch. Just for the
>> sake of bisecting, it seems like a good idea to update gnulib in a
>> separate prerequisite patch.
> Good point - especially since I _will_ be backporting this series, and
> we tend to eschew gnulib submodule updates on backports not tied to a
> stable release branch.
>
>> ACK to the new function. I do think the gnulib update should be a
>> separate commit, though.
> Do you need to see a v3, or should I just go ahead and make that split?
>

Just making the split is fine. We all know what it will look like :-)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]