From: Julio Faracco <jcfaracco@gmail.com>

New element 'resolution' with parameters 'x' and 'y' were added to
support this settings for VGA, QXL, Virtio and Bochs XMLs. A new
structure was created as Acceleration element has. It is easy to parse
this property. Example:

    <model type='qxl'>
      <resolution x='800' y='600'\>
    </model>

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/conf/domain_conf.c  | 75 ++++++++++++++++++++++++++++++++++++++++-
 src/conf/domain_conf.h  |  5 +++
 src/conf/virconftypes.h |  3 ++
 3 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7a342bb91..9db8fd9697 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15311,6 +15311,53 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
     return def;
 }
 
+static virDomainVideoResolutionDefPtr
+virDomainVideoResolutionDefParseXML(xmlNodePtr node)
+{
+    xmlNodePtr cur;
+    virDomainVideoResolutionDefPtr def;
+    VIR_AUTOFREE(char *) x = NULL;
+    VIR_AUTOFREE(char *) y = NULL;
+
+    cur = node->children;
+    while (cur != NULL) {
+        if (cur->type == XML_ELEMENT_NODE) {
+            if (!x && !y &&
+                virXMLNodeNameEqual(cur, "resolution")) {
+                x = virXMLPropString(cur, "x");
+                y = virXMLPropString(cur, "y");
+            }
+        }
+        cur = cur->next;
+    }
+
+    if (!x || !y)
+        return NULL;
+
+    if (VIR_ALLOC(def) < 0)
+        goto cleanup;
+
+    if (x) {
+        if (virStrToLong_uip(x, NULL, 10, &def->x) < 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("cannot parse video x-resolution '%s'"), x);
+            goto cleanup;
+        }
+    }
+
+    if (y) {
+        if (virStrToLong_uip(y, NULL, 10, &def->y) < 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("cannot parse video y-resolution '%s'"), y);
+            goto cleanup;
+        }
+    }
+
+ cleanup:
+    return def;
+}
+
+
 static virDomainVideoDriverDefPtr
 virDomainVideoDriverDefParseXML(xmlNodePtr node)
 {
@@ -15389,6 +15436,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
                 }
 
                 def->accel = virDomainVideoAccelDefParseXML(cur);
+                def->res = virDomainVideoResolutionDefParseXML(cur);
             }
             if (virXMLNodeNameEqual(cur, "driver")) {
                 if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
@@ -15463,6 +15511,17 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
         }
     }
 
+    if (def->res) {
+        if (def->type != VIR_DOMAIN_VIDEO_TYPE_VGA &&
+            def->type != VIR_DOMAIN_VIDEO_TYPE_QXL &&
+            def->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+            def->type != VIR_DOMAIN_VIDEO_TYPE_BOCHS) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("model resolution is not supported"));
+            goto error;
+        }
+    }
+
     if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
         goto error;
 
@@ -26443,6 +26502,18 @@ virDomainVideoAccelDefFormat(virBufferPtr buf,
     virBufferAddLit(buf, "/>\n");
 }
 
+static void
+virDomainVideoResolutionDefFormat(virBufferPtr buf,
+                                  virDomainVideoResolutionDefPtr def)
+{
+    virBufferAddLit(buf, "<resolution");
+    if (def->x && def->y) {
+        virBufferAsprintf(buf, " x='%u' y='%u'",
+                          def->x, def->y);
+    }
+    virBufferAddLit(buf, "/>\n");
+}
+
 static int
 virDomainVideoDefFormat(virBufferPtr buf,
                         virDomainVideoDefPtr def,
@@ -26486,11 +26557,13 @@ virDomainVideoDefFormat(virBufferPtr buf,
         virBufferAsprintf(buf, " heads='%u'", def->heads);
     if (def->primary)
         virBufferAddLit(buf, " primary='yes'");
-    if (def->accel) {
+    if (def->accel || def->res) {
         virBufferAddLit(buf, ">\n");
         virBufferAdjustIndent(buf, 2);
         if (def->accel)
             virDomainVideoAccelDefFormat(buf, def->accel);
+        if (def->res)
+            virDomainVideoResolutionDefFormat(buf, def->res);
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</model>\n");
     } else {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 33cef5b75c..a164f26d57 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1407,6 +1407,10 @@ struct _virDomainVideoAccelDef {
     int accel3d; /* enum virTristateBool */
 };
 
+struct _virDomainVideoResolutionDef {
+    unsigned int x;
+    unsigned int y;
+};
 
 struct _virDomainVideoDriverDef {
    virDomainVideoVGAConf vgaconf;
@@ -1422,6 +1426,7 @@ struct _virDomainVideoDef {
     bool primary;
     virDomainVideoAccelDefPtr accel;
     virDomainVideoDriverDefPtr driver;
+    virDomainVideoResolutionDefPtr res;
     virDomainDeviceInfo info;
     virDomainVirtioOptionsPtr virtio;
 };
diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
index a15cfb5f9e..462842f324 100644
--- a/src/conf/virconftypes.h
+++ b/src/conf/virconftypes.h
@@ -324,6 +324,9 @@ typedef virDomainVcpuDef *virDomainVcpuDefPtr;
 typedef struct _virDomainVideoAccelDef virDomainVideoAccelDef;
 typedef virDomainVideoAccelDef *virDomainVideoAccelDefPtr;
 
+typedef struct _virDomainVideoResolutionDef virDomainVideoResolutionDef;
+typedef virDomainVideoResolutionDef *virDomainVideoResolutionDefPtr;
+
 typedef struct _virDomainVideoDef virDomainVideoDef;
 typedef virDomainVideoDef *virDomainVideoDefPtr;
 
-- 
2.20.1

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

On 8/30/19 5:40 PM, jcfaracco@gmail.com wrote:
> From: Julio Faracco <jcfaracco@gmail.com>
> 
> New element 'resolution' with parameters 'x' and 'y' were added to
> support this settings for VGA, QXL, Virtio and Bochs XMLs. A new
> structure was created as Acceleration element has. It is easy to parse
> this property. Example:
> 
>     <model type='qxl'>
>       <resolution x='800' y='600'\>

The ending slash should be /

>     </model>
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  src/conf/domain_conf.c  | 75 ++++++++++++++++++++++++++++++++++++++++-
>  src/conf/domain_conf.h  |  5 +++
>  src/conf/virconftypes.h |  3 ++
>  3 files changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b7a342bb91..9db8fd9697 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15311,6 +15311,53 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>      return def;
>  }
>  
> +static virDomainVideoResolutionDefPtr
> +virDomainVideoResolutionDefParseXML(xmlNodePtr node)
> +{
> +    xmlNodePtr cur;
> +    virDomainVideoResolutionDefPtr def;
> +    VIR_AUTOFREE(char *) x = NULL;
> +    VIR_AUTOFREE(char *) y = NULL;
> +
> +    cur = node->children;
> +    while (cur != NULL) {
> +        if (cur->type == XML_ELEMENT_NODE) {
> +            if (!x && !y &&
> +                virXMLNodeNameEqual(cur, "resolution")) {
> +                x = virXMLPropString(cur, "x");
> +                y = virXMLPropString(cur, "y");
> +            }
> +        }
> +        cur = cur->next;
> +    }
> +
> +    if (!x || !y)
> +        return NULL;
> +
> +    if (VIR_ALLOC(def) < 0)
> +        goto cleanup;
> +
> +    if (x) {
> +        if (virStrToLong_uip(x, NULL, 10, &def->x) < 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("cannot parse video x-resolution '%s'"), x);
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (y) {
> +        if (virStrToLong_uip(y, NULL, 10, &def->y) < 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("cannot parse video y-resolution '%s'"), y);
> +            goto cleanup;
> +        }
> +    }
> +
> + cleanup:
> +    return def;
> +}
> +
> +
>  static virDomainVideoDriverDefPtr
>  virDomainVideoDriverDefParseXML(xmlNodePtr node)
>  {
> @@ -15389,6 +15436,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  }
>  
>                  def->accel = virDomainVideoAccelDefParseXML(cur);
> +                def->res = virDomainVideoResolutionDefParseXML(cur);
>              }
>              if (virXMLNodeNameEqual(cur, "driver")) {
>                  if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
> @@ -15463,6 +15511,17 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
>          }
>      }
>  
> +    if (def->res) {
> +        if (def->type != VIR_DOMAIN_VIDEO_TYPE_VGA &&
> +            def->type != VIR_DOMAIN_VIDEO_TYPE_QXL &&
> +            def->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
> +            def->type != VIR_DOMAIN_VIDEO_TYPE_BOCHS) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("model resolution is not supported"));
> +            goto error;
> +        }
> +    }
> +

As I mention in the cover letter response I think all checks like this
can be removed. But for future reference we are trying to move bits like
this out of Parse* time and into the explicit Validation functions.

- Cole

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