[PATCH 04/10] util: preserve exit status from mount namespace callback

[Eric Blake <eblake@xxxxxxxxxx>]

The documentation of namespace callbacks was inconsistent on whether
it preserved positive return values.  Now that we have a dedicated
EXIT_CANCELED to flag all errors before getting to the callback,
it is possible to use positive return values (not that any of the
current callers do, but it better matches the docs).

Also, while vircommand.c is careful to close fds that a child should
not have, it's still better to be in the practice of setting
FD_CLOEXEC up front.

* src/util/virprocess.c (virProcessRunInMountNamespace): Tweak
return value to pass back non-zero status.  Avoid leaking pipe fds
to other threads.
* src/util/virprocess.h: Fix comment.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
 src/util/virprocess.c | 19 +++++++++++--------
 src/util/virprocess.h |  2 +-
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 68c4c14..bd406db 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -923,9 +923,9 @@ static int virProcessNamespaceHelper(int errfd,

 /* Run cb(opaque) in the mount namespace of pid.  Return -1 with error
  * message raised if we fail to run the child, if the child dies from
- * a signal, or if the child has status 1; otherwise return the exit
- * status of the child. The callback will be run in a child process
- * so must be careful to only use async signal safe functions.
+ * a signal, or if the child has status EXIT_CANCELED; otherwise return
+ * the exit status of the child. The callback will be run in a child
+ * process so must be careful to only use async signal safe functions.
  */
 int
 virProcessRunInMountNamespace(pid_t pid,
@@ -936,7 +936,7 @@ virProcessRunInMountNamespace(pid_t pid,
     pid_t child = -1;
     int errfd[2] = { -1, -1 };

-    if (pipe(errfd) < 0) {
+    if (pipe2(errfd, O_CLOEXEC) < 0) {
         virReportSystemError(errno, "%s",
                              _("Cannot create pipe for child"));
         return -1;
@@ -946,7 +946,7 @@ virProcessRunInMountNamespace(pid_t pid,

     if (ret < 0 || child < 0) {
         if (child == 0)
-            _exit(1);
+            _exit(EXIT_CANCELED);

         /* parent */
         virProcessAbort(child);
@@ -958,13 +958,16 @@ virProcessRunInMountNamespace(pid_t pid,
         ret = virProcessNamespaceHelper(errfd[1], pid,
                                         cb, opaque);
         VIR_FORCE_CLOSE(errfd[1]);
-        _exit(ret < 0 ? 1 : 0);
+        _exit(ret < 0 ? EXIT_CANCELED : ret);
     } else {
         char *buf = NULL;
-        VIR_FORCE_CLOSE(errfd[1]);
+        int status;

+        VIR_FORCE_CLOSE(errfd[1]);
         ignore_value(virFileReadHeaderFD(errfd[0], 1024, &buf));
-        ret = virProcessWait(child, NULL);
+        ret = virProcessWait(child, &status);
+        if (!ret)
+            ret = status == EXIT_CANCELED ? -1 : status;
         VIR_FREE(buf);
     }

diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index b96dbd4..abe3635 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -67,7 +67,7 @@ int virProcessSetMaxFiles(pid_t pid, unsigned int files);
  * pid.  This function must use only async-signal-safe functions, as
  * it gets run after a fork of a multi-threaded process.  The return
  * value of this function is passed to _exit(), except that a
- * negative value is treated as an error.  */
+ * negative value is treated as EXIT_CANCELED.  */
 typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);

 int virProcessRunInMountNamespace(pid_t pid,
-- 
1.8.5.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list