[libvirt] [PATCH 0/5] Fix snapshot xml test suite and add new test

To cover the case fixed in 5a66c667ff5cae61c2ad2e646c8eb3eedc67f925 we need to upgrade the testsuite first. Peter Krempa (5): domainsnapshotxml2xmltest: Clean up labels and use bool instead of int domainsnapshotxml2xmltest: Allow for better testing of snapshots domainsnapshotxml2xml: Move files with conflicting names domainsnapshotxml2xmltest: Add existing files as new tests domainsnapshotxml2xmltest: Add test case for empty driver element .../disk_driver_name_null.xml | 10 ++ tests/domainsnapshotxml2xmlin/external_vm.xml | 1 - tests/domainsnapshotxml2xmlin/noparent.xml | 1 + .../disk_driver_name_null.xml | 9 + tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 67 +------- .../disk_snapshot_redefine.xml | 80 +++++++++ tests/domainsnapshotxml2xmlout/empty.xml | 8 + tests/domainsnapshotxml2xmlout/external_vm.xml | 39 ----- .../external_vm_redefine.xml | 44 +++++ .../name_and_description.xml | 5 + tests/domainsnapshotxml2xmlout/noparent.xml | 2 +- tests/domainsnapshotxml2xmltest.c | 190 ++++++++++++++++----- 12 files changed, 309 insertions(+), 147 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml create mode 100644 tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml create mode 100644 tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml create mode 100644 tests/domainsnapshotxml2xmlout/empty.xml create mode 100644 tests/domainsnapshotxml2xmlout/external_vm_redefine.xml create mode 100644 tests/domainsnapshotxml2xmlout/name_and_description.xml -- 1.8.4.3

The 'internal' variable holds only two states; convert it to a boolean and the 'fail' label should be called 'cleanup'. --- tests/domainsnapshotxml2xmltest.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index 9225119..c787f73 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -23,7 +23,7 @@ static virQEMUDriver driver; static int -testCompareXMLToXMLFiles(const char *inxml, const char *uuid, int internal) +testCompareXMLToXMLFiles(const char *inxml, const char *uuid, bool internal) { char *inXmlData = NULL; char *actual = NULL; @@ -33,7 +33,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *uuid, int internal) VIR_DOMAIN_SNAPSHOT_PARSE_DISKS); if (virtTestLoadFile(inxml, &inXmlData) < 0) - goto fail; + goto cleanup; if (internal) flags |= VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL; @@ -41,21 +41,22 @@ testCompareXMLToXMLFiles(const char *inxml, const char *uuid, int internal) driver.xmlopt, QEMU_EXPECTED_VIRT_TYPES, flags))) - goto fail; + goto cleanup; if (!(actual = virDomainSnapshotDefFormat(uuid, def, VIR_DOMAIN_XML_SECURE, internal))) - goto fail; + goto cleanup; if (STRNEQ(inXmlData, actual)) { virtTestDifference(stderr, inXmlData, actual); - goto fail; + goto cleanup; } ret = 0; - fail: + +cleanup: VIR_FREE(inXmlData); VIR_FREE(actual); virDomainSnapshotDefFree(def); @@ -65,9 +66,10 @@ testCompareXMLToXMLFiles(const char *inxml, const char *uuid, int internal) struct testInfo { const char *name; const char *uuid; - int internal; + bool internal; }; + static int testCompareXMLToXMLHelper(const void *data) { @@ -95,8 +97,10 @@ mymain(void) if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) + if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) { + virObjectUnref(driver.caps); return EXIT_FAILURE; + } # define DO_TEST(name, uuid, internal) \ do { \ @@ -111,14 +115,14 @@ mymain(void) * values for these envvars */ setenv("PATH", "/bin", 1); - DO_TEST("all_parameters", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 1); - DO_TEST("disk_snapshot", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 1); - DO_TEST("full_domain", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 1); - DO_TEST("noparent_nodescription_noactive", NULL, 0); - DO_TEST("noparent_nodescription", NULL, 1); - DO_TEST("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 0); - DO_TEST("metadata", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 0); - DO_TEST("external_vm", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 0); + DO_TEST("all_parameters", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", true); + DO_TEST("disk_snapshot", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); + DO_TEST("full_domain", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); + DO_TEST("noparent_nodescription_noactive", NULL, false); + DO_TEST("noparent_nodescription", NULL, true); + DO_TEST("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false); + DO_TEST("metadata", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); + DO_TEST("external_vm", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.8.4.3

On 12/04/2013 10:55 AM, Peter Krempa wrote:
The 'internal' variable holds only two states; convert it to a boolean and the 'fail' label should be called 'cleanup'. --- tests/domainsnapshotxml2xmltest.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)
@@ -95,8 +97,10 @@ mymain(void) if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE;
- if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) + if (!(driver.xmlopt = virQEMUDriverCreateXMLConf(&driver))) { + virObjectUnref(driver.caps); return EXIT_FAILURE; + }
Is this a memory leak fix not mentioned in the commit message? Fine to include it, but mentioning it would be nice. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Until now the test was only testing redefinition of snapshot XMLs stored in tests/domainsnapshotxml2xmlout. This patch adds new infrastructure to allow testing of files that may differ and will allow to utilize files in tests/domainsnapshotxml2xmlin as new tests too. --- tests/domainsnapshotxml2xmltest.c | 161 ++++++++++++++++++++++++++++++-------- 1 file changed, 128 insertions(+), 33 deletions(-) diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index c787f73..bc41a11 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -8,6 +8,8 @@ #include <sys/types.h> #include <fcntl.h> +#include <regex.h> + #include "testutils.h" #ifdef WITH_QEMU @@ -22,21 +24,79 @@ static virQEMUDriver driver; +/* This regex will skip the following XML constructs in test files + * that are dynamically generated and thus problematic to test: + * <name>1234352345</name> if the snapshot has no name, + * <creationTime>23523452345</creationTime>, + * <state>nostate</state> as the backend code doesn't fill this + */ +static const char *testSnapshotXMLVariableLineRegexStr = + "(<(name|creationTime)>[0-9]+</(name|creationTime)>|" + "<state>nostate</state>)"; + +regex_t *testSnapshotXMLVariableLineRegex = NULL; + +static char * +testFilterXML(char *xml) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char **xmlLines = NULL; + char **xmlLine; + char *ret = NULL; + + if (!(xmlLines = virStringSplit(xml, "\n", 0))) { + VIR_FREE(xml); + goto cleanup; + } + VIR_FREE(xml); + + for (xmlLine = xmlLines; *xmlLine; xmlLine++) { + if (regexec(testSnapshotXMLVariableLineRegex, + *xmlLine, 0, NULL, 0) == 0) + continue; + + virBufferStrcat(&buf, *xmlLine, "\n", NULL); + } + + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + + ret = virBufferContentAndReset(&buf); + +cleanup: + virBufferFreeAndReset(&buf); + virStringFreeList(xmlLines); + return ret; +} + static int -testCompareXMLToXMLFiles(const char *inxml, const char *uuid, bool internal) +testCompareXMLToXMLFiles(const char *inxml, + const char *outxml, + const char *uuid, + bool internal, + bool redefine) { char *inXmlData = NULL; + char *outXmlData = NULL; char *actual = NULL; int ret = -1; virDomainSnapshotDefPtr def = NULL; - unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE | - VIR_DOMAIN_SNAPSHOT_PARSE_DISKS); + unsigned int flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; + + if (internal) + flags |= VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL; + + if (redefine) + flags |= VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; if (virtTestLoadFile(inxml, &inXmlData) < 0) goto cleanup; - if (internal) - flags |= VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL; + if (virtTestLoadFile(outxml, &outXmlData) < 0) + goto cleanup; + if (!(def = virDomainSnapshotDefParseString(inXmlData, driver.caps, driver.xmlopt, QEMU_EXPECTED_VIRT_TYPES, @@ -48,9 +108,16 @@ testCompareXMLToXMLFiles(const char *inxml, const char *uuid, bool internal) internal))) goto cleanup; + if (!redefine) { + if (!(actual = testFilterXML(actual))) + goto cleanup; + + if (!(outXmlData = testFilterXML(outXmlData))) + goto cleanup; + } - if (STRNEQ(inXmlData, actual)) { - virtTestDifference(stderr, inXmlData, actual); + if (STRNEQ(outXmlData, actual)) { + virtTestDifference(stderr, outXmlData, actual); goto cleanup; } @@ -58,15 +125,18 @@ testCompareXMLToXMLFiles(const char *inxml, const char *uuid, bool internal) cleanup: VIR_FREE(inXmlData); + VIR_FREE(outXmlData); VIR_FREE(actual); virDomainSnapshotDefFree(def); return ret; } struct testInfo { - const char *name; + const char *inxml; + const char *outxml; const char *uuid; bool internal; + bool redefine; }; @@ -74,18 +144,9 @@ static int testCompareXMLToXMLHelper(const void *data) { const struct testInfo *info = data; - char *xml_in = NULL; - int ret = -1; - - if (virAsprintf(&xml_in, "%s/domainsnapshotxml2xmlout/%s.xml", - abs_srcdir, info->name) < 0) - goto cleanup; - ret = testCompareXMLToXMLFiles(xml_in, info->uuid, info->internal); - -cleanup: - VIR_FREE(xml_in); - return ret; + return testCompareXMLToXMLFiles(info->inxml, info->outxml, info->uuid, + info->internal, info->redefine); } @@ -102,28 +163,62 @@ mymain(void) return EXIT_FAILURE; } -# define DO_TEST(name, uuid, internal) \ - do { \ - const struct testInfo info = {name, uuid, internal}; \ - if (virtTestRun("SNAPSHOT XML-2-XML " name, \ - testCompareXMLToXMLHelper, &info) < 0) \ - ret = -1; \ + if (VIR_ALLOC(testSnapshotXMLVariableLineRegex) < 0) + goto cleanup; + + if (regcomp(testSnapshotXMLVariableLineRegex, + testSnapshotXMLVariableLineRegexStr, + REG_EXTENDED | REG_NOSUB) != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "failed to compile test regex"); + goto cleanup; + } + + +# define DO_TEST(prefix, name, inpath, outpath, uuid, internal, redefine) \ + do { \ + const struct testInfo info = {abs_srcdir "/" inpath "/" name ".xml", \ + abs_srcdir "/" outpath "/" name ".xml", \ + uuid, internal, redefine}; \ + if (virtTestRun("SNAPSHOT XML-2-XML " prefix " " name, \ + testCompareXMLToXMLHelper, &info) < 0) \ + ret = -1; \ } while (0) +# define DO_TEST_IN(name, uuid) DO_TEST("in->in", name,\ + "domainsnapshotxml2xmlin",\ + "domainsnapshotxml2xmlin",\ + uuid, false, false) + +# define DO_TEST_OUT(name, uuid, internal) DO_TEST("out->out", name,\ + "domainsnapshotxml2xmlout",\ + "domainsnapshotxml2xmlout",\ + uuid, internal, true) + +# define DO_TEST_INOUT(name, uuid, internal, redefine) \ + DO_TEST("in->out", name,\ + "domainsnapshotxml2xmlin",\ + "domainsnapshotxml2xmlout",\ + uuid, internal, redefine) + /* Unset or set all envvars here that are copied in qemudBuildCommandLine * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected * values for these envvars */ setenv("PATH", "/bin", 1); - DO_TEST("all_parameters", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", true); - DO_TEST("disk_snapshot", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); - DO_TEST("full_domain", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); - DO_TEST("noparent_nodescription_noactive", NULL, false); - DO_TEST("noparent_nodescription", NULL, true); - DO_TEST("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false); - DO_TEST("metadata", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); - DO_TEST("external_vm", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); + DO_TEST_OUT("all_parameters", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", true); + DO_TEST_OUT("disk_snapshot", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); + DO_TEST_OUT("full_domain", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); + DO_TEST_OUT("noparent_nodescription_noactive", NULL, false); + DO_TEST_OUT("noparent_nodescription", NULL, true); + DO_TEST_OUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false); + DO_TEST_OUT("metadata", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); + DO_TEST_OUT("external_vm", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); +cleanup: + if (testSnapshotXMLVariableLineRegex) + regfree(testSnapshotXMLVariableLineRegex); + VIR_FREE(testSnapshotXMLVariableLineRegex); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.8.4.3

On 12/04/2013 10:55 AM, Peter Krempa wrote:
Until now the test was only testing redefinition of snapshot XMLs stored in tests/domainsnapshotxml2xmlout. This patch adds new infrastructure to allow testing of files that may differ and will allow to utilize files in tests/domainsnapshotxml2xmlin as new tests too. --- tests/domainsnapshotxml2xmltest.c | 161 ++++++++++++++++++++++++++++++-------- 1 file changed, 128 insertions(+), 33 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The new tests that will be added later would colide with files of existing tests. Move and rename those files. --- .../{disk_snapshot.xml => disk_snapshot_redefine.xml} | 0 .../{external_vm.xml => external_vm_redefine.xml} | 0 tests/domainsnapshotxml2xmltest.c | 4 ++-- 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/domainsnapshotxml2xmlout/{disk_snapshot.xml => disk_snapshot_redefine.xml} (100%) rename tests/domainsnapshotxml2xmlout/{external_vm.xml => external_vm_redefine.xml} (100%) diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml similarity index 100% rename from tests/domainsnapshotxml2xmlout/disk_snapshot.xml rename to tests/domainsnapshotxml2xmlout/disk_snapshot_redefine.xml diff --git a/tests/domainsnapshotxml2xmlout/external_vm.xml b/tests/domainsnapshotxml2xmlout/external_vm_redefine.xml similarity index 100% rename from tests/domainsnapshotxml2xmlout/external_vm.xml rename to tests/domainsnapshotxml2xmlout/external_vm_redefine.xml diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index bc41a11..d8055e8 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -207,13 +207,13 @@ mymain(void) setenv("PATH", "/bin", 1); DO_TEST_OUT("all_parameters", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", true); - DO_TEST_OUT("disk_snapshot", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); + DO_TEST_OUT("disk_snapshot_redefine", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); DO_TEST_OUT("full_domain", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true); DO_TEST_OUT("noparent_nodescription_noactive", NULL, false); DO_TEST_OUT("noparent_nodescription", NULL, true); DO_TEST_OUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false); DO_TEST_OUT("metadata", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); - DO_TEST_OUT("external_vm", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); + DO_TEST_OUT("external_vm_redefine", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); cleanup: if (testSnapshotXMLVariableLineRegex) -- 1.8.4.3

On 12/04/2013 10:55 AM, Peter Krempa wrote:
The new tests that will be added later would colide with files of
s/colide/collide/
existing tests. Move and rename those files. --- .../{disk_snapshot.xml => disk_snapshot_redefine.xml} | 0 .../{external_vm.xml => external_vm_redefine.xml} | 0 tests/domainsnapshotxml2xmltest.c | 4 ++-- 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/domainsnapshotxml2xmlout/{disk_snapshot.xml => disk_snapshot_redefine.xml} (100%) rename tests/domainsnapshotxml2xmlout/{external_vm.xml => external_vm_redefine.xml} (100%)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

There were plenty snapshot XMLs in the tests/domainsnapshotxml2xmlin directory that actually weren't used in XML testing. The upgraded domainsnapshotxml2xml test now allows us to use them. --- tests/domainsnapshotxml2xmlin/external_vm.xml | 1 - tests/domainsnapshotxml2xmlin/noparent.xml | 1 + tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 15 +++++++++++++++ tests/domainsnapshotxml2xmlout/empty.xml | 8 ++++++++ tests/domainsnapshotxml2xmlout/external_vm.xml | 5 +++++ tests/domainsnapshotxml2xmlout/name_and_description.xml | 5 +++++ tests/domainsnapshotxml2xmlout/noparent.xml | 2 +- tests/domainsnapshotxml2xmltest.c | 10 ++++++++++ 8 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlout/disk_snapshot.xml create mode 100644 tests/domainsnapshotxml2xmlout/empty.xml create mode 100644 tests/domainsnapshotxml2xmlout/external_vm.xml create mode 100644 tests/domainsnapshotxml2xmlout/name_and_description.xml diff --git a/tests/domainsnapshotxml2xmlin/external_vm.xml b/tests/domainsnapshotxml2xmlin/external_vm.xml index 3bcd150..6be626c 100644 --- a/tests/domainsnapshotxml2xmlin/external_vm.xml +++ b/tests/domainsnapshotxml2xmlin/external_vm.xml @@ -6,5 +6,4 @@ <parent> <name>earlier_snap</name> </parent> - <creationTime>1272917631</creationTime> </domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlin/noparent.xml b/tests/domainsnapshotxml2xmlin/noparent.xml index cbac0d8..672a0af 100644 --- a/tests/domainsnapshotxml2xmlin/noparent.xml +++ b/tests/domainsnapshotxml2xmlin/noparent.xml @@ -2,6 +2,7 @@ <name>my snap name</name> <description>!@#$%^</description> <state>running</state> + <memory snapshot='internal'/> <creationTime>1272917631</creationTime> <domain> <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml new file mode 100644 index 0000000..1a1fc02 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -0,0 +1,15 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <disks> + <disk name='/dev/HostVG/QEMUGuest1'/> + <disk name='hdb' snapshot='no'/> + <disk name='hdc' snapshot='internal'/> + <disk name='hdd' snapshot='external'> + <driver type='qed'/> + </disk> + <disk name='hde' snapshot='external'> + <source file='/path/to/new'/> + </disk> + </disks> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/empty.xml b/tests/domainsnapshotxml2xmlout/empty.xml new file mode 100644 index 0000000..41538f7 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/empty.xml @@ -0,0 +1,8 @@ +<domainsnapshot> + <name>1386166249</name> + <state>nostate</state> + <creationTime>1386166249</creationTime> + <domain> + <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> + </domain> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/external_vm.xml b/tests/domainsnapshotxml2xmlout/external_vm.xml new file mode 100644 index 0000000..9da369b --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/external_vm.xml @@ -0,0 +1,5 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <memory snapshot='external' file='/dev/HostVG/GuestMemory'/> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/name_and_description.xml b/tests/domainsnapshotxml2xmlout/name_and_description.xml new file mode 100644 index 0000000..435ab79 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/name_and_description.xml @@ -0,0 +1,5 @@ +<domainsnapshot> + <name>snap1</name> + <description>A longer description!</description> + <state>nostate</state> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/noparent.xml b/tests/domainsnapshotxml2xmlout/noparent.xml index 0cbbb65..d4360f0 100644 --- a/tests/domainsnapshotxml2xmlout/noparent.xml +++ b/tests/domainsnapshotxml2xmlout/noparent.xml @@ -1,7 +1,7 @@ <domainsnapshot> <name>my snap name</name> <description>!@#$%^</description> - <state>running</state> + <state>nostate</state> <creationTime>1272917631</creationTime> <memory snapshot='internal'/> <domain> diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index d8055e8..9960959 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -215,6 +215,16 @@ mymain(void) DO_TEST_OUT("metadata", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); DO_TEST_OUT("external_vm_redefine", "c7a5fdbd-edaf-9455-926a-d65c16db1809", false); + DO_TEST_INOUT("empty", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false, false); + DO_TEST_INOUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false, false); + DO_TEST_INOUT("external_vm", NULL, false, false); + DO_TEST_INOUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false, false); + DO_TEST_INOUT("disk_snapshot", NULL, false, false); + + DO_TEST_IN("name_and_description", NULL); + DO_TEST_IN("description_only", NULL); + DO_TEST_IN("name_only", NULL); + cleanup: if (testSnapshotXMLVariableLineRegex) regfree(testSnapshotXMLVariableLineRegex); -- 1.8.4.3

On 12/04/2013 10:55 AM, Peter Krempa wrote:
There were plenty snapshot XMLs in the tests/domainsnapshotxml2xmlin directory that actually weren't used in XML testing. The upgraded domainsnapshotxml2xml test now allows us to use them.
They WERE used in domainsnapshotschematest, for RNG validation, but I agree that enhancing the xml2xml test to also test them through the C code and not just RNG validation is a good move.
--- tests/domainsnapshotxml2xmlin/external_vm.xml | 1 - tests/domainsnapshotxml2xmlin/noparent.xml | 1 + tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 15 +++++++++++++++ tests/domainsnapshotxml2xmlout/empty.xml | 8 ++++++++ tests/domainsnapshotxml2xmlout/external_vm.xml | 5 +++++ tests/domainsnapshotxml2xmlout/name_and_description.xml | 5 +++++ tests/domainsnapshotxml2xmlout/noparent.xml | 2 +- tests/domainsnapshotxml2xmltest.c | 10 ++++++++++ 8 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlout/disk_snapshot.xml create mode 100644 tests/domainsnapshotxml2xmlout/empty.xml create mode 100644 tests/domainsnapshotxml2xmlout/external_vm.xml create mode 100644 tests/domainsnapshotxml2xmlout/name_and_description.xml
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Commit 5a66c667ff5cae61c2ad2e646c8eb3eedc67f925 fixed a NULL dereference if the disk driver element was empty. Add a test for this case. --- tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml | 10 ++++++++++ tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml | 9 +++++++++ tests/domainsnapshotxml2xmltest.c | 1 + 3 files changed, 20 insertions(+) create mode 100644 tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml create mode 100644 tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml diff --git a/tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml b/tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml new file mode 100644 index 0000000..78eee9c --- /dev/null +++ b/tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml @@ -0,0 +1,10 @@ +<domainsnapshot> + <name>asdf</name> + <description>adsf</description> + <disks> + <disk name='vda' snapshot='external'> + <source file='/tmp/foo'/> + <driver/> + </disk> + </disks> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml b/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml new file mode 100644 index 0000000..41961f1 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml @@ -0,0 +1,9 @@ +<domainsnapshot> + <name>asdf</name> + <description>adsf</description> + <disks> + <disk name='vda' snapshot='external'> + <source file='/tmp/foo'/> + </disk> + </disks> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index 9960959..921c7ad 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -220,6 +220,7 @@ mymain(void) DO_TEST_INOUT("external_vm", NULL, false, false); DO_TEST_INOUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false, false); DO_TEST_INOUT("disk_snapshot", NULL, false, false); + DO_TEST_INOUT("disk_driver_name_null", NULL, false, false); DO_TEST_IN("name_and_description", NULL); DO_TEST_IN("description_only", NULL); -- 1.8.4.3

On 12/04/2013 10:55 AM, Peter Krempa wrote:
Commit 5a66c667ff5cae61c2ad2e646c8eb3eedc67f925 fixed a NULL dereference if the disk driver element was empty. Add a test for this case. --- tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml | 10 ++++++++++ tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml | 9 +++++++++ tests/domainsnapshotxml2xmltest.c | 1 + 3 files changed, 20 insertions(+) create mode 100644 tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml create mode 100644 tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 12/04/13 19:21, Eric Blake wrote:
On 12/04/2013 10:55 AM, Peter Krempa wrote:
Commit 5a66c667ff5cae61c2ad2e646c8eb3eedc67f925 fixed a NULL dereference if the disk driver element was empty. Add a test for this case. --- tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml | 10 ++++++++++ tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml | 9 +++++++++ tests/domainsnapshotxml2xmltest.c | 1 + 3 files changed, 20 insertions(+) create mode 100644 tests/domainsnapshotxml2xmlin/disk_driver_name_null.xml create mode 100644 tests/domainsnapshotxml2xmlout/disk_driver_name_null.xml
ACK.
I've fixed the commit messages as requested and pushed the series. Thanks for the review. Peter
participants (2)
-
Eric Blake
-
Peter Krempa