Re: [Qemu-devel] [PATCH 1/3] qtest: Enable creation of multiple qemu instances

[Blue Swirl]

On Thu, Dec 13, 2012 at 10:02 PM, Jason Baron <address@hidden> wrote:
> From: Jason Baron <address@hidden>
>
> 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 the child pid instead,
> so we can remove that limitation.
>
> Signed-off-by: Jason Baron <address@hidden>
> ---
>  tests/libqtest.c |   31 +++++++++++++++++--------------
>  1 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 71b84c1..f3dd4e4 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -101,6 +101,10 @@ static pid_t qtest_qemu_pid(QTestState *s)
>      return pid;
>  }
>
> +#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
> +#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
> +#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"

These filenames are too predictable from security point of view,
please change the code to use mkstemp() instead. That would also solve
the original file name conflict problem.

> +
>  QTestState *qtest_init(const char *extra_args)
>  {
>      QTestState *s;
> @@ -113,25 +117,16 @@ QTestState *qtest_init(const char *extra_args)
>      qemu_binary = getenv("QTEST_QEMU_BINARY");
>      g_assert(qemu_binary != NULL);
>
> -    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());
> -
> -    sock = init_socket(s->socket_path);
> -    qmpsock = init_socket(s->qmp_socket_path);
> -
>      pid = fork();
>      if (pid == 0) {
>          command = g_strdup_printf("%s "
> -                                  "-qtest unix:%s,nowait "
> +                                  "-qtest unix:" QTEST_FILE_TEMP ",nowait "
>                                    "-qtest-log /dev/null "
> -                                  "-qmp unix:%s,nowait "
> -                                  "-pidfile %s "
> +                                  "-qmp unix:" QTEST_QMP_FILE_TEMP ",nowait "
> +                                  "-pidfile " QTEST_PID_FILE_TEMP " "
>                                    "-machine accel=qtest "
> -                                  "%s", qemu_binary, s->socket_path,
> -                                  s->qmp_socket_path, pid_file,
> +                                  "%s", qemu_binary, getpid(),
> +                                  getpid(), getpid(),
>                                    extra_args ?: "");
>
>          ret = system(command);
> @@ -139,6 +134,14 @@ QTestState *qtest_init(const char *extra_args)
>          g_free(command);
>      }
>
> +    s = g_malloc(sizeof(*s));
> +    s->socket_path = g_strdup_printf(QTEST_FILE_TEMP, pid);
> +    s->qmp_socket_path = g_strdup_printf(QTEST_QMP_FILE_TEMP, pid);
> +    pid_file = g_strdup_printf(QTEST_PID_FILE_TEMP, pid);
> +
> +    sock = init_socket(s->socket_path);
> +    qmpsock = init_socket(s->qmp_socket_path);
> +
>      s->fd = socket_accept(sock);
>      s->qmp_fd = socket_accept(qmpsock);
>
> --
> 1.7.1
>
>