From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Add some non-null annotations to qemuMonitorOpen and also check that the error callback is set, since it is mandatory --- src/qemu/qemu_monitor.c | 7 ++++++- src/qemu/qemu_monitor.h | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6ce1839..290f150 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -683,11 +683,16 @@ qemuMonitorOpen(virDomainObjPtr vm, { qemuMonitorPtr mon; - if (!cb || !cb->eofNotify) { + if (!cb->eofNotify) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("EOF notify callback must be supplied")); return NULL; } + if (!cb->errorNotify) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error notify callback must be supplied")); + return NULL; + } if (qemuMonitorInitialize() < 0) return NULL; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index ad8d2f1..2033473 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -145,7 +145,8 @@ char *qemuMonitorUnescapeArg(const char *in); qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, virDomainChrSourceDefPtr config, int json, - qemuMonitorCallbacksPtr cb); + qemuMonitorCallbacksPtr cb) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4); void qemuMonitorClose(qemuMonitorPtr mon); -- 1.7.11.2

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; To be able to test the QEMU monitor code, we need to have a fake QEMU monitor server. This introduces a simple (dumb) framework that can do this. The test case registers a series of items to be sent back as replies to commands that will be executed. A thread runs the event loop looking for incoming replies and sending back this pre-registered data. This allows testing all QEMU monitor code that deals with parsing responses and errors from QEMU, without needing QEMU around --- cfg.mk | 2 +- tests/Makefile.am | 22 +- tests/qemumonitortestutils.c | 499 +++++++++++++++++++++++++++++++++++++++++++ tests/qemumonitortestutils.h | 41 ++++ 4 files changed, 560 insertions(+), 4 deletions(-) create mode 100644 tests/qemumonitortestutils.c create mode 100644 tests/qemumonitortestutils.h diff --git a/cfg.mk b/cfg.mk index d2e54e3..224f89f 100644 --- a/cfg.mk +++ b/cfg.mk @@ -741,7 +741,7 @@ exclude_file_name_regexp--sc_copyright_address = \ exclude_file_name_regexp--sc_flags_usage = ^(docs/|src/util/virnetdevtap\.c$$) exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \ - ^src/rpc/gendispatch\.pl$$ + ^(src/rpc/gendispatch\.pl$$|tests/) exclude_file_name_regexp--sc_po_check = ^(docs/|src/rpc/gendispatch\.pl$$) diff --git a/tests/Makefile.am b/tests/Makefile.am index 60d322d..22a7a26 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -215,12 +215,17 @@ endif EXTRA_DIST += $(test_scripts) +test_libraries = libshunload.la +if WITH_QEMU +test_libraries += libqemumonitortestutils.la +endif + if WITH_TESTS noinst_PROGRAMS = $(test_programs) $(test_helpers) -noinst_LTLIBRARIES = libshunload.la +noinst_LTLIBRARIES = $(test_libraries) else check_PROGRAMS = $(test_programs) $(test_helpers) -check_LTLIBRARIES = libshunload.la +check_LTLIBRARIES = $(test_libraries) endif TESTS = $(test_programs) \ @@ -294,8 +299,18 @@ EXTRA_DIST += xml2sexprtest.c sexpr2xmltest.c xmconfigtest.c \ testutilsxen.c testutilsxen.h endif +QEMUMONITORTESTUTILS_SOURCES = \ + qemumonitortestutils.c \ + qemumonitortestutils.h \ + $(NULL) + if WITH_QEMU +libqemumonitortestutils_la_SOURCES = $(QEMUMONITORTESTUTILS_SOURCES) +libqemumonitortestutils_la_CFLAGS = \ + -Dabs_builddir="\"`pwd`\"" $(AM_CFLAGS) + + qemu_LDADDS = ../src/libvirt_driver_qemu_impl.la if WITH_NETWORK qemu_LDADDS += ../src/libvirt_driver_network_impl.la @@ -338,7 +353,8 @@ domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) else EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \ - qemumonitortest.c testutilsqemu.c testutilsqemu.h + qemumonitortest.c testutilsqemu.c testutilsqemu.h \ + $(QEMUMONITORTESTUTILS_SOURCES) endif if WITH_LXC diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c new file mode 100644 index 0000000..76b11e6 --- /dev/null +++ b/tests/qemumonitortestutils.c @@ -0,0 +1,499 @@ +/* + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <time.h> + +#include "qemumonitortestutils.h" + +#include "threads.h" +#include "qemu/qemu_monitor.h" +#include "rpc/virnetsocket.h" +#include "memory.h" +#include "util.h" +#include "logging.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +typedef struct _qemuMonitorTestItem qemuMonitorTestItem; +typedef qemuMonitorTestItem *qemuMonitorTestItemPtr; + +struct _qemuMonitorTestItem { + char *command_name; + char *response; +}; + +struct _qemuMonitorTest { + virMutex lock; + virThread thread; + + bool json; + bool quit; + bool running; + + char *incoming; + size_t incomingLength; + size_t incomingCapacity; + + char *outgoing; + size_t outgoingLength; + size_t outgoingCapacity; + + virNetSocketPtr server; + virNetSocketPtr client; + + qemuMonitorPtr mon; + + size_t nitems; + qemuMonitorTestItemPtr *items; + + virDomainObjPtr vm; +}; + + +static void qemuMonitorTestItemFree(qemuMonitorTestItemPtr item); + +/* + * Appends data for a reply onto the outgoing buffer + */ +static int qemuMonitorTestAddReponse(qemuMonitorTestPtr test, + const char *response) +{ + size_t want = strlen(response) + 2; + size_t have = test->outgoingCapacity - test->outgoingLength; + + if (have < want) { + size_t need = want - have; + if (VIR_EXPAND_N(test->outgoing, test->outgoingCapacity, need) < 0) { + virReportOOMError(); + return -1; + } + } + + want -= 2; + memcpy(test->outgoing + test->outgoingLength, + response, + want); + memcpy(test->outgoing + test->outgoingLength + want, + "\r\n", + 2); + test->outgoingLength += want + 2; + return 0; +} + + +/* + * Processes a single line, looking for a matching expected + * item to reply with, else replies with an error + */ +static int qemuMonitorTestProcessCommandJSON(qemuMonitorTestPtr test, + const char *cmdstr) +{ + virJSONValuePtr val; + const char *cmdname; + int ret = -1; + + if (!(val = virJSONValueFromString(cmdstr))) + return -1; + + if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Missing command name in %s", cmdstr); + goto cleanup; + } + + if (test->nitems == 0 || + STRNEQ(test->items[0]->command_name, cmdname)) { + ret = qemuMonitorTestAddReponse(test, + "{ \"error\": " + " { \"desc\": \"Unexpected command\", " + " \"class\": \"UnexpectedCommand\" } }"); + } else { + ret = qemuMonitorTestAddReponse(test, + test->items[0]->response); + qemuMonitorTestItemFree(test->items[0]); + if (test->nitems == 1) { + VIR_FREE(test->items); + test->nitems = 0; + } else { + memmove(test->items, + test->items + 1, + sizeof(test->items[0]) * (test->nitems - 1)); + VIR_SHRINK_N(test->items, test->nitems, 1); + } + } + +cleanup: + virJSONValueFree(val); + return ret; +} + + +static int qemuMonitorTestProcessCommandText(qemuMonitorTestPtr test, + const char *cmdstr) +{ + char *tmp; + char *cmdname; + int ret = -1; + + if (!(cmdname = strdup(cmdstr))) { + virReportOOMError(); + return -1; + } + if (!(tmp = strchr(cmdname, ' '))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Cannot find command name in '%s'", cmdstr); + goto cleanup; + } + *tmp = '\0'; + + if (test->nitems == 0 || + STRNEQ(test->items[0]->command_name, cmdname)) { + ret = qemuMonitorTestAddReponse(test, + "unexpected command"); + } else { + ret = qemuMonitorTestAddReponse(test, + test->items[0]->response); + qemuMonitorTestItemFree(test->items[0]); + if (test->nitems == 1) { + VIR_FREE(test->items); + test->nitems = 0; + } else { + memmove(test->items, + test->items + 1, + sizeof(test->items[0]) * (test->nitems - 1)); + VIR_SHRINK_N(test->items, test->nitems, 1); + } + } + +cleanup: + VIR_FREE(cmdname); + return ret; +} + +static int qemuMonitorTestProcessCommand(qemuMonitorTestPtr test, + const char *cmdstr) +{ + if (test->json) + return qemuMonitorTestProcessCommandJSON(test ,cmdstr); + else + return qemuMonitorTestProcessCommandText(test ,cmdstr); +} + +/* + * Handles read/write of monitor data on the monitor server side + */ +static void qemuMonitorTestIO(virNetSocketPtr sock, + int events, + void *opaque) +{ + qemuMonitorTestPtr test = opaque; + bool err = false; + + virMutexLock(&test->lock); + if (events & VIR_EVENT_HANDLE_WRITABLE) { + ssize_t ret; + if ((ret = virNetSocketWrite(sock, + test->outgoing, + test->outgoingLength)) < 0) { + err = true; + goto cleanup; + } + + memmove(test->outgoing, + test->outgoing + ret, + test->outgoingLength - ret); + test->outgoingLength -= ret; + + if ((test->outgoingCapacity - test->outgoingLength) > 1024) + VIR_SHRINK_N(test->outgoing, test->outgoingCapacity, 1024); + } + + if (events & VIR_EVENT_HANDLE_READABLE) { + ssize_t ret, used; + char *t1, *t2; + + if ((test->incomingCapacity - test->incomingLength) < 1024) { + if (VIR_EXPAND_N(test->incoming, test->incomingCapacity, 1024) < 0) { + err = true; + goto cleanup; + } + } + + if ((ret = virNetSocketRead(sock, + test->incoming + test->incomingLength, + (test->incomingCapacity - test->incomingLength) - 1)) < 0) { + err = true; + goto cleanup; + } + test->incomingLength += ret; + test->incoming[test->incomingLength] = '\0'; + + /* Look to see if we've got a complete line, and + * if so, handle that command + */ + t1 = test->incoming; + while ((t2 = strstr(t1, "\r\n"))) { + *t2 = '\0'; + + if (qemuMonitorTestProcessCommand(test, t1) < 0) { + err = true; + goto cleanup; + } + + t1 = t2 + 2; + } + used = t1 - test->incoming; + memmove(test->incoming, t1, test->incomingLength - used); + test->incomingLength -= used; + if ((test->incomingCapacity - test->incomingLength) > 1024) { + VIR_SHRINK_N(test->incoming, + test->incomingCapacity, + 1024); + } + } + + if (events & (VIR_EVENT_HANDLE_HANGUP | + VIR_EVENT_HANDLE_ERROR)) + err = true; + +cleanup: + if (err) { + virNetSocketRemoveIOCallback(sock); + virNetSocketClose(sock); + virObjectUnref(test->client); + test->client = NULL; + } else { + events = VIR_EVENT_HANDLE_READABLE; + + if (test->outgoingLength) + events |= VIR_EVENT_HANDLE_WRITABLE; + + virNetSocketUpdateIOCallback(sock, events); + } + virMutexUnlock(&test->lock); +} + + +static void qemuMonitorTestWorker(void *opaque) +{ + qemuMonitorTestPtr test = opaque; + + virMutexLock(&test->lock); + + while (!test->quit) { + virMutexUnlock(&test->lock); + + if (virEventRunDefaultImpl() < 0) { + test->quit = true; + break; + } + + virMutexLock(&test->lock); + } + + test->running = false; + + virMutexUnlock(&test->lock); + return; +} + +static void qemuMonitorTestItemFree(qemuMonitorTestItemPtr item) +{ + if (!item) + return; + + VIR_FREE(item->command_name); + VIR_FREE(item->response); + + VIR_FREE(item); +} + + +void qemuMonitorTestFree(qemuMonitorTestPtr test) +{ + size_t i; + + if (!test) + return; + + virMutexLock(&test->lock); + if (test->running) { + test->quit = true; + } + virMutexUnlock(&test->lock); + + if (test->client) { + virNetSocketRemoveIOCallback(test->client); + virNetSocketClose(test->client); + virObjectUnref(test->client); + } + + virObjectUnref(test->server); + if (test->mon) { + qemuMonitorUnlock(test->mon); + qemuMonitorClose(test->mon); + } + + virObjectUnref(test->vm); + + if (test->running) + virThreadJoin(&test->thread); + + for (i = 0 ; i < test->nitems ; i++) + qemuMonitorTestItemFree(test->items[i]); + VIR_FREE(test->items); + + virMutexDestroy(&test->lock); + VIR_FREE(test); +} + + +int +qemuMonitorTestAddItem(qemuMonitorTestPtr test, + const char *command_name, + const char *response) +{ + qemuMonitorTestItemPtr item; + + if (VIR_ALLOC(item) < 0) + goto no_memory; + + if (!(item->command_name = strdup(command_name)) || + !(item->response = strdup(response))) + goto no_memory; + + virMutexLock(&test->lock); + if (VIR_EXPAND_N(test->items, test->nitems, 1) < 0) { + virMutexUnlock(&test->lock); + goto no_memory; + } + test->items[test->nitems - 1] = item; + + virMutexUnlock(&test->lock); + + return 0; + +no_memory: + virReportOOMError(); + qemuMonitorTestItemFree(item); + return -1; +} + + +static void qemuMonitorTestEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ +} + +static void qemuMonitorTestErrorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ +} + + +static qemuMonitorCallbacks qemuCallbacks = { + .eofNotify = qemuMonitorTestEOFNotify, + .errorNotify = qemuMonitorTestErrorNotify, +}; + +qemuMonitorTestPtr qemuMonitorTestNew(bool json, virCapsPtr caps) +{ + qemuMonitorTestPtr test; + const char *path = abs_builddir "/qemumonitorjsontest.sock"; + virDomainChrSourceDef src; + + memset(&src, 0, sizeof(src)); + src.type = VIR_DOMAIN_CHR_TYPE_UNIX; + src.data.nix.path = (char *)path; + src.data.nix.listen = false; + + if (VIR_ALLOC(test) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&test->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Cannot initialize mutex"); + VIR_FREE(test); + return NULL; + } + + test->json = json; + if (!(test->vm = virDomainObjNew(caps))) + goto error; + + if (virNetSocketNewListenUNIX(path, + 0700, + getuid(), + getgid(), + &test->server) < 0) + goto error; + + + if (virNetSocketListen(test->server, 1) < 0) + goto error; + + if (!(test->mon = qemuMonitorOpen(test->vm, + &src, + true, + &qemuCallbacks))) + goto error; + qemuMonitorLock(test->mon); + + if (virNetSocketAccept(test->server, &test->client) < 0) + goto error; + if (!test->client) + goto error; + + if (virNetSocketAddIOCallback(test->client, + VIR_EVENT_HANDLE_READABLE, + qemuMonitorTestIO, + test, + NULL) < 0) + goto error; + + virMutexLock(&test->lock); + if (virThreadCreate(&test->thread, + true, + qemuMonitorTestWorker, + test) < 0) { + virMutexUnlock(&test->lock); + goto error; + } + test->running = true; + virMutexUnlock(&test->lock); + + return test; + +error: + qemuMonitorTestFree(test); + return NULL; +} + +qemuMonitorPtr qemuMonitorTestGetMonitor(qemuMonitorTestPtr test) +{ + return test->mon; +} diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h new file mode 100644 index 0000000..0e9117c --- /dev/null +++ b/tests/qemumonitortestutils.h @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#ifndef __VIR_QEMU_MONITOR_TEST_UTILS_H__ +# define __VIR_QEMU_MONITOR_TEST_UTILS_H__ + +# include "capabilities.h" +# include "qemu/qemu_monitor.h" + +typedef struct _qemuMonitorTest qemuMonitorTest; +typedef qemuMonitorTest *qemuMonitorTestPtr; + +int +qemuMonitorTestAddItem(qemuMonitorTestPtr test, + const char *command_name, + const char *response); + +qemuMonitorTestPtr qemuMonitorTestNew(bool json, + virCapsPtr caps); + +void qemuMonitorTestFree(qemuMonitorTestPtr test); + +qemuMonitorPtr qemuMonitorTestGetMonitor(qemuMonitorTestPtr test); + +#endif /* __VIR_QEMU_MONITOR_TEST_UTILS_H__ */ -- 1.7.11.2

On Mon, Aug 20, 2012 at 08:11:14AM -0600, Eric Blake wrote: That was an intentionally heavy hammer. We shouldn't waste translators time on messages in the tests/ directory. In fact we should probably write a syntax-check rule to *forbid* use of _(...) in tests/ directory completely, rather than just whitelisted the existing rule. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Take advantage of the previously added monitor helpers to create a test suite for the QEMU JSON monitor impl. As a proof of concept, this tests the 'qemuMonitorGetStatus' implementation --- .gitignore | 1 + tests/Makefile.am | 11 +++- tests/qemumonitorjsontest.c | 150 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 tests/qemumonitorjsontest.c diff --git a/.gitignore b/.gitignore index 17dcae4..661509e 100644 --- a/.gitignore +++ b/.gitignore @@ -140,6 +140,7 @@ /tests/openvzutilstest /tests/qemuargv2xmltest /tests/qemuhelptest +/tests/qemumonitorjsontest /tests/qemumonitortest /tests/qemuxmlnstest /tests/qparamtest diff --git a/tests/Makefile.am b/tests/Makefile.am index 22a7a26..884bb7d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -109,7 +109,7 @@ endif if WITH_QEMU test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ - qemumonitortest + qemumonitortest qemumonitorjsontest endif if WITH_LXC @@ -346,6 +346,14 @@ qemuhelptest_LDADD = $(qemu_LDADDS) qemumonitortest_SOURCES = qemumonitortest.c testutils.c testutils.h qemumonitortest_LDADD = $(qemu_LDADDS) +qemumonitorjsontest_SOURCES = \ + qemumonitorjsontest.c \ + testutils.c testutils.h \ + testutilsqemu.c testutilsqemu.h \ + $(NULL) +qemumonitorjsontest_LDADD = $(qemu_LDADDS) libqemumonitortestutils.la +qemumonitorjsontest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) + domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h @@ -354,6 +362,7 @@ else EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \ qemumonitortest.c testutilsqemu.c testutilsqemu.h \ + qemumonitorjsontest.c \ $(QEMUMONITORTESTUTILS_SOURCES) endif diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c new file mode 100644 index 0000000..ea69feb --- /dev/null +++ b/tests/qemumonitorjsontest.c @@ -0,0 +1,150 @@ +/* + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#include <config.h> + +#include "testutils.h" +#include "testutilsqemu.h" +#include "qemumonitortestutils.h" +#include "threads.h" +#include "virterror_internal.h" + + +#define VIR_FROM_THIS VIR_FROM_NONE + +static int +testQemuMonitorJSONGetStatus(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + bool running = false; + virDomainPausedReason reason = 0; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "query-status", + "{ " + " \"return\": { " + " \"status\": \"running\", " + " \"singlestep\": false, " + " \"running\": true " + " } " + "}") < 0) + goto cleanup; + if (qemuMonitorTestAddItem(test, "query-status", + "{ " + " \"return\": { " + " \"singlestep\": false, " + " \"running\": false " + " } " + "}") < 0) + goto cleanup; + if (qemuMonitorTestAddItem(test, "query-status", + "{ " + " \"return\": { " + " \"status\": \"inmigrate\", " + " \"singlestep\": false, " + " \"running\": false " + " } " + "}") < 0) + goto cleanup; + + if (qemuMonitorGetStatus(qemuMonitorTestGetMonitor(test), + &running, &reason) < 0) + goto cleanup; + + if (!running) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Running was not true"); + goto cleanup; + } + + if (reason != VIR_DOMAIN_PAUSED_UNKNOWN) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Reason was unexpectedly set to %d", reason); + goto cleanup; + } + + if (qemuMonitorGetStatus(qemuMonitorTestGetMonitor(test), + &running, &reason) < 0) + goto cleanup; + + if (running) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Running was not false"); + goto cleanup; + } + + if (reason != VIR_DOMAIN_PAUSED_UNKNOWN) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Reason was unexpectedly set to %d", reason); + goto cleanup; + } + + if (qemuMonitorGetStatus(qemuMonitorTestGetMonitor(test), + &running, &reason) < 0) + goto cleanup; + + if (running) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Running was not false"); + goto cleanup; + } + + if (reason != VIR_DOMAIN_PAUSED_MIGRATION) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Reason was unexpectedly set to %d", reason); + goto cleanup; + } + + ret = 0; + +cleanup: + qemuMonitorTestFree(test); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + virCapsPtr caps; + + if (virThreadInitialize() < 0) + exit(EXIT_FAILURE); + + if (!(caps = testQemuCapsInit())) + exit(EXIT_FAILURE); + + virEventRegisterDefaultImpl(); + +#define DO_TEST(name) \ + if (virtTestRun(# name, 1, testQemuMonitorJSON ## name, caps) < 0) \ + ret = -1 + + DO_TEST(GetStatus); + + virCapabilitiesFree(caps); + + return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.7.11.2

On 08/20/2012 07:49 AM, Daniel P. Berrange wrote: Given this error, why not ATTRIBUTE_NONNULL(1) as well? query-version has existed since 0.14.0, so we are safe using it if we assume JSON. Hmm, weren't there versions, like 1.1, which lacked micro? /me researches... That's true for the 'qemu -help' output, but it appears that the QMP command has always provided micro, even when it is 0. So this should be safe. Why is it an error if package is non-NULL but package data was not present? Can't we just leave *package=NULL in that case, rather than erroring out? After all, when package is NULL, we don't care whether package data was present. Shouldn't we test typical values in use by actual qemu? For example, with RHEL, I see something like "package":"(qemu-kvm-0.12.1.2)". Overall, I like the idea. But do we have any code that uses the new monitor command, besides the testsuite? -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Add a new qemuMonitorGetMachines() method to support invocation of the 'query-machines' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2 --- src/qemu/qemu_monitor.c | 30 +++++++++++++ src/qemu/qemu_monitor.h | 15 +++++++ src/qemu/qemu_monitor_json.c | 101 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++ tests/qemumonitorjsontest.c | 76 ++++++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 43c3a8c..d2534a9 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3005,3 +3005,33 @@ int qemuMonitorGetVersion(qemuMonitorPtr mon, return qemuMonitorJSONGetVersion(mon, major, minor, micro, package); } + +int qemuMonitorGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines) +{ + VIR_DEBUG("mon=%p machines=%p", + mon, machines); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetMachines(mon, machines); +} + +void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine) +{ + if (!machine) + return; + VIR_FREE(machine->name); + VIR_FREE(machine->alias); + VIR_FREE(machine); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 573f6c1..364e0ad 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -581,6 +581,21 @@ int qemuMonitorGetVersion(qemuMonitorPtr mon, char **package) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + +typedef struct _qemuMonitorMachineInfo qemuMonitorMachineInfo; +typedef qemuMonitorMachineInfo *qemuMonitorMachineInfoPtr; + +struct _qemuMonitorMachineInfo { + char *name; + bool isDefault; + char *alias; +}; + +int qemuMonitorGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines); + +void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c03b524..73cfcb7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3907,3 +3907,104 @@ cleanup: virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + qemuMonitorMachineInfoPtr *infolist = NULL; + int n = 0; + size_t i; + + *machines = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-machines", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply data was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(infolist, n) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + qemuMonitorMachineInfoPtr info; + + if (VIR_ALLOC(info) < 0) { + virReportOOMError(); + goto cleanup; + } + + infolist[i] = info; + + if (!(tmp = virJSONValueObjectGetString(child, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply data was missing 'name'")); + goto cleanup; + } + + if (!(info->name = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + + if (virJSONValueObjectHasKey(child, "is-default") && + virJSONValueObjectGetBoolean(child, "is-default", &info->isDefault) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply has malformed 'is-default' data")); + goto cleanup; + } + + if (virJSONValueObjectHasKey(child, "alias")) { + if (!(tmp = virJSONValueObjectGetString(child, "alias"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply has malformed 'alias' data")); + goto cleanup; + } + if (!(info->alias = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + } + + ret = n; + *machines = infolist; + +cleanup: + if (ret < 0 && infolist) { + for (i = 0 ; i < n ; i++) + qemuMonitorMachineInfoFree(infolist[i]); + VIR_FREE(infolist); + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 26bfc2c..0e29155 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -291,4 +291,8 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, char **package) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines) + ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 21d5252..66f89fc 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -224,6 +224,81 @@ cleanup: } static int +testQemuMonitorJSONGetMachines(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + qemuMonitorMachineInfoPtr *info; + int ninfo; + const char *null = NULL; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "query-machines", + "{ " + " \"return\": [ " + " { " + " \"name\": \"pc-1.0\" " + " }, " + " { " + " \"name\": \"pc-1.1\" " + " }, " + " { " + " \"name\": \"pc-1.2\", " + " \"is-default\": true, " + " \"alias\": \"pc\" " + " } " + " ]" + "}") < 0) + goto cleanup; + + if ((ninfo = qemuMonitorGetMachines(qemuMonitorTestGetMonitor(test), + &info)) < 0) + goto cleanup; + + if (ninfo != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "ninfo %d is not 3", ninfo); + goto cleanup; + } + +#define CHECK(i, wantname, wantisDefault, wantalias) \ + do { \ + if (STRNEQ(info[i]->name, (wantname))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "name %s is not %s", \ + info[i]->name, (wantname)); \ + goto cleanup; \ + } \ + if (info[i]->isDefault != (wantisDefault)) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "isDefault %d is not %d", \ + info[i]->isDefault, (wantisDefault)); \ + goto cleanup; \ + } \ + if (STRNEQ_NULLABLE(info[i]->alias, (wantalias))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "alias %s is not %s", \ + info[i]->alias, NULLSTR(wantalias)); \ + goto cleanup; \ + } \ + } while (0) + + CHECK(0, "pc-1.0", false, null); + CHECK(1, "pc-1.1", false, null); + CHECK(2, "pc-1.2", true, "pc"); + + ret = 0; + +cleanup: + qemuMonitorTestFree(test); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -243,6 +318,7 @@ mymain(void) DO_TEST(GetStatus); DO_TEST(GetVersion); + DO_TEST(GetMachines); virCapabilitiesFree(caps); -- 1.7.11.2

On Mon, Aug 20, 2012 at 10:54:47AM -0600, Eric Blake wrote: I hit some GCC-wierdness when doing that: CC qemumonitorjsontest-qemumonitorjsontest.o qemumonitorjsontest.c: In function 'testQemuMonitorJSONGetMachines': qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 1) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 1) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 2) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 2) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 1) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 1) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 2) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 2) [-Werror=nonnull] qemumonitorjsontest.c:289:5: error: invalid application of 'sizeof' to a void type [-Werror=pointer-arith] Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|