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 <ebl...@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.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to