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

[Peter Krempa]

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 at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190708/98d50f51/attachment-0001.sig>