Move code, that parses graphics listens, to it's own function. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/conf/domain_conf.c | 79 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d4c78fd..a0ef3d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10675,38 +10675,19 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, } -/* Parse the XML definition for a graphics device */ -static virDomainGraphicsDefPtr -virDomainGraphicsDefParseXML(xmlNodePtr node, - xmlXPathContextPtr ctxt, - unsigned int flags) +static int +virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) { - virDomainGraphicsDefPtr def; - char *type = NULL; int nListens; xmlNodePtr *listenNodes = NULL; char *listenAddr = NULL; xmlNodePtr save = ctxt->node; - - if (VIR_ALLOC(def) < 0) - return NULL; + int ret = -1; ctxt->node = node; - - type = virXMLPropString(node, "type"); - - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing graphics device type")); - goto error; - } - - if ((def->type = virDomainGraphicsTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown graphics device type '%s'"), type); - goto error; - } - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP || def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { @@ -10723,10 +10704,10 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, goto error; for (i = 0; i < nListens; i++) { - int ret = virDomainGraphicsListenDefParseXML(&def->listens[i], - listenNodes[i], - flags); - if (ret < 0) + int rv = virDomainGraphicsListenDefParseXML(&def->listens[i], + listenNodes[i], + flags); + if (rv < 0) goto error; def->nListens++; } @@ -10780,6 +10761,43 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, } } + ret = 0; + error: + VIR_FREE(listenNodes); + VIR_FREE(listenAddr); + ctxt->node = save; + return ret; +} + + +/* Parse the XML definition for a graphics device */ +static virDomainGraphicsDefPtr +virDomainGraphicsDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + virDomainGraphicsDefPtr def; + char *type = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + type = virXMLPropString(node, "type"); + if (!type) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing graphics device type")); + goto error; + } + + if ((def->type = virDomainGraphicsTypeFromString(type)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown graphics device type '%s'"), type); + goto error; + } + + if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0) + goto error; + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { char *port = virXMLPropString(node, "port"); char *websocket = virXMLPropString(node, "websocket"); @@ -11230,10 +11248,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, cleanup: VIR_FREE(type); - VIR_FREE(listenNodes); - VIR_FREE(listenAddr); - ctxt->node = save; return def; error: -- 2.7.4

Refactor the listen parser to use only one loop. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/conf/domain_conf.c | 117 +++++++++++++++++++++---------------------------- 1 file changed, 50 insertions(+), 67 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a0ef3d9..9d48d07 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10681,86 +10681,69 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def, xmlXPathContextPtr ctxt, unsigned int flags) { - int nListens; xmlNodePtr *listenNodes = NULL; - char *listenAddr = NULL; xmlNodePtr save = ctxt->node; + virDomainGraphicsListenDefPtr address = NULL; + char *listenAddr = NULL; + int nListens; int ret = -1; ctxt->node = node; - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP || - def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - /* parse the <listen> subelements for graphics types that support it */ - nListens = virXPathNodeSet("./listen", ctxt, &listenNodes); - if (nListens < 0) - goto error; + if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC && + def->type != VIR_DOMAIN_GRAPHICS_TYPE_RDP && + def->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + ret = 0; + goto error; + } - if (nListens > 0) { - size_t i; + /* parse the <listen> subelements for graphics types that support it */ + nListens = virXPathNodeSet("./listen", ctxt, &listenNodes); + if (nListens < 0) + goto error; + + if (nListens > 0) { + size_t i; - if (VIR_ALLOC_N(def->listens, nListens) < 0) + if (VIR_ALLOC_N(def->listens, nListens) < 0) + goto error; + + for (i = 0; i < nListens; i++) { + if (virDomainGraphicsListenDefParseXML(&def->listens[i], + listenNodes[i], + flags) < 0) goto error; - for (i = 0; i < nListens; i++) { - int rv = virDomainGraphicsListenDefParseXML(&def->listens[i], - listenNodes[i], - flags); - if (rv < 0) - goto error; - 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 (listenAddr && !listenAddr[0]) - VIR_FREE(listenAddr); - - if (listenAddr) { - if (def->nListens == 0) { - /* There were no <listen> elements, so we can just - * directly set listenAddr as listens[0]->address */ - if (virDomainGraphicsListenSetAddress(def, 0, listenAddr, - -1, true) < 0) - goto error; - } else { - /* There is at least 1 listen element, so we look for - * the first listen of type='address', and make sure - * its address matches the listen attribute from - * graphics. */ - bool matched = false; - const char *found = NULL; - size_t i; - - for (i = 0; i < nListens; i++) { - if (virDomainGraphicsListenGetType(def, i) - == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) { - found = virDomainGraphicsListenGetAddress(def, i); - if (STREQ_NULLABLE(found, listenAddr)) - matched = true; - break; - } - } - if (found && !matched) { - virReportError(VIR_ERR_XML_ERROR, - _("graphics listen attribute %s must match address " - "attribute of first listen element (found %s)"), - listenAddr, found); - goto error; - } else if (!found) { - /* quietly ignore listen address if none of the listens - * are of type address */ - VIR_FREE(listenAddr); - } - } + 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 (listenAddr && !listenAddr[0]) + VIR_FREE(listenAddr); + + if (STREQ_NULLABLE(address->address, listenAddr) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("graphics listen attribute %s must match address " + "attribute of first listen element (found %s)"), + listenAddr, address->address); + goto error; + } + + /* There were no <listen> elements, so we can just + * directly set listenAddr as listens[0]->address */ + if (listenAddr && def->nListens == 0 && + virDomainGraphicsListenSetAddress(def, 0, listenAddr, -1, true) < 0) + goto error; + ret = 0; error: VIR_FREE(listenNodes); -- 2.7.4

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/conf/domain_conf.c | 835 +++++++++++++++++++++++++++---------------------- 1 file changed, 453 insertions(+), 382 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9d48d07..fd9316f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10753,480 +10753,551 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def, } -/* Parse the XML definition for a graphics device */ -static virDomainGraphicsDefPtr -virDomainGraphicsDefParseXML(xmlNodePtr node, - xmlXPathContextPtr ctxt, - unsigned int flags) +static int +virDomainGraphicsDefParseXMLVnc(virDomainGraphicsDefPtr def, + xmlNodePtr node, + unsigned int flags) { - virDomainGraphicsDefPtr def; - char *type = NULL; - - if (VIR_ALLOC(def) < 0) - return NULL; - - type = virXMLPropString(node, "type"); - if (!type) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing graphics device type")); - goto error; - } - - if ((def->type = virDomainGraphicsTypeFromString(type)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown graphics device type '%s'"), type); - goto error; - } - - if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0) - goto error; - - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - char *port = virXMLPropString(node, "port"); - char *websocket = virXMLPropString(node, "websocket"); - char *sharePolicy = virXMLPropString(node, "sharePolicy"); - char *autoport; + char *port = virXMLPropString(node, "port"); + char *websocket = virXMLPropString(node, "websocket"); + char *sharePolicy = virXMLPropString(node, "sharePolicy"); + char *autoport; + int ret = -1; - if (port) { - if (virStrToLong_i(port, NULL, 10, &def->data.vnc.port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse vnc port %s"), port); - VIR_FREE(port); - goto error; - } + if (port) { + if (virStrToLong_i(port, NULL, 10, &def->data.vnc.port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse vnc port %s"), port); VIR_FREE(port); - /* Legacy compat syntax, used -1 for auto-port */ - if (def->data.vnc.port == -1) { - if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) - def->data.vnc.port = 0; - def->data.vnc.autoport = true; - } - } else { - def->data.vnc.port = 0; + goto error; + } + VIR_FREE(port); + /* Legacy compat syntax, used -1 for auto-port */ + if (def->data.vnc.port == -1) { + if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) + def->data.vnc.port = 0; def->data.vnc.autoport = true; } + } else { + def->data.vnc.port = 0; + def->data.vnc.autoport = true; + } - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { - if (STREQ(autoport, "yes")) { - if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) - def->data.vnc.port = 0; - def->data.vnc.autoport = true; - } - VIR_FREE(autoport); + if ((autoport = virXMLPropString(node, "autoport")) != NULL) { + if (STREQ(autoport, "yes")) { + if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) + def->data.vnc.port = 0; + def->data.vnc.autoport = true; } + VIR_FREE(autoport); + } - if (websocket) { - if (virStrToLong_i(websocket, - NULL, 10, - &def->data.vnc.websocket) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse vnc WebSocket port %s"), websocket); - VIR_FREE(websocket); - goto error; - } + if (websocket) { + if (virStrToLong_i(websocket, + NULL, 10, + &def->data.vnc.websocket) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse vnc WebSocket port %s"), websocket); VIR_FREE(websocket); + goto error; } + VIR_FREE(websocket); + } - if (sharePolicy) { - int policy = - virDomainGraphicsVNCSharePolicyTypeFromString(sharePolicy); + if (sharePolicy) { + int policy = + virDomainGraphicsVNCSharePolicyTypeFromString(sharePolicy); - if (policy < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown vnc display sharing policy '%s'"), sharePolicy); - VIR_FREE(sharePolicy); - goto error; - } else { - def->data.vnc.sharePolicy = policy; - } + if (policy < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown vnc display sharing policy '%s'"), sharePolicy); VIR_FREE(sharePolicy); + goto error; + } else { + def->data.vnc.sharePolicy = policy; } + VIR_FREE(sharePolicy); + } - def->data.vnc.socket = virXMLPropString(node, "socket"); - def->data.vnc.keymap = virXMLPropString(node, "keymap"); + def->data.vnc.socket = virXMLPropString(node, "socket"); + def->data.vnc.keymap = virXMLPropString(node, "keymap"); - if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth, - def->type) < 0) - goto error; - } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - char *fullscreen = virXMLPropString(node, "fullscreen"); + if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth, + def->type) < 0) + goto error; - if (fullscreen != NULL) { - if (STREQ(fullscreen, "yes")) { - def->data.sdl.fullscreen = true; - } else if (STREQ(fullscreen, "no")) { - def->data.sdl.fullscreen = false; - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown fullscreen value '%s'"), fullscreen); - VIR_FREE(fullscreen); - goto error; - } - VIR_FREE(fullscreen); - } else { + ret = 0; + error: + return ret; +} + + +static int +virDomainGraphicsDefParseXMLSdl(virDomainGraphicsDefPtr def, + xmlNodePtr node) +{ + char *fullscreen = virXMLPropString(node, "fullscreen"); + + if (fullscreen != NULL) { + if (STREQ(fullscreen, "yes")) { + def->data.sdl.fullscreen = true; + } else if (STREQ(fullscreen, "no")) { def->data.sdl.fullscreen = false; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown fullscreen value '%s'"), fullscreen); + VIR_FREE(fullscreen); + return -1; } - def->data.sdl.xauth = virXMLPropString(node, "xauth"); - def->data.sdl.display = virXMLPropString(node, "display"); - } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) { - char *port = virXMLPropString(node, "port"); - char *autoport; - char *replaceUser; - char *multiUser; + VIR_FREE(fullscreen); + } else { + def->data.sdl.fullscreen = false; + } + def->data.sdl.xauth = virXMLPropString(node, "xauth"); + def->data.sdl.display = virXMLPropString(node, "display"); - if (port) { - if (virStrToLong_i(port, NULL, 10, &def->data.rdp.port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse rdp port %s"), port); - VIR_FREE(port); - goto error; - } - /* Legacy compat syntax, used -1 for auto-port */ - if (def->data.rdp.port == -1) - def->data.rdp.autoport = true; + return 0; +} + + +static int +virDomainGraphicsDefParseXMLRdp(virDomainGraphicsDefPtr def, + xmlNodePtr node, + unsigned int flags) +{ + char *port = virXMLPropString(node, "port"); + char *autoport; + char *replaceUser; + char *multiUser; + int ret = -1; + if (port) { + if (virStrToLong_i(port, NULL, 10, &def->data.rdp.port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse rdp port %s"), port); VIR_FREE(port); - } else { - def->data.rdp.port = 0; - def->data.rdp.autoport = true; + goto error; } + /* Legacy compat syntax, used -1 for auto-port */ + if (def->data.rdp.port == -1) + def->data.rdp.autoport = true; - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { - if (STREQ(autoport, "yes")) - def->data.rdp.autoport = true; + VIR_FREE(port); + } else { + def->data.rdp.port = 0; + def->data.rdp.autoport = true; + } - VIR_FREE(autoport); - } + if ((autoport = virXMLPropString(node, "autoport")) != NULL) { + if (STREQ(autoport, "yes")) + def->data.rdp.autoport = true; - if (def->data.rdp.autoport && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) - def->data.rdp.port = 0; + VIR_FREE(autoport); + } - if ((replaceUser = virXMLPropString(node, "replaceUser")) != NULL) { - if (STREQ(replaceUser, "yes")) - def->data.rdp.replaceUser = true; - VIR_FREE(replaceUser); - } + if (def->data.rdp.autoport && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) + def->data.rdp.port = 0; - if ((multiUser = virXMLPropString(node, "multiUser")) != NULL) { - if (STREQ(multiUser, "yes")) - def->data.rdp.multiUser = true; - VIR_FREE(multiUser); - } + if ((replaceUser = virXMLPropString(node, "replaceUser")) != NULL) { + if (STREQ(replaceUser, "yes")) + def->data.rdp.replaceUser = true; + VIR_FREE(replaceUser); + } - } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP) { - char *fullscreen = virXMLPropString(node, "fullscreen"); + if ((multiUser = virXMLPropString(node, "multiUser")) != NULL) { + if (STREQ(multiUser, "yes")) + def->data.rdp.multiUser = true; + VIR_FREE(multiUser); + } - if (fullscreen != NULL) { - if (STREQ(fullscreen, "yes")) { - def->data.desktop.fullscreen = true; - } else if (STREQ(fullscreen, "no")) { - def->data.desktop.fullscreen = false; - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown fullscreen value '%s'"), fullscreen); - VIR_FREE(fullscreen); - goto error; - } - VIR_FREE(fullscreen); - } else { + ret = 0; + error: + return ret; +} + + +static int +virDomainGraphicsDefParseXMLDesktop(virDomainGraphicsDefPtr def, + xmlNodePtr node) +{ + char *fullscreen = virXMLPropString(node, "fullscreen"); + + if (fullscreen != NULL) { + if (STREQ(fullscreen, "yes")) { + def->data.desktop.fullscreen = true; + } else if (STREQ(fullscreen, "no")) { def->data.desktop.fullscreen = false; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown fullscreen value '%s'"), fullscreen); + VIR_FREE(fullscreen); + return -1; } + VIR_FREE(fullscreen); + } else { + def->data.desktop.fullscreen = false; + } - def->data.desktop.display = virXMLPropString(node, "display"); - } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - xmlNodePtr cur; - char *port = virXMLPropString(node, "port"); - char *tlsPort; - char *autoport; - char *defaultMode; - int defaultModeVal; + def->data.desktop.display = virXMLPropString(node, "display"); + return 0; +} - if (port) { - if (virStrToLong_i(port, NULL, 10, &def->data.spice.port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse spice port %s"), port); - VIR_FREE(port); - goto error; - } + +static int +virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDefPtr def, + xmlNodePtr node, + unsigned int flags) +{ + xmlNodePtr cur; + char *port = virXMLPropString(node, "port"); + char *tlsPort; + char *autoport; + char *defaultMode; + int defaultModeVal; + int ret = -1; + + if (port) { + if (virStrToLong_i(port, NULL, 10, &def->data.spice.port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse spice port %s"), port); VIR_FREE(port); - } else { - def->data.spice.port = 0; + goto error; } + VIR_FREE(port); + } else { + def->data.spice.port = 0; + } - tlsPort = virXMLPropString(node, "tlsPort"); - if (tlsPort) { - if (virStrToLong_i(tlsPort, NULL, 10, &def->data.spice.tlsPort) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse spice tlsPort %s"), tlsPort); - VIR_FREE(tlsPort); - goto error; - } + tlsPort = virXMLPropString(node, "tlsPort"); + if (tlsPort) { + if (virStrToLong_i(tlsPort, NULL, 10, &def->data.spice.tlsPort) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse spice tlsPort %s"), tlsPort); VIR_FREE(tlsPort); - } else { - def->data.spice.tlsPort = 0; + goto error; } + VIR_FREE(tlsPort); + } else { + def->data.spice.tlsPort = 0; + } - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { - if (STREQ(autoport, "yes")) - def->data.spice.autoport = true; - VIR_FREE(autoport); - } + if ((autoport = virXMLPropString(node, "autoport")) != NULL) { + if (STREQ(autoport, "yes")) + def->data.spice.autoport = true; + VIR_FREE(autoport); + } - def->data.spice.defaultMode = VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY; + def->data.spice.defaultMode = VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY; - if ((defaultMode = virXMLPropString(node, "defaultMode")) != NULL) { - if ((defaultModeVal = virDomainGraphicsSpiceChannelModeTypeFromString(defaultMode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown default spice channel mode %s"), - defaultMode); - VIR_FREE(defaultMode); - goto error; - } - def->data.spice.defaultMode = defaultModeVal; + if ((defaultMode = virXMLPropString(node, "defaultMode")) != NULL) { + if ((defaultModeVal = virDomainGraphicsSpiceChannelModeTypeFromString(defaultMode)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown default spice channel mode %s"), + defaultMode); VIR_FREE(defaultMode); + goto error; } + def->data.spice.defaultMode = defaultModeVal; + VIR_FREE(defaultMode); + } - if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) { - /* Legacy compat syntax, used -1 for auto-port */ - def->data.spice.autoport = true; - } + if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) { + /* Legacy compat syntax, used -1 for auto-port */ + def->data.spice.autoport = true; + } - if (def->data.spice.autoport && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { - def->data.spice.port = 0; - def->data.spice.tlsPort = 0; - } + if (def->data.spice.autoport && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { + def->data.spice.port = 0; + def->data.spice.tlsPort = 0; + } - def->data.spice.keymap = virXMLPropString(node, "keymap"); + def->data.spice.keymap = virXMLPropString(node, "keymap"); - if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth, - def->type) < 0) - goto error; + if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth, + def->type) < 0) + goto error; - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(cur->name, BAD_CAST "channel")) { - char *name, *mode; - int nameval, modeval; - name = virXMLPropString(cur, "name"); - mode = virXMLPropString(cur, "mode"); + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "channel")) { + char *name, *mode; + int nameval, modeval; + name = virXMLPropString(cur, "name"); + mode = virXMLPropString(cur, "mode"); - if (!name || !mode) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("spice channel missing name/mode")); - VIR_FREE(name); - VIR_FREE(mode); - goto error; - } + if (!name || !mode) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("spice channel missing name/mode")); + VIR_FREE(name); + VIR_FREE(mode); + goto error; + } - if ((nameval = virDomainGraphicsSpiceChannelNameTypeFromString(name)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown spice channel name %s"), - name); - VIR_FREE(name); - VIR_FREE(mode); - goto error; - } - if ((modeval = virDomainGraphicsSpiceChannelModeTypeFromString(mode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown spice channel mode %s"), - mode); - VIR_FREE(name); - VIR_FREE(mode); - goto error; - } + if ((nameval = virDomainGraphicsSpiceChannelNameTypeFromString(name)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown spice channel name %s"), + name); VIR_FREE(name); VIR_FREE(mode); + goto error; + } + if ((modeval = virDomainGraphicsSpiceChannelModeTypeFromString(mode)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown spice channel mode %s"), + mode); + VIR_FREE(name); + VIR_FREE(mode); + goto error; + } + VIR_FREE(name); + VIR_FREE(mode); - def->data.spice.channels[nameval] = modeval; - } else if (xmlStrEqual(cur->name, BAD_CAST "image")) { - char *compression = virXMLPropString(cur, "compression"); - int compressionVal; + def->data.spice.channels[nameval] = modeval; + } else if (xmlStrEqual(cur->name, BAD_CAST "image")) { + char *compression = virXMLPropString(cur, "compression"); + int compressionVal; - if (!compression) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("spice image missing compression")); - goto error; - } + if (!compression) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("spice image missing compression")); + goto error; + } - if ((compressionVal = - virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown spice image compression %s"), - compression); - VIR_FREE(compression); - goto error; - } + if ((compressionVal = + virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown spice image compression %s"), + compression); VIR_FREE(compression); + goto error; + } + VIR_FREE(compression); - def->data.spice.image = compressionVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "jpeg")) { - char *compression = virXMLPropString(cur, "compression"); - int compressionVal; + def->data.spice.image = compressionVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "jpeg")) { + char *compression = virXMLPropString(cur, "compression"); + int compressionVal; - if (!compression) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("spice jpeg missing compression")); - goto error; - } + if (!compression) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("spice jpeg missing compression")); + goto error; + } - if ((compressionVal = - virDomainGraphicsSpiceJpegCompressionTypeFromString(compression)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown spice jpeg compression %s"), - compression); - VIR_FREE(compression); - goto error; - } + if ((compressionVal = + virDomainGraphicsSpiceJpegCompressionTypeFromString(compression)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown spice jpeg compression %s"), + compression); VIR_FREE(compression); + goto error; + } + VIR_FREE(compression); - def->data.spice.jpeg = compressionVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "zlib")) { - char *compression = virXMLPropString(cur, "compression"); - int compressionVal; + def->data.spice.jpeg = compressionVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "zlib")) { + char *compression = virXMLPropString(cur, "compression"); + int compressionVal; - if (!compression) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("spice zlib missing compression")); - goto error; - } + if (!compression) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("spice zlib missing compression")); + goto error; + } - if ((compressionVal = - virDomainGraphicsSpiceZlibCompressionTypeFromString(compression)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown spice zlib compression %s"), - compression); - VIR_FREE(compression); - goto error; - } + if ((compressionVal = + virDomainGraphicsSpiceZlibCompressionTypeFromString(compression)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown spice zlib compression %s"), + compression); VIR_FREE(compression); + goto error; + } + VIR_FREE(compression); - def->data.spice.zlib = compressionVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "playback")) { - char *compression = virXMLPropString(cur, "compression"); - int compressionVal; - - if (!compression) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("spice playback missing compression")); - goto error; - } + def->data.spice.zlib = compressionVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "playback")) { + char *compression = virXMLPropString(cur, "compression"); + int compressionVal; - if ((compressionVal = - virTristateSwitchTypeFromString(compression)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unknown spice playback compression")); - VIR_FREE(compression); - goto error; + if (!compression) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("spice playback missing compression")); + goto error; + } - } + if ((compressionVal = + virTristateSwitchTypeFromString(compression)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unknown spice playback compression")); VIR_FREE(compression); + goto error; - def->data.spice.playback = compressionVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "streaming")) { - char *mode = virXMLPropString(cur, "mode"); - int modeVal; + } + VIR_FREE(compression); - if (!mode) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("spice streaming missing mode")); - goto error; - } - if ((modeVal = - virDomainGraphicsSpiceStreamingModeTypeFromString(mode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unknown spice streaming mode")); - VIR_FREE(mode); - goto error; + def->data.spice.playback = compressionVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "streaming")) { + char *mode = virXMLPropString(cur, "mode"); + int modeVal; - } + if (!mode) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("spice streaming missing mode")); + goto error; + } + if ((modeVal = + virDomainGraphicsSpiceStreamingModeTypeFromString(mode)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unknown spice streaming mode")); VIR_FREE(mode); + goto error; - def->data.spice.streaming = modeVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "clipboard")) { - char *copypaste = virXMLPropString(cur, "copypaste"); - int copypasteVal; + } + VIR_FREE(mode); - if (!copypaste) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("spice clipboard missing copypaste")); - goto error; - } + def->data.spice.streaming = modeVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "clipboard")) { + char *copypaste = virXMLPropString(cur, "copypaste"); + int copypasteVal; - if ((copypasteVal = - virTristateBoolTypeFromString(copypaste)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown copypaste value '%s'"), copypaste); - VIR_FREE(copypaste); - goto error; - } + if (!copypaste) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("spice clipboard missing copypaste")); + goto error; + } + + if ((copypasteVal = + virTristateBoolTypeFromString(copypaste)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown copypaste value '%s'"), copypaste); VIR_FREE(copypaste); + goto error; + } + VIR_FREE(copypaste); - def->data.spice.copypaste = copypasteVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "filetransfer")) { - char *enable = virXMLPropString(cur, "enable"); - int enableVal; + def->data.spice.copypaste = copypasteVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "filetransfer")) { + char *enable = virXMLPropString(cur, "enable"); + int enableVal; - if (!enable) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("spice filetransfer missing enable")); - goto error; - } + if (!enable) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("spice filetransfer missing enable")); + goto error; + } - if ((enableVal = - virTristateBoolTypeFromString(enable)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown enable value '%s'"), enable); - VIR_FREE(enable); - goto error; - } + if ((enableVal = + virTristateBoolTypeFromString(enable)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown enable value '%s'"), enable); VIR_FREE(enable); + goto error; + } + VIR_FREE(enable); - def->data.spice.filetransfer = enableVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "gl")) { - char *enable = virXMLPropString(cur, "enable"); - int enableVal; + def->data.spice.filetransfer = enableVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "gl")) { + char *enable = virXMLPropString(cur, "enable"); + int enableVal; - if (!enable) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("spice gl element missing enable")); - goto error; - } + if (!enable) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("spice gl element missing enable")); + goto error; + } - if ((enableVal = - virTristateBoolTypeFromString(enable)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown enable value '%s'"), enable); - VIR_FREE(enable); - goto error; - } + if ((enableVal = + virTristateBoolTypeFromString(enable)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown enable value '%s'"), enable); VIR_FREE(enable); + goto error; + } + VIR_FREE(enable); - def->data.spice.gl = enableVal; - } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")) { - char *mode = virXMLPropString(cur, "mode"); - int modeVal; + def->data.spice.gl = enableVal; + } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")) { + char *mode = virXMLPropString(cur, "mode"); + int modeVal; - if (!mode) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("spice mouse missing mode")); - goto error; - } + if (!mode) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("spice mouse missing mode")); + goto error; + } - if ((modeVal = virDomainGraphicsSpiceMouseModeTypeFromString(mode)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown mouse mode value '%s'"), - mode); - VIR_FREE(mode); - goto error; - } + if ((modeVal = virDomainGraphicsSpiceMouseModeTypeFromString(mode)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown mouse mode value '%s'"), + mode); VIR_FREE(mode); - - def->data.spice.mousemode = modeVal; + goto error; } + VIR_FREE(mode); + + def->data.spice.mousemode = modeVal; } - cur = cur->next; } + cur = cur->next; + } + + ret = 0; + error: + return ret; +} + + +/* Parse the XML definition for a graphics device */ +static virDomainGraphicsDefPtr +virDomainGraphicsDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + virDomainGraphicsDefPtr def; + char *type = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + type = virXMLPropString(node, "type"); + if (!type) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing graphics device type")); + goto error; + } + + if ((def->type = virDomainGraphicsTypeFromString(type)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown graphics device type '%s'"), type); + goto error; + } + + if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0) + goto error; + + switch (def->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (virDomainGraphicsDefParseXMLVnc(def, node, flags) < 0) + goto error; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + if (virDomainGraphicsDefParseXMLSdl(def, node) < 0) + goto error; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + if (virDomainGraphicsDefParseXMLRdp(def, node, flags) < 0) + goto error; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + if (virDomainGraphicsDefParseXMLDesktop(def, node) < 0) + goto error; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + if (virDomainGraphicsDefParseXMLSpice(def, node, flags) < 0) + goto error; + break; } cleanup: -- 2.7.4

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/conf/domain_conf.c | 100 ++++++++++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 55 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fd9316f..42050b0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10761,17 +10761,15 @@ virDomainGraphicsDefParseXMLVnc(virDomainGraphicsDefPtr def, char *port = virXMLPropString(node, "port"); char *websocket = virXMLPropString(node, "websocket"); char *sharePolicy = virXMLPropString(node, "sharePolicy"); - char *autoport; + char *autoport = virXMLPropString(node, "autoport"); int ret = -1; if (port) { if (virStrToLong_i(port, NULL, 10, &def->data.vnc.port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse vnc port %s"), port); - VIR_FREE(port); goto error; } - VIR_FREE(port); /* Legacy compat syntax, used -1 for auto-port */ if (def->data.vnc.port == -1) { if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) @@ -10783,13 +10781,12 @@ virDomainGraphicsDefParseXMLVnc(virDomainGraphicsDefPtr def, def->data.vnc.autoport = true; } - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { + if (autoport) { if (STREQ(autoport, "yes")) { if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) def->data.vnc.port = 0; def->data.vnc.autoport = true; } - VIR_FREE(autoport); } if (websocket) { @@ -10798,10 +10795,8 @@ virDomainGraphicsDefParseXMLVnc(virDomainGraphicsDefPtr def, &def->data.vnc.websocket) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse vnc WebSocket port %s"), websocket); - VIR_FREE(websocket); goto error; } - VIR_FREE(websocket); } if (sharePolicy) { @@ -10810,13 +10805,12 @@ virDomainGraphicsDefParseXMLVnc(virDomainGraphicsDefPtr def, if (policy < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown vnc display sharing policy '%s'"), sharePolicy); - VIR_FREE(sharePolicy); + _("unknown vnc display sharing policy '%s'"), + sharePolicy); goto error; } else { def->data.vnc.sharePolicy = policy; } - VIR_FREE(sharePolicy); } def->data.vnc.socket = virXMLPropString(node, "socket"); @@ -10828,6 +10822,10 @@ virDomainGraphicsDefParseXMLVnc(virDomainGraphicsDefPtr def, ret = 0; error: + VIR_FREE(port); + VIR_FREE(autoport); + VIR_FREE(websocket); + VIR_FREE(sharePolicy); return ret; } @@ -10837,6 +10835,7 @@ virDomainGraphicsDefParseXMLSdl(virDomainGraphicsDefPtr def, xmlNodePtr node) { char *fullscreen = virXMLPropString(node, "fullscreen"); + int ret = -1; if (fullscreen != NULL) { if (STREQ(fullscreen, "yes")) { @@ -10846,17 +10845,19 @@ virDomainGraphicsDefParseXMLSdl(virDomainGraphicsDefPtr def, } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown fullscreen value '%s'"), fullscreen); - VIR_FREE(fullscreen); - return -1; + goto error; } - VIR_FREE(fullscreen); } else { def->data.sdl.fullscreen = false; } + def->data.sdl.xauth = virXMLPropString(node, "xauth"); def->data.sdl.display = virXMLPropString(node, "display"); - return 0; + ret = 0; + error: + VIR_FREE(fullscreen); + return ret; } @@ -10866,52 +10867,44 @@ virDomainGraphicsDefParseXMLRdp(virDomainGraphicsDefPtr def, unsigned int flags) { char *port = virXMLPropString(node, "port"); - char *autoport; - char *replaceUser; - char *multiUser; + char *autoport = virXMLPropString(node, "autoport"); + char *replaceUser = virXMLPropString(node, "replaceUser"); + char *multiUser = virXMLPropString(node, "multiUser"); int ret = -1; if (port) { if (virStrToLong_i(port, NULL, 10, &def->data.rdp.port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse rdp port %s"), port); - VIR_FREE(port); goto error; } /* Legacy compat syntax, used -1 for auto-port */ if (def->data.rdp.port == -1) def->data.rdp.autoport = true; - VIR_FREE(port); } else { def->data.rdp.port = 0; def->data.rdp.autoport = true; } - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { - if (STREQ(autoport, "yes")) - def->data.rdp.autoport = true; - - VIR_FREE(autoport); - } + if (autoport && STREQ(autoport, "yes")) + def->data.rdp.autoport = true; if (def->data.rdp.autoport && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) def->data.rdp.port = 0; - if ((replaceUser = virXMLPropString(node, "replaceUser")) != NULL) { - if (STREQ(replaceUser, "yes")) - def->data.rdp.replaceUser = true; - VIR_FREE(replaceUser); - } + if (replaceUser && STREQ(replaceUser, "yes")) + def->data.rdp.replaceUser = true; - if ((multiUser = virXMLPropString(node, "multiUser")) != NULL) { - if (STREQ(multiUser, "yes")) - def->data.rdp.multiUser = true; - VIR_FREE(multiUser); - } + if (multiUser && STREQ(multiUser, "yes")) + def->data.rdp.multiUser = true; ret = 0; error: + VIR_FREE(port); + VIR_FREE(autoport); + VIR_FREE(replaceUser); + VIR_FREE(multiUser); return ret; } @@ -10921,6 +10914,7 @@ virDomainGraphicsDefParseXMLDesktop(virDomainGraphicsDefPtr def, xmlNodePtr node) { char *fullscreen = virXMLPropString(node, "fullscreen"); + int ret = -1; if (fullscreen != NULL) { if (STREQ(fullscreen, "yes")) { @@ -10930,16 +10924,18 @@ virDomainGraphicsDefParseXMLDesktop(virDomainGraphicsDefPtr def, } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown fullscreen value '%s'"), fullscreen); - VIR_FREE(fullscreen); - return -1; + goto error; } - VIR_FREE(fullscreen); } else { def->data.desktop.fullscreen = false; } def->data.desktop.display = virXMLPropString(node, "display"); - return 0; + + ret = 0; + error: + VIR_FREE(fullscreen); + return ret; } @@ -10950,9 +10946,9 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDefPtr def, { xmlNodePtr cur; char *port = virXMLPropString(node, "port"); - char *tlsPort; - char *autoport; - char *defaultMode; + char *tlsPort = virXMLPropString(node, "tlsPort"); + char *autoport = virXMLPropString(node, "autoport"); + char *defaultMode = virXMLPropString(node, "defaultMode"); int defaultModeVal; int ret = -1; @@ -10960,45 +10956,35 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDefPtr def, if (virStrToLong_i(port, NULL, 10, &def->data.spice.port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse spice port %s"), port); - VIR_FREE(port); goto error; } - VIR_FREE(port); } else { def->data.spice.port = 0; } - tlsPort = virXMLPropString(node, "tlsPort"); if (tlsPort) { if (virStrToLong_i(tlsPort, NULL, 10, &def->data.spice.tlsPort) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse spice tlsPort %s"), tlsPort); - VIR_FREE(tlsPort); goto error; } - VIR_FREE(tlsPort); } else { def->data.spice.tlsPort = 0; } - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { - if (STREQ(autoport, "yes")) - def->data.spice.autoport = true; - VIR_FREE(autoport); - } + if (autoport && STREQ(autoport, "yes")) + def->data.spice.autoport = true; def->data.spice.defaultMode = VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY; - if ((defaultMode = virXMLPropString(node, "defaultMode")) != NULL) { + if (defaultMode) { if ((defaultModeVal = virDomainGraphicsSpiceChannelModeTypeFromString(defaultMode)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown default spice channel mode %s"), defaultMode); - VIR_FREE(defaultMode); goto error; } def->data.spice.defaultMode = defaultModeVal; - VIR_FREE(defaultMode); } if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) { @@ -11245,6 +11231,10 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDefPtr def, ret = 0; error: + VIR_FREE(port); + VIR_FREE(tlsPort); + VIR_FREE(autoport); + VIR_FREE(defaultMode); return ret; } -- 2.7.4

On Mon, Apr 04, 2016 at 03:20:24PM +0200, Pavel Hrdina wrote: I would go even further and delete the pos argument too. Every single caller uses 0 and only calls it when def->nListens == 0. This would become APPEND_ELEMENT_COPY. ACK Jan

On Mon, Apr 04, 2016 at 03:20:29PM +0200, Pavel Hrdina wrote: I prefer the original version where the function only touches the one single ListenDef it is parsing. Jan

Move generation of vnc unix socket to qemuProcessPrepareDomain which is the correct place to do those things. Now we can also test it. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/qemu/qemu_command.c | 20 ++++--------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_process.c | 20 +++++++++++-- ...qemuxml2argv-graphics-vnc-auto-unix-socket.args | 22 ++++++++++++++ .../qemuxml2argv-graphics-vnc-auto-unix-socket.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ 6 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9335f63..67d3336 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7219,8 +7219,7 @@ static int qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - virDomainGraphicsDefPtr graphics, - const char *domainLibDir) + virDomainGraphicsDefPtr graphics) { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr listen = NULL; @@ -7235,14 +7234,8 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; } - if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { - if (!graphics->data.vnc.socket && - virAsprintf(&graphics->data.vnc.socket, - "%s/vnc.sock", domainLibDir) == -1) - goto error; - + if (graphics->data.vnc.socket) { virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); - } else { if (!graphics->data.vnc.autoport && (graphics->data.vnc.port < 5900 || @@ -7613,8 +7606,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainGraphicsDefPtr graphics, - const char *domainLibDir) + virDomainGraphicsDefPtr graphics) { switch ((virDomainGraphicsType) graphics->type) { case VIR_DOMAIN_GRAPHICS_TYPE_SDL: @@ -7646,8 +7638,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, - graphics, domainLibDir); + return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, graphics); case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics); @@ -9199,7 +9190,6 @@ qemuBuildCommandLine(virConnectPtr conn, virBitmapPtr nodeset, size_t *nnicindexes, int **nicindexes, - const char *domainLibDir, const char *domainChannelTargetDir) { size_t i; @@ -9360,7 +9350,7 @@ qemuBuildCommandLine(virConnectPtr conn, for (i = 0; i < def->ngraphics; ++i) { if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps, - def->graphics[i], domainLibDir) < 0) + def->graphics[i]) < 0) goto error; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 7c13d45..677fc05 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -74,10 +74,9 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, virBitmapPtr nodeset, size_t *nnicindexes, int **nicindexes, - const char *domainLibDir, const char *domainChannelTargetDir) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11) - ATTRIBUTE_NONNULL(17) ATTRIBUTE_NONNULL(18); + ATTRIBUTE_NONNULL(17); /* Generate '-device' string for chardev device */ int diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6a4fb8c..cfd8a90 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4957,6 +4957,7 @@ qemuProcessPrepareDomain(virConnectPtr conn, size_t i; char *nodeset = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) @@ -4996,6 +4997,22 @@ qemuProcessPrepareDomain(virConnectPtr conn, if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0) goto cleanup; + /* Generate socket paths for graphics */ + for (i = 0; i < vm->def->ngraphics; i++) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (cfg->vncAutoUnixSocket && !graphics->data.vnc.socket) { + if (virAsprintf(&graphics->data.vnc.socket, + "%s/vnc.sock", priv->libDir) < 0) + goto cleanup; + + continue; + } + } + } + /* "volume" type disk's source must be translated before * cgroup and security setting. */ @@ -5033,6 +5050,7 @@ qemuProcessPrepareDomain(virConnectPtr conn, cleanup: VIR_FREE(nodeset); virObjectUnref(caps); + virObjectUnref(cfg); return ret; } @@ -5230,7 +5248,6 @@ qemuProcessLaunch(virConnectPtr conn, qemuCheckFips(), priv->autoNodeset, &nnicindexes, &nicindexes, - priv->libDir, priv->channelTargetDir))) goto cleanup; @@ -5652,7 +5669,6 @@ qemuProcessCreatePretendCmd(virConnectPtr conn, priv->autoNodeset, NULL, NULL, - priv->libDir, priv->channelTargetDir); cleanup: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.args new file mode 100644 index 0000000..cfa63b1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-vnc unix:/tmp/lib/domain--1-QEMUGuest1/vnc.sock \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.xml new file mode 100644 index 0000000..af961a5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc'/> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f8d0f56..7afe5c3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -903,6 +903,10 @@ mymain(void) VIR_FREE(driver.config->vncSASLdir); VIR_FREE(driver.config->vncTLSx509certdir); + driver.config->vncAutoUnixSocket = true; + DO_TEST("graphics-vnc-auto-unix-socket", QEMU_CAPS_VNC); + driver.config->vncAutoUnixSocket = false; + DO_TEST("graphics-sdl", QEMU_CAPS_SDL); DO_TEST("graphics-sdl-fullscreen", QEMU_CAPS_SDL); DO_TEST("nographics", NONE); -- 2.7.4

On 04/04/2016 09:20 AM, Pavel Hrdina wrote: A pre-existing bug, but we shouldn't be forcing usage of socket= here if the user already specified a listen address. So, check nListens == 0 first. But that's for a separate patch - Cole

If in qemu.conf vncAutoUnixSocket is enabled remove all other listens parsed from XML config. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 10 ++++++ ...-graphics-vnc-auto-unix-socket-with-listen.args | 22 +++++++++++++ ...v-graphics-vnc-auto-unix-socket-with-listen.xml | 36 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 69 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f38b0f3..65ab0f5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1418,6 +1418,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, virQEMUDriverPtr driver = opaque; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virQEMUCapsPtr qemuCaps = NULL; + size_t i; int ret = -1; if (def->os.bootloader || def->os.bootloaderArgs) { @@ -1446,6 +1447,15 @@ qemuDomainDefPostParse(virDomainDefPtr def, !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) return ret; + for (i = 0; i < def->ngraphics; i++) { + virDomainGraphicsDefPtr graphics = def->graphics[i]; + + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + if (cfg->vncAutoUnixSocket) + virDomainGraphicsListenClear(graphics); + } + } + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.args new file mode 100644 index 0000000..cfa63b1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-vnc unix:/tmp/lib/domain--1-QEMUGuest1/vnc.sock \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.xml new file mode 100644 index 0000000..af8442e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-auto-unix-socket-with-listen.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7afe5c3..475cb47 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -905,6 +905,7 @@ mymain(void) driver.config->vncAutoUnixSocket = true; DO_TEST("graphics-vnc-auto-unix-socket", QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-auto-unix-socket-with-listen", QEMU_CAPS_VNC); driver.config->vncAutoUnixSocket = false; DO_TEST("graphics-sdl", QEMU_CAPS_SDL); -- 2.7.4