xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>
Subject: [PATCH 06/11] libxl: Expose functions for helping with subprocesses.
Date: Thu, 25 Mar 2010 19:04:09 +0000	[thread overview]
Message-ID: <1269543854-7780-7-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1269543854-7780-1-git-send-email-ian.jackson@eu.citrix.com>

 * Expose libxl_fork in libxl_utils.h
 * Expose libxl_pipe in libxl_utils.h
 * Make libxl_exec put SIGPIPE back (so that libxl callers may
    have SIGPIPE ignored)

xl would like to use libxl_fork (which is like fork(2) except that it
logs errors) and also a similar function libxl_pipe.  So put these in
libxl_utils.[ch] and use them in libxl.c as appropriate, to avoid
having to duplicate code between xl and libxl.

Also, make sure that subprocesses spawned by libxl have SIGPIPE set
back to SIG_DFL as they are entitled to expect.  This means that a
libxl caller which sets SIGPIPE to SIG_IGN is no longer buggy.  (This
is relevant for xl migration, because xl would like to be such a
caller.)

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tools/libxl/libxl.c       |   12 +++---------
 tools/libxl/libxl_exec.c  |   18 +++++-------------
 tools/libxl/libxl_utils.c |   23 +++++++++++++++++++++++
 tools/libxl/libxl_utils.h |    4 ++++
 4 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 59859ab..711f0a0 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1238,15 +1238,9 @@ int libxl_device_disk_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_di
                 char buf[1024], *dev;
                 dev = get_blktap2_device(ctx, disk->physpath, device_disk_string_of_phystype(disk->phystype));
                 if (dev == NULL) {
-                    if (pipe(p) < 0) {
-                        XL_LOG(ctx, XL_LOG_ERROR, "Failed to create a pipe");
-                        return -1;
-                    }
-                    rc = fork();
-                    if (rc < 0) {
-                        XL_LOG(ctx, XL_LOG_ERROR, "Failed to fork a new process");
-                        return -1;
-                    } else if (!rc) { /* child */
+                    rc= libxl_pipe(ctx, p);  if (rc==-1) return -1;
+                    rc= libxl_fork(ctx);  if (rc==-1) return -1;
+                    if (!rc) { /* child */
                         int null_r, null_w;
                         char *args[4];
                         args[0] = "tapdisk2";
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index e8cc74c..6d4a5c5 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -30,19 +30,6 @@
 #include "libxl.h"
 #include "libxl_internal.h"
 
-static pid_t libxl_fork(struct libxl_ctx *ctx)
-{
-    pid_t pid;
-
-    pid = fork();
-    if (pid == -1) {
-        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "fork failed");
-        return -1;
-    }
-
-    return pid;
-}
-
 static int call_waitpid(pid_t (*waitpid_cb)(pid_t, int *, int), pid_t pid, int *status, int options)
 {
     return (waitpid_cb) ? waitpid_cb(pid, status, options) : waitpid(pid, status, options);
@@ -61,6 +48,11 @@ void libxl_exec(int stdinfd, int stdoutfd, int stderrfd, char *arg0, char **args
         dup2(stderrfd, STDERR_FILENO);
     for (i = 4; i < 256; i++)
         close(i);
+
+    signal(SIGPIPE, SIG_DFL);
+    /* in case our caller set it to IGN.  subprocesses are entitled
+     * to assume they got DFL. */
+
     execv(arg0, args);
     _exit(-1);
 }
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 3a1f095..c6989ce 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -280,3 +280,26 @@ int libxl_read_file_contents(struct libxl_ctx *ctx, const char *filename,
 
 READ_WRITE_EXACTLY(read, 1, /* */)
 READ_WRITE_EXACTLY(write, 0, const)
+
+
+pid_t libxl_fork(struct libxl_ctx *ctx)
+{
+    pid_t pid;
+
+    pid = fork();
+    if (pid == -1) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "fork failed");
+        return -1;
+    }
+
+    return pid;
+}
+
+int libxl_pipe(struct libxl_ctx *ctx, int pipes[2])
+{
+    if (pipe(pipes) < 0) {
+        XL_LOG(ctx, XL_LOG_ERROR, "Failed to create a pipe");
+        return -1;
+    }
+    return 0;
+}
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 65e7e31..fe8b975 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -44,5 +44,9 @@ int libxl_write_exactly(struct libxl_ctx *ctx, int fd, const void *data,
    * logged using filename (which is only used for logging) and what
    * (which may be 0). */
     
+pid_t libxl_fork(struct libxl_ctx *ctx);
+int libxl_pipe(struct libxl_ctx *ctx, int pipes[2]);
+  /* Just like fork(2), pipe(2), but log errors. */
+
 #endif
 
-- 
1.5.6.5

  parent reply	other threads:[~2010-03-25 19:04 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
2010-03-25 19:04 ` [PATCH 01/11] libxl: Make logging functions preserve errno Ian Jackson
2010-03-25 19:04 ` [PATCH 02/11] libxl: Report error if logfile rotation fails Ian Jackson
2010-03-25 19:04 ` [PATCH 03/11] libxl: New utility functions in for reading and writing files Ian Jackson
2010-03-25 19:04 ` [PATCH 04/11] libxl: libxl_domain_restore: Put fd back to blocking mode Ian Jackson
2010-03-25 19:04 ` [PATCH 05/11] libxl: Provide libxl_domain_rename Ian Jackson
2010-03-26 10:01   ` Vincent Hanquez
2010-03-26 10:23     ` Ian Jackson
2010-03-26 10:34       ` Vincent Hanquez
2010-03-26 10:40         ` Ian Jackson
2010-04-01 11:00         ` Stefano Stabellini
2010-03-25 19:04 ` Ian Jackson [this message]
2010-03-25 19:04 ` [PATCH 07/11] libxl: Expose libxl_report_exitstatus Ian Jackson
2010-03-25 19:04 ` [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user Ian Jackson
2010-03-26 10:06   ` Vincent Hanquez
2010-03-26 10:20     ` Ian Jackson
2010-03-26 10:29       ` Vincent Hanquez
2010-03-26 10:34         ` Ian Jackson
2010-03-26 10:41           ` Vincent Hanquez
2010-03-26 10:45             ` Ian Jackson
2010-03-26 11:27               ` Vincent Hanquez
2010-03-26 11:56                 ` Ian Jackson
2010-04-01 11:04       ` Stefano Stabellini
2010-03-25 19:04 ` [PATCH 09/11] xl: New savefile format. Save domain config when saving a domain Ian Jackson
2010-04-01 11:08   ` Stefano Stabellini
2010-03-25 19:04 ` [PATCH 10/11] xl: Domain creation logging fixes Ian Jackson
2010-03-25 19:04 ` [PATCH 11/11] xl: Migration support Ian Jackson
2010-04-01 11:10   ` Stefano Stabellini
2010-04-01 11:10 ` [PATCH 00/11] Migration support for "xl" Stefano Stabellini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1269543854-7780-7-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).