From patchwork Fri Jul 5 09:59:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andre Przywara X-Patchwork-Id: 11032355 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9224813A4 for ; Fri, 5 Jul 2019 09:59:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8075228A80 for ; Fri, 5 Jul 2019 09:59:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 74EF128A86; Fri, 5 Jul 2019 09:59:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 070F828A80 for ; Fri, 5 Jul 2019 09:59:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728315AbfGEJ7W (ORCPT ); Fri, 5 Jul 2019 05:59:22 -0400 Received: from foss.arm.com ([217.140.110.172]:34690 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728240AbfGEJ7V (ORCPT ); Fri, 5 Jul 2019 05:59:21 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C2F7B14FF; Fri, 5 Jul 2019 02:59:20 -0700 (PDT) Received: from donnerap.arm.com (donnerap.cambridge.arm.com [10.1.197.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 024143F246; Fri, 5 Jul 2019 02:59:19 -0700 (PDT) From: Andre Przywara To: Will Deacon , Julien Thierry Cc: kvm@vger.kernel.org, Dave Martin Subject: [PATCH kvmtool 1/2] term: Avoid busy loop with unconnected pseudoterminals Date: Fri, 5 Jul 2019 10:59:13 +0100 Message-Id: <20190705095914.151056-2-andre.przywara@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190705095914.151056-1-andre.przywara@arm.com> References: <20190705095914.151056-1-andre.przywara@arm.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Currently when kvmtool is creating a pseudoterminal (--tty x), the terminal thread will consume 100% of its CPU time as long as no slave is connected to the other end. This is due to the fact that poll() unconditonally sets the POLLHUP bit in revents and returns immediately, regardless of the events we are querying for. There does not seem to be a solution to this with just poll() alone. Using the TIOCPKT ioctl sounds promising, but doesn't help either, as poll still detects the HUP condition. So apart from chickening out with some poll() timeout tricks, inotify seems to be the way to go: Each time poll() returns with the POLLHUP bit set, we disable this file descriptor in the poll() array and rely on the inotify IN_OPEN watch to fire on the slave end of the pseudoterminal. We then enable the file descriptor again. Signed-off-by: Andre Przywara --- term.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/term.c b/term.c index b8a70fe2..7fbd98c6 100644 --- a/term.c +++ b/term.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "kvm/read-write.h" #include "kvm/term.h" @@ -21,6 +22,8 @@ static struct termios orig_term; static int term_fds[TERM_MAX_DEVS][2]; +static int inotify_fd; + static pthread_t term_poll_thread; /* ctrl-a is used for escape */ @@ -100,7 +103,7 @@ bool term_readable(int term) static void *term_poll_thread_loop(void *param) { - struct pollfd fds[TERM_MAX_DEVS]; + struct pollfd fds[TERM_MAX_DEVS + 1]; struct kvm *kvm = (struct kvm *) param; int i; @@ -111,11 +114,42 @@ static void *term_poll_thread_loop(void *param) fds[i].events = POLLIN; fds[i].revents = 0; } + fds[i].fd = inotify_fd; + fds[i].events = POLLIN; + fds[i].revents = 0; while (1) { + int i; + /* Poll with infinite timeout */ - if(poll(fds, TERM_MAX_DEVS, -1) < 1) + if(poll(fds, TERM_MAX_DEVS + 1, -1) < 1) break; + + for (i = 0; i < TERM_MAX_DEVS; i++) { + /* + * Check for unconnected pseudoterminals. They will + * make poll() return immediately, so we have to + * disable those fds and rely on inotify to tell us + * when the slave side gets opened. + */ + if (fds[i].revents == POLLHUP) + fds[i].fd = ~fds[i].fd; + } + if (fds[TERM_MAX_DEVS].revents) { /* inotify fd */ + struct inotify_event event; + + /* + * Just enable all fds that we previously disabled, + * still unconnected ones will be disabled again on + * the next poll() call. + */ + for (i = 0; i < TERM_MAX_DEVS; i++) + if (fds[i].fd < 0) + fds[i].fd = ~fds[i].fd; + /* Consume at least one inotify event. */ + i = read(inotify_fd, &event, sizeof(event)); + } + kvm__arch_read_term(kvm); } @@ -154,7 +188,11 @@ static void term_set_tty(int term) close(slave); - pr_info("Assigned terminal %d to pty %s\n", term, new_pty); + pr_info("Assigned terminal %d to pty %s", term, new_pty); + + if (!inotify_fd) + inotify_fd = inotify_init(); + inotify_add_watch(inotify_fd, new_pty, IN_OPEN); term_fds[term][TERM_FD_IN] = term_fds[term][TERM_FD_OUT] = master; } @@ -194,6 +232,10 @@ static int term_init(struct kvm *kvm) term.c_lflag &= ~(ICANON | ECHO | ISIG); tcsetattr(STDIN_FILENO, TCSANOW, &term); + if (!inotify_fd) + inotify_fd = inotify_init(); + if (inotify_fd < 0) + die("Unable to initialise inotify\n"); /* Use our own blocking thread to read stdin, don't require a tick */ if(pthread_create(&term_poll_thread, NULL, term_poll_thread_loop,kvm)) From patchwork Fri Jul 5 09:59:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andre Przywara X-Patchwork-Id: 11032357 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1EE3713A4 for ; Fri, 5 Jul 2019 09:59:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0D05E28A80 for ; Fri, 5 Jul 2019 09:59:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 014D928A86; Fri, 5 Jul 2019 09:59:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 75C2D28A80 for ; Fri, 5 Jul 2019 09:59:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727624AbfGEJ72 (ORCPT ); Fri, 5 Jul 2019 05:59:28 -0400 Received: from foss.arm.com ([217.140.110.172]:34696 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727990AbfGEJ7W (ORCPT ); Fri, 5 Jul 2019 05:59:22 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C2C751509; Fri, 5 Jul 2019 02:59:21 -0700 (PDT) Received: from donnerap.arm.com (donnerap.cambridge.arm.com [10.1.197.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 021053F246; Fri, 5 Jul 2019 02:59:20 -0700 (PDT) From: Andre Przywara To: Will Deacon , Julien Thierry Cc: kvm@vger.kernel.org, Dave Martin Subject: [PATCH kvmtool 2/2] Add detach feature Date: Fri, 5 Jul 2019 10:59:14 +0100 Message-Id: <20190705095914.151056-3-andre.przywara@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190705095914.151056-1-andre.przywara@arm.com> References: <20190705095914.151056-1-andre.przywara@arm.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP At the moment a kvmtool process started on a terminal has no way of detaching from the terminal without killing the guest. Existing workarounds are starting kvmtool in a screen/tmux session or using a pseudoterminal (--tty 0), both of which have to be done upon guest creation. Introduce a terminal command to create a pseudoterminal during the guest's runtime and redirect the console output to that. This will be triggered by typing the letter 'd' after the kvmtool escape sequence (default Ctrl+a). This also daemonises kvmtool, so gives back the shell prompt, and the user can log out without affecting the guest. Naively daemonising kvmtool at that point doesn't work, though, as the fork() doesn't inherit the threads, so they keep running in the grandparent process and would be killed by its exit. The trick used here is to do the double fork() already right at the beginning of kvmtool's runtime, before spawning the first thread. We then don't end the parent and grandparent processes yet, instead let them block until the user actually requests the detach. This will let all the threads be created in the grandchild process, but keeps kvmtool still attached to the terminal until the user requests a detach. Signed-off-by: Andre Przywara --- builtin-run.c | 3 ++ include/kvm/kvm.h | 2 ++ term.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+) diff --git a/builtin-run.c b/builtin-run.c index f8dc6c72..fa391419 100644 --- a/builtin-run.c +++ b/builtin-run.c @@ -592,6 +592,9 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv) } } + /* Fork twice already now to create the threads in the right process. */ + kvm__prepare_daemonize(); + if (!kvm->cfg.guest_name) { if (kvm->cfg.custom_rootfs) { kvm->cfg.guest_name = kvm->cfg.custom_rootfs_name; diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h index 7a738183..801f9474 100644 --- a/include/kvm/kvm.h +++ b/include/kvm/kvm.h @@ -90,6 +90,8 @@ struct kvm { void kvm__set_dir(const char *fmt, ...); const char *kvm__get_dir(void); +int kvm__prepare_daemonize(void); + int kvm__init(struct kvm *kvm); struct kvm *kvm__new(void); int kvm__recommended_cpus(struct kvm *kvm); diff --git a/term.c b/term.c index 7fbd98c6..df8328f8 100644 --- a/term.c +++ b/term.c @@ -22,6 +22,7 @@ static struct termios orig_term; static int term_fds[TERM_MAX_DEVS][2]; +static int daemon_fd; static int inotify_fd; static pthread_t term_poll_thread; @@ -29,6 +30,92 @@ static pthread_t term_poll_thread; /* ctrl-a is used for escape */ #define term_escape_char 0x01 +/* This needs to be called *before* we create any threads. */ +int kvm__prepare_daemonize(void) +{ + pid_t pid; + char dummy; + int child_pipe[2], parent_pipe[2]; + + if (pipe(parent_pipe)) + return -1; + + pid = fork(); + if (pid < 0) + return pid; + if (pid > 0) { /* parent process */ + + close(parent_pipe[1]); + + /* Block until we are told to exit. */ + if (read(parent_pipe[0], &dummy, 1) != 1) + perror("reading exit pipe"); + + exit(0); + } + + close(parent_pipe[0]); + if (pipe(child_pipe)) + return -1; + daemon_fd = child_pipe[1]; + + /* Become a session leader. */ + setsid(); + pid = fork(); + if (pid > 0) { + close(child_pipe[1]); + + /* Block until we are told to exit. */ + if (read(child_pipe[0], &dummy, 1) != 1) + perror("reading child exit pipe"); + + if (write(parent_pipe[1], &dummy, 1) != 1) + pr_warning("could not kill daemon's parent"); + + exit(0); + } + close(child_pipe[0]); + close(parent_pipe[1]); + + /* Only the grandchild returns here, to do the actual work. */ + return 0; +} + +static void term_set_tty(int term); + +static void detach_terminal(int term) +{ + char dummy = 0; + + /* Detaching only make sense if we use the process' terminal. */ + if (term_fds[term][TERM_FD_IN] != STDIN_FILENO) + return; + + /* Clean up just this terminal, leave the others alone. */ + tcsetattr(term_fds[term][TERM_FD_IN], TCSANOW, &orig_term); + + /* Redirect this terminal to a PTY */ + term_set_tty(term); + + /* + * Replace STDIN/STDOUT with this new PTY. This will automatically + * transfer all the other serial terminals over. + */ + dup2(term_fds[term][TERM_FD_IN], STDIN_FILENO); + dup2(term_fds[term][TERM_FD_OUT], STDOUT_FILENO); + + /* Tell the (waiting) child process to exit now. */ + if (write(daemon_fd, &dummy, 1) != 1) + pr_warning("could not kill daemon's parent"); + + /* To not hog the current directory unnecessarily. */ + if (chdir("/")) + perror("changing to root directory"); + umask(0); + + close(STDERR_FILENO); +} + int term_getc(struct kvm *kvm, int term) { static bool term_got_escape = false; @@ -41,6 +128,10 @@ int term_getc(struct kvm *kvm, int term) term_got_escape = false; if (c == 'x') kvm__reboot(kvm); + if (c == 'd') { + detach_terminal(term); + return -1; + } if (c == term_escape_char) return c; }