* [PATCH 0/2] Small fixes for nstool @ 2024-11-06 3:03 David Gibson 2024-11-06 3:03 ` [PATCH 1/2] test: Rename propagating signal handler David Gibson ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: David Gibson @ 2024-11-06 3:03 UTC (permalink / raw) To: Stefano Brivio, passt-dev; +Cc: David Gibson While working on the exeter tests, I noticed a couple of things missing in nstool. These make sense standalone, even if we don't have an urgent need for them without the exeter tests. David Gibson (2): test: Rename propagating signal handler test: Make nstool hold robust against interruptions to control clients test/nstool.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) -- 2.47.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] test: Rename propagating signal handler 2024-11-06 3:03 [PATCH 0/2] Small fixes for nstool David Gibson @ 2024-11-06 3:03 ` David Gibson 2024-11-06 3:03 ` [PATCH 2/2] test: Make nstool hold robust against interruptions to control clients David Gibson 2024-11-07 14:54 ` [PATCH 0/2] Small fixes for nstool Stefano Brivio 2 siblings, 0 replies; 7+ messages in thread From: David Gibson @ 2024-11-06 3:03 UTC (permalink / raw) To: Stefano Brivio, passt-dev; +Cc: David Gibson nstool in "exec" mode will propagate some signals (specifically SIGTERM) to the process in the namespace it executes. The signal handler which accomplishes this is called simply sig_handler(). However, it turns out we're going to need some other signal handlers, so rename this to the more specific sig_propagate(). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- test/nstool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index fc357d8..3f75edd 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -346,7 +346,7 @@ static int openns(const char *fmt, ...) } static pid_t sig_pid; -static void sig_handler(int signum) +static void sig_propagate(int signum) { int err; @@ -358,7 +358,7 @@ static void sig_handler(int signum) static void wait_for_child(pid_t pid) { struct sigaction sa = { - .sa_handler = sig_handler, + .sa_handler = sig_propagate, .sa_flags = SA_RESETHAND, }; int status, err; -- @@ -346,7 +346,7 @@ static int openns(const char *fmt, ...) } static pid_t sig_pid; -static void sig_handler(int signum) +static void sig_propagate(int signum) { int err; @@ -358,7 +358,7 @@ static void sig_handler(int signum) static void wait_for_child(pid_t pid) { struct sigaction sa = { - .sa_handler = sig_handler, + .sa_handler = sig_propagate, .sa_flags = SA_RESETHAND, }; int status, err; -- 2.47.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] test: Make nstool hold robust against interruptions to control clients 2024-11-06 3:03 [PATCH 0/2] Small fixes for nstool David Gibson 2024-11-06 3:03 ` [PATCH 1/2] test: Rename propagating signal handler David Gibson @ 2024-11-06 3:03 ` David Gibson 2024-11-07 14:54 ` [PATCH 0/2] Small fixes for nstool Stefano Brivio 2 siblings, 0 replies; 7+ messages in thread From: David Gibson @ 2024-11-06 3:03 UTC (permalink / raw) To: Stefano Brivio, passt-dev; +Cc: David Gibson Currently nstool die()s on essentially any error. In most cases that's fine for our purposes. However, it's a problem when in "hold" mode and getting an IO error on an accept()ed socket. This could just indicate that the control client aborted prematurely, in which case we don't want to kill of the namespace we're holding. Adjust these to print an error, close() the control client socket and carry on. In addition, we need to explicitly ignore SIGPIPE in order not to be killed by an abruptly closed client connection. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- test/nstool.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 3f75edd..7ab5d2a 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -31,10 +31,15 @@ #define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) -#define die(...) \ - do { \ - fprintf(stderr, __VA_ARGS__); \ - exit(1); \ +#define die(...) \ + do { \ + fprintf(stderr, "nstool: " __VA_ARGS__); \ + exit(1); \ + } while (0) + +#define err(...) \ + do { \ + fprintf(stderr, "nstool: " __VA_ARGS__); \ } while (0) struct ns_type { @@ -156,6 +161,9 @@ static int connect_ctl(const char *sockpath, bool wait, static void cmd_hold(int argc, char *argv[]) { + struct sigaction sa = { + .sa_handler = SIG_IGN, + }; int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNIX); struct sockaddr_un addr; const char *sockpath = argv[1]; @@ -185,6 +193,10 @@ static void cmd_hold(int argc, char *argv[]) if (!getcwd(info.cwd, sizeof(info.cwd))) die("getcwd(): %s\n", strerror(errno)); + rc = sigaction(SIGPIPE, &sa, NULL); + if (rc) + die("sigaction(SIGPIPE): %s\n", strerror(errno)); + do { int afd = accept(fd, NULL, NULL); char buf; @@ -193,17 +205,21 @@ static void cmd_hold(int argc, char *argv[]) die("accept(): %s\n", strerror(errno)); rc = write(afd, &info, sizeof(info)); - if (rc < 0) - die("write(): %s\n", strerror(errno)); + if (rc < 0) { + err("holder write() to control socket: %s\n", + strerror(errno)); + } if ((size_t)rc < sizeof(info)) - die("short write() on control socket\n"); + err("holder short write() on control socket\n"); rc = read(afd, &buf, sizeof(buf)); - if (rc < 0) - die("read(): %s\n", strerror(errno)); + if (rc < 0) { + err("holder read() on control socket: %s\n", + strerror(errno)); + } close(afd); - } while (rc == 0); + } while (rc <= 0); unlink(sockpath); } -- @@ -31,10 +31,15 @@ #define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) -#define die(...) \ - do { \ - fprintf(stderr, __VA_ARGS__); \ - exit(1); \ +#define die(...) \ + do { \ + fprintf(stderr, "nstool: " __VA_ARGS__); \ + exit(1); \ + } while (0) + +#define err(...) \ + do { \ + fprintf(stderr, "nstool: " __VA_ARGS__); \ } while (0) struct ns_type { @@ -156,6 +161,9 @@ static int connect_ctl(const char *sockpath, bool wait, static void cmd_hold(int argc, char *argv[]) { + struct sigaction sa = { + .sa_handler = SIG_IGN, + }; int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNIX); struct sockaddr_un addr; const char *sockpath = argv[1]; @@ -185,6 +193,10 @@ static void cmd_hold(int argc, char *argv[]) if (!getcwd(info.cwd, sizeof(info.cwd))) die("getcwd(): %s\n", strerror(errno)); + rc = sigaction(SIGPIPE, &sa, NULL); + if (rc) + die("sigaction(SIGPIPE): %s\n", strerror(errno)); + do { int afd = accept(fd, NULL, NULL); char buf; @@ -193,17 +205,21 @@ static void cmd_hold(int argc, char *argv[]) die("accept(): %s\n", strerror(errno)); rc = write(afd, &info, sizeof(info)); - if (rc < 0) - die("write(): %s\n", strerror(errno)); + if (rc < 0) { + err("holder write() to control socket: %s\n", + strerror(errno)); + } if ((size_t)rc < sizeof(info)) - die("short write() on control socket\n"); + err("holder short write() on control socket\n"); rc = read(afd, &buf, sizeof(buf)); - if (rc < 0) - die("read(): %s\n", strerror(errno)); + if (rc < 0) { + err("holder read() on control socket: %s\n", + strerror(errno)); + } close(afd); - } while (rc == 0); + } while (rc <= 0); unlink(sockpath); } -- 2.47.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Small fixes for nstool 2024-11-06 3:03 [PATCH 0/2] Small fixes for nstool David Gibson 2024-11-06 3:03 ` [PATCH 1/2] test: Rename propagating signal handler David Gibson 2024-11-06 3:03 ` [PATCH 2/2] test: Make nstool hold robust against interruptions to control clients David Gibson @ 2024-11-07 14:54 ` Stefano Brivio 2024-11-07 23:30 ` David Gibson 2 siblings, 1 reply; 7+ messages in thread From: Stefano Brivio @ 2024-11-07 14:54 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev On Wed, 6 Nov 2024 14:03:20 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > While working on the exeter tests, I noticed a couple of things > missing in nstool. These make sense standalone, even if we don't have > an urgent need for them without the exeter tests. > > David Gibson (2): > test: Rename propagating signal handler > test: Make nstool hold robust against interruptions to control clients Applied. Actually, one much-needed improvement for nstool in the current test framework (at least for my usage) would be to make it terminate when needed. A while ago, 'killall -9 nstool' entered my shell history and now it's right there with 'git rebase --continue': $ sort ~/.bash_history | uniq -c | sort -nr | grep -A1 'killall -9 nstool' 192 killall -9 nstool 192 git rebase --continue -- Stefano ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Small fixes for nstool 2024-11-07 14:54 ` [PATCH 0/2] Small fixes for nstool Stefano Brivio @ 2024-11-07 23:30 ` David Gibson 2024-11-08 0:05 ` Stefano Brivio 0 siblings, 1 reply; 7+ messages in thread From: David Gibson @ 2024-11-07 23:30 UTC (permalink / raw) To: Stefano Brivio; +Cc: passt-dev [-- Attachment #1: Type: text/plain, Size: 1334 bytes --] On Thu, Nov 07, 2024 at 03:54:43PM +0100, Stefano Brivio wrote: > On Wed, 6 Nov 2024 14:03:20 +1100 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > While working on the exeter tests, I noticed a couple of things > > missing in nstool. These make sense standalone, even if we don't have > > an urgent need for them without the exeter tests. > > > > David Gibson (2): > > test: Rename propagating signal handler > > test: Make nstool hold robust against interruptions to control clients > > Applied. > > Actually, one much-needed improvement for nstool in the current test > framework (at least for my usage) would be to make it terminate when > needed. > > A while ago, 'killall -9 nstool' entered my shell history and now it's > right there with 'git rebase --continue': > > $ sort ~/.bash_history | uniq -c | sort -nr | grep -A1 'killall -9 nstool' > 192 killall -9 nstool > 192 git rebase --continue Yeah, I have something similar. But, I don't currently know what exactly the circumstances are that lead to those stale nstools, so it would involve a fair bit of debugging. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Small fixes for nstool 2024-11-07 23:30 ` David Gibson @ 2024-11-08 0:05 ` Stefano Brivio 2024-11-08 2:32 ` David Gibson 0 siblings, 1 reply; 7+ messages in thread From: Stefano Brivio @ 2024-11-08 0:05 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev On Fri, 8 Nov 2024 10:30:00 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > On Thu, Nov 07, 2024 at 03:54:43PM +0100, Stefano Brivio wrote: > > On Wed, 6 Nov 2024 14:03:20 +1100 > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > While working on the exeter tests, I noticed a couple of things > > > missing in nstool. These make sense standalone, even if we don't have > > > an urgent need for them without the exeter tests. > > > > > > David Gibson (2): > > > test: Rename propagating signal handler > > > test: Make nstool hold robust against interruptions to control clients > > > > Applied. > > > > Actually, one much-needed improvement for nstool in the current test > > framework (at least for my usage) would be to make it terminate when > > needed. > > > > A while ago, 'killall -9 nstool' entered my shell history and now it's > > right there with 'git rebase --continue': > > > > $ sort ~/.bash_history | uniq -c | sort -nr | grep -A1 'killall -9 nstool' > > 192 killall -9 nstool > > 192 git rebase --continue > > Yeah, I have something similar. But, I don't currently know what > exactly the circumstances are that lead to those stale nstools, so it > would involve a fair bit of debugging. Start tests, the ones using nstool (not the "ugly" ones), then ^C ^D until you're out of tmux, and nstool is _always_ there at that point. -- Stefano ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Small fixes for nstool 2024-11-08 0:05 ` Stefano Brivio @ 2024-11-08 2:32 ` David Gibson 0 siblings, 0 replies; 7+ messages in thread From: David Gibson @ 2024-11-08 2:32 UTC (permalink / raw) To: Stefano Brivio; +Cc: passt-dev [-- Attachment #1: Type: text/plain, Size: 1852 bytes --] On Fri, Nov 08, 2024 at 01:05:43AM +0100, Stefano Brivio wrote: > On Fri, 8 Nov 2024 10:30:00 +1100 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Thu, Nov 07, 2024 at 03:54:43PM +0100, Stefano Brivio wrote: > > > On Wed, 6 Nov 2024 14:03:20 +1100 > > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > > > While working on the exeter tests, I noticed a couple of things > > > > missing in nstool. These make sense standalone, even if we don't have > > > > an urgent need for them without the exeter tests. > > > > > > > > David Gibson (2): > > > > test: Rename propagating signal handler > > > > test: Make nstool hold robust against interruptions to control clients > > > > > > Applied. > > > > > > Actually, one much-needed improvement for nstool in the current test > > > framework (at least for my usage) would be to make it terminate when > > > needed. > > > > > > A while ago, 'killall -9 nstool' entered my shell history and now it's > > > right there with 'git rebase --continue': > > > > > > $ sort ~/.bash_history | uniq -c | sort -nr | grep -A1 'killall -9 nstool' > > > 192 killall -9 nstool > > > 192 git rebase --continue > > > > Yeah, I have something similar. But, I don't currently know what > > exactly the circumstances are that lead to those stale nstools, so it > > would involve a fair bit of debugging. > > Start tests, the ones using nstool (not the "ugly" ones), then ^C ^D > until you're out of tmux, and nstool is _always_ there at that point. Huh. I usually exit with ^B-& then ^C and that only occasionally leaves things behind. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-11-08 2:32 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-11-06 3:03 [PATCH 0/2] Small fixes for nstool David Gibson 2024-11-06 3:03 ` [PATCH 1/2] test: Rename propagating signal handler David Gibson 2024-11-06 3:03 ` [PATCH 2/2] test: Make nstool hold robust against interruptions to control clients David Gibson 2024-11-07 14:54 ` [PATCH 0/2] Small fixes for nstool Stefano Brivio 2024-11-07 23:30 ` David Gibson 2024-11-08 0:05 ` Stefano Brivio 2024-11-08 2:32 ` David Gibson
Code repositories for project(s) associated with this public inbox https://passt.top/passt 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 IMAP folder(s).