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

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