[PATCH 5/5] conf: replace validation with variables passed to virXMLParse()

Kristina Hanicova posted 5 patches 4 years, 6 months ago
[PATCH 5/5] conf: replace validation with variables passed to virXMLParse()
Posted by Kristina Hanicova 4 years, 6 months ago
virXMLParse() now allows to validate xml against schema directly,
eliminating the need to do it individually in each function.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
---
 src/conf/backup_conf.c     | 13 ++-----------
 src/conf/checkpoint_conf.c | 12 ++----------
 src/conf/snapshot_conf.c   | 15 ++-------------
 3 files changed, 6 insertions(+), 34 deletions(-)

diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index 9307357d84..8e378a5d26 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -281,7 +281,8 @@ virDomainBackupDefParseString(const char *xmlStr,
     g_autoptr(xmlDoc) xml = NULL;
     int keepBlanksDefault = xmlKeepBlanksDefault(0);
 
-    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), NULL, false))) {
+    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), "domainbackup.rng",
+                           !(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)))) {
         xmlKeepBlanksDefault(keepBlanksDefault);
         ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml),
                                           xmlopt, flags);
@@ -306,16 +307,6 @@ virDomainBackupDefParseNode(xmlDocPtr xml,
         return NULL;
     }
 
-    if (!(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)) {
-        if (!(schema = virFileFindResource("domainbackup.rng",
-                                           abs_top_srcdir "/docs/schemas",
-                                           PKGDATADIR "/schemas")))
-            return NULL;
-
-        if (virXMLValidateAgainstSchema(schema, xml) < 0)
-            return NULL;
-    }
-
     if (!(ctxt = virXMLXPathContextNew(xml)))
         return NULL;
 
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
index dd0e6035fa..ccb01b87f9 100644
--- a/src/conf/checkpoint_conf.c
+++ b/src/conf/checkpoint_conf.c
@@ -200,15 +200,6 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml,
         return NULL;
     }
 
-    /* This is a new enough API to make schema validation unconditional */
-    schema = virFileFindResource("domaincheckpoint.rng",
-                                 abs_top_srcdir "/docs/schemas",
-                                 PKGDATADIR "/schemas");
-    if (!schema)
-        return NULL;
-    if (virXMLValidateAgainstSchema(schema, xml) < 0)
-        return NULL;
-
     if (!(ctxt = virXMLXPathContextNew(xml)))
         return NULL;
 
@@ -226,7 +217,8 @@ virDomainCheckpointDefParseString(const char *xmlStr,
     xmlDocPtr xml;
     int keepBlanksDefault = xmlKeepBlanksDefault(0);
 
-    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), NULL, false))) {
+    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"),
+                           "domaincheckpoint.rng", true))) {
         xmlKeepBlanksDefault(keepBlanksDefault);
         ret = virDomainCheckpointDefParseNode(xml, xmlDocGetRootElement(xml),
                                               xmlopt, parseOpaque, flags);
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 3282627044..6d3c59f98e 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -432,18 +432,6 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml,
         return NULL;
     }
 
-    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE) {
-        g_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;
-    }
-
     if (!(ctxt = virXMLXPathContextNew(xml)))
         return NULL;
 
@@ -462,7 +450,8 @@ virDomainSnapshotDefParseString(const char *xmlStr,
     xmlDocPtr xml;
     int keepBlanksDefault = xmlKeepBlanksDefault(0);
 
-    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), NULL, false))) {
+    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), "domainsnapshot.rng",
+                           flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE))) {
         xmlKeepBlanksDefault(keepBlanksDefault);
         ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml),
                                             xmlopt, parseOpaque,
-- 
2.31.1

Re: [PATCH 5/5] conf: replace validation with variables passed to virXMLParse()
Posted by Jano Tomko 4 years, 6 months ago
On a %A in %Y, Kristina Hanicova wrote:
> virXMLParse() now allows to validate xml against schema directly,

s/allows to validate/allows validating/

> eliminating the need to do it individually in each function.
> 
> Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
> ---
>  src/conf/backup_conf.c     | 13 ++-----------
>  src/conf/checkpoint_conf.c | 12 ++----------
>  src/conf/snapshot_conf.c   | 15 ++-------------
>  3 files changed, 6 insertions(+), 34 deletions(-)
> 
> diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
> index 9307357d84..8e378a5d26 100644
> --- a/src/conf/backup_conf.c
> +++ b/src/conf/backup_conf.c
> @@ -281,7 +281,8 @@ virDomainBackupDefParseString(const char *xmlStr,
>      g_autoptr(xmlDoc) xml = NULL;
>      int keepBlanksDefault = xmlKeepBlanksDefault(0);
>  
> -    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), NULL, false))) {
> +    if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), "domainbackup.rng",
> +                           !(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)))) {
>          xmlKeepBlanksDefault(keepBlanksDefault);
>          ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml),
>                                            xmlopt, flags);
> @@ -306,16 +307,6 @@ virDomainBackupDefParseNode(xmlDocPtr xml,
>          return NULL;
>      }
>  
> -    if (!(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)) {
> -        if (!(schema = virFileFindResource("domainbackup.rng",
> -                                           abs_top_srcdir "/docs/schemas",
> -                                           PKGDATADIR "/schemas")))
> -            return NULL;
> -
> -        if (virXMLValidateAgainstSchema(schema, xml) < 0)
> -            return NULL;
> -    }
> -
>      if (!(ctxt = virXMLXPathContextNew(xml)))
>          return NULL;
>  

The declaration of 'schema' also needs to go, otherwise clang complains:
../src/conf/backup_conf.c:303:22: error: unused variable 'schema'
[-Werror,-Wunused-variable]
    g_autofree char *schema = NULL;
                     ^
1 error generated.

> diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
> index dd0e6035fa..ccb01b87f9 100644
> --- a/src/conf/checkpoint_conf.c
> +++ b/src/conf/checkpoint_conf.c
> @@ -200,15 +200,6 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml,
>          return NULL;
>      }
>  
> -    /* This is a new enough API to make schema validation unconditional */
> -    schema = virFileFindResource("domaincheckpoint.rng",
> -                                 abs_top_srcdir "/docs/schemas",
> -                                 PKGDATADIR "/schemas");
> -    if (!schema)
> -        return NULL;
> -    if (virXMLValidateAgainstSchema(schema, xml) < 0)
> -        return NULL;
> -

same here.

>      if (!(ctxt = virXMLXPathContextNew(xml)))
>          return NULL;
>  

For the whole series:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano