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

2012-12-20 Thread Jason Baron 
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
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-XX, 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);
 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

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

2012-12-20 Thread Blue Swirl 
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.

 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-XX, 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.

  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

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

2012-12-20 Thread Jason Baron 
On Thu, Dec 20, 2012 at 08:07:02PM +, 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?

 
  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-XX, 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.

   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

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

2012-12-20 Thread Blue Swirl 
On Thu, Dec 20, 2012 at 8:26 PM, Jason Baron jba...@redhat.com wrote:
 On Thu, Dec 20, 2012 at 08:07:02PM +, 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-XX, 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

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

2012-12-20 Thread Markus Armbruster 
Jason Baron jba...@redhat.com writes:

 On Thu, Dec 20, 2012 at 08:07:02PM +, Blue Swirl wrote:
 On Thu, Dec 20, 2012 at 5:14 PM, Jason Baron jba...@redhat.com wrote:
  From: Jason Baron jba...@redhat.com
[...]
  @@ -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.

s/might not be as/isn't/

SUSv7:

[EPERM]
The file named by path is a directory, and either the calling
process does not have appropriate privileges, or the implementation
prohibits using unlink() on directories.

[...]