[libvirt] [PATCH] Updated to deal with specifying user IDs to that do not map to usernames

[Daniel P. Berrange]

On Tue, Jun 07, 2016 at 04:16:46PM +0100, Daniel P. Berrange wrote:
> On Tue, Jun 07, 2016 at 05:15:54PM +0200, Peter Krempa wrote:
> > On Tue, Jun 07, 2016 at 15:50:38 +0100, Daniel Berrange wrote:
> > > On Tue, Jun 07, 2016 at 08:24:14AM +0200, Peter Krempa wrote:
> > > > On Mon, Jun 06, 2016 at 14:25:23 -0500, Roy Keene wrote:
> > > > > Patch to libvirt master to avoid failing when a user ID is specified, 
> > > > > e.g. for <seclabel type='dac'>, that does not map to a user name.
> > > > > 
> > > > > This is useful if you want to run each VM as a separate user and not 
> > > > > bother creating an /etc/passwd entry for each UID.
> > > > 
> > > > For this use case you shall prefix the name with a +. Please refer to
> > > > the documentation on seclabels.
> > > 
> > > Empirically that does not currently work:
> > > 
> > > # virsh dumpxml serial | grep --after 2 '<seclabel'
> > >   <seclabel type='static' model='dac' relabel='no'>
> > >     <label>+21421:+12421421</label>
> > >   </seclabel>
> > > 
> > > # virsh start serial
> > > error: Failed to start domain serial
> > > error: Cannot access storage file '/var/lib/libvirt/images/demo.qcow2' (as uid:21421, gid:12421421): Success
> > > 
> > > # ls -al /var/lib/libvirt/images/demo.qcow2
> > > -rw-r--r--. 1 21421 12421421 197120 Apr 30  2015 /var/lib/libvirt/images/demo.qcow2
> > > 
> > > 
> > > Looking at the libvirtd logs we see
> > > 
> > > 2016-06-07 14:49:13.724+0000: 13490: debug : qemuDomainCheckDiskPresence:3954 : Checking for disk presence
> > > 2016-06-07 14:49:16.551+0000: 13490: error : virGetUserEnt:801 : Failed to find user record for uid '21421'
> > > 2016-06-07 14:49:16.551+0000: 13490: error : virStorageFileGetMetadataRecurse:3114 : Cannot access storage file '/var/lib/libvirt/images/demo.qcow2' (as uid:21421, gid:12421421): Success
> > > 
> > > 
> > > So even though the QEMU driver has honoured the '+' syntax, some of the
> > > things QEMU is calling appears to be trying to resolve the UID back into
> > > a user password record and failing.
> > 
> > Indeed.  This is caused by calling virFileAccessibleAs which calls
> > virGetGroupList so that it can add all groups for the given UID which is
> > very strange. The group list is then set after forking at attempting to
> > check file presence. I hate root squashed NFS.
> > 
> > I guess we could just skip reporting the error if we can't get the
> > group list in that case and just set the provided numerical UID/GID and
> > try it that way.
> 
> Yep, if all we're doing is trying to get supplementary groups, then
> lack of an /etc/passwd or /etc/group entry really just semantically
> means zero supplementary groups are needed.

Roy, perhaps test if the following change makes it work for you:

diff --git a/src/util/virutil.c b/src/util/virutil.c
index d80d994..20a426d 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1109,8 +1109,11 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
     if (uid == (uid_t)-1)
         return 0;
 
-    if (virGetUserEnt(uid, &user, &primary, NULL) < 0)
-        return -1;
+    if (virGetUserEnt(uid, &user, &primary, NULL) < 0) {
+        VIR_INFO("No passwd entry for uid %llu, setting empty group list"
+                 (unsigned long long)uid);
+        return 0;
+    }
 
     ret = mgetgroups(user, primary, list);
     if (ret < 0) {


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|