[libvirt] [PATCH v3 3/4] conf: Add XML resolution support for video models
Cole Robinson
crobinso at redhat.com
Thu Oct 10 17:37:39 UTC 2019
On 10/7/19 11:35 PM, jcfaracco at gmail.com wrote:
> From: Julio Faracco <jcfaracco at gmail.com>
>
> XML need to support both properties together. This commit adds XML
> support for video model if they are set. Domain configuration is able
> to parse this properties as 'x' and 'y'. Code is not using same label as
> QEMU: 'xres' and 'yres'.
>
Compared to v2, the commit message here dropped the XML example, which I
find useful; can make it easier to grep git logs in some instances
> Signed-off-by: Julio Faracco <jcfaracco at gmail.com>
> ---
> src/conf/domain_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 5 +++
> src/conf/virconftypes.h | 3 ++
> 3 files changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a53cd6a725..950c4522f9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15403,6 +15403,52 @@ 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)
> {
> @@ -15482,6 +15528,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
> }
>
> def->accel = virDomainVideoAccelDefParseXML(cur);
> + def->res = virDomainVideoResolutionDefParseXML(cur);
> }
> if (virXMLNodeNameEqual(cur, "driver")) {
> if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
> @@ -15569,6 +15616,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;
> + }
> + }
> +
I mentioned this in v2: IMO this check should be removed, it's just a
whitelist duplicating qemu logic which could easily go out of date. But
if you want to keep it, it should go in a Validate callback,
specifically virDomainVideoDefValidate. We are trying to avoid adding
more of these checks inline in the XML parser
You can CC me on v4 and I'll review it
Thanks,
Cole
More information about the libvir-list
mailing list