Commit 91f4ebbac81bc3829da6d5a71d7520a6fc9e358e (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: 91f4ebbac81bc3829da6d5a71d7520a6fc9e358e
Resolves: https://issues.redhat.com/browse/RHEL-36721
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/util/virfile.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index c4d22921ce..d820172405 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -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"));
-- 
2.45.1

On Wed, May 22, 2024 at 18:00:18 +0200, Peter Krempa wrote:
> Commit 91f4ebbac81bc3829da6d5a71d7520a6fc9e358e (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: 91f4ebbac81bc3829da6d5a71d7520a6fc9e358e
> Resolves: https://issues.redhat.com/browse/RHEL-36721
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/util/virfile.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>