snapshot: Don't expose testsuite-only state in snapshot XML

None of the existing drivers actually use the 0-valued 'nostate'
snapshot state; rather, it was a fluke of implementation. In fact,
some drivers, like qemu, actively reject 'nostate' as invalid during a
snapshot redefine. Normally, a driver computes the state post-parse
from the current domain, and thus virDomainSnapshotGetXMLDesc() will
never expose the state. However, since the testsuite lacks any
associated domain to copy state from, and lacks post-parse processing
that normal drivers have, the testsuite output had several spots with
the state, coupled with a regex filter to ignore the oddity.

It is better to follow the lead of other XML defaults, by not
outputting anything during format if post-parse defaults have not been
applied, and rejecting the default value during parsing. The testsuite
needs a bit of an update, by adding another flag for when to simulate
a post-parse action of setting a snapshot state, but none of the
drivers are impacted other than rejecting XML that was previously
already suspicious in nature.

Similarly, don't expose creation time 0 (for now, only possible if a
user redefined a snapshot to claim creation at the Epoch, but also
happens once setting the creation time is deferred to a post-parse
handler).

This is also a step towards cleaning up snapshot_conf.c to separate
its existing post-parse work (namely, setting the creationTime and
default snapshot name) from the pure parsing work, so that we can get
rid of the testsuite hack of regex filtering of the XML and instead
have more accurate testing of our parser/formatter code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
index 2680887095a4cb7d06ec86fb85d0656a3978d01c..8863d9957890d0b194ef636ebc5581d6d4eff1a1 100644 (file)
--- a/docs/schemas/domainsnapshot.rng
+++ b/docs/schemas/domainsnapshot.rng
@@ -103,7 +103,6 @@
 
   <define name='state'>
     <choice>
-      <value>nostate</value>
       <value>running</value>
       <value>blocked</value>
       <value>paused</value>

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ce543cbaf7bd92980308bedd8ba78931a3e9b29d..08f335646c698295d9fad9fa870c93c08dd22ce6 100644 (file)
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -245,7 +245,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
             goto cleanup;
         }
         def->state = virDomainSnapshotStateTypeFromString(state);
-        if (def->state < 0) {
+        if (def->state <= 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("Invalid state '%s' in domain snapshot XML"),
                            state);
@@ -810,8 +810,9 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
     if (def->common.description)
         virBufferEscapeString(buf, "<description>%s</description>\n",
                               def->common.description);
-    virBufferAsprintf(buf, "<state>%s</state>\n",
-                      virDomainSnapshotStateTypeToString(def->state));
+    if (def->state)
+        virBufferAsprintf(buf, "<state>%s</state>\n",
+                          virDomainSnapshotStateTypeToString(def->state));
 
     if (def->common.parent) {
         virBufferAddLit(buf, "<parent>\n");
@@ -821,8 +822,9 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
         virBufferAddLit(buf, "</parent>\n");
     }
 
-    virBufferAsprintf(buf, "<creationTime>%lld</creationTime>\n",
-                      def->common.creationTime);
+    if (def->common.creationTime)
+        virBufferAsprintf(buf, "<creationTime>%lld</creationTime>\n",
+                          def->common.creationTime);
 
     if (def->memory) {
         virBufferAsprintf(buf, "<memory snapshot='%s'",

diff --git a/tests/domainsnapshotxml2xmlout/empty.xml b/tests/domainsnapshotxml2xmlout/empty.xml
index 41538f7b7468b85f61f4ff1d2a35f78f197f5e60..0ae32e932a98d6bb2fde4c1c41113af5b1ccda9e 100644 (file)
--- a/tests/domainsnapshotxml2xmlout/empty.xml
+++ b/tests/domainsnapshotxml2xmlout/empty.xml
@@ -1,6 +1,5 @@
 <domainsnapshot>
   <name>1386166249</name>
-  <state>nostate</state>
   <creationTime>1386166249</creationTime>
   <domain>
     <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid>

diff --git a/tests/domainsnapshotxml2xmlout/name_and_description.xml b/tests/domainsnapshotxml2xmlout/name_and_description.xml
index 435ab79aa9481c93ee527016032050274c1ce8e1..90ce7747413b475c613d4a30af8b4fd54964cded 100644 (file)
--- a/tests/domainsnapshotxml2xmlout/name_and_description.xml
+++ b/tests/domainsnapshotxml2xmlout/name_and_description.xml
@@ -1,5 +1,4 @@
 <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 d4360f0dc11988139b63a05ed21eb14af467128f..0cbbb658d8690c9465f04c00ba580a180ab43514 100644 (file)
--- a/tests/domainsnapshotxml2xmlout/noparent.xml
+++ b/tests/domainsnapshotxml2xmlout/noparent.xml
@@ -1,7 +1,7 @@
 <domainsnapshot>
   <name>my snap name</name>
   <description>!@#$%^</description>
-  <state>nostate</state>
+  <state>running</state>
   <creationTime>1272917631</creationTime>
   <memory snapshot='internal'/>
   <domain>

diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c
index 056219e849409c4e3280150d3045613c9d1e6502..191612d2160eb12ba63a1ed3a89bc78ae2665065 100644 (file)
--- a/tests/domainsnapshotxml2xmltest.c
+++ b/tests/domainsnapshotxml2xmltest.c
@@ -24,18 +24,17 @@ 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
+ * <creationTime>23523452345</creationTime>
  */
 static const char *testSnapshotXMLVariableLineRegexStr =
-    "(<(name|creationTime)>[0-9]+</(name|creationTime)>|"
-    "<state>nostate</state>)";
+    "<(name|creationTime)>[0-9]+</(name|creationTime)>";
 
 regex_t *testSnapshotXMLVariableLineRegex = NULL;
 
 enum {
     TEST_INTERNAL = 1 << 0, /* Test use of INTERNAL parse/format flag */
     TEST_REDEFINE = 1 << 1, /* Test use of REDEFINE parse flag */
+    TEST_RUNNING = 1 << 2, /* Set snapshot state to running after parse */
 };
 
 static char *
@@ -109,6 +108,11 @@ testCompareXMLToXMLFiles(const char *inxml,
             goto cleanup;
         formatflags |= VIR_DOMAIN_SNAPSHOT_FORMAT_CURRENT;
     }
+    if (flags & TEST_RUNNING) {
+        if (def->state)
+            goto cleanup;
+        def->state = VIR_DOMAIN_RUNNING;
+    }
 
     if (!(actual = virDomainSnapshotDefFormat(uuid, def, driver.caps,
                                               driver.xmlopt,
@@ -222,7 +226,8 @@ mymain(void)
                 0);
 
     DO_TEST_INOUT("empty", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 0);
-    DO_TEST_INOUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 0);
+    DO_TEST_INOUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8",
+                  TEST_RUNNING);
     DO_TEST_INOUT("external_vm", NULL, 0);
     DO_TEST_INOUT("disk_snapshot", NULL, 0);
     DO_TEST_INOUT("disk_driver_name_null", NULL, 0);