On Tue, Mar 11, 2014 at 12:09:09PM +0200, Marcel Apfelbaum wrote:
> @@ -78,12 +79,16 @@ static int socket_accept(int sock)
> struct sockaddr_un addr;
> socklen_t addrlen;
> int ret;
> + struct timeval timeout = { .tv_sec = SOCKET_TIMEOUT,
> + .tv_usec = 0 };
> +
> + setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (void *)&timeout,
> + sizeof(timeout));
>
> addrlen = sizeof(addr);
> do {
> ret = accept(sock, (struct sockaddr *)&addr, &addrlen);
> } while (ret == -1 && errno == EINTR);
> - g_assert_no_errno(ret);
> close(sock);
Did you mean to leave SO_RCVTIMEO set after this function completes?
> @@ -91,7 +96,7 @@ static int socket_accept(int sock)
>
> static void kill_qemu(QTestState *s)
> {
> - if (s->qemu_pid != -1) {
> + if (s && s->qemu_pid != -1) {
> kill(s->qemu_pid, SIGTERM);
> waitpid(s->qemu_pid, NULL, 0);
> }
This is a bug in libqtest.c, please don't silence the crash.
kill_qemu() gets called from the SIGABRT signal handler but I forgot
that global_qtest isn't initialized yet while qtest_init() executes.
In other words, the cleanup is broken if we fail inside qtest_init().
Can you drop this hunk and I'll send a patch to fix the underlying
issue?
> @@ -153,6 +158,8 @@ QTestState *qtest_init(const char *extra_args)
> g_free(socket_path);
> g_free(qmp_socket_path);
>
> + g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> +
We probably shouldn't socket_accept() s->qmp_fd if s->fd already failed.
Otherwise we'll wait another 5 seconds for the timeout to explire:
s->fd = socket_accept(sock);
if (s->fd >= 0) {
s->qmp_fd = socket_accept(qmpsock);
}
