diff mbox

tests/libqtest.c: Delete possible stale unix sockets

Message ID 1490963801-27870-1-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell March 31, 2017, 12:36 p.m. UTC
Occasionally if a test crashes or is interrupted by the user
at the wrong moment it could leave behind a stale UNIX
socket in /tmp/. This will then cause a subsequent test
run to fail spuriously with
 tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
if it happens to reuse the same PID.

Defend against this by deleting any stray stale socket before
trying to open the new ones for this test.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This seems like an easy way to shut up this infrequent but irritating
error case...

 tests/libqtest.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Philippe Mathieu-Daudé March 31, 2017, 1:32 p.m. UTC | #1
On 03/31/2017 09:36 AM, Peter Maydell wrote:
> Occasionally if a test crashes or is interrupted by the user
> at the wrong moment it could leave behind a stale UNIX
> socket in /tmp/. This will then cause a subsequent test
> run to fail spuriously with
>  tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
> if it happens to reuse the same PID.
>
> Defend against this by deleting any stray stale socket before
> trying to open the new ones for this test.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
> This seems like an easy way to shut up this infrequent but irritating
> error case...
>
>  tests/libqtest.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index a5c3d2b..99b1195 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -167,6 +167,14 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>      socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
>      qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
>
> +    /* It's possible that if an earlier test run crashed it might
> +     * have left a stale unix socket lying around. Delete any
> +     * stale old socket to avoid spurious test failures with
> +     * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
> +     */
> +    unlink(socket_path);
> +    unlink(qmp_socket_path);
> +
>      sock = init_socket(socket_path);
>      qmpsock = init_socket(qmp_socket_path);
>
>
Eric Blake March 31, 2017, 2:04 p.m. UTC | #2
On 03/31/2017 07:36 AM, Peter Maydell wrote:
> Occasionally if a test crashes or is interrupted by the user
> at the wrong moment it could leave behind a stale UNIX
> socket in /tmp/. This will then cause a subsequent test
> run to fail spuriously with
>  tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
> if it happens to reuse the same PID.
> 
> Defend against this by deleting any stray stale socket before
> trying to open the new ones for this test.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This seems like an easy way to shut up this infrequent but irritating
> error case...

There is a related failure in qemu-iotests that I hit yesterday:

+mkfifo: cannot create fifo
'/home/eblake/qemu/tests/qemu-iotests/scratch/qmp-out-3812_0': File exists

that is probably also worth addressing, but that can be a separate patch.

Reviewed-by: Eric Blake <eblake@redhat.com>
Stefan Hajnoczi April 3, 2017, 6:01 p.m. UTC | #3
On Fri, Mar 31, 2017 at 01:36:41PM +0100, Peter Maydell wrote:
> Occasionally if a test crashes or is interrupted by the user
> at the wrong moment it could leave behind a stale UNIX
> socket in /tmp/. This will then cause a subsequent test
> run to fail spuriously with
>  tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
> if it happens to reuse the same PID.
> 
> Defend against this by deleting any stray stale socket before
> trying to open the new ones for this test.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This seems like an easy way to shut up this infrequent but irritating
> error case...
> 
>  tests/libqtest.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Markus Armbruster April 11, 2017, 3:14 p.m. UTC | #4
Peter Maydell <peter.maydell@linaro.org> writes:

> Occasionally if a test crashes or is interrupted by the user
> at the wrong moment it could leave behind a stale UNIX
> socket in /tmp/. This will then cause a subsequent test
> run to fail spuriously with
>  tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
> if it happens to reuse the same PID.
>
> Defend against this by deleting any stray stale socket before
> trying to open the new ones for this test.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This seems like an easy way to shut up this infrequent but irritating
> error case...
>
>  tests/libqtest.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index a5c3d2b..99b1195 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -167,6 +167,14 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>      socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
>      qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
>  
> +    /* It's possible that if an earlier test run crashed it might
> +     * have left a stale unix socket lying around. Delete any
> +     * stale old socket to avoid spurious test failures with
> +     * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
> +     */
> +    unlink(socket_path);
> +    unlink(qmp_socket_path);
> +
>      sock = init_socket(socket_path);
>      qmpsock = init_socket(qmp_socket_path);

No objection to this work-around for tests leaving crap behind, but I
think we should also try harder not to leave crap behind.  Say give each
test its own scratch directory, make sure no test writes outside it, and
have the harness remove it afterwards.
Peter Maydell April 11, 2017, 3:34 p.m. UTC | #5
On 11 April 2017 at 16:14, Markus Armbruster <armbru@redhat.com> wrote:
> No objection to this work-around for tests leaving crap behind, but I
> think we should also try harder not to leave crap behind.  Say give each
> test its own scratch directory, make sure no test writes outside it, and
> have the harness remove it afterwards.

That would all be nice, yes :-)

thanks
-- PMM
diff mbox

Patch

diff --git a/tests/libqtest.c b/tests/libqtest.c
index a5c3d2b..99b1195 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -167,6 +167,14 @@  QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
     qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
 
+    /* It's possible that if an earlier test run crashed it might
+     * have left a stale unix socket lying around. Delete any
+     * stale old socket to avoid spurious test failures with
+     * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
+     */
+    unlink(socket_path);
+    unlink(qmp_socket_path);
+
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);