On Tue, Jun 25, 2013 at 11:38:15AM +0200, Michal Privoznik wrote:
As my punishment for the break in 7f15ebc7 (fixed in 752596b5dd)
I'm
introducing this test to make sure it won't happen again. Currently,
only test for <graphics/> is supported.
---
.gitignore | 1 +
tests/Makefile.am | 11 +-
tests/qemuhotplugtest.c | 208 +++++++++++++++++++++
.../qemuhotplug-disk-cdrom-nochange.xml | 6 +
.../qemuhotplug-graphics-spice-listen.xml | 11 ++
.../qemuhotplug-graphics-spice-nochange.xml | 11 ++
...qemuhotplug-graphics-spice-timeout-nochange.xml | 1 +
...qemuhotplug-graphics-spice-timeout-password.xml | 1 +
.../qemuxml2argv-graphics-spice-listen-network.xml | 45 +++++
9 files changed, 293 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuhotplugtest.c
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-disk-cdrom-nochange.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-listen.xml
create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-nochange.xml
create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-timeout-nochange.xml
create mode 100644
tests/qemuhotplugtestdata/qemuhotplug-graphics-spice-timeout-password.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-listen-network.xml
ACK
diff --git a/.gitignore b/.gitignore
index 7a28941..3efc2e4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -167,6 +167,7 @@
/tests/openvzutilstest
/tests/qemuargv2xmltest
/tests/qemuhelptest
+/tests/qemuhotplugtest
/tests/qemumonitorjsontest
/tests/qemumonitortest
/tests/qemuxmlnstest
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9c9c802..a019eb9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -155,7 +155,7 @@ endif
if WITH_QEMU
test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
- qemumonitortest qemumonitorjsontest
+ qemumonitortest qemumonitorjsontest qemuhotplugtest
endif
if WITH_LXC
@@ -405,6 +405,13 @@ qemumonitorjsontest_SOURCES = \
$(NULL)
qemumonitorjsontest_LDADD = $(qemu_LDADDS) libqemumonitortestutils.la
+qemuhotplugtest_SOURCES = \
+ qemuhotplugtest.c \
+ testutils.c testutils.h \
+ testutilsqemu.c testutilsqemu.h \
+ $(NULL)
+qemuhotplugtest_LDADD = $(qemu_LDADDS) libqemumonitortestutils.la
+
domainsnapshotxml2xmltest_SOURCES = \
domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \
testutils.c testutils.h
@@ -413,7 +420,7 @@ else
EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \
qemumonitortest.c testutilsqemu.c testutilsqemu.h \
- qemumonitorjsontest.c \
+ qemumonitorjsontest.c qemuhotplugtest.c \
$(QEMUMONITORTESTUTILS_SOURCES)
endif
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
new file mode 100644
index 0000000..ed3ca7f
--- /dev/null
+++ b/tests/qemuhotplugtest.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright (C) 2011-2013 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 "qemu/qemu_conf.h"
+#include "qemu/qemu_hotplug.h"
+#include "qemumonitortestutils.h"
+#include "testutils.h"
+#include "testutilsqemu.h"
+#include "virerror.h"
+#include "virstring.h"
+#include "virthread.h"
+#include "virfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+static virQEMUDriver driver;
+
+struct qemuHotplugTestData {
+ const char *domain_filename;
+ const char *device_filename;
+ bool fail;
+ const char *const *mon;
+};
+
+static int
+qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
+ virDomainObjPtr *vm,
+ const char *filename)
+{
+ int ret = -1;
+
+ if (!(*vm = virDomainObjNew(xmlopt)))
+ goto cleanup;
+
+ if (!((*vm)->def = virDomainDefParseFile(filename,
+ driver.caps,
+ driver.xmlopt,
+ QEMU_EXPECTED_VIRT_TYPES,
+ 0)))
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ return ret;
+}
+
+static int
+testQemuHotplug(const void *data)
+{
+ int ret = -1;
+ struct qemuHotplugTestData *test = (struct qemuHotplugTestData *) data;
+ char *domain_filename = NULL;
+ char *device_filename = NULL;
+ char *device_xml = NULL;
+ const char *const *tmp;
+ bool fail = test->fail;
+ virDomainObjPtr vm = NULL;
+ virDomainDeviceDefPtr dev = NULL;
+ virCapsPtr caps = NULL;
+ qemuMonitorTestPtr test_mon = NULL;
+ qemuDomainObjPrivatePtr priv;
+
+ if (virAsprintf(&domain_filename,
"%s/qemuxml2argvdata/qemuxml2argv-%s.xml",
+ abs_srcdir, test->domain_filename) < 0 ||
+ virAsprintf(&device_filename,
"%s/qemuhotplugtestdata/qemuhotplug-%s.xml",
+ abs_srcdir, test->device_filename) < 0)
+ goto cleanup;
+
+ if (!(caps = virQEMUDriverGetCapabilities(&driver, false)))
+ goto cleanup;
+
+ if (qemuHotplugCreateObjects(driver.xmlopt, &vm, domain_filename) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (virtTestLoadFile(device_filename, &device_xml) < 0)
+ goto cleanup;
+
+ if (!(dev = virDomainDeviceDefParse(device_xml, vm->def,
+ caps, driver.xmlopt,
+ VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
+
+ /* Now is the best time to feed the spoofed monitor with predefined
+ * replies. */
+ if (!(test_mon = qemuMonitorTestNew(true, driver.xmlopt)))
+ goto cleanup;
+
+ tmp = test->mon;
+ while (tmp && *tmp) {
+ const char *command_name;
+ const char *response;
+
+ if (!(command_name = *tmp++) ||
+ !(response = *tmp++))
+ break;
+ if (qemuMonitorTestAddItem(test_mon, command_name, response) < 0)
+ goto cleanup;
+ }
+
+ priv->mon = qemuMonitorTestGetMonitor(test_mon);
+ priv->monJSON = true;
+
+ /* XXX We need to unlock the monitor here, as
+ * qemuDomainObjEnterMonitorInternal (called from qemuDomainChangeGraphics)
+ * tries to lock it again */
+ virObjectUnlock(priv->mon);
+
+ /* XXX Ideally, we would call qemuDomainUpdateDeviceLive here. But that
+ * would require us to provide virConnectPtr and virDomainPtr (they're used
+ * in case of updating a disk device. So for now, we will proceed with
+ * breaking the function into pieces. If we ever learn how to fake those
+ * required object, we can replace this code then. */
+ switch (dev->type) {
+ case VIR_DOMAIN_DEVICE_GRAPHICS:
+ ret = qemuDomainChangeGraphics(&driver, vm, dev->data.graphics);
+ break;
+ default:
+ if (virTestGetVerbose())
+ fprintf(stderr, "device type '%s' cannot be updated",
+ virDomainDeviceTypeToString(dev->type));
+ break;
+ }
+
+cleanup:
+ VIR_FREE(domain_filename);
+ VIR_FREE(device_filename);
+ VIR_FREE(device_xml);
+ /* don't dispose test monitor with VM */
+ priv->mon = NULL;
+ virObjectUnref(vm);
+ virDomainDeviceDefFree(dev);
+ virObjectUnref(caps);
+ qemuMonitorTestFree(test_mon);
+ return ((ret < 0 && fail) || (!ret && !fail)) ? 0 : -1;
+}
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+#if !WITH_YAJL
+ fputs("libvirt not compiled with yajl, skipping this test\n", stderr);
+ return EXIT_AM_SKIP;
+#endif
+
+ if (virThreadInitialize() < 0 ||
+ !(driver.caps = testQemuCapsInit()) ||
+ !(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver)))
+ return EXIT_FAILURE;
+
+ virEventRegisterDefaultImpl();
+
+ driver.config = virQEMUDriverConfigNew(false);
+ VIR_FREE(driver.config->spiceListen);
+ VIR_FREE(driver.config->vncListen);
+
+ /* some dummy values from 'config file' */
+ if (VIR_STRDUP_QUIET(driver.config->spicePassword, "123456") < 0)
+ return EXIT_FAILURE;
+
+#define DO_TEST(file, dev, fial, ...) \
+ do { \
+ const char *my_mon[] = { __VA_ARGS__, NULL}; \
+ struct qemuHotplugTestData data = \
+ {.domain_filename = file, .device_filename = dev, .fail = fial, \
+ .mon = my_mon}; \
+ if (virtTestRun(#file, 1, testQemuHotplug, &data) < 0) \
+ ret = -1; \
+ } while (0)
What's with the 'fail' parameter you're passing across test cases.
AFAICT, no test needs to be aware of the fail status of any earlier
test. You're re-creating the fake monitor for each test case so
no state is shared between tests. Just setting the 'ret = -1' here
is sufficient
+
+ DO_TEST("graphics-spice", "graphics-spice-nochange", false,
NULL);
+ DO_TEST("graphics-spice-timeout",
"graphics-spice-timeout-nochange", false,
+ "set_password", "{\"return\":{}}",
"expire_password", "{\"return\":{}}");
+ DO_TEST("graphics-spice-timeout",
"graphics-spice-timeout-password", false,
+ "set_password", "{\"return\":{}}",
"expire_password", "{\"return\":{}}");
+ DO_TEST("graphics-spice", "graphics-spice-listen", true, NULL);
+ DO_TEST("graphics-spice-listen-network",
"graphics-spice-listen-network", false,
+ "set_password", "{\"return\":{}}",
"expire_password", "{\"return\":{}}");
+ /* Strange huh? Currently, only graphics can be testet :-P */
+ DO_TEST("disk-cdrom", "disk-cdrom-nochange", true, NULL);
+
+ virObjectUnref(driver.caps);
+ virObjectUnref(driver.xmlopt);
+ return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
Everything looks good, aside from the 'fail' flag there. ACK if that
is just removed.
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 :|