| Message ID | 20230129182414.583349-3-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Various win32 fixes & teach 'getfd' QMP command to import sockets | expand |
On 29/01/2023 19.24, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > socat "PIPE:"" on Windows are named pipes, not fifo path names. > > Fixes: commit 68406d10859 ("tests/unit: cleanups for test-io-channel-command") > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > tests/unit/test-io-channel-command.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c > index 096224962c..e76ef2daaa 100644 > --- a/tests/unit/test-io-channel-command.c > +++ b/tests/unit/test-io-channel-command.c > @@ -31,8 +31,12 @@ static char *socat = NULL; > > static void test_io_channel_command_fifo(bool async) > { > +#ifdef WIN32 > + const gchar *fifo = TEST_FIFO; Question from a Windows ignorant: Won't this cause a race condition in case someone is trying to run tests in parallel (i.e. shouldn't there be a random part in the name)? Or are these named pipes local to the current process? Thomas > +#else > g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL); > g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL); > +#endif > g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat, fifo); > g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat, fifo); > g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1); > @@ -57,7 +61,9 @@ static void test_io_channel_command_fifo(bool async) > object_unref(OBJECT(src)); > object_unref(OBJECT(dst)); > > +#ifndef WIN32 > g_rmdir(tmpdir); > +#endif > } > >
On 29/1/23 19:24, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > socat "PIPE:"" on Windows are named pipes, not fifo path names. > > Fixes: commit 68406d10859 ("tests/unit: cleanups for test-io-channel-command") > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > tests/unit/test-io-channel-command.c | 6 ++++++ > 1 file changed, 6 insertions(+) This doesn't apply cleanly on top of c906e6fbaa ("tests/unit: drop hacky race avoidance in test-io-channel-command").
Hi On Mon, Feb 6, 2023 at 12:09 PM Thomas Huth <thuth@redhat.com> wrote: > > On 29/01/2023 19.24, marcandre.lureau@redhat.com wrote: > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > socat "PIPE:"" on Windows are named pipes, not fifo path names. > > > > Fixes: commit 68406d10859 ("tests/unit: cleanups for test-io-channel-command") > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > tests/unit/test-io-channel-command.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c > > index 096224962c..e76ef2daaa 100644 > > --- a/tests/unit/test-io-channel-command.c > > +++ b/tests/unit/test-io-channel-command.c > > @@ -31,8 +31,12 @@ static char *socat = NULL; > > > > static void test_io_channel_command_fifo(bool async) > > { > > +#ifdef WIN32 > > + const gchar *fifo = TEST_FIFO; > > Question from a Windows ignorant: Won't this cause a race condition in case > someone is trying to run tests in parallel (i.e. shouldn't there be a random > part in the name)? Or are these named pipes local to the current process? > You are right, they are global. Furthermore, socat PIPE: is actually not using windows named-pipes, at least directly (despite debugging log saying something like that). It's actually mkfifo() cygwin/newlib implementation that does some magic (in msys2). It looks like it creates some kind of "fake" file/links with metadata and actually creates the named pipe later on demand. So it still maps to some kind of file, and it looks like create/open races would still be possible, even if "cygwin" mkfifo was called before socat. The alternative would be to create pipe() beforehand and handing those fd to socat, perhaps. I might give it a try. thanks > Thomas > > > +#else > > g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL); > > g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL); > > +#endif > > g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat, fifo); > > g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat, fifo); > > g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1); > > @@ -57,7 +61,9 @@ static void test_io_channel_command_fifo(bool async) > > object_unref(OBJECT(src)); > > object_unref(OBJECT(dst)); > > > > +#ifndef WIN32 > > g_rmdir(tmpdir); > > +#endif > > } > > > > > >
diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c index 096224962c..e76ef2daaa 100644 --- a/tests/unit/test-io-channel-command.c +++ b/tests/unit/test-io-channel-command.c @@ -31,8 +31,12 @@ static char *socat = NULL; static void test_io_channel_command_fifo(bool async) { +#ifdef WIN32 + const gchar *fifo = TEST_FIFO; +#else g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XXXXXX", NULL); g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL); +#endif g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat, fifo); g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat, fifo); g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1); @@ -57,7 +61,9 @@ static void test_io_channel_command_fifo(bool async) object_unref(OBJECT(src)); object_unref(OBJECT(dst)); +#ifndef WIN32 g_rmdir(tmpdir); +#endif }