[libvirt] [PATCH 3/8] snapshot: Add internal option to validate XML against schema

Eric Blake posted 8 patches 5 years, 4 months ago
[libvirt] [PATCH 3/8] snapshot: Add internal option to validate XML against schema
Posted by Eric Blake 5 years, 4 months ago
Similar to VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; for now used only by
the testsuite, but will be exposed in the public API in the next
patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 src/conf/snapshot_conf.h              |  1 +
 src/conf/snapshot_conf.c              | 13 +++++++++++++
 tests/qemudomainsnapshotxml2xmltest.c |  3 ++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 08b2a3b955..a0c87e7ade 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -92,6 +92,7 @@ typedef enum {
     VIR_DOMAIN_SNAPSHOT_PARSE_DISKS    = 1 << 1,
     VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL = 1 << 2,
     VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE  = 1 << 3,
+    VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE = 1 << 4,
 } virDomainSnapshotParseFlags;

 typedef enum {
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index c7f29360e7..daea6c616d 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -25,6 +25,7 @@
 #include <sys/stat.h>
 #include <unistd.h>

+#include "configmake.h"
 #include "internal.h"
 #include "virbitmap.h"
 #include "virbuffer.h"
@@ -409,6 +410,18 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
         goto cleanup;
     }

+    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE) {
+        VIR_AUTOFREE(char *) schema = NULL;
+
+        schema = virFileFindResource("domainsnapshot.rng",
+                                     abs_top_srcdir "/docs/schemas",
+                                     PKGDATADIR "/schemas");
+        if (!schema)
+            return NULL;
+        if (virXMLValidateAgainstSchema(schema, xml) < 0)
+            return NULL;
+    }
+
     ctxt = xmlXPathNewContext(xml);
     if (ctxt == NULL) {
         virReportOOMError();
diff --git a/tests/qemudomainsnapshotxml2xmltest.c b/tests/qemudomainsnapshotxml2xmltest.c
index 10ea9cf8d2..55cb8575f7 100644
--- a/tests/qemudomainsnapshotxml2xmltest.c
+++ b/tests/qemudomainsnapshotxml2xmltest.c
@@ -35,7 +35,8 @@ testCompareXMLToXMLFiles(const char *inxml,
     char *outXmlData = NULL;
     char *actual = NULL;
     int ret = -1;
-    unsigned int parseflags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
+    unsigned int parseflags = (VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
+                               VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE);
     unsigned int formatflags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE;
     bool cur = false;
     VIR_AUTOUNREF(virDomainSnapshotDefPtr) def = NULL;
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] snapshot: Add internal option to validate XML against schema
Posted by Peter Krempa 5 years, 4 months ago
On Fri, Jul 05, 2019 at 23:37:30 -0500, Eric Blake wrote:
> Similar to VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; for now used only by
> the testsuite, but will be exposed in the public API in the next
> patch.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  src/conf/snapshot_conf.h              |  1 +
>  src/conf/snapshot_conf.c              | 13 +++++++++++++
>  tests/qemudomainsnapshotxml2xmltest.c |  3 ++-
>  3 files changed, 16 insertions(+), 1 deletion(-)

[...]

> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index c7f29360e7..daea6c616d 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c

[....]

> @@ -409,6 +410,18 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
>          goto cleanup;

^^^

>      }
> 
> +    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE) {
> +        VIR_AUTOFREE(char *) schema = NULL;
> +
> +        schema = virFileFindResource("domainsnapshot.rng",
> +                                     abs_top_srcdir "/docs/schemas",
> +                                     PKGDATADIR "/schemas");
> +        if (!schema)
> +            return NULL;

This would skip the cleanup section while the above block does not.
While this is not a problem code wise, it's semantically wrong.


> +        if (virXMLValidateAgainstSchema(schema, xml) < 0)
> +            return NULL;
> +    }
> +
>      ctxt = xmlXPathNewContext(xml);
>      if (ctxt == NULL) {
>          virReportOOMError();
> diff --git a/tests/qemudomainsnapshotxml2xmltest.c b/tests/qemudomainsnapshotxml2xmltest.c
> index 10ea9cf8d2..55cb8575f7 100644
> --- a/tests/qemudomainsnapshotxml2xmltest.c
> +++ b/tests/qemudomainsnapshotxml2xmltest.c
> @@ -35,7 +35,8 @@ testCompareXMLToXMLFiles(const char *inxml,
>      char *outXmlData = NULL;
>      char *actual = NULL;
>      int ret = -1;
> -    unsigned int parseflags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
> +    unsigned int parseflags = (VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
> +                               VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE);

We schema-check all the snapshot examples explitly and additionally this
does prevent us from ever testing upgrade from invalid snapshot XML.

ACK if you use "goto cleanup" in the implementation and drop the test
change.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list