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