* [PATCH iproute2] ipbatch: fix use of 'ip netns exec'
@ 2013-07-05 16:05 Nicolas Dichtel
2013-07-05 20:49 ` Ben Hutchings
0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Dichtel @ 2013-07-05 16:05 UTC (permalink / raw)
To: shemminger; +Cc: netdev, junwei.zhang, Nicolas Dichtel
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().
Signed-off-by: JunweiZhang <junwei.zhang@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
ip/ipnetns.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
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)
--
1.8.2.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2] ipbatch: fix use of 'ip netns exec'
2013-07-05 16:05 [PATCH iproute2] ipbatch: fix use of 'ip netns exec' Nicolas Dichtel
@ 2013-07-05 20:49 ` Ben Hutchings
2013-07-08 9:41 ` Nicolas Dichtel
0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2013-07-05 20:49 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: shemminger, netdev, junwei.zhang
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.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2] ipbatch: fix use of 'ip netns exec'
2013-07-05 20:49 ` Ben Hutchings
@ 2013-07-08 9:41 ` Nicolas Dichtel
2013-07-08 13:57 ` Ben Hutchings
0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Dichtel @ 2013-07-08 9:41 UTC (permalink / raw)
To: Ben Hutchings; +Cc: shemminger, netdev, junwei.zhang
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."
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH iproute2] ipbatch: fix use of 'ip netns exec'
2013-07-08 9:41 ` Nicolas Dichtel
@ 2013-07-08 13:57 ` Ben Hutchings
0 siblings, 0 replies; 4+ messages in thread
From: Ben Hutchings @ 2013-07-08 13:57 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: shemminger, netdev, junwei.zhang
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.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-08 13:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-05 16:05 [PATCH iproute2] ipbatch: fix use of 'ip netns exec' Nicolas Dichtel
2013-07-05 20:49 ` Ben Hutchings
2013-07-08 9:41 ` Nicolas Dichtel
2013-07-08 13:57 ` Ben Hutchings
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).