[libvirt] [PATCH 0/3] Patchset to avoid uid problems related to root-squash NFS

[Laine Stump]

The first two patches add some utility functions, obsoleting the
earlier two patches I sent to the list on Jan 8:

 https://www.redhat.com/archives/libvir-list/2010-January/msg00114.html

The third patch uses these new utilities to rework the storage volume
build functions to eliminate problems with pools located on
root-squashing NFS servers.

Note that the method of setting uid and gid of the created volumes
remains the same in all cases where the pool containing the volume
isn't type netfs - the volume will be created, then chowned. Only in
the case that the volume will be in a pool of type netfs do we use a
new method - first fork() and setuid/setgid in the child process
before creating the volume. After returning from the child process, a
couple extra steps are taken - 1) check if the child failed to create
the new volume, and if so, try again as root, and 2) check if the
volume really was created with the desired uid/gid, and if not, try
setting it with chown. (This final step solves the problem of a parent
directory that has the setgid bit on - simple calling setgid on the
process that creates the file will then have no effect on the gid of
the new file - it must be set explicitly after creation).

The one problem I have with this final patch is the way that I
determine if the pool containing the volume is of type netfs. The
virStorageVolDef itself has no information about the pool; that
information is only known a few layers up the call chain when someone
is getting the buildVol or .buildVolFrom memor of a virStorageBackend
struct. The way that I found to maintain the information of pool type
(or more precisely, whether this is a netfs pool or something else)
was to make buildVol and buildVolFrom point to different functions in
the case of virStorageBackenNetFilesystem (it previously pointed to
the same function as for type dir and fs). Since that first level
function is just a stub that calls
_virStorageBackendFileSystemVolBuild with a flag that is always 0, and
since _virStorageBackendFileSystemVolBuild then calls the actual
"create_func" with that same flag, AND since flag is currently unused
by any of these functions, I just have the toplevel stub function set
the bit "VIR_STORAGE_BUILD_NETFS" in the flag, and use that
information when the time comes in the individual create_funcs.

I don't like this because as far as I know, the flags are really there
to allow for future expansion in the public API, and I've now
subverted that usage 2/3 of the way down the call chain.

So, I would appreciate either some help in deciding the best way for
lower level functions (like the functions pointed to by create_func)
to learn the type of the pool that will contain the volume pointed to
by the virStorageVolDefPtr, or alternately affirmation that my
subversion of the existing flags is acceptable.

P.S. I have tested creation of raw and qcow volumes in several
differnet scenarios (eg, parent directory isn't owned by the desired
uid, creating a need to fall back to original method, parent directory
has setgid bit on, etc.) with liberal DEBUGs throughout the code and
verified that pretty much every path works.