Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. Xen>
  3. Devel
[PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec
roger.pau at entel
Nov 18, 2011, 3:59 AM
Post #1 of 6 (707 views)
Permalink
# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1321609614 -3600
# Node ID ec94a7e4983ad6338db20fa0777a85507b582607
# Parent 9e8abd626484f82a95d0edc07834ae287bc9467a
libxl: add libxl__forkexec function to libxl_exec

Add a new function to libxl_exec that performs a fork and executes
the passed arguments using libxl__exec.

Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>

diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_exec.c
--- a/tools/libxl/libxl_exec.c Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_exec.c Fri Nov 18 10:46:54 2011 +0100
@@ -450,6 +450,40 @@ int libxl__spawn_check(libxl__gc *gc, li
return ERROR_FAIL;
}

+int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
+ int stderrfd, char **args)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ int status;
+ int rc = 0;
+ pid_t pid = fork();
+
+ switch (pid) {
+ case -1:
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "fork failed\n");
+ rc = -1;
+ break;
+ case 0:
+ libxl__exec(stdinfd, stdoutfd, stderrfd, args[0], args);
+ /* libxl__exec never returns */
+ default:
+ while (waitpid(pid, &status, 0) < 0) {
+ if (errno != EINTR) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "waitpid failed\n");
+ rc = -1;
+ break;
+ }
+ }
+ if (WIFEXITED(status)) {
+ rc = WEXITSTATUS(status);
+ } else {
+ rc = -1;
+ }
+ break;
+ }
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Fri Sep 30 14:38:55 2011 +0200
+++ b/tools/libxl/libxl_internal.h Fri Nov 18 10:46:54 2011 +0100
@@ -400,6 +400,20 @@ _hidden int libxl__spawn_check(libxl__gc
_hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
const char *arg0, char **args); // logs errors, never returns

+/*
+ * libxl__forkexec - Executes a file synchronously
+ * gc: allocation pool
+ * stdinfd, stdoutfd, stderrfd: fds to pass to libxl__exec
+ * args: file to execute and arguments to pass in the following format
+ * args[0]: file to execute
+ * args[1]: first argument to pass to the called program
+ * args[n]: nth argument to pass to the called program
+ *
+ * Returns 0 on success, and < 0 on error.
+ */
+_hidden int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
+ int stderrfd, char **args);
+
/* from xl_create */
_hidden int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info, uint32_t *domid);
_hidden int libxl__domain_build(libxl__gc *gc,

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec [ In reply to ]
Ian.Campbell at citrix
Nov 18, 2011, 5:31 AM
Post #2 of 6 (692 views)
Permalink
On Fri, 2011-11-18 at 11:59 +0000, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@entel.upc.edu>
> # Date 1321609614 -3600
> # Node ID ec94a7e4983ad6338db20fa0777a85507b582607
> # Parent 9e8abd626484f82a95d0edc07834ae287bc9467a
> libxl: add libxl__forkexec function to libxl_exec
>
> Add a new function to libxl_exec that performs a fork and executes
> the passed arguments using libxl__exec.
>
> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
>
> diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_exec.c
> --- a/tools/libxl/libxl_exec.c Fri Sep 30 14:38:55 2011 +0200
> +++ b/tools/libxl/libxl_exec.c Fri Nov 18 10:46:54 2011 +0100
> @@ -450,6 +450,40 @@ int libxl__spawn_check(libxl__gc *gc, li
> return ERROR_FAIL;
> }
>
> +int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
> + int stderrfd, char **args)
> +{
> + libxl_ctx *ctx = libxl__gc_owner(gc);
> + int status;
> + int rc = 0;
> + pid_t pid = fork();
> +
> + switch (pid) {
> + case -1:
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "fork failed\n");
> + rc = -1;
> + break;
> + case 0:
> + libxl__exec(stdinfd, stdoutfd, stderrfd, args[0], args);
> + /* libxl__exec never returns */
> + default:
> + while (waitpid(pid, &status, 0) < 0) {
> + if (errno != EINTR) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "waitpid failed\n");
> + rc = -1;
> + break;
> + }
> + }
> + if (WIFEXITED(status)) {
> + rc = WEXITSTATUS(status);
> + } else {
> + rc = -1;
> + }
> + break;
> + }
> + return rc;
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff -r 9e8abd626484 -r ec94a7e4983a tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h Fri Sep 30 14:38:55 2011 +0200
> +++ b/tools/libxl/libxl_internal.h Fri Nov 18 10:46:54 2011 +0100
> @@ -400,6 +400,20 @@ _hidden int libxl__spawn_check(libxl__gc
> _hidden void libxl__exec(int stdinfd, int stdoutfd, int stderrfd,
> const char *arg0, char **args); // logs errors, never returns
>
> +/*
> + * libxl__forkexec - Executes a file synchronously
> + * gc: allocation pool
> + * stdinfd, stdoutfd, stderrfd: fds to pass to libxl__exec
> + * args: file to execute and arguments to pass in the following format
> + * args[0]: file to execute
> + * args[1]: first argument to pass to the called program
> + * args[n]: nth argument to pass to the called program

args[n] must be NULL, right?

> + *
> + * Returns 0 on success, and < 0 on error.
> + */
> +_hidden int libxl__forkexec(libxl__gc *gc, int stdinfd, int stdoutfd,
> + int stderrfd, char **args);
> +
> /* from xl_create */
> _hidden int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info, uint32_t *domid);
> _hidden int libxl__domain_build(libxl__gc *gc,
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec [ In reply to ]
roger.pau at entel
Nov 18, 2011, 6:50 AM
Post #3 of 6 (688 views)
Permalink
2011/11/18 Ian Campbell <Ian.Campbell@citrix.com>:
> args[n] must be NULL, right?

My bad, fixed it already.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec [ In reply to ]
Ian.Jackson at eu
Nov 24, 2011, 10:11 AM
Post #4 of 6 (689 views)
Permalink
Roger Pau Monne writes ("[Xen-devel] [PATCH 3 of 9 v2] libxl: add libxl__forkexe> + while (waitpid(pid, &status, 0) < 0) {
> + if (errno != EINTR) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "waitpid failed\n");
> + rc = -1;
> + break;
> + }
> + }

It is good to see the exit status actually being retrieved and
checked. But:

> + if (WIFEXITED(status)) {
> + rc = WEXITSTATUS(status);
> + } else {
> + rc = -1;
> + }

I don't think this is the right thing to do with it.

Full and proper checking of a subprocess exit status involves:
* checking for WIFSIGNALED too and perhaps calling strsignal
* printing a message somewhere

I have found that the best way to deal with this is to split this up
into two parts:
- Functions which pass back the wait status without interpreting it,
and leaves checking it up to the caller
- A generic function for reporting the meaning of an exit status
to the log

It's also OK to have convenience functions which bundle these two
together, but the actual checking of an exit status involves logging.

Conveniently, I have already written the second function for you:
libxl_report_child_exitstatus :-).

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec [ In reply to ]
roger.pau at entel
Dec 1, 2011, 6:29 AM
Post #5 of 6 (682 views)
Permalink
2011/11/24 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> Conveniently, I have already written the second function for you:
> libxl_report_child_exitstatus :-).

Why does the libxl_report_child_exitstatus function print
"unexpectedly exited status zero", shouldn't it print something like
"child exited successfully"? Or I'm not supposed to call it if
WIFEXITED(status) && WEXITSTATUS(status) == 0?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec [ In reply to ]
Ian.Jackson at eu
Dec 1, 2011, 6:57 AM
Post #6 of 6 (682 views)
Permalink
Roger Pau Monné writes ("Re: [Xen-devel] [PATCH 3 of 9 v2] libxl: add libxl__forkexec function to libxl_exec"):
> 2011/11/24 Ian Jackson <Ian.Jackson@eu.citrix.com>:
> > Conveniently, I have already written the second function for you:
> > libxl_report_child_exitstatus :-).
>
> Why does the libxl_report_child_exitstatus function print
> "unexpectedly exited status zero", shouldn't it print something like
> "child exited successfully"? Or I'm not supposed to call it if
> WIFEXITED(status) && WEXITSTATUS(status) == 0?

You're not supposed to call it in that case, unless you weren't
expecting the process to exit.

And you can write
WIFEXITED(status) && WEXITSTATUS(status) == 0
more simply as
status==0
so this is quite easy to do.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal