From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34930 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PyQI0-0003aT-Qn for qemu-devel@nongnu.org; Sat, 12 Mar 2011 09:59:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PyQHz-0003tN-8o for qemu-devel@nongnu.org; Sat, 12 Mar 2011 09:59:32 -0500 Received: from mail-yw0-f45.google.com ([209.85.213.45]:39489) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PyQHz-0003tC-3N for qemu-devel@nongnu.org; Sat, 12 Mar 2011 09:59:31 -0500 Received: by ywl41 with SMTP id 41so1863716ywl.4 for ; Sat, 12 Mar 2011 06:59:30 -0800 (PST) Message-ID: <4D7B8A50.5020303@codemonkey.ws> Date: Sat, 12 Mar 2011 08:59:28 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp References: <1299884745-521-1-git-send-email-aliguori@us.ibm.com> <1299884745-521-15-git-send-email-aliguori@us.ibm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org, Adam Litke , Luiz Capitulino , Markus Armbruster , Avi Kivity On 03/12/2011 05:23 AM, Blue Swirl wrote: > On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori wrote: >> This provides a glib-test based testing framework for QMP >> >> Signed-off-by: Anthony Liguori >> >> diff --git a/Makefile b/Makefile >> index 5170675..1d363d7 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -72,6 +72,8 @@ defconfig: >> >> -include config-all-devices.mak >> >> +TOOLS += test-libqmp >> + >> build-all: $(DOCS) $(TOOLS) recurse-all >> >> config-host.h: config-host.h-timestamp >> @@ -205,6 +207,15 @@ check-qlist: check-qlist.o qlist.o qint.o $(CHECK_PROG_DEPS) >> check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS) >> check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o $(CHECK_PROG_DEPS) >> >> +LIBQMP_OBJS := qmp-types.o libqmp.o error.o libqmp-core.o >> +LIBQMP_OBJS += qmp-marshal-types-core.o qmp-marshal-types.o >> +LIBQMP_OBJS += qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o >> +LIBQMP_OBJS += qerror.o >> +LIBQMP_OBJS += json-streamer.o json-lexer.o json-parser.o >> +LIBQMP_OBJS += $(oslib-obj-y) $(trace-obj-y) qemu-malloc.o >> + >> +test-libqmp: test-libqmp.o $(LIBQMP_OBJS) qemu-timer-common.o >> + >> clean: >> # avoid old build problems by removing potentially incorrect old files >> rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h >> diff --git a/test-libqmp.c b/test-libqmp.c >> new file mode 100644 > I'd put this to tests/. tests/ lives outside of the QEMU build system today. It's also very TCG specific. How about taking the current contents of tests/ and moving it to tests/tcg and moving test-libqmp and check-*.c to tests/? >> index 0000000..9b73987 >> --- /dev/null >> +++ b/test-libqmp.c >> @@ -0,0 +1,170 @@ >> +/* >> + * QAPI >> + * >> + * Copyright IBM, Corp. 2011 >> + * >> + * Authors: >> + * Anthony Liguori >> + * >> + * This work is licensed under the terms of the GNU LGPL, version 2. See >> + * the COPYING.LIB file in the top-level directory. >> + */ >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include "config-host.h" >> +#include "libqmp.h" >> +#include "qerror.h" >> + >> +#define g_assert_noerr(err) g_assert(err == NULL); >> +#define g_assert_anyerr(err) g_assert(err != NULL); >> +#define g_assert_cmperr(err, op, type) do { \ >> + g_assert_anyerr(err); \ >> + g_assert_cmpstr(error_get_field(err, "class"), op, type); \ >> +} while (0) >> + >> +static pid_t last_qemu_pid = -1; >> + >> +static QmpSession *qemu(const char *fmt, ...) >> +{ >> + char buffer0[4096]; >> + char buffer1[4096]; >> + const char *pid_filename = "/tmp/test-libqmp-qemu.pid"; >> + const char *path = "/tmp/test-libqmp-qemu.sock"; > Very insecure filenames. This disappears in round 3 when I introduce discovery support in libqmp. Even so, I don't think security is a major concern here. >> +static void wait_for_pid_exit(pid_t pid) >> +{ >> + FILE *f = NULL; >> + >> + /* This is ugly but I don't know of a better way */ > man waitpid? waitpid only works for child processes. Since we launch with -daemonize, that the QEMU instance is no longer a child process. We use -daemonize because it avoids the race condition where we try to connect to a QMP socket but QEMU hasn't created the socket yet. It also means that we can just use system() to invoke QEMU which makes life a whole lot simpler. Regards, Anthony Liguori