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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list