Re: [Qemu-devel] [PATCH 14/15] qapi: add test-libqmp

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/12/2011 05:23 AM, Blue Swirl wrote:
> On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> This provides a glib-test based testing framework for QMP
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>
>> 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<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.  See
>> + * the COPYING.LIB file in the top-level directory.
>> + */
>> +#include<stdio.h>
>> +#include<sys/socket.h>
>> +#include<netinet/in.h>
>> +#include<netinet/tcp.h>
>> +#include<arpa/inet.h>
>> +#include<sys/un.h>
>> +#include<stdlib.h>
>> +#include<glib.h>
>> +#include<sys/wait.h>
>> +#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