On 8/30/19 5:40 PM, jcfaracco@xxxxxxxxx wrote: > From: Julio Faracco <jcfaracco@xxxxxxxxx> > > 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@xxxxxxxxx> > --- > 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list