Hi
On Wed, May 11, 2016 at 5:08 PM, Pavel Hrdina <phrdina@xxxxxxxxxx> wrote:
> Move the compatibility code out of virDomainGraphicsListensParseXML()
> into virDomainGraphicsListenDefParseXML(). This also fixes a small
> inconsistency between the code and error message itself.
>
> Before this patch we would search first listen element that is
> type='address' to validate listen and address attributes. After this
> patch we always take the first listen element regardless of the type.
>
> This shouldn't break anything since all drivers supports only one
> listen.
>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 85 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 51 insertions(+), 34 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index df2258a..45d2789 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10640,18 +10640,36 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node,
> return 0;
> }
>
> +
> +/**
> + * virDomainGraphicsListenDefParseXML:
> + * @def: listen def pointer to be filled
> + * @node: xml node of <listen/> element
> + * @parent: xml node of <graphics/> element
> + * @flags: bit-wise or of VIR_DOMAIN_DEF_PARSE_*
> + *
> + * Parses current <listen/> element from @node to @def. For backward
> + * compatibility the @parent element should contain node of <graphics/> element
> + * for the first <listen/> element in order to validate attributes from both
> + * elements.
> + */
> static int
> virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
> xmlNodePtr node,
> + xmlNodePtr parent,
> unsigned int flags)
> {
> int ret = -1;
> - char *type = virXMLPropString(node, "type");
> - char *address = virXMLPropString(node, "address");
> - char *network = virXMLPropString(node, "network");
> + char *type = virXMLPropString(node, "type");
> + char *address = virXMLPropString(node, "address");
> + char *network = virXMLPropString(node, "network");
code-style change only
> char *fromConfig = virXMLPropString(node, "fromConfig");
> + char *addressCompat = NULL;
> int tmp, typeVal;
>
> + if (parent)
> + addressCompat = virXMLPropString(parent, "listen");
> +
> if (!type) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("graphics listen type must be specified"));
> @@ -10665,9 +10683,21 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
> }
> def->type = typeVal;
>
> - /* address is recognized if either type='address', or if
> - * type='network' and we're looking at live XML (i.e. *not*
> - * inactive). It is otherwise ignored. */
> + if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) {
> + if (address && addressCompat && STRNEQ(address, addressCompat)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("graphics 'listen' attribute '%s' must match "
> + "'address' attribute of first listen element "
> + "(found '%s')"), addressCompat, address);
> + goto error;
> + }
> +
> + if (!address) {
> + address = addressCompat;
> + addressCompat = NULL;
> + }
> + }
> +
> if (address && address[0] &&
> (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS ||
> (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK &&
> @@ -10707,6 +10737,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
> VIR_FREE(address);
> VIR_FREE(network);
> VIR_FREE(fromConfig);
> + VIR_FREE(addressCompat);
> return ret;
> }
>
> @@ -10719,8 +10750,7 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def,
> {
> xmlNodePtr *listenNodes = NULL;
> xmlNodePtr save = ctxt->node;
> - virDomainGraphicsListenDefPtr address = NULL;
> - char *listenAddr = NULL;
> + virDomainGraphicsListenDef newListen = {0};
> char *socketPath = NULL;
> int nListens;
> int ret = -1;
> @@ -10747,44 +10777,31 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def,
> for (i = 0; i < nListens; i++) {
> if (virDomainGraphicsListenDefParseXML(&def->listens[i],
> listenNodes[i],
> + i == 0 ? node : NULL,
> flags) < 0)
> goto error;
>
> - if (!address &&
> - def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS)
> - address = &def->listens[i];
> -
> def->nListens++;
> }
> VIR_FREE(listenNodes);
> - }
> -
> - /* listen attribute of <graphics> is also supported by these,
> - * but must match the 'address' attribute of the first listen
> - * that is type='address' (if present) */
> - listenAddr = virXMLPropString(node, "listen");
> - if (STREQ_NULLABLE(listenAddr, ""))
> - VIR_FREE(listenAddr);
> + } else {
> + /* If no <listen/> element was found in XML for backward compatibility
> + * we should try to parse 'listen' attribute from <graphics/> element. */
> + newListen.type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS;
> + newListen.address = virXMLPropString(node, "listen");
> + if (STREQ_NULLABLE(newListen.address, ""))
> + VIR_FREE(newListen.address);
>
> - if (address && listenAddr &&
> - STRNEQ_NULLABLE(address->address, listenAddr)) {
> - virReportError(VIR_ERR_XML_ERROR,
> - _("graphics listen attribute %s must match address "
> - "attribute of first listen element (found %s)"),
> - listenAddr, address->address);
> - goto error;
> + if (newListen.address &&
> + VIR_APPEND_ELEMENT(def->listens, def->nListens, newListen) < 0)
> + goto error;
> }
>
> - /* There were no <listen> elements, so we can just
> - * directly set listenAddr as listens[0]->address */
> - if (listenAddr && def->nListens == 0 &&
> - virDomainGraphicsListenAppendAddress(def, listenAddr) < 0)
> - goto error;
> -
> ret = 0;
> error:
> + if (ret < 0)
> + virDomainGraphicsListenDefClear(&newListen);
> VIR_FREE(listenNodes);
> - VIR_FREE(listenAddr);
> VIR_FREE(socketPath);
> ctxt->node = save;
> return ret;
> --
> 2.8.2
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
--
Marc-André Lureau
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list