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

[Anthony Liguori]

On 03/12/2011 05:23 AM, Blue Swirl wrote:
> On Sat, Mar 12, 2011 at 1:05 AM, Anthony Liguori<address@hidden>  wrote:
> > This provides a glib-test based testing framework for QMP
> >
> > Signed-off-by: Anthony Liguori<address@hidden>
> >
> > 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<address@hidden>
> > + *
> > + * 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