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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux