[libvirt] [PATCH] conf: Do not validate resolution XML if 'x' or/and 'y' are 0.

jcfaracco@gmail.com posted 1 patch 4 years, 5 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20191020034520.29721-1-jcfaracco@gmail.com
src/conf/domain_conf.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
[libvirt] [PATCH] conf: Do not validate resolution XML if 'x' or/and 'y' are 0.
Posted by jcfaracco@gmail.com 4 years, 5 months ago
From: Julio Faracco <jcfaracco@gmail.com>

There is an issue with <resolution/> when of 'x' or 'y' settings are 0.
Function virDomainVideoResolutionDefParseXML() will validate this XML,
but both 'x' and 'y' will be removed. One example, if someone defines
this settings:

    <model ...>
      <resolution x='1024' y='0'/>
    <model/>

After applying this settings, funcion libvirt will remove both
resolutions because virDomainVideoResolutionDefFormat() requires 'x' and
'y' higher than 0. So, the example above will become:

    <model ...>
      <resolution/>
    <model/>

Now, libvirt only adds <resolution/> with 'x' and 'y' if boths strings
are not NULL AND they are higher than 0.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/conf/domain_conf.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 88e93f6fb8..d89d8059ce 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15375,7 +15375,7 @@ virDomainVideoResolutionDefParseXML(xmlNodePtr node)
         if (virStrToLong_uip(x, NULL, 10, &def->x) < 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("cannot parse video x-resolution '%s'"), x);
-            goto cleanup;
+            goto error;
         }
     }
 
@@ -15383,12 +15383,21 @@ virDomainVideoResolutionDefParseXML(xmlNodePtr node)
         if (virStrToLong_uip(y, NULL, 10, &def->y) < 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("cannot parse video y-resolution '%s'"), y);
-            goto cleanup;
+            goto error;
         }
     }
 
+    /* QEMU ignores 'xres' or/and 'yres' with value 0. */
+    if (!def->x || !def->y)
+        goto error;
+
  cleanup:
     return def;
+
+ error:
+    VIR_FREE(def);
+    def = NULL;
+    goto cleanup;
 }
 
 static virDomainVideoDriverDefPtr
-- 
2.20.1

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

Re: [libvirt] [PATCH] conf: Do not validate resolution XML if 'x' or/and 'y' are 0.
Posted by Peter Krempa 4 years, 5 months ago
On Sun, Oct 20, 2019 at 00:45:20 -0300, jcfaracco@gmail.com wrote:
> From: Julio Faracco <jcfaracco@gmail.com>
> 
> There is an issue with <resolution/> when of 'x' or 'y' settings are 0.
> Function virDomainVideoResolutionDefParseXML() will validate this XML,
> but both 'x' and 'y' will be removed. One example, if someone defines
> this settings:
> 
>     <model ...>
>       <resolution x='1024' y='0'/>
>     <model/>
> 
> After applying this settings, funcion libvirt will remove both
> resolutions because virDomainVideoResolutionDefFormat() requires 'x' and
> 'y' higher than 0. So, the example above will become:
> 
>     <model ...>
>       <resolution/>
>     <model/>
> 
> Now, libvirt only adds <resolution/> with 'x' and 'y' if boths strings
> are not NULL AND they are higher than 0.

Johnathon posted a more complete fix:

https://www.redhat.com/archives/libvir-list/2019-October/msg01256.html

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