[libvirt] [PATCH 0/7] Resolve issues saving domain to NFS root squashed client

[John Ferlan]

Lots of details here:

https://bugzilla.redhat.com/show_bug.cgi?id=1158034

During my investigation I uncovered some other "issues" which are covered
by the first 4 patches. The issues are error path related.

The first two patches could be combined, but for now they're apart so
that it's proveable that they resolve a single issue. Hopefully mkletzan
can take a look at these to ensure they fit with the "spirit" of the
changes they're fixing. With these fixes in place the error from virfile
through qemu will be displayed...

The third and fourth patches can be combined.  Realistically only the
third one is necessary as it resolves an issue introduced by the ref'd
gnulib change (eblake has already read about it). The fourth takes eblake's
suggestion to me in order to take the next step to avoid the throw-away
waitpid loop in virFileOpenForked and replace it by more inline logic.
Since this is code introduced by laine, I'm hoping he'll be able to take
a look-see. It's not required for the series, but nice to have. The
third patch allows us to be able to get the "correct" EACCES error back
from the vfork'd child, while the forth tries to make it clearer.

The fifth and sixth patches resolve the problem from the above ref'd
bz. Essentially what's happening is that when there's a root squashed
nfs client to which the 'virsh save $dom $file' is being saved, the
attempt by the second virOpenFileAs to open or chmod the $file can
fail because the file generated by the first call may not be able to
be opened, written, or chown'd due to whatever was set by that first call.
The fifth patch is there because I found that if a 'save' file existed
initially and a 'virsh save $dom $file' failed - the $file was deleted
(and of course the subsequent 'virsh save $dom $file' would succeed).
Since we didn't create the file, we shouldn't delete it 

The seventh patch isn't required, but I was hoping it might generate
some discussion as to whether it's the right thing to do. If we don't
set the VIR_FILE_OPEN_FORCE_MODE flag on the virFileOpenAs call, then
the subsequent virFileOpenForceOwnerMode will not change the protection
bits. So why pass something different if the end result is not change?
I looked at other virOpenFileAs paths that are creating and found only
one where the bit was set in virStorageBackendCreateRaw.  Perhaps this
patch generates other patches to "really" save mode bits when CREAT is
being used.

An eight patch was considered, but I dropped it. I was going to add
an S_IROTH to the bits being passed on that nfs root squash path. This
was being considered because the above ref'd bz uses an nfs pool in
order to save the file into. That pool used different protections on
files when first created and once/if a pool-refresh or libvirtd restart/
reload is done - the pool will fail to start because of the save file's
protections. Setting other+read "resolved" or "work-around" that, but
I don't really think that's the right thing to do.  Altough, it is
NFS and well not necessarily secure.


John Ferlan (7):
  qemu: Save/restore error in error path for qemuDomainSaveInternal
  qemu: Adjust EndAsyncJob for qemuDomainSaveInternal error path
  virfile: Need to check for ENOTCONN from recvfd failure
  virfile: Adjust error path for virFileOpenForked
  qemu: Don't unconditionally delete file in qemuOpenFileAs
  qemu: remove failed create prior to root squash share path
  qemu: qemuOpenFileAs - set flag VIR_FILE_OPEN_FORCE_MODE

 src/qemu/qemu_driver.c | 24 +++++++++++++++---
 src/util/virfile.c     | 66 +++++++++++++++++++++++++++++---------------------
 2 files changed, 60 insertions(+), 30 deletions(-)

-- 
2.1.0