[libvirt] [PATCH v2 03/10] conf: Adding resolution property for model element

[Cole Robinson]

On 8/30/19 5:40 PM, jcfaracco at gmail.com wrote:
> From: Julio Faracco <jcfaracco at 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 at 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