From 7c3370d4fe3fa6cda8655f109e4659afc8ca4269 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Fri, 8 May 2009 12:34:17 +0200 Subject: slirp: Avoid zombie processes after fork_exec Slirp uses fork_exec for spawning service processes, and QEMU uses this for running smbd. As SIGCHLD is not handled, these processes become zombies on termination. Fix this by installing a proper signal handler, but also make sure we disable the signal while waiting on forked network setup/shutdown scripts. Signed-off-by: Jan Kiszka Signed-off-by: Mark McLoughlin --- vl.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'vl.c') diff --git a/vl.c b/vl.c index fcf8532..a5f966d 100644 --- a/vl.c +++ b/vl.c @@ -4783,7 +4783,12 @@ static void termsig_handler(int signal) qemu_system_shutdown_request(); } -static void termsig_setup(void) +static void sigchld_handler(int signal) +{ + waitpid(-1, NULL, WNOHANG); +} + +static void sighandler_setup(void) { struct sigaction act; @@ -4792,6 +4797,10 @@ static void termsig_setup(void) sigaction(SIGINT, &act, NULL); sigaction(SIGHUP, &act, NULL); sigaction(SIGTERM, &act, NULL); + + act.sa_handler = sigchld_handler; + act.sa_flags = SA_NOCLDSTOP; + sigaction(SIGCHLD, &act, NULL); } #endif @@ -5918,7 +5927,7 @@ int main(int argc, char **argv, char **envp) #ifndef _WIN32 /* must be after terminal init, SDL library changes signal handlers */ - termsig_setup(); + sighandler_setup(); #endif /* Maintain compatibility with multiple stdio monitors */ -- cgit v1.1 From cda94b27821726df74eead0701d8401c1acda6ec Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Thu, 28 May 2009 14:07:31 +0100 Subject: Revert "Fix output of uninitialized strings" This reverts commit 8cf07dcbe7691dbe4f47563058659dba6ef66b05. This is a sorry saga. This commit: 8e4416af45 net: Add parameter checks for VLAN clients broken '-net socket' and this commit: ffad4116b9 net: Fix -net socket parameter checks fixed the problem but introduced another problem which this commit: 8cf07dcbe7 Fix output of uninitialized strings fixed that final problem, but causing us to lose some error reporting information in the process. Meanwhile Jan posted a patch to mostly re-do ffad4116b9 in a way that fixes the original issue, but without losing the error reporting information. So, let's revert 8cf07dcbe7 and apply Jan's patch. Signed-off-by: Mark McLoughlin --- vl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'vl.c') diff --git a/vl.c b/vl.c index a5f966d..ff438d0 100644 --- a/vl.c +++ b/vl.c @@ -2228,7 +2228,8 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque) NULL }; if (check_params(params, str) < 0) { - fprintf(stderr, "qemu: unknown parameter in '%s'\n", str); + fprintf(stderr, "qemu: unknown parameter '%s' in '%s'\n", + buf, str); return -1; } -- cgit v1.1 From 0aa7a205c899c516d906673efbe9457f7af0dd3c Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Fri, 8 May 2009 12:34:17 +0200 Subject: net: Real fix for check_params users OK, last try: 8e4416af45 broke -net socket, ffad4116b9 tried to fix it but broke error reporting of invalid parameters. So this patch widely reverts ffad4116b9 again and intead fixes those callers of check_params that originally suffered from overwritten buffers by using separate ones. Signed-off-by: Jan Kiszka Signed-off-by: Mark McLoughlin --- vl.c | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) (limited to 'vl.c') diff --git a/vl.c b/vl.c index ff438d0..659e9f7 100644 --- a/vl.c +++ b/vl.c @@ -1836,45 +1836,34 @@ int get_param_value(char *buf, int buf_size, return 0; } -int check_params(const char * const *params, const char *str) +int check_params(char *buf, int buf_size, + const char * const *params, const char *str) { - int name_buf_size = 1; const char *p; - char *name_buf; - int i, len; - int ret = 0; - - for (i = 0; params[i] != NULL; i++) { - len = strlen(params[i]) + 1; - if (len > name_buf_size) { - name_buf_size = len; - } - } - name_buf = qemu_malloc(name_buf_size); + int i; p = str; while (*p != '\0') { - p = get_opt_name(name_buf, name_buf_size, p, '='); + p = get_opt_name(buf, buf_size, p, '='); if (*p != '=') { - ret = -1; - break; + return -1; } p++; - for(i = 0; params[i] != NULL; i++) - if (!strcmp(params[i], name_buf)) + for (i = 0; params[i] != NULL; i++) { + if (!strcmp(params[i], buf)) { break; + } + } if (params[i] == NULL) { - ret = -1; - break; + return -1; } p = get_opt_value(NULL, 0, p); - if (*p != ',') + if (*p != ',') { break; + } p++; } - - qemu_free(name_buf); - return ret; + return 0; } /***********************************************************/ @@ -2227,7 +2216,7 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque) "cache", "format", "serial", "werror", NULL }; - if (check_params(params, str) < 0) { + if (check_params(buf, sizeof(buf), params, str) < 0) { fprintf(stderr, "qemu: unknown parameter '%s' in '%s'\n", buf, str); return -1; -- cgit v1.1 From 10ae5a7a98f7c1d901fbb6658324e9c4c4fec8cf Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Fri, 8 May 2009 12:34:18 +0200 Subject: net: Improve parameter error reporting As host network devices can also be instantiated via the monitor, errors should then be reported to the related monitor instead of stderr. This requires larger refactoring, so this patch starts small with introducing a helper to catch both cases and convert net_client_init as well as net_slirp_redir. Signed-off-by: Jan Kiszka Signed-off-by: Mark McLoughlin --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'vl.c') diff --git a/vl.c b/vl.c index 659e9f7..94027c0 100644 --- a/vl.c +++ b/vl.c @@ -2699,7 +2699,7 @@ static int usb_device_add(const char *devname, int is_hotplug) } else if (strstart(devname, "net:", &p)) { int nic = nb_nics; - if (net_client_init("nic", p) < 0) + if (net_client_init(NULL, "nic", p) < 0) return -1; nd_table[nic].model = "usb"; dev = usb_net_init(&nd_table[nic]); -- cgit v1.1