This commit partially reverts

    commit c360ea28dc267802690e129fbad08ca2f22a44e9
    Refs: v6.2.0-rc1-1-gc360ea28dc
    Author:     Rafael Fonseca <r4f4rfs@gmail.com>
    AuthorDate: Fri Mar 27 18:40:47 2020 +0100
    Commit:     Michal Prívozník <mprivozn@redhat.com>
    CommitDate: Mon Mar 30 09:48:22 2020 +0200

    util: virdaemon: fix compilation on mingw

    The daemons are not supported on Win32 and therefore were not compiled
    in that platform. However, with the daemon code sharing, all the code in
    utils *is* compiled and it failed because `waitpid`, `fork`, and
    `setsid` are not available. So, as before, let's not build them on
    Win32 and make the code more portable by using existing vir* wrappers.

Not compiling virDaemonForkIntoBackground on Win32 is good, but the
second part of the original patch incorrectly replaced waitpid and fork
with our virProcessWait and virFork APIs. These APIs are more than just
simple wrappers and we don't want any of the extra functionality.
Especially virFork would reset any setup made before
virDaemonForkIntoBackground is called, such as logging, signal handling,
etc.

As a result of the change the additional fix in v6.2.0-67-ga87e4788d2
(util: virdaemon: fix waiting for child processes) is no longer
needed and it is effectively reverted by this commit.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/util/virdaemon.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/util/virdaemon.c b/src/util/virdaemon.c
index 99530fd146..6182ca3c21 100644
--- a/src/util/virdaemon.c
+++ b/src/util/virdaemon.c
@@ -20,7 +20,9 @@
 
 #include <config.h>
 
+#include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <stdbool.h>
@@ -30,8 +32,6 @@
 #include "virfile.h"
 #include "virlog.h"
 #include "viralloc.h"
-#include "virprocess.h"
-#include "vircommand.h"
 
 #include "configmake.h"
 
@@ -44,7 +44,7 @@ virDaemonForkIntoBackground(const char *argv0)
     if (virPipeQuiet(statuspipe) < 0)
         return -1;
 
-    pid_t pid = virFork();
+    pid_t pid = fork();
     switch (pid) {
     case 0:
         {
@@ -73,7 +73,7 @@ virDaemonForkIntoBackground(const char *argv0)
             if (setsid() < 0)
                 goto cleanup;
 
-            nextpid = virFork();
+            nextpid = fork();
             switch (nextpid) {
             case 0: /* grandchild */
                 return statuspipe[1];
@@ -97,14 +97,15 @@ virDaemonForkIntoBackground(const char *argv0)
     default:
         {
             /* parent */
-            int exitstatus = 0;
+            int got, exitstatus = 0;
             int ret;
             char status;
 
             VIR_FORCE_CLOSE(statuspipe[1]);
 
             /* We wait to make sure the first child forked successfully */
-            if (virProcessWait(pid, &exitstatus, 0) < 0 ||
+            if ((got = waitpid(pid, &exitstatus, 0)) < 0 ||
+                got != pid ||
                 exitstatus != 0) {
                 goto error;
             }
-- 
2.26.1

On 4/16/20 3:49 PM, Jiri Denemark wrote:
> This commit partially reverts
> 
>      commit c360ea28dc267802690e129fbad08ca2f22a44e9
>      Refs: v6.2.0-rc1-1-gc360ea28dc
>      Author:     Rafael Fonseca <r4f4rfs@gmail.com>
>      AuthorDate: Fri Mar 27 18:40:47 2020 +0100
>      Commit:     Michal Prívozník <mprivozn@redhat.com>
>      CommitDate: Mon Mar 30 09:48:22 2020 +0200
> 
>      util: virdaemon: fix compilation on mingw
> 
>      The daemons are not supported on Win32 and therefore were not compiled
>      in that platform. However, with the daemon code sharing, all the code in
>      utils *is* compiled and it failed because `waitpid`, `fork`, and
>      `setsid` are not available. So, as before, let's not build them on
>      Win32 and make the code more portable by using existing vir* wrappers.
> 
> Not compiling virDaemonForkIntoBackground on Win32 is good, but the
> second part of the original patch incorrectly replaced waitpid and fork
> with our virProcessWait and virFork APIs. These APIs are more than just
> simple wrappers and we don't want any of the extra functionality.
> Especially virFork would reset any setup made before
> virDaemonForkIntoBackground is called, such as logging, signal handling,
> etc.
> 
> As a result of the change the additional fix in v6.2.0-67-ga87e4788d2
> (util: virdaemon: fix waiting for child processes) is no longer
> needed and it is effectively reverted by this commit.
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>   src/util/virdaemon.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)

Ooops.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal