On Thu, Dec 20, 2012 at 8:26 PM, Jason Baron <jba...@redhat.com> wrote:
> On Thu, Dec 20, 2012 at 08:07:02PM +0000, Blue Swirl wrote:
>> On Thu, Dec 20, 2012 at 5:14 PM, Jason Baron <jba...@redhat.com> wrote:
>> > From: Jason Baron <jba...@redhat.com>
>> >
>> > Currently, the qtest harness can only spawn 1 qemu instance at a time
>> > because
>> > the parent pid is used to create the socket files. Use 'mkdtemp()' in
>>
>> But mkdtemp() is not available on Win32.
>
> So this case important even for qtest?
Well, there's $(EXESUF) handling for qtest also. But it does not seem
to build now:
LINK tests/test-thread-pool.exe
tests/test-thread-pool.o: In function `test_cancel':
/src/qemu/tests/test-thread-pool.c:177: undefined reference to
`___sync_val_compare_and_swap_4'
tests/test-thread-pool.o: In function `worker_cb':
/src/qemu/tests/test-thread-pool.c:18: undefined reference to
`___sync_fetch_and_add_4'
>
>>
>> > combination with the parent pid to avoid conflicts.
>> >
>> > Signed-off-by: Jason Baron <jba...@redhat.com>
>> > ---
>> > tests/libqtest.c | 15 +++++++++------
>> > 1 files changed, 9 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/tests/libqtest.c b/tests/libqtest.c
>> > index 71b84c1..57665c9 100644
>> > --- a/tests/libqtest.c
>> > +++ b/tests/libqtest.c
>> > @@ -41,6 +41,7 @@ struct QTestState
>> > GString *rx;
>> > gchar *pid_file;
>> > char *socket_path, *qmp_socket_path;
>> > + char *tmp_dir;
>> > };
>> >
>> > #define g_assert_no_errno(ret) do { \
>> > @@ -105,7 +106,6 @@ QTestState *qtest_init(const char *extra_args)
>> > {
>> > QTestState *s;
>> > int sock, qmpsock, ret, i;
>> > - gchar *pid_file;
>> > gchar *command;
>> > const char *qemu_binary;
>> > pid_t pid;
>> > @@ -115,9 +115,11 @@ QTestState *qtest_init(const char *extra_args)
>> >
>> > s = g_malloc(sizeof(*s));
>> >
>> > - s->socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
>> > - s->qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
>> > - pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
>> > + s->tmp_dir = g_strdup_printf("/tmp/qtest-%d-XXXXXX", getpid());
>> > + g_assert(mkdtemp(s->tmp_dir) != NULL);
>> > + s->socket_path = g_strdup_printf("%s/%s", s->tmp_dir, "sock");
>> > + s->qmp_socket_path = g_strdup_printf("%s/%s", s->tmp_dir, "qmp");
>> > + s->pid_file = g_strdup_printf("%s/%s", s->tmp_dir, "pid");
>> >
>> > sock = init_socket(s->socket_path);
>> > qmpsock = init_socket(s->qmp_socket_path);
>> > @@ -131,7 +133,7 @@ QTestState *qtest_init(const char *extra_args)
>> > "-pidfile %s "
>> > "-machine accel=qtest "
>> > "%s", qemu_binary, s->socket_path,
>> > - s->qmp_socket_path, pid_file,
>> > + s->qmp_socket_path, s->pid_file,
>> > extra_args ?: "");
>> >
>> > ret = system(command);
>> > @@ -143,7 +145,6 @@ QTestState *qtest_init(const char *extra_args)
>> > s->qmp_fd = socket_accept(qmpsock);
>> >
>> > s->rx = g_string_new("");
>> > - s->pid_file = pid_file;
>> > for (i = 0; i < MAX_IRQ; i++) {
>> > s->irq_level[i] = false;
>> > }
>> > @@ -172,9 +173,11 @@ void qtest_quit(QTestState *s)
>> > unlink(s->pid_file);
>> > unlink(s->socket_path);
>> > unlink(s->qmp_socket_path);
>> > + unlink(s->tmp_dir);
>>
>> -EISDIR, rmdir() would be needed instead.
>>
>
> 'unlink()' tested fine on Linux. But yes, it might not be as portable.
>
> I looked at tempnam() and mktemp(), but they both generate linker warnings.
> 'mkstemp()' could be used but its awkward to delete the file right after
> its created so that bind() can create it. Plus, it could be a greater
> security risk in that the filename is easier to determine.
>
> We could write our own random file string generator then, if mkdtemp(),
> isn't ok.
Or introduce qemu_mkdtemp() which resolves either to mkdtemp() or to a
custom implementation:
http://stackoverflow.com/questions/278439/creating-a-temporary-directory-in-windows
>
>> > g_free(s->pid_file);
>> > g_free(s->socket_path);
>> > g_free(s->qmp_socket_path);
>> > + g_free(s->tmp_dir);
>> > }
>> >
>> > static void socket_sendf(int fd, const char *fmt, va_list ap)
>> > --
>> > 1.7.1
>> >
