From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vot7p-00070O-Cr for qemu-devel@nongnu.org; Fri, 06 Dec 2013 05:59:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vot7j-000082-6o for qemu-devel@nongnu.org; Fri, 06 Dec 2013 05:59:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:6985) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vot7j-00007L-03 for qemu-devel@nongnu.org; Fri, 06 Dec 2013 05:59:07 -0500 From: Markus Armbruster References: <1385031835-4472-1-git-send-email-stefanha@redhat.com> <1385031835-4472-2-git-send-email-stefanha@redhat.com> Date: Fri, 06 Dec 2013 11:59:01 +0100 In-Reply-To: <1385031835-4472-2-git-send-email-stefanha@redhat.com> (Stefan Hajnoczi's message of "Thu, 21 Nov 2013 12:03:54 +0100") Message-ID: <87d2lajle2.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/2] qtest: unlink QEMU pid file after startup List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org, Andreas Faerber , Gerd Hoffmann Stefan Hajnoczi writes: > After starting the QEMU process and initializing the QMP connection, we > can read the pid file and unlink it. > > Just stash away the pid instead of the pid filename. This way we can > avoid pid file leaks since running tests may abort(3) without cleanup. > > Signed-off-by: Stefan Hajnoczi > --- > tests/libqtest.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 359d571..dd93be8 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -43,8 +43,8 @@ struct QTestState > int qmp_fd; > bool irq_level[MAX_IRQ]; > GString *rx; > - gchar *pid_file; /* QEMU PID file */ > int child_pid; /* Child process created to execute QEMU */ > + pid_t qemu_pid; /* QEMU process spawned by our child */ > char *socket_path, *qmp_socket_path; > }; > > @@ -90,13 +90,13 @@ static int socket_accept(int sock) > return ret; > } > > -static pid_t qtest_qemu_pid(QTestState *s) > +static pid_t read_pid_file(const char *pid_file) > { > FILE *f; > char buffer[1024]; > pid_t pid = -1; > > - f = fopen(s->pid_file, "r"); > + f = fopen(pid_file, "r"); > if (f) { > if (fgets(buffer, sizeof(buffer), f)) { > pid = atoi(buffer); > @@ -147,7 +147,6 @@ QTestState *qtest_init(const char *extra_args) > s->qmp_fd = socket_accept(qmpsock); > > s->rx = g_string_new(""); > - s->pid_file = pid_file; > s->child_pid = pid; > for (i = 0; i < MAX_IRQ; i++) { > s->irq_level[i] = false; > @@ -157,8 +156,12 @@ QTestState *qtest_init(const char *extra_args) > qtest_qmp_discard_response(s, ""); > qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); > > + s->qemu_pid = read_pid_file(pid_file); > + unlink(pid_file); unlink() can fail, but I guess there's not much useful you can do when it fails. Also, no worse than before your patch (see last hunk). > + g_free(pid_file); > + > if (getenv("QTEST_STOP")) { > - kill(qtest_qemu_pid(s), SIGSTOP); > + kill(s->qemu_pid, SIGSTOP); > } > > return s; > @@ -168,19 +171,16 @@ void qtest_quit(QTestState *s) > { > int status; > > - pid_t pid = qtest_qemu_pid(s); > - if (pid != -1) { > - kill(pid, SIGTERM); > - waitpid(pid, &status, 0); > + if (s->qemu_pid != -1) { > + kill(s->qemu_pid, SIGTERM); > + waitpid(s->qemu_pid, &status, 0); > } > > close(s->fd); > close(s->qmp_fd); > g_string_free(s->rx, true); > - unlink(s->pid_file); > unlink(s->socket_path); > unlink(s->qmp_socket_path); > - g_free(s->pid_file); > g_free(s->socket_path); > g_free(s->qmp_socket_path); > g_free(s);