1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/5] Fix up some issues from x and y resolution patches
  4. View patch

[libvirt] [PATCH v2 5/5] conf: report errors when parsing video acceleration

Jonathon Jongsma posted 5 patches 5 years, 1 month ago
There is a newer version of this series
[libvirt] [PATCH v2 5/5] conf: report errors when parsing video acceleration
Posted by Jonathon Jongsma 5 years, 1 month ago 
In order to differentiate between a configuration that does not specify
any video acceleration and one that is incorrectly specified, change the
signature of virDomainVideoAccelDefParseXML() to return an error
response.

If any of the values are invalid, report an error rather than returning
a partially-specified accel object. If no 'acceleration' node is found,
do not report an error since it is optional.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/conf/domain_conf.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b3f32d0b63..1c3fc5c4ce 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15285,8 +15285,8 @@ virDomainVideoDefaultType(const virDomainDef *def)
     }
 }
 
-static virDomainVideoAccelDefPtr
-virDomainVideoAccelDefParseXML(xmlNodePtr node)
+static int
+virDomainVideoAccelDefParseXML(xmlNodePtr node, virDomainVideoAccelDefPtr *accel)
 {
     xmlNodePtr cur;
     g_autofree virDomainVideoAccelDefPtr def = NULL;
@@ -15295,21 +15295,25 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
     g_autofree char *accel3d = NULL;
     g_autofree char *rendernode = NULL;
 
+    *accel = NULL;
     cur = node->children;
     while (cur != NULL) {
-        if (cur->type == XML_ELEMENT_NODE) {
-            if (!accel3d && !accel2d &&
-                virXMLNodeNameEqual(cur, "acceleration")) {
-                accel3d = virXMLPropString(cur, "accel3d");
-                accel2d = virXMLPropString(cur, "accel2d");
-                rendernode = virXMLPropString(cur, "rendernode");
-            }
+        if ((cur->type == XML_ELEMENT_NODE) &&
+            virXMLNodeNameEqual(cur, "acceleration")) {
+            accel3d = virXMLPropString(cur, "accel3d");
+            accel2d = virXMLPropString(cur, "accel2d");
+            rendernode = virXMLPropString(cur, "rendernode");
+            break;
         }
         cur = cur->next;
     }
 
+    /* no acceleration node was specified */
+    if (cur == NULL)
+        return 0;
+
     if (!accel3d && !accel2d && !rendernode)
-        return NULL;
+        return -1;
 
     def = g_new0(virDomainVideoAccelDef, 1);
 
@@ -15317,7 +15321,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
         if ((val = virTristateBoolTypeFromString(accel3d)) <= 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("unknown accel3d value '%s'"), accel3d);
-            goto cleanup;
+            return -1;
         }
         def->accel3d = val;
     }
@@ -15326,7 +15330,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
         if ((val = virTristateBoolTypeFromString(accel2d)) <= 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("unknown accel2d value '%s'"), accel2d);
-            goto cleanup;
+            return -1;
         }
         def->accel2d = val;
     }
@@ -15334,8 +15338,8 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
     if (rendernode)
         def->rendernode = virFileSanitizePath(rendernode);
 
- cleanup:
-    return g_steal_pointer(&def);
+    *accel = g_steal_pointer(&def);
+    return 0;
 }
 
 static int
@@ -15469,7 +15473,11 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
                     VIR_FREE(primary);
                 }
 
-                def->accel = virDomainVideoAccelDefParseXML(cur);
+                if (virDomainVideoAccelDefParseXML(cur, &def->accel) < 0) {
+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                   "%s", _("Cannot parse video acceleration"));
+                    goto error;
+                }
                 if (virDomainVideoResolutionDefParseXML(cur, &def->res) < 0) {
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                    "%s", _("Cannot parse video resolution"));
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/5] conf: report errors when parsing video acceleration
Posted by Cole Robinson 5 years ago 
On 10/23/19 1:46 PM, Jonathon Jongsma wrote:
> In order to differentiate between a configuration that does not specify
> any video acceleration and one that is incorrectly specified, change the
> signature of virDomainVideoAccelDefParseXML() to return an error
> response.
> 
> If any of the values are invalid, report an error rather than returning
> a partially-specified accel object. If no 'acceleration' node is found,
> do not report an error since it is optional.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
>  src/conf/domain_conf.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b3f32d0b63..1c3fc5c4ce 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15285,8 +15285,8 @@ virDomainVideoDefaultType(const virDomainDef *def)
>      }
>  }
>  
> -static virDomainVideoAccelDefPtr
> -virDomainVideoAccelDefParseXML(xmlNodePtr node)
> +static int
> +virDomainVideoAccelDefParseXML(xmlNodePtr node, virDomainVideoAccelDefPtr *accel)
>  {
>      xmlNodePtr cur;
>      g_autofree virDomainVideoAccelDefPtr def = NULL;
> @@ -15295,21 +15295,25 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>      g_autofree char *accel3d = NULL;
>      g_autofree char *rendernode = NULL;
>  
> +    *accel = NULL;
>      cur = node->children;
>      while (cur != NULL) {
> -        if (cur->type == XML_ELEMENT_NODE) {
> -            if (!accel3d && !accel2d &&
> -                virXMLNodeNameEqual(cur, "acceleration")) {
> -                accel3d = virXMLPropString(cur, "accel3d");
> -                accel2d = virXMLPropString(cur, "accel2d");
> -                rendernode = virXMLPropString(cur, "rendernode");
> -            }
> +        if ((cur->type == XML_ELEMENT_NODE) &&
> +            virXMLNodeNameEqual(cur, "acceleration")) {
> +            accel3d = virXMLPropString(cur, "accel3d");
> +            accel2d = virXMLPropString(cur, "accel2d");
> +            rendernode = virXMLPropString(cur, "rendernode");
> +            break;
>          }
>          cur = cur->next;
>      }
>  

THe virXMLNodeNameEqual check should be moved to the parent, same as
mentioned in patch 4

> +    /* no acceleration node was specified */
> +    if (cur == NULL)
> +        return 0;
> +
>      if (!accel3d && !accel2d && !rendernode)
> -        return NULL;
> +        return -1;
>  
>      def = g_new0(virDomainVideoAccelDef, 1);
>  
> @@ -15317,7 +15321,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>          if ((val = virTristateBoolTypeFromString(accel3d)) <= 0) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("unknown accel3d value '%s'"), accel3d);
> -            goto cleanup;
> +            return -1;
>          }
>          def->accel3d = val;
>      }
> @@ -15326,7 +15330,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>          if ((val = virTristateBoolTypeFromString(accel2d)) <= 0) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                             _("unknown accel2d value '%s'"), accel2d);
> -            goto cleanup;
> +            return -1;
>          }
>          def->accel2d = val;
>      }
> @@ -15334,8 +15338,8 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>      if (rendernode)
>          def->rendernode = virFileSanitizePath(rendernode);
>  
> - cleanup:
> -    return g_steal_pointer(&def);
> +    *accel = g_steal_pointer(&def);
> +    return 0;
>  }
>  
>  static int
> @@ -15469,7 +15473,11 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
>                      VIR_FREE(primary);
>                  }
>  
> -                def->accel = virDomainVideoAccelDefParseXML(cur);
> +                if (virDomainVideoAccelDefParseXML(cur, &def->accel) < 0) {
> +                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                   "%s", _("Cannot parse video acceleration"));
> +                    goto error;
> +                }
>                  if (virDomainVideoResolutionDefParseXML(cur, &def->res) < 0) {
>                      virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                     "%s", _("Cannot parse video resolution"));
> 

Same error overwriting issue here as in patch 4

- Cole

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.