virFileOpenForked: Fix handling of return value from virSocketSendFD()

Commit 91f4ebbac8 (v10.0.0-185-g91f4ebbac8)
changed the return value of virSocketSendFD() from 0 to 1 on success.

Unfortunately in 'virFileOpenForked' the return value was used to report
the error back to the main process from the fork'd child. As process
return codes are positive only, the code negates the value of 'ret' and
reports it. This resulted in the parent thinking the process exited with
failure:

 # virsh save avocado-vt-vm1 /mnt/save
 error: Failed to save domain 'avocado-vt-vm1' to /mnt/save
 error: Error from child process creating '/mnt/save': Unknown error 255

This error reproduces on NFS mounts with 'root_squash' enabled. I've
also observed it in one specific migration case when root_squash NFS is
used with following error:

  Failed to open file '/var/lib/libvirt/images/alpine.qcow2': Unknown error 255'

To fix the issue the code is refactored so that it doesn't actually
touch the 'ret' variable needlessly and assigns to it only on failure
cases, which prevents the '1' to be propagated to the parent process as
'255' after negating and storing in the process return code.

Fixes: 91f4ebbac8
Resolves: https://issues.redhat.com/browse/RHEL-36721
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This commit is contained in:
Peter Krempa 2024-05-22 17:43:03 +02:00
parent f63cbc7365
commit 65cdc37a7e

View File

@ -2441,8 +2441,7 @@ virFileOpenForked(const char *path,
created = true;
/* File is successfully open. Set permissions if requested. */
ret = virFileOpenForceOwnerMode(path, fd, mode, uid, gid, flags);
if (ret < 0) {
if (virFileOpenForceOwnerMode(path, fd, mode, uid, gid, flags) < 0) {
ret = -errno;
virReportSystemError(errno,
_("child process failed to force owner mode file '%1$s'"),
@ -2450,9 +2449,7 @@ virFileOpenForked(const char *path,
goto childerror;
}
ret = virSocketSendFD(pair[1], fd);
if (ret < 0) {
if (virSocketSendFD(pair[1], fd) < 0) {
ret = -errno;
virReportSystemError(errno, "%s",
_("child process failed to send fd to parent"));