| Message ID | 1373040345-11116-1-git-send-email-nicolas.dichtel@6wind.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Fri, 2013-07-05 at 18:05 +0200, Nicolas Dichtel wrote: > From: JunweiZhang <junwei.zhang@6wind.com> > > execvp() does not return when the command succeed, hence all commands in the > batch file after the line 'ip netns exec' are not executed. > > Let's fork before calling execvp(). A Unix shell forks every command it runs, so why should ip do this too? [...] > + return WIFEXITED(status) ? EXIT_SUCCESS : EXIT_FAILURE; [...] So you throw away the original exit code of the child process. I suspect your actual problem has to do with the exit code of the child, and your shell script contains 'set -e'. Ben.
Le 05/07/2013 22:49, Ben Hutchings a écrit : > On Fri, 2013-07-05 at 18:05 +0200, Nicolas Dichtel wrote: >> From: JunweiZhang <junwei.zhang@6wind.com> >> >> execvp() does not return when the command succeed, hence all commands in the >> batch file after the line 'ip netns exec' are not executed. >> >> Let's fork before calling execvp(). > > A Unix shell forks every command it runs, so why should ip do this too? Just to show the problem: $ cat test.batch netns add netns1 netns exec netns1 ip l netns $ ip -b test.batch 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 2: sit0: <NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT link/sit 0.0.0.0 brd 0.0.0.0 All command after 'netns exec' are never executed. With the patch: $ ip -b test.batch 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 2: sit0: <NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT link/sit 0.0.0.0 brd 0.0.0.0 netns1 Now, existing netns are displayed. > > [...] >> + return WIFEXITED(status) ? EXIT_SUCCESS : EXIT_FAILURE; > [...] > > So you throw away the original exit code of the child process. Right, should use WEXITSTATUS(). I will send a v2. > > I suspect your actual problem has to do with the exit code of the child, > and your shell script contains 'set -e'. Maybe I miss something, but man execvp says: "The exec() functions only return if an error has have occurred." -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-07-08 at 11:41 +0200, Nicolas Dichtel wrote: > Le 05/07/2013 22:49, Ben Hutchings a écrit : > > On Fri, 2013-07-05 at 18:05 +0200, Nicolas Dichtel wrote: > >> From: JunweiZhang <junwei.zhang@6wind.com> > >> > >> execvp() does not return when the command succeed, hence all commands in the > >> batch file after the line 'ip netns exec' are not executed. > >> > >> Let's fork before calling execvp(). > > > > A Unix shell forks every command it runs, so why should ip do this too? > Just to show the problem: > > $ cat test.batch > netns add netns1 > netns exec netns1 ip l > netns > $ ip -b test.batch [...] Sorry, I totally missed that there is this 'batch' sub-command in ip. I think you are right that it should fork in this case. Ben.
diff --git a/ip/ipnetns.c b/ip/ipnetns.c index fa2b681..a4a68a7 100644 --- a/ip/ipnetns.c +++ b/ip/ipnetns.c @@ -138,6 +138,7 @@ static int netns_exec(int argc, char **argv) const char *name, *cmd; char net_path[MAXPATHLEN]; int netns; + int pid, status; if (argc < 1) { fprintf(stderr, "No netns name specified\n"); @@ -185,10 +186,17 @@ static int netns_exec(int argc, char **argv) /* Setup bind mounts for config files in /etc */ bind_etc(name); - if (execvp(cmd, argv + 1) < 0) + pid = fork(); + if (pid < 0) + return EXIT_FAILURE; + else if (pid > 0) + waitpid(pid, &status, 0); + else if (execvp(cmd, argv + 1) < 0) { fprintf(stderr, "exec of \"%s\" failed: %s\n", cmd, strerror(errno)); - return EXIT_FAILURE; + return EXIT_FAILURE; + } + return WIFEXITED(status) ? EXIT_SUCCESS : EXIT_FAILURE; } static int is_pid(const char *str)