Thomas Huth <[email protected]> writes:
> On 26/10/2022 18.18, Alex Bennée wrote: >> Daniel P. Berrangé <[email protected]> writes: >> >>> CC'ing Marc-André as original author of the change >>> >>> On Tue, Oct 25, 2022 at 01:57:23PM +0100, Alex Bennée wrote: >>>> >>>> Juan Quintela <[email protected]> writes: >>>> >>>>> Previous commit removed the creation of the fifo. Without it, I get >>>>> random failure during tests with high load, please consider >>>>> reintroduce it. >>>>> >>>>> My guess is that there is a race between the two socats when we leave >>>>> them to create the channel, better return to the previous behavior. >>>>> >>>>> I can't reproduce the problem when I run ./test-io-channel-command >>>>> test alone, I need to do the make check. And any (unrelated) change >>>>> can make it dissapear. >>>> >>>> I was chasing a similar problem with this test although I don't see it >>>> timeout while running (I don't think our unit tests time out). I'm >>>> provisionally queuing this to testing/next unless anyone objects. >>> >>> It won't build on Win32 since that platform lacks mkfifo. >>> >>> The test normally works since socat will call mknod to create >>> the fifo. >>> >>> I think the problem is that we have a race condition where the >>> client socat runs before the server socat, and so won't see the >>> fifo. This will be where high load triggers problems. >> Ok I shall drop the patch from testing/next - we need a better >> solution. > > Could we maybe at least revert the patch that introduced the problem? > ... the failing test is annoying ... I'm trying to fix it now but I haven't been able to get one of the socat's to not race with the other: --8<---------------cut here---------------start------------->8--- modified tests/unit/test-io-channel-command.c @@ -19,6 +19,7 @@ */ #include "qemu/osdep.h" +#include <glib/gstdio.h> #include "io/channel-command.h" #include "io-channel-helpers.h" #include "qapi/error.h" @@ -33,25 +34,26 @@ static char *socat = NULL; static void test_io_channel_command_fifo(bool async) { + g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL); + g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO); + g_autoptr(GString) srcargs = g_string_new(socat); + g_autoptr(GString) dstargs = g_string_new(socat); + g_auto(GStrv) srcargv; + g_auto(GStrv) dstargv; QIOChannel *src, *dst; QIOChannelTest *test; - const char *srcargv[] = { - socat, "-", SOCAT_SRC, NULL, - }; - const char *dstargv[] = { - socat, SOCAT_DST, "-", NULL, - }; - if (!socat) { - g_test_skip("socat is not found in PATH"); - return; - } + g_string_append_printf(srcargs, " - PIPE:%s,wronly,creat=1,excl=1", fifo); + g_string_append_printf(dstargs, " PIPE:%s,rdonly,creat=0 -", fifo); + + srcargv = g_strsplit(srcargs->str, " ", -1); + dstargv = g_strsplit(dstargs->str, " ", -1); - unlink(TEST_FIFO); - src = QIO_CHANNEL(qio_channel_command_new_spawn(srcargv, + src = QIO_CHANNEL(qio_channel_command_new_spawn((const char**) srcargv, O_WRONLY, &error_abort)); - dst = QIO_CHANNEL(qio_channel_command_new_spawn(dstargv, + + dst = QIO_CHANNEL(qio_channel_command_new_spawn((const char**) dstargv, O_RDONLY, &error_abort)); @@ -62,17 +64,27 @@ static void test_io_channel_command_fifo(bool async) object_unref(OBJECT(src)); object_unref(OBJECT(dst)); - unlink(TEST_FIFO); + g_rmdir(tmpdir); } static void test_io_channel_command_fifo_async(void) { + if (!socat) { + g_test_skip("socat is not found in PATH"); + return; + } + test_io_channel_command_fifo(true); } static void test_io_channel_command_fifo_sync(void) { + if (!socat) { + g_test_skip("socat is not found in PATH"); + return; + } + test_io_channel_command_fifo(false); } --8<---------------cut here---------------end--------------->8--- > > Thomas -- Alex Bennée
