[libvirt] [PATCHv3 1/2] conf: add <listen> subelement to domain <graphics> element

Once it's plugged in, the <listen element will be an optional replacement for the "listen", "port", "tlsPort", and "autoport" attributes that graphics elements already have. If the <listen> type='address', it will have an attribute called 'address' which will contain an IP address or dns name that the guest's display server should listen on. If, however, type='network', the <listen> element should have an attribute called 'network' that will be set to the name of a network configuration to get the IP address from. * docs/schemas/domain.rng: updated to allow the <listen> element * docs/formatdomain.html.in: document the <listen> element and its attributes. * src/conf/domain_conf.[hc]: 1) The domain parser, formatter, and data structure are modified to support 0 or more <listen> subelements to each <graphics> element. The old style "legacy" attributes are also still accepted, and will be stored internally just as if they were a separate <listen> element. On output (i.e. format), the first <listen> element will be duplicated in the legacy attributes of the <graphic> element. 2) The following attributes have been removed from the unions in virDomainGRaphicsDef for graphics types vnc, rdp, and spice: listen port tlsPort autoport These attributes are all present in the <listen> subelement (aka virDomainGraphicsListenDef) 3) Several helper functions were written to provide simple access (both Get and Set) to the listen elements. * src/libvirt_private.syms: export the listen helper functions * src/qemu/qemu_command.c, src/qemu/qemu_driver.c, src/qemu/qemu_hotplug.c, src/qemu/qemu_migration.c, src/qemu/qemu_process.c, src/vbox/vbox_tmpl.c, src/vmx/vmx.c, src/xenxs/xen_sxpr.c, src/xenxs/xen_xm.c Modify all these files to use the listen helper functions rather than directly referencing the (now missing) listen, port, tlsPort, and autoport attributes. There can be multiple <listen> elements to a single <graphics>, but the drivers all currently only support one, so all replacements of direct access with a helper function indicate index "0". * tests/* - only 3 of these are new files added explicitly to test the new <listen> element. All the others have been modified to reflect the fact that any "legacy" attributes passed in to the domain parse will be saved in a <listen> element (i.e. one of the virDomainGraphicsListenDefs), and during the domain format function, both the <listen> element as well as the legacy attributes will be output. --- docs/formatdomain.html.in | 70 ++ docs/schemas/domain.rng | 46 ++ src/conf/domain_conf.c | 681 ++++++++++++++++---- src/conf/domain_conf.h | 66 ++- src/libvirt_private.syms | 12 + src/qemu/qemu_command.c | 67 ++- src/qemu/qemu_driver.c | 7 +- src/qemu/qemu_hotplug.c | 32 +- src/qemu/qemu_migration.c | 10 +- src/qemu/qemu_process.c | 26 +- src/vbox/vbox_tmpl.c | 49 +- src/vmx/vmx.c | 37 +- src/xenxs/xen_sxpr.c | 52 +- src/xenxs/xen_xm.c | 79 ++- .../qemuxml2argv-graphics-listen-network.xml | 32 + .../qemuxml2argv-graphics-listen-network2.xml | 33 + .../qemuxml2argv-graphics-spice-compression.xml | 1 + .../qemuxml2argv-graphics-spice-qxl-vga.xml | 1 + .../qemuxml2argv-graphics-spice-timeout.xml | 4 +- .../qemuxml2argv-graphics-spice.xml | 1 + .../qemuxml2argv-graphics-vnc-sasl.xml | 4 +- .../qemuxml2argv-graphics-vnc-tls.xml | 4 +- .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 4 +- tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 4 +- .../qemuxml2xmlout-graphics-listen-network2.xml | 33 + tests/qemuxml2xmltest.c | 2 + tests/sexpr2xmldata/sexpr2xml-curmem.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 4 +- .../sexpr2xml-fv-serial-dev-2-ports.xml | 4 +- .../sexpr2xml-fv-serial-dev-2nd-port.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 4 +- .../sexpr2xml-fv-serial-tcp-telnet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-fv.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 4 +- .../sexpr2xml-pv-vfb-new-vncdisplay.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml | 4 +- tests/sexpr2xmldata/sexpr2xml-pv-vfb-orig.xml | 4 +- .../sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml | 4 +- tests/vmx2xmldata/vmx2xml-graphics-vnc.xml | 4 +- tests/xmconfigdata/test-escape-paths.xml | 4 +- tests/xmconfigdata/test-fullvirt-force-hpet.xml | 4 +- tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 4 +- tests/xmconfigdata/test-fullvirt-localtime.xml | 4 +- tests/xmconfigdata/test-fullvirt-net-ioemu.xml | 4 +- tests/xmconfigdata/test-fullvirt-net-netfront.xml | 4 +- tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 4 +- tests/xmconfigdata/test-fullvirt-old-cdrom.xml | 4 +- tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 4 +- .../test-fullvirt-serial-dev-2-ports.xml | 4 +- .../test-fullvirt-serial-dev-2nd-port.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-file.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-null.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-pty.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 4 +- .../test-fullvirt-serial-tcp-telnet.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-tcp.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-udp.xml | 4 +- tests/xmconfigdata/test-fullvirt-serial-unix.xml | 4 +- tests/xmconfigdata/test-fullvirt-sound.xml | 4 +- tests/xmconfigdata/test-fullvirt-usbmouse.xml | 4 +- .../test-fullvirt-usbtablet-no-bus.xml | 4 +- tests/xmconfigdata/test-fullvirt-usbtablet.xml | 4 +- tests/xmconfigdata/test-fullvirt-utc.xml | 4 +- tests/xmconfigdata/test-no-source-cdrom.xml | 4 +- tests/xmconfigdata/test-paravirt-net-e1000.xml | 4 +- tests/xmconfigdata/test-paravirt-net-vifname.xml | 4 +- .../test-paravirt-new-pvfb-vncdisplay.xml | 4 +- tests/xmconfigdata/test-paravirt-new-pvfb.xml | 4 +- .../test-paravirt-old-pvfb-vncdisplay.xml | 4 +- tests/xmconfigdata/test-paravirt-old-pvfb.xml | 4 +- tests/xmconfigdata/test-pci-devs.xml | 4 +- 91 files changed, 1274 insertions(+), 343 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f40afc4..2bfd72b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1965,6 +1965,12 @@ qemu-kvm -net nic,model=? /dev/null <graphics type='vnc' port='5904'/> <graphics type='rdp' autoport='yes' multiUser='yes' /> <graphics type='desktop' fullscreen='yes'/> + <graphics type='vnc'> + <listen type='address' address='1.2.3.4' autoport='yes'/> + </graphics> + <graphics type='spice'> + <listen type='network' network='rednet' port='5920' tlsPort='5980'/> + </graphics> </devices> ...</pre> @@ -2110,6 +2116,70 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <p> + Rather than putting the information used to setup the listening + socket for graphics types <code>vnc</code> and <code>spice</code> in + the <code>listen</code>, <code>port</code>, <code>tlsPort</code>, + and <code>autoport</code> attributes, a separate subelement + of <code><graphics></code>, + called <code><listen></code> can be specified (see the + examples above)<span class="since">since + 0.9.4</span>. <code><listen></code> accepts the following + attributes: + </p> + <dl> + <dt><code>type</code></dt> + <dd>Set to either <code>address</code> + or <code>network</code>. This tells whether this listen + element is specifying the address to be used directly, or by + naming a network (which will then be used to determine an + appropriate address for listening). + </dd> + </dl> + <dl> + <dt><code>address</code></dt> + <dd>if <code>type='address'</code>, the <code>address</code> + attribute will contain either an IP address or hostname (which + will be resolved to an IP address via a DNS query) to listen + on. In the "live" XML of a running domain, this attribute be + set to the IP address used for listening, even + if <code>type='network'</code>. + </dd> + </dl> + <dl> + <dt><code>network</code></dt> + <dd>if <code>type='network'</code>, the <code>network</code> + attribute will contain the name of a network in libvirt's list + of configured networks. The named network configuration will + be examined to determine an appropriate listen address. For + example, if the network has an IPv4 address in its + configuration (e.g. if it has a forward type + of <code>route</code>, <code>nat</code>, or no forward type + (isolated)), the first IPv4 address listed in the network's + configuration will be used. If the network is describing a + host bridge, the first IPv4 address associated with that + bridge device will be used, and if the network is describing + one of the 'direct' (macvtap) modes, the first IPv4 address of + the first forward dev will be used. + </dd> + </dl> + <dl> + <dt><code>port</code></dt> + <dd>The port to listen on.</dd> + </dl> + <dl> + <dt><code>tlsPort</code></dt> + <dd>The port to listen on for TLS connections (<code>graphics + type='spice'</code> only).</dd> + </dl> + <dl> + <dt><code>autoport</code></dt> + <dd>If set to 'yes', a listen port will be determined + automatically at runtime, and reflected in the domain's live + XML. + </dd> + </dl> + <h4><a name="elementsVideo">Video devices</a></h4> <p> A video device. diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index aa4ce69..b860c08 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1289,6 +1289,7 @@ </choice> </attribute> </optional> + <ref name="listenElements"/> </group> <group> <attribute name="type"> @@ -1342,6 +1343,7 @@ </attribute> </optional> <interleave> + <ref name="listenElements"/> <zeroOrMore> <element name="channel"> <attribute name="name"> @@ -1478,6 +1480,7 @@ <ref name="addrIPorName"/> </attribute> </optional> + <ref name="listenElements"/> </group> <group> <attribute name="type"> @@ -1500,6 +1503,49 @@ </choice> </element> </define> + + <define name="listenElements"> + <zeroOrMore> + <element name="listen"> + <optional> + <attribute name="type"> + <choice> + <value>address</value> + <value>network</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="address"> + <ref name="addrIPorName"/> + </attribute> + </optional> + <optional> + <attribute name="network"> + <text/> + </attribute> + </optional> + <optional> + <attribute name="port"> + <ref name="PortNumber"/> + </attribute> + </optional> + <optional> + <attribute name="tlsPort"> + <ref name="PortNumber"/> + </attribute> + </optional> + <optional> + <attribute name="autoport"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + </element> + </zeroOrMore> + </define> <!-- A video adapter description, allowing configuration of device model, number of virtual heads, and video ram size diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 072c970..579aa56 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -331,6 +331,11 @@ VIR_ENUM_IMPL(virDomainGraphics, VIR_DOMAIN_GRAPHICS_TYPE_LAST, "desktop", "spice") +VIR_ENUM_IMPL(virDomainGraphicsListen, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST, + "default", + "address", + "network") + VIR_ENUM_IMPL(virDomainGraphicsAuthConnected, VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST, "default", @@ -625,14 +630,26 @@ virDomainGraphicsAuthDefClear(virDomainGraphicsAuthDefPtr def) /* Don't free def */ } +static void +virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->address); + VIR_FREE(def->network); + return; +} + void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) { + int ii; + if (!def) return; switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - VIR_FREE(def->data.vnc.listenAddr); VIR_FREE(def->data.vnc.socket); VIR_FREE(def->data.vnc.keymap); virDomainGraphicsAuthDefClear(&def->data.vnc.auth); @@ -644,7 +661,6 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - VIR_FREE(def->data.rdp.listenAddr); break; case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: @@ -652,12 +668,15 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - VIR_FREE(def->data.spice.listenAddr); VIR_FREE(def->data.spice.keymap); virDomainGraphicsAuthDefClear(&def->data.spice.auth); break; } + for (ii = 0; ii < def->nListens; ii++) + virDomainGraphicsListenDefClear(&def->listens[ii]); + VIR_FREE(def->listens); + VIR_FREE(def); } @@ -4010,19 +4029,118 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node, return 0; } +static int +virDomainGraphicsListenParseXML(virDomainGraphicsListenDefPtr def, + xmlNodePtr node, + enum virDomainGraphicsType graphicsType, + unsigned int flags) +{ + int ret = -1; + char *type = virXMLPropString(node, "type"); + char *address = virXMLPropString(node, "address"); + char *network = virXMLPropString(node, "network"); + char *port = virXMLPropString(node, "port"); + char *tlsPort = virXMLPropString(node, "tlsPort"); + char *autoport = virXMLPropString(node, "autoport"); + + if (type && (def->type = virDomainGraphicsListenTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("unknown graphics listen type '%s'"), type); + goto error; + } + + if (address && address[0]) { + def->address = address; + address = NULL; + } + + if (network && network[0]) { + if (def->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("network attribute not allowed when listen type is not network")); + goto error; + } + def->network = network; + network = NULL; + } + + if (port) { + if (virStrToLong_i(port, NULL, 10, &def->port) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse listen port %s"), port); + goto error; + } + /* legacy syntax for graphics type=vnc used -1 for + * autoport. We need to maintain it here because the legacy + * attributes in <graphics> must match those in + * <listen>. */ + if ((graphicsType == VIR_DOMAIN_GRAPHICS_TYPE_VNC) && + (def->port == -1)) { + if (flags & VIR_DOMAIN_XML_INACTIVE) + def->port = 0; + def->autoport = true; + } + } else { + def->autoport = true; + } + + if (tlsPort && + (graphicsType == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + if (virStrToLong_i(tlsPort, NULL, 10, &def->tlsPort) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse spice tlsPort %s"), tlsPort); + goto error; + } + } + + if (autoport && STREQ(autoport, "yes")) { + if (flags & VIR_DOMAIN_XML_INACTIVE) { + def->port = 0; + def->tlsPort = 0; + } + def->autoport = true; + } + + ret = 0; +error: + if (ret < 0) + virDomainGraphicsListenDefClear(def); + VIR_FREE(type); + VIR_FREE(address); + VIR_FREE(network); + VIR_FREE(port); + VIR_FREE(tlsPort); + VIR_FREE(autoport); + return ret; +} + /* Parse the XML definition for a graphics device */ static virDomainGraphicsDefPtr -virDomainGraphicsDefParseXML(xmlNodePtr node, unsigned int flags) +virDomainGraphicsDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) { virDomainGraphicsDefPtr def; char *type = NULL; + int nListens; + xmlNodePtr *listenNodes = NULL; + char *port = NULL; + char *tlsPort = NULL; + char *autoport = NULL; + char *listenAddr = NULL; + int portVal = 0; + int tlsPortVal = 0; + bool autoportVal = false; + xmlNodePtr save = ctxt->node; if (VIR_ALLOC(def) < 0) { virReportOOMError(); return NULL; } + ctxt->node = node; + type = virXMLPropString(node, "type"); if (!type) { @@ -4037,46 +4155,136 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, unsigned int flags) goto error; } - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - char *port = virXMLPropString(node, "port"); - char *autoport; + if ((def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { - if (port) { - if (virStrToLong_i(port, NULL, 10, &def->data.vnc.port) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse vnc port %s"), port); - VIR_FREE(port); + /* parse the <listen> subelements for graphics types that support it */ + nListens = virXPathNodeSet("./listen", ctxt, &listenNodes); + if (nListens < 0) + goto error; + + if (nListens > 0) { + int ii; + + if (VIR_ALLOC_N(def->listens, nListens) < 0) { + virReportOOMError(); + goto error; + } + + for (ii = 0; ii < nListens; ii++) { + int ret = virDomainGraphicsListenParseXML(&def->listens[ii], + listenNodes[ii], + def->type, flags); + if (ret < 0) + goto error; + def->nListens++; + } + VIR_FREE(listenNodes); + } + + /* and now parse the "legacy" port/autoport/tlsPort/listen attributes */ + if ((port = virXMLPropString(node, "port"))) { + if (virStrToLong_i(port, NULL, 10, &portVal) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse graphics port %s"), port); goto error; } - VIR_FREE(port); - /* Legacy compat syntax, used -1 for auto-port */ - if (def->data.vnc.port == -1) { + + /* Legacy compat syntax, used -1 for autoport, but only when type=vnc */ + if ((def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) && + (portVal == -1)) { if (flags & VIR_DOMAIN_XML_INACTIVE) - def->data.vnc.port = 0; - def->data.vnc.autoport = 1; + portVal = 0; + autoportVal = true; } + } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + /* When port isn't specified for spice, for some + * reason it defaults to 5900, rather than turning on + * autoport like the others. */ + portVal = 5900; } else { - def->data.vnc.port = 0; - def->data.vnc.autoport = 1; + autoportVal = true; } - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { + /* tlsPort is only used for spice */ + if ((tlsPort = virXMLPropString(node, "tlsPort")) + && (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + if (virStrToLong_i(tlsPort, NULL, 10, &tlsPortVal) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse spice tlsPort %s"), tlsPort); + goto error; + } + } + + if ((autoport = virXMLPropString(node, "autoport"))) { if (STREQ(autoport, "yes")) { - if (flags & VIR_DOMAIN_XML_INACTIVE) - def->data.vnc.port = 0; - def->data.vnc.autoport = 1; + if (flags & VIR_DOMAIN_XML_INACTIVE) { + portVal = 0; + tlsPort = 0; + } + autoportVal = true;; } - VIR_FREE(autoport); } - def->data.vnc.listenAddr = virXMLPropString(node, "listen"); + listenAddr = virXMLPropString(node, "listen"); + if (listenAddr && !listenAddr[0]) + VIR_FREE(listenAddr); + + if (def->nListens == 0) { + /* There were no <listen> elements, so we can just + * directly set these attributes as listens[0] */ + if (portVal && (virDomainGraphicsListenSetPort(def, 0, portVal) < 0)) + goto error; + if ((def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) && + tlsPortVal && (virDomainGraphicsListenSetTlsPort(def, 0, tlsPortVal) < 0)) + goto error; + if (autoportVal && (virDomainGraphicsListenSetAutoport(def, 0, autoportVal) < 0)) + goto error; + if (listenAddr && (virDomainGraphicsListenSetAddress(def, 0, listenAddr, -1) < 0)) + goto error; + } else { + /* There's already a listens[0] element, so we just verify + * that it matches these legacy attributes from <graphics>. */ + if (port && (portVal != virDomainGraphicsListenGetPort(def, 0))) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("graphics port attribute %s must match port " + "attribute of first listen element (found %d)"), + port, virDomainGraphicsListenGetPort(def, 0)); + goto error; + } + if ((def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) && + tlsPort && (tlsPortVal != virDomainGraphicsListenGetTlsPort(def, 0))) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("graphics tlsPort attribute %s must match tlsPort " + "attribute of first listen element (found %d)"), + tlsPort, virDomainGraphicsListenGetTlsPort(def, 0)); + goto error; + } + if (autoport && (autoportVal != virDomainGraphicsListenGetAutoport(def, 0))) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("graphics autoport attribute %s must match autoport " + "attribute of first listen element (found %s)"), + autoport, + virDomainGraphicsListenGetAutoport(def, 0) ? "yes" : "no"); + goto error; + } + if (listenAddr && STRNEQ_NULLABLE(listenAddr, + virDomainGraphicsListenGetAddress(def, 0))) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("graphics listen attribute %s must match address " + "attribute of first listen element (found %s)"), + listenAddr, virDomainGraphicsListenGetAddress(def, 0)); + goto error; + } + } + } + + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + def->data.vnc.socket = virXMLPropString(node, "socket"); def->data.vnc.keymap = virXMLPropString(node, "keymap"); - if (def->data.vnc.listenAddr && - !def->data.vnc.listenAddr[0]) - VIR_FREE(def->data.vnc.listenAddr); - if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth, def->type) < 0) goto error; @@ -4100,33 +4308,9 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, unsigned int flags) 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; - if (port) { - if (virStrToLong_i(port, NULL, 10, &def->data.rdp.port) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse rdp port %s"), port); - VIR_FREE(port); - goto error; - } - VIR_FREE(port); - } else { - def->data.rdp.port = 0; - def->data.rdp.autoport = 1; - } - - if ((autoport = virXMLPropString(node, "autoport")) != NULL) { - if (STREQ(autoport, "yes")) { - if (flags & VIR_DOMAIN_XML_INACTIVE) - def->data.rdp.port = 0; - def->data.rdp.autoport = 1; - } - VIR_FREE(autoport); - } - if ((replaceUser = virXMLPropString(node, "replaceUser")) != NULL) { if (STREQ(replaceUser, "yes")) { def->data.rdp.replaceUser = 1; @@ -4141,11 +4325,6 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, unsigned int flags) VIR_FREE(multiUser); } - def->data.rdp.listenAddr = virXMLPropString(node, "listen"); - - if (def->data.rdp.listenAddr && - !def->data.rdp.listenAddr[0]) - VIR_FREE(def->data.rdp.listenAddr); } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP) { char *fullscreen = virXMLPropString(node, "fullscreen"); @@ -4167,53 +4346,9 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, unsigned int flags) 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; - if (port) { - if (virStrToLong_i(port, NULL, 10, &def->data.spice.port) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse spice port %s"), port); - VIR_FREE(port); - goto error; - } - VIR_FREE(port); - } else { - def->data.spice.port = 5900; - } - - tlsPort = virXMLPropString(node, "tlsPort"); - if (tlsPort) { - if (virStrToLong_i(tlsPort, NULL, 10, &def->data.spice.tlsPort) < 0) { - virDomainReportError(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")) { - if (flags & VIR_DOMAIN_XML_INACTIVE) { - def->data.spice.port = 0; - def->data.spice.tlsPort = 0; - } - def->data.spice.autoport = 1; - } - VIR_FREE(autoport); - } - - def->data.spice.listenAddr = virXMLPropString(node, "listen"); def->data.spice.keymap = virXMLPropString(node, "keymap"); - if (def->data.spice.listenAddr && - !def->data.spice.listenAddr[0]) - VIR_FREE(def->data.spice.listenAddr); - if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth, def->type) < 0) goto error; @@ -4387,7 +4522,13 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, unsigned int flags) cleanup: VIR_FREE(type); + VIR_FREE(listenNodes); + VIR_FREE(port); + VIR_FREE(tlsPort); + VIR_FREE(autoport); + VIR_FREE(listenAddr); + ctxt->node = save; return def; error: @@ -5256,7 +5397,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "graphics")) { dev->type = VIR_DOMAIN_DEVICE_GRAPHICS; - if (!(dev->data.graphics = virDomainGraphicsDefParseXML(node, flags))) + if (!(dev->data.graphics = virDomainGraphicsDefParseXML(node, ctxt, flags))) goto error; } else { virDomainReportError(VIR_ERR_XML_ERROR, @@ -6505,6 +6646,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { virDomainGraphicsDefPtr graphics = virDomainGraphicsDefParseXML(nodes[i], + ctxt, flags); if (!graphics) goto error; @@ -9411,12 +9553,61 @@ virDomainGraphicsAuthDefFormatAttr(virBufferPtr buf, virDomainGraphicsAuthConnectedTypeToString(def->connected)); } + +static void +virDomainGraphicsListenDefFormat(virBufferPtr buf, + virDomainGraphicsListenDefPtr def, + enum virDomainGraphicsType graphicsType, + unsigned int flags) +{ + virBufferAddLit(buf, " <listen"); + + if (def->type) { + virBufferAsprintf(buf, " type='%s'", + virDomainGraphicsListenTypeToString(def->type)); + } + + if (def->address && + ((def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) + || !(flags & VIR_DOMAIN_XML_INACTIVE))) { + /* address may also be set to show current status when type='network', + * but we don't want to print that if INACTIVE data is requested. */ + virBufferAsprintf(buf, " address='%s'", def->address); + } + + if (def->network && + (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) { + virBufferAsprintf(buf, " network='%s'", def->network); + } + + /* For VNC definitions, if the INACTIVE XML was requested and + * autoport is on, we don't print output the port. (Not sure why + * the same thing isn't done for RDP and SPICE, but the existing + * code that this is mimicking doesn't do it.) */ + if (def->port && + !((graphicsType == VIR_DOMAIN_GRAPHICS_TYPE_VNC) && + def->autoport && (flags & VIR_DOMAIN_XML_INACTIVE))) { + virBufferAsprintf(buf, " port='%d'", def->port); + } + if (def->tlsPort) + virBufferAsprintf(buf, " tlsPort='%d'", def->tlsPort); + if (def->autoport) + virBufferAddLit(buf, " autoport='yes'"); + + virBufferAddLit(buf, "/>\n"); +} + + static int virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsDefPtr def, unsigned int flags) { const char *type = virDomainGraphicsTypeToString(def->type); + int port = virDomainGraphicsListenGetPort(def, 0); + int tlsPort = virDomainGraphicsListenGetTlsPort(def, 0); + bool autoport = virDomainGraphicsListenGetAutoport(def, 0); + const char *listenAddr = virDomainGraphicsListenGetAddress(def, 0); int children = 0; int i; @@ -9435,19 +9626,15 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " socket='%s'", def->data.vnc.socket); } else { - if (def->data.vnc.port && - (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_XML_INACTIVE))) - virBufferAsprintf(buf, " port='%d'", - def->data.vnc.port); - else if (def->data.vnc.autoport) + if (port && !(autoport && (flags & VIR_DOMAIN_XML_INACTIVE))) + virBufferAsprintf(buf, " port='%d'", port); + else if (autoport) virBufferAddLit(buf, " port='-1'"); - virBufferAsprintf(buf, " autoport='%s'", - def->data.vnc.autoport ? "yes" : "no"); + virBufferAsprintf(buf, " autoport='%s'", autoport ? "yes" : "no"); - if (def->data.vnc.listenAddr) - virBufferAsprintf(buf, " listen='%s'", - def->data.vnc.listenAddr); + if (listenAddr) + virBufferAsprintf(buf, " listen='%s'", listenAddr); } if (def->data.vnc.keymap) @@ -9471,13 +9658,12 @@ virDomainGraphicsDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - if (def->data.rdp.port) - virBufferAsprintf(buf, " port='%d'", - def->data.rdp.port); - else if (def->data.rdp.autoport) + if (port) + virBufferAsprintf(buf, " port='%d'", port); + else if (autoport) virBufferAddLit(buf, " port='0'"); - if (def->data.rdp.autoport) + if (autoport) virBufferAsprintf(buf, " autoport='yes'"); if (def->data.rdp.replaceUser) @@ -9486,8 +9672,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (def->data.rdp.multiUser) virBufferAsprintf(buf, " multiUser='yes'"); - if (def->data.rdp.listenAddr) - virBufferAsprintf(buf, " listen='%s'", def->data.rdp.listenAddr); + if (listenAddr) + virBufferAsprintf(buf, " listen='%s'", listenAddr); break; @@ -9502,20 +9688,17 @@ virDomainGraphicsDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if (def->data.spice.port) - virBufferAsprintf(buf, " port='%d'", - def->data.spice.port); + if (port) + virBufferAsprintf(buf, " port='%d'", port); - if (def->data.spice.tlsPort) - virBufferAsprintf(buf, " tlsPort='%d'", - def->data.spice.tlsPort); + if (tlsPort) + virBufferAsprintf(buf, " tlsPort='%d'", tlsPort); virBufferAsprintf(buf, " autoport='%s'", - def->data.spice.autoport ? "yes" : "no"); + autoport ? "yes" : "no"); - if (def->data.spice.listenAddr) - virBufferAsprintf(buf, " listen='%s'", - def->data.spice.listenAddr); + if (listenAddr) + virBufferAsprintf(buf, " listen='%s'", listenAddr); if (def->data.spice.keymap) virBufferEscapeString(buf, " keymap='%s'", @@ -9526,6 +9709,17 @@ virDomainGraphicsDefFormat(virBufferPtr buf, } + if ((def->nListens > 0) && def->listens) { + int ii; + for (ii = 0; ii < def->nListens; ii++) { + if (!children) { + virBufferAddLit(buf, ">\n"); + children = 1; + } + virDomainGraphicsListenDefFormat(buf, &def->listens[ii], def->type, flags); + } + } + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { int mode = def->data.spice.channels[i]; @@ -11383,3 +11577,230 @@ virDomainNetGetActualDirectVirtPortProfile(virDomainNetDefPtr iface) return NULL; return iface->data.network.actual->data.direct.virtPortProfile; } + +/* Return listens[ii] from the appropriate union for the graphics + * type, or NULL if this is an unsuitable type, or the index is out of + * bounds. If force0 is TRUE, ii == 0, and there is no listen array, + * allocate one with a single item. */ +static virDomainGraphicsListenDefPtr +virDomainGraphicsGetListen(virDomainGraphicsDefPtr def, size_t ii, bool force0) +{ + if ((def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + + if (!def->listens && (ii == 0) && force0) { + if (VIR_ALLOC(def->listens) < 0) + virReportOOMError(); + else + def->nListens = 1; + } + + if (!def->listens || (def->nListens <= ii)) + return NULL; + + return &def->listens[ii]; + } + + /* it's a type that has no listens array */ + return NULL; +} + +/* Access functions for the fields in a virDomainGraphicsDef's + * "listens" array. + * + * NB: For simple backward compatibility with existing code, any of + * the "Set" functions will auto-create listens[0] to store the new + * setting, when necessary. Auto-creation beyond the first item is not + * supported. + * + * Return values: All "Get" functions return the requested item, or + * 0/NULL. (in the case of returned const char *, the caller should + * make a copy if they want to keep it around). All "Set" functions + * return 0 on success, -1 on failure. */ + +bool +virDomainGraphicsListenGetAutoport(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo) + return false; + return listenInfo->autoport; +} + + +int +virDomainGraphicsListenSetAutoport(virDomainGraphicsDefPtr def, size_t ii, bool val) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo) + return -1; + listenInfo->autoport = val; + return 0; +} + + +int +virDomainGraphicsListenGetPort(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo) + return 0; + return listenInfo->port; +} + + +int +virDomainGraphicsListenSetPort(virDomainGraphicsDefPtr def, size_t ii, int val) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo) + return -1; + listenInfo->port = val; + return 0; +} + + +int +virDomainGraphicsListenGetTlsPort(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo || (def->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) + return 0; + return listenInfo->tlsPort; +} + + +int +virDomainGraphicsListenSetTlsPort(virDomainGraphicsDefPtr def, size_t ii, int val) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo || (def->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) + return -1; + listenInfo->tlsPort = val; + return 0; +} + + +int +virDomainGraphicsListenGetType(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo) + return VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_DEFAULT; + return listenInfo->type; +} + + +/* NB: This function assumes type has not previously been set. It + * *will not* free any existing address or network based on a change + * in value of type. */ +int +virDomainGraphicsListenSetType(virDomainGraphicsDefPtr def, size_t ii, int val) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo) + return -1; + listenInfo->type = val; + return 0; +} + + +const char * +virDomainGraphicsListenGetAddress(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo || + (listenInfo->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS)) + return NULL; + return listenInfo->address; +} + + +/* Make a copy of up to len characters of address, and store it in + * listens[ii].address */ +int +virDomainGraphicsListenSetAddress(virDomainGraphicsDefPtr def, + size_t ii, const char *address, int len) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo) + return -1; + + listenInfo->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; + + if (!address) { + listenInfo->address = NULL; + return 0; + } + + listenInfo->address = (len == -1) ? strdup(address) : strndup(address, len); + if (!listenInfo->address) { + virReportOOMError(); + return -1; + } + + return 0; +} + + +const char * +virDomainGraphicsListenGetNetwork(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo || + (listenInfo->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) + return NULL; + return listenInfo->network; +} + + +/* Make a copy of up to len characters of address, and store it in + * listens[ii].network */ +int +virDomainGraphicsListenSetNetwork(virDomainGraphicsDefPtr def, + size_t ii, const char *network, int len) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo) + return -1; + + listenInfo->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK; + + if (!network) { + listenInfo->network = NULL; + return 0; + } + + listenInfo->network = (len == -1) ? strdup(network) : strndup(network, len); + if (!listenInfo->network) { + virReportOOMError(); + return -1; + } + + return 0; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 212f0c5..1f3f92e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -760,15 +760,31 @@ enum virDomainGraphicsSpiceClipboardCopypaste { VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_LAST }; +enum virDomainGraphicsListenType { + VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_DEFAULT = 0, + VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS, + VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK, + + VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST, +}; + +typedef struct _virDomainGraphicsListenDef virDomainGraphicsListenDef; +typedef virDomainGraphicsListenDef *virDomainGraphicsListenDefPtr; +struct _virDomainGraphicsListenDef { + int type; /* enum virDomainGraphicsListenType */ + char *address; + char *network; + int port; + int tlsPort; + bool autoport; +}; + typedef struct _virDomainGraphicsDef virDomainGraphicsDef; typedef virDomainGraphicsDef *virDomainGraphicsDefPtr; struct _virDomainGraphicsDef { int type; union { struct { - int port; - unsigned int autoport :1; - char *listenAddr; char *keymap; char *socket; virDomainGraphicsAuthDef auth; @@ -779,9 +795,6 @@ struct _virDomainGraphicsDef { int fullscreen; } sdl; struct { - int port; - char *listenAddr; - unsigned int autoport :1; unsigned int replaceUser :1; unsigned int multiUser :1; } rdp; @@ -790,12 +803,8 @@ struct _virDomainGraphicsDef { unsigned int fullscreen :1; } desktop; struct { - int port; - int tlsPort; - char *listenAddr; char *keymap; virDomainGraphicsAuthDef auth; - unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; int image; int jpeg; @@ -805,6 +814,11 @@ struct _virDomainGraphicsDef { int copypaste; } spice; } data; + /* nListens and listens are only useful if type is vnc, rdp, or + * spice. They've been extracted from the union only to simplify + * parsing code.*/ + size_t nListens; + virDomainGraphicsListenDefPtr listens; }; enum virDomainHostdevMode { @@ -1484,6 +1498,37 @@ int virDomainNetIndexByMac(virDomainDefPtr def, const unsigned char *mac); int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net); int virDomainNetRemoveByMac(virDomainDefPtr def, const unsigned char *mac); +bool virDomainGraphicsListenGetAutoport(virDomainGraphicsDefPtr def, size_t ii) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenSetAutoport(virDomainGraphicsDefPtr def, size_t ii, + bool val) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenGetPort(virDomainGraphicsDefPtr def, size_t ii) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenSetPort(virDomainGraphicsDefPtr def, size_t ii, int val) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenGetTlsPort(virDomainGraphicsDefPtr def, size_t ii) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenSetTlsPort(virDomainGraphicsDefPtr def, size_t ii, + int val) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenGetType(virDomainGraphicsDefPtr def, size_t ii) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenSetType(virDomainGraphicsDefPtr def, size_t ii, int val) + ATTRIBUTE_NONNULL(1); +const char *virDomainGraphicsListenGetAddress(virDomainGraphicsDefPtr def, + size_t ii) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenSetAddress(virDomainGraphicsDefPtr def, + size_t ii, const char *address, int len) + ATTRIBUTE_NONNULL(1); +const char *virDomainGraphicsListenGetNetwork(virDomainGraphicsDefPtr def, + size_t ii) + ATTRIBUTE_NONNULL(1); +int virDomainGraphicsListenSetNetwork(virDomainGraphicsDefPtr def, + size_t ii, const char *network, int len) + ATTRIBUTE_NONNULL(1); + int virDomainNetGetActualType(virDomainNetDefPtr iface); char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface); char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface); @@ -1644,6 +1689,7 @@ VIR_ENUM_DECL(virDomainHostdevSubsys) VIR_ENUM_DECL(virDomainInput) VIR_ENUM_DECL(virDomainInputBus) VIR_ENUM_DECL(virDomainGraphics) +VIR_ENUM_DECL(virDomainGraphicsListen) VIR_ENUM_DECL(virDomainGraphicsAuthConnected) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelName) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelMode) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 853ee62..25c34ec 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -299,6 +299,18 @@ virDomainGetRootFilesystem; virDomainGraphicsAuthConnectedTypeFromString; virDomainGraphicsAuthConnectedTypeToString; virDomainGraphicsDefFree; +virDomainGraphicsListenGetAddress; +virDomainGraphicsListenGetAutoport; +virDomainGraphicsListenGetNetwork; +virDomainGraphicsListenGetPort; +virDomainGraphicsListenGetTlsPort; +virDomainGraphicsListenGetType; +virDomainGraphicsListenSetAddress; +virDomainGraphicsListenSetAutoport; +virDomainGraphicsListenSetNetwork; +virDomainGraphicsListenSetPort; +virDomainGraphicsListenSetTlsPort; +virDomainGraphicsListenSetType; virDomainGraphicsSpiceChannelModeTypeFromString; virDomainGraphicsSpiceChannelModeTypeToString; virDomainGraphicsSpiceChannelNameTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f8fd4ee..4a39078 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4110,6 +4110,8 @@ qemuBuildCommandLine(virConnectPtr conn, if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + int port = virDomainGraphicsListenGetPort(def->graphics[0], 0); + const char *listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); virBuffer opt = VIR_BUFFER_INITIALIZER; if (def->graphics[0]->data.vnc.socket || @@ -4125,20 +4127,20 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->data.vnc.socket); } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { - const char *addr = def->graphics[0]->data.vnc.listenAddr ? - def->graphics[0]->data.vnc.listenAddr : - driver->vncListen; - bool escapeAddr = strchr(addr, ':') != NULL; + bool escapeAddr; + + if (!listenAddr) + listenAddr = driver->vncListen; + + escapeAddr = strchr(listenAddr, ':') != NULL; if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", addr); + virBufferAsprintf(&opt, "[%s]", listenAddr); else - virBufferAdd(&opt, addr, -1); - virBufferAsprintf(&opt, ":%d", - def->graphics[0]->data.vnc.port - 5900); + virBufferAdd(&opt, listenAddr, -1); + virBufferAsprintf(&opt, ":%d", port - 5900); } else { - virBufferAsprintf(&opt, "%d", - def->graphics[0]->data.vnc.port - 5900); + virBufferAsprintf(&opt, "%d", port - 5900); } if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { @@ -4219,6 +4221,9 @@ qemuBuildCommandLine(virConnectPtr conn, } else if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { virBuffer opt = VIR_BUFFER_INITIALIZER; + int port = virDomainGraphicsListenGetPort(def->graphics[0], 0); + int tlsPort = virDomainGraphicsListenGetTlsPort(def->graphics[0], 0); + const char *listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -4226,15 +4231,15 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port); + virBufferAsprintf(&opt, "port=%u", port); - if (driver->spiceTLS && def->graphics[0]->data.spice.tlsPort != -1) - virBufferAsprintf(&opt, ",tls-port=%u", def->graphics[0]->data.spice.tlsPort); + if (driver->spiceTLS && tlsPort != -1) + virBufferAsprintf(&opt, ",tls-port=%u", tlsPort); - if (def->graphics[0]->data.spice.listenAddr) - virBufferAsprintf(&opt, ",addr=%s", def->graphics[0]->data.spice.listenAddr); - else if (driver->spiceListen) - virBufferAsprintf(&opt, ",addr=%s", driver->spiceListen); + if (!listenAddr) + listenAddr = driver->spiceListen; + if (listenAddr) + virBufferAsprintf(&opt, ",addr=%s", listenAddr); /* In the password case we set it via monitor command, to avoid * making it visible on CLI, so there's no use of password=XXX @@ -5976,7 +5981,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, /* -vnc unix:/some/big/path */ vnc->data.vnc.socket = strdup(val + 5); if (!vnc->data.vnc.socket) { - VIR_FREE(vnc); + virDomainGraphicsDefFree(vnc); goto no_memory; } } else { @@ -5985,34 +5990,40 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, * -vnc [2001:1:2:3:4:5:1234:1234]:4 * -vnc some.host.name:4 */ + int port; char *opts; const char *sep = ":"; + if (val[0] == '[') sep = "]:"; tmp = strstr(val, sep); if (!tmp) { - VIR_FREE(vnc); + virDomainGraphicsDefFree(vnc); qemuReportError(VIR_ERR_INTERNAL_ERROR, _("missing VNC port number in '%s'"), val); goto error; } - if (virStrToLong_i(tmp+strlen(sep), &opts, 10, - &vnc->data.vnc.port) < 0) { - VIR_FREE(vnc); + if (virStrToLong_i(tmp+strlen(sep), &opts, 10, &port) < 0) { + virDomainGraphicsDefFree(vnc); qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse VNC port '%s'"), tmp+1); goto error; } + if (virDomainGraphicsListenSetPort(vnc, 0, port + 5900) < 0) { + virDomainGraphicsDefFree(vnc); + goto error; + } + if (val[0] == '[') - vnc->data.vnc.listenAddr = strndup(val+1, tmp-(val+1)); + virDomainGraphicsListenSetAddress(vnc, 0, + val+1, tmp-(val+1)); else - vnc->data.vnc.listenAddr = strndup(val, tmp-val); - if (!vnc->data.vnc.listenAddr) { - VIR_FREE(vnc); + virDomainGraphicsListenSetAddress(vnc, 0, + val, tmp-val); + if (!virDomainGraphicsListenGetAddress(vnc, 0)) { + virDomainGraphicsDefFree(vnc); goto no_memory; } - vnc->data.vnc.port += 5900; - vnc->data.vnc.autoport = 0; } if (VIR_REALLOC_N(def->graphics, def->ngraphics+1) < 0) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5df58b1..a6f7989 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4209,8 +4209,11 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, } for (i = 0 ; i < def->ngraphics ; i++) { if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - def->graphics[i]->data.vnc.autoport) - def->graphics[i]->data.vnc.port = QEMU_VNC_PORT_MIN; + virDomainGraphicsListenGetAutoport(def->graphics[i], 0) && + (virDomainGraphicsListenSetPort(def->graphics[i], 0, + QEMU_VNC_PORT_MIN) < 0)) { + goto cleanup; + } } if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9746cb3..18b5164 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1052,6 +1052,9 @@ qemuDomainChangeGraphics(struct qemud_driver *driver, virDomainGraphicsDefPtr dev) { virDomainGraphicsDefPtr olddev = qemuDomainFindGraphics(vm, dev); + int oldPort, newPort, oldTlsPort, newTlsPort; + bool oldAutoport, newAutoport; + const char *oldListenAddr, *newListenAddr; int ret = -1; if (!olddev) { @@ -1060,17 +1063,25 @@ qemuDomainChangeGraphics(struct qemud_driver *driver, return -1; } + oldAutoport = virDomainGraphicsListenGetAutoport(olddev, 0); + newAutoport = virDomainGraphicsListenGetAutoport(dev, 0); + oldPort = virDomainGraphicsListenGetPort(olddev, 0); + newPort = virDomainGraphicsListenGetPort(dev, 0); + oldTlsPort = virDomainGraphicsListenGetTlsPort(olddev, 0); + newTlsPort = virDomainGraphicsListenGetTlsPort(dev, 0); + oldListenAddr = virDomainGraphicsListenGetAddress(olddev, 0); + newListenAddr = virDomainGraphicsListenGetAddress(dev, 0); + switch (dev->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - if ((olddev->data.vnc.autoport != dev->data.vnc.autoport) || - (!dev->data.vnc.autoport && - (olddev->data.vnc.port != dev->data.vnc.port))) { + + if ((oldAutoport != newAutoport) || + (!newAutoport && (oldPort != newPort))) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change port settings on vnc graphics")); return -1; } - if (STRNEQ_NULLABLE(olddev->data.vnc.listenAddr, - dev->data.vnc.listenAddr)) { + if (STRNEQ_NULLABLE(oldListenAddr,newListenAddr)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change listen address setting on vnc graphics")); return -1; @@ -1109,17 +1120,14 @@ qemuDomainChangeGraphics(struct qemud_driver *driver, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - if ((olddev->data.spice.autoport != dev->data.spice.autoport) || - (!dev->data.spice.autoport && - (olddev->data.spice.port != dev->data.spice.port)) || - (!dev->data.spice.autoport && - (olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) { + if ((oldAutoport != newAutoport) || + (!newAutoport && (oldPort != newPort)) || + (!newAutoport && (oldTlsPort != newTlsPort))) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change port settings on spice graphics")); return -1; } - if (STRNEQ_NULLABLE(olddev->data.spice.listenAddr, - dev->data.spice.listenAddr)) { + if (STRNEQ_NULLABLE(oldListenAddr, newListenAddr)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change listen address setting on spice graphics")); return -1; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 9f6b372..679a957 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -196,8 +196,8 @@ qemuMigrationCookieGraphicsAlloc(struct qemud_driver *driver, mig->type = def->type; if (mig->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - mig->port = def->data.vnc.port; - listenAddr = def->data.vnc.listenAddr; + mig->port = virDomainGraphicsListenGetPort(def, 0); + listenAddr = virDomainGraphicsListenGetAddress(def, 0); if (!listenAddr) listenAddr = driver->vncListen; @@ -205,12 +205,12 @@ qemuMigrationCookieGraphicsAlloc(struct qemud_driver *driver, !(mig->tlsSubject = qemuDomainExtractTLSSubject(driver->vncTLSx509certdir))) goto error; } else { - mig->port = def->data.spice.port; + mig->port = virDomainGraphicsListenGetPort(def, 0); if (driver->spiceTLS) - mig->tlsPort = def->data.spice.tlsPort; + mig->tlsPort = virDomainGraphicsListenGetTlsPort(def, 0); else mig->tlsPort = -1; - listenAddr = def->data.spice.listenAddr; + listenAddr = virDomainGraphicsListenGetAddress(def, 0); if (!listenAddr) listenAddr = driver->spiceListen; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 646215e..29aba3a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2568,16 +2568,17 @@ int qemuProcessStart(virConnectPtr conn, if (vm->def->ngraphics == 1) { if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && !vm->def->graphics[0]->data.vnc.socket && - vm->def->graphics[0]->data.vnc.autoport) { + virDomainGraphicsListenGetAutoport(vm->def->graphics[0], 0)) { int port = qemuProcessNextFreePort(driver, QEMU_VNC_PORT_MIN); if (port < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused VNC port")); goto cleanup; } - vm->def->graphics[0]->data.vnc.port = port; + if (virDomainGraphicsListenSetPort(vm->def->graphics[0], 0, port) < 0) + goto cleanup; } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - vm->def->graphics[0]->data.spice.autoport) { + virDomainGraphicsListenGetAutoport(vm->def->graphics[0], 0)) { int port = qemuProcessNextFreePort(driver, QEMU_VNC_PORT_MIN); int tlsPort = -1; if (port < 0) { @@ -2596,8 +2597,10 @@ int qemuProcessStart(virConnectPtr conn, } } - vm->def->graphics[0]->data.spice.port = port; - vm->def->graphics[0]->data.spice.tlsPort = tlsPort; + if (virDomainGraphicsListenSetPort(vm->def->graphics[0], 0, port) < 0) + goto cleanup; + if (virDomainGraphicsListenSetTlsPort(vm->def->graphics[0], 0, tlsPort) < 0) + goto cleanup; } } @@ -3098,14 +3101,17 @@ retry: */ if ((vm->def->ngraphics == 1) && vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.vnc.port); + virDomainGraphicsListenGetAutoport(vm->def->graphics[0], 0)) { + qemuProcessReturnPort(driver, + virDomainGraphicsListenGetPort(vm->def->graphics[0], 0)); } if ((vm->def->ngraphics == 1) && vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - vm->def->graphics[0]->data.spice.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.port); - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.tlsPort); + virDomainGraphicsListenGetAutoport(vm->def->graphics[0], 0)) { + qemuProcessReturnPort(driver, + virDomainGraphicsListenGetPort(vm->def->graphics[0], 0)); + qemuProcessReturnPort(driver, + virDomainGraphicsListenGetTlsPort(vm->def->graphics[0], 0)); } vm->taint = 0; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index ee0720a..553b28f 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2461,17 +2461,27 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { char *netAddressUtf8 = NULL; PRBool allowMultiConnection = PR_FALSE; PRBool reuseSingleConnection = PR_FALSE; + + /* We must set the graphics type before + * calling any of the + * virDomainGraphicsListen functions, as + * they return NULL for types that don't + * allow setting up listen addresses/ports. */ + def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP; + #if VBOX_API_VERSION < 3001 PRUint32 VRDPport = 0; VRDxServer->vtbl->GetPort(VRDxServer, &VRDPport); if (VRDPport) { - def->graphics[def->ngraphics]->data.rdp.port = VRDPport; + virDomainGraphicsListenSetPort(def->graphics[def->ngraphics], + 0, VRDPport); #elif VBOX_API_VERSION < 4000 /* 3001 <= VBOX_API_VERSION < 4000 */ PRUnichar *VRDPport = NULL; VRDxServer->vtbl->GetPorts(VRDxServer, &VRDPport); if (VRDPport) { /* even if vbox supports mutilpe ports, single port for now here */ - def->graphics[def->ngraphics]->data.rdp.port = PRUnicharToInt(VRDPport); + virDomainGraphicsListenSetPort(def->graphics[def->ngraphics], 0, + PRUnicharToInt(VRDPport)); VBOX_UTF16_FREE(VRDPport); #else /* VBOX_API_VERSION >= 4000 */ PRUnichar *VRDEPortsKey = NULL; @@ -2481,15 +2491,14 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { VBOX_UTF16_FREE(VRDEPortsKey); if (VRDEPortsValue) { /* even if vbox supports mutilpe ports, single port for now here */ - def->graphics[def->ngraphics]->data.rdp.port = PRUnicharToInt(VRDEPortsValue); + virDomainGraphicsListenSetPort(def->graphics[def->ngraphics], 0, + PRUnicharToInt(VRDEPortsValue)); VBOX_UTF16_FREE(VRDEPortsValue); #endif /* VBOX_API_VERSION >= 4000 */ } else { - def->graphics[def->ngraphics]->data.rdp.autoport = 1; + virDomainGraphicsListenSetAutoport(def->graphics[def->ngraphics], 0, true); } - def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP; - #if VBOX_API_VERSION >= 4000 PRUnichar *VRDENetAddressKey = NULL; VBOX_UTF8_TO_UTF16("TCP/Address", &VRDENetAddressKey); @@ -2501,7 +2510,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (netAddressUtf16) { VBOX_UTF16_TO_UTF8(netAddressUtf16, &netAddressUtf8); if (STRNEQ(netAddressUtf8, "")) - def->graphics[def->ngraphics]->data.rdp.listenAddr = strdup(netAddressUtf8); + virDomainGraphicsListenSetAddress(def->graphics[def->ngraphics], 0, + netAddressUtf8, -1); VBOX_UTF16_FREE(netAddressUtf16); VBOX_UTF8_FREE(netAddressUtf8); } @@ -4532,16 +4542,18 @@ vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) machine->vtbl->GetVRDEServer(machine, &VRDxServer); #endif /* VBOX_API_VERSION >= 4000 */ if (VRDxServer) { + int port = virDomainGraphicsListenGetPort(def->graphics[i], 0); + const char *listenAddr + = virDomainGraphicsListenGetAddress(def->graphics[i], 0); + VRDxServer->vtbl->SetEnabled(VRDxServer, PR_TRUE); VIR_DEBUG("VRDP Support turned ON."); #if VBOX_API_VERSION < 3001 - if (def->graphics[i]->data.rdp.port) { - VRDxServer->vtbl->SetPort(VRDxServer, - def->graphics[i]->data.rdp.port); - VIR_DEBUG("VRDP Port changed to: %d", - def->graphics[i]->data.rdp.port); - } else if (def->graphics[i]->data.rdp.autoport) { + if (port) { + VRDxServer->vtbl->SetPort(VRDxServer, port); + VIR_DEBUG("VRDP Port changed to: %d", port); + } else if (virDomainGraphicsListenGetAutoport(def->graphics[i], 0)) { /* Setting the port to 0 will reset its value to * the default one which is 3389 currently */ @@ -4550,14 +4562,14 @@ vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } #elif VBOX_API_VERSION < 4000 /* 3001 <= VBOX_API_VERSION < 4000 */ PRUnichar *portUtf16 = NULL; - portUtf16 = PRUnicharFromInt(def->graphics[i]->data.rdp.port); + portUtf16 = PRUnicharFromInt(port); VRDxServer->vtbl->SetPorts(VRDxServer, portUtf16); VBOX_UTF16_FREE(portUtf16); #else /* VBOX_API_VERSION >= 4000 */ PRUnichar *VRDEPortsKey = NULL; PRUnichar *VRDEPortsValue = NULL; VBOX_UTF8_TO_UTF16("TCP/Ports", &VRDEPortsKey); - VRDEPortsValue = PRUnicharFromInt(def->graphics[i]->data.rdp.port); + VRDEPortsValue = PRUnicharFromInt(port); VRDxServer->vtbl->SetVRDEProperty(VRDxServer, VRDEPortsKey, VRDEPortsValue); VBOX_UTF16_FREE(VRDEPortsKey); @@ -4576,14 +4588,13 @@ vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VIR_DEBUG("VRDP set to allow multiple connection"); } - if (def->graphics[i]->data.rdp.listenAddr) { + if (listenAddr) { #if VBOX_API_VERSION >= 4000 PRUnichar *netAddressKey = NULL; #endif PRUnichar *netAddressUtf16 = NULL; - VBOX_UTF8_TO_UTF16(def->graphics[i]->data.rdp.listenAddr, - &netAddressUtf16); + VBOX_UTF8_TO_UTF16(listenAddr, &netAddressUtf16); #if VBOX_API_VERSION < 4000 VRDxServer->vtbl->SetNetAddress(VRDxServer, netAddressUtf16); @@ -4594,7 +4605,7 @@ vboxAttachDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VBOX_UTF16_FREE(netAddressKey); #endif /* VBOX_API_VERSION >= 4000 */ VIR_DEBUG("VRDP listen address is set to: %s", - def->graphics[i]->data.rdp.listenAddr); + listenAddr); VBOX_UTF16_FREE(netAddressUtf16); } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index b37f03b..1ed41c8 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1755,6 +1755,7 @@ virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def) { bool enabled = false; long long port = 0; + char *listenAddr = NULL; if (def == NULL || *def != NULL) { VMX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -1780,7 +1781,7 @@ virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def) if (virVMXGetConfigLong(conf, "RemoteDisplay.vnc.port", &port, -1, true) < 0 || virVMXGetConfigString(conf, "RemoteDisplay.vnc.ip", - &(*def)->data.vnc.listenAddr, true) < 0 || + &listenAddr, true) < 0 || virVMXGetConfigString(conf, "RemoteDisplay.vnc.keymap", &(*def)->data.vnc.keymap, true) < 0 || virVMXGetConfigString(conf, "RemoteDisplay.vnc.password", @@ -1788,24 +1789,33 @@ virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def) goto failure; } + if (listenAddr) { + if (virDomainGraphicsListenSetAddress(*def, 0, listenAddr, -1) < 0) + goto failure; + VIR_FREE(listenAddr); + } + if (port < 0) { VIR_WARN("VNC is enabled but VMX entry 'RemoteDisplay.vnc.port' " "is missing, the VNC port is unknown"); - (*def)->data.vnc.port = 0; - (*def)->data.vnc.autoport = 1; + /* port is already defaulted to 0, no need to set it */ + if (virDomainGraphicsListenSetAutoport(*def, 0, true) < 0) + goto failure; } else { if (port < 5900 || port > 5964) { VIR_WARN("VNC port %lld it out of [5900..5964] range", port); } - (*def)->data.vnc.port = port; - (*def)->data.vnc.autoport = 0; + /* autoport is already defaulted to false, no need to set it */ + if (virDomainGraphicsListenSetPort(*def, 0, port) < 0) + goto failure; } return 0; failure: + VIR_FREE(listenAddr); virDomainGraphicsDefFree(*def); *def = NULL; @@ -3196,6 +3206,7 @@ virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, int virVMXFormatVNC(virDomainGraphicsDefPtr def, virBufferPtr buffer) { + const char *listenAddr; if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { VMX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; @@ -3203,22 +3214,20 @@ virVMXFormatVNC(virDomainGraphicsDefPtr def, virBufferPtr buffer) virBufferAsprintf(buffer, "RemoteDisplay.vnc.enabled = \"true\"\n"); - if (def->data.vnc.autoport) { + if (virDomainGraphicsListenGetAutoport(def, 0)) { VIR_WARN("VNC autoport is enabled, but the automatically assigned " "VNC port cannot be read back"); } else { - if (def->data.vnc.port < 5900 || def->data.vnc.port > 5964) { - VIR_WARN("VNC port %d it out of [5900..5964] range", - def->data.vnc.port); - } + int port = virDomainGraphicsListenGetPort(def, 0); + if (port < 5900 || port > 5964) + VIR_WARN("VNC port %d it out of [5900..5964] range", port); - virBufferAsprintf(buffer, "RemoteDisplay.vnc.port = \"%d\"\n", - def->data.vnc.port); + virBufferAsprintf(buffer, "RemoteDisplay.vnc.port = \"%d\"\n", port); } - if (def->data.vnc.listenAddr != NULL) { + if ((listenAddr = virDomainGraphicsListenGetAddress(def, 0))) { virBufferAsprintf(buffer, "RemoteDisplay.vnc.ip = \"%s\"\n", - def->data.vnc.listenAddr); + listenAddr); } if (def->data.vnc.keymap != NULL) { diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 4245a64..610119a 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -745,13 +745,17 @@ xenParseSxprGraphicsOld(virDomainDefPtr def, if (port == -1 && xendConfigVersion < 2) port = 5900 + def->id; - if ((unused && STREQ(unused, "1")) || port == -1) - graphics->data.vnc.autoport = 1; - graphics->data.vnc.port = port; + if (((unused && STREQ(unused, "1")) || port == -1) && + (virDomainGraphicsListenSetAutoport(graphics, 0, true) < 0)) { + goto error; + } + + if (virDomainGraphicsListenSetPort(graphics, 0, port) < 0) + goto error; if (listenAddr && - !(graphics->data.vnc.listenAddr = strdup(listenAddr))) - goto no_memory; + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, -1)) + goto error; if (vncPasswd && !(graphics->data.vnc.auth.passwd = strdup(vncPasswd))) @@ -794,6 +798,7 @@ xenParseSxprGraphicsOld(virDomainDefPtr def, no_memory: virReportOOMError(); +error: virDomainGraphicsDefFree(graphics); return -1; } @@ -858,16 +863,19 @@ xenParseSxprGraphicsNew(virDomainDefPtr def, port = val; } - if ((unused && STREQ(unused, "1")) || port == -1) - graphics->data.vnc.autoport = 1; + if (((unused && STREQ(unused, "1")) || port == -1) && + (virDomainGraphicsListenSetAutoport(graphics, 0, true) < 0)) { + goto error; + } if (port >= 0 && port < 5900) port += 5900; - graphics->data.vnc.port = port; + if (virDomainGraphicsListenSetPort(graphics, 0, port) < 0) + goto error; if (listenAddr && - !(graphics->data.vnc.listenAddr = strdup(listenAddr))) - goto no_memory; + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, -1)) + goto error; if (vncPasswd && !(graphics->data.vnc.auth.passwd = strdup(vncPasswd))) @@ -1437,6 +1445,8 @@ static int xenFormatSxprGraphicsNew(virDomainGraphicsDefPtr def, virBufferPtr buf) { + const char *listenAddr; + if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_SDL && def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { XENXS_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -1456,15 +1466,17 @@ xenFormatSxprGraphicsNew(virDomainGraphicsDefPtr def, virBufferAsprintf(buf, "(xauthority '%s')", def->data.sdl.xauth); } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virBufferAddLit(buf, "(type vnc)"); - if (def->data.vnc.autoport) { + if (virDomainGraphicsListenGetAutoport(def, 0)) { virBufferAddLit(buf, "(vncunused 1)"); } else { virBufferAddLit(buf, "(vncunused 0)"); - virBufferAsprintf(buf, "(vncdisplay %d)", def->data.vnc.port-5900); + virBufferAsprintf(buf, "(vncdisplay %d)", + virDomainGraphicsListenGetPort(def, 0) - 5900); } - if (def->data.vnc.listenAddr) - virBufferAsprintf(buf, "(vnclisten '%s')", def->data.vnc.listenAddr); + listenAddr = virDomainGraphicsListenGetAddress(def, 0); + if (listenAddr) + virBufferAsprintf(buf, "(vnclisten '%s')", listenAddr); if (def->data.vnc.auth.passwd) virBufferAsprintf(buf, "(vncpasswd '%s')", def->data.vnc.auth.passwd); if (def->data.vnc.keymap) @@ -1482,6 +1494,8 @@ xenFormatSxprGraphicsOld(virDomainGraphicsDefPtr def, virBufferPtr buf, int xendConfigVersion) { + const char *listenAddr; + if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_SDL && def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { XENXS_ERROR(VIR_ERR_INTERNAL_ERROR, @@ -1499,15 +1513,17 @@ xenFormatSxprGraphicsOld(virDomainGraphicsDefPtr def, } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { virBufferAddLit(buf, "(vnc 1)"); if (xendConfigVersion >= 2) { - if (def->data.vnc.autoport) { + if (virDomainGraphicsListenGetAutoport(def, 0)) { virBufferAddLit(buf, "(vncunused 1)"); } else { virBufferAddLit(buf, "(vncunused 0)"); - virBufferAsprintf(buf, "(vncdisplay %d)", def->data.vnc.port-5900); + virBufferAsprintf(buf, "(vncdisplay %d)", + virDomainGraphicsListenGetPort(def, 0) - 5900); } - if (def->data.vnc.listenAddr) - virBufferAsprintf(buf, "(vnclisten '%s')", def->data.vnc.listenAddr); + listenAddr = virDomainGraphicsListenGetAddress(def, 0); + if (listenAddr) + virBufferAsprintf(buf, "(vnclisten '%s')", listenAddr); if (def->data.vnc.auth.passwd) virBufferAsprintf(buf, "(vncpasswd '%s')", def->data.vnc.auth.passwd); if (def->data.vnc.keymap) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 0ad2179..0951647 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -213,6 +213,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, int vmlocaltime = 0; unsigned long count; char *script = NULL; + char *listenAddr = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -852,16 +853,27 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_VNC; if (xenXMConfigGetBool(conf, "vncunused", &val, 1) < 0) goto cleanup; - graphics->data.vnc.autoport = val ? 1 : 0; + if (virDomainGraphicsListenSetAutoport(graphics, 0, val) < 0) + goto cleanup; - if (!graphics->data.vnc.autoport) { + if (!virDomainGraphicsListenGetAutoport(graphics, 0)) { unsigned long vncdisplay; if (xenXMConfigGetULong(conf, "vncdisplay", &vncdisplay, 0) < 0) goto cleanup; - graphics->data.vnc.port = (int)vncdisplay + 5900; + if (virDomainGraphicsListenSetPort(graphics, 0, + (int)vncdisplay + 5900) < 0) + goto cleanup; } - if (xenXMConfigCopyStringOpt(conf, "vnclisten", &graphics->data.vnc.listenAddr) < 0) + + if (xenXMConfigCopyStringOpt(conf, "vnclisten", &listenAddr) < 0) goto cleanup; + if (listenAddr && + virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1) < 0) { + goto cleanup; + } + VIR_FREE(listenAddr); + if (xenXMConfigCopyStringOpt(conf, "vncpasswd", &graphics->data.vnc.auth.passwd) < 0) goto cleanup; if (xenXMConfigCopyStringOpt(conf, "keymap", &graphics->data.vnc.keymap) < 0) @@ -928,11 +940,15 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { if (STRPREFIX(key, "vncunused=")) { - if (STREQ(key + 10, "1")) - graphics->data.vnc.autoport = 1; + if (STREQ(key + 10, "1") && + (virDomainGraphicsListenSetAutoport(graphics, + 0, true) < 0)) { + goto cleanup; + } } else if (STRPREFIX(key, "vnclisten=")) { - if (!(graphics->data.vnc.listenAddr = strdup(key + 10))) - goto no_memory; + if (virDomainGraphicsListenSetAddress(graphics, 0, + key+10, -1) < 0) + goto cleanup; } else if (STRPREFIX(key, "vncpasswd=")) { if (!(graphics->data.vnc.auth.passwd = strdup(key + 10))) goto no_memory; @@ -940,7 +956,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (!(graphics->data.vnc.keymap = strdup(key + 7))) goto no_memory; } else if (STRPREFIX(key, "vncdisplay=")) { - graphics->data.vnc.port = strtol(key+11, NULL, 10) + 5900; + int port = strtol(key+11, NULL, 10) + 5900; + if (virDomainGraphicsListenSetPort(graphics, 0, + port) < 0) + goto cleanup; } } else { if (STRPREFIX(key, "display=")) { @@ -1067,6 +1086,7 @@ cleanup: virDomainDiskDefFree(disk); virDomainDefFree(def); VIR_FREE(script); + VIR_FREE(listenAddr); return NULL; } @@ -1646,21 +1666,29 @@ virConfPtr xenFormatXM(virConnectPtr conn, def->graphics[0]->data.sdl.xauth) < 0) goto no_memory; } else { + int autoport; + int port; + const char *listenAddr; + if (xenXMConfigSetInt(conf, "sdl", 0) < 0) goto no_memory; if (xenXMConfigSetInt(conf, "vnc", 1) < 0) goto no_memory; - if (xenXMConfigSetInt(conf, "vncunused", - def->graphics[0]->data.vnc.autoport ? 1 : 0) < 0) + + autoport = virDomainGraphicsListenGetAutoport(def->graphics[0], 0) ? 1 : 0; + if (xenXMConfigSetInt(conf, "vncunused", autoport) < 0) goto no_memory; - if (!def->graphics[0]->data.vnc.autoport && - xenXMConfigSetInt(conf, "vncdisplay", - def->graphics[0]->data.vnc.port - 5900) < 0) + + port = virDomainGraphicsListenGetPort(def->graphics[0], 0); + if (!autoport && + xenXMConfigSetInt(conf, "vncdisplay", port - 5900) < 0) goto no_memory; - if (def->graphics[0]->data.vnc.listenAddr && - xenXMConfigSetString(conf, "vnclisten", - def->graphics[0]->data.vnc.listenAddr) < 0) + + listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + if (listenAddr && + xenXMConfigSetString(conf, "vnclisten", listenAddr) < 0) goto no_memory; + if (def->graphics[0]->data.vnc.auth.passwd && xenXMConfigSetString(conf, "vncpasswd", def->graphics[0]->data.vnc.auth.passwd) < 0) @@ -1683,15 +1711,16 @@ virConfPtr xenFormatXM(virConnectPtr conn, virBufferAsprintf(&buf, ",xauthority=%s", def->graphics[0]->data.sdl.xauth); } else { + bool autoport = virDomainGraphicsListenGetAutoport(def->graphics[0], 0); + int port = virDomainGraphicsListenGetPort(def->graphics[0], 0); + const char *listenAddr + = virDomainGraphicsListenGetAddress(def->graphics[0], 0); virBufferAddLit(&buf, "type=vnc"); - virBufferAsprintf(&buf, ",vncunused=%d", - def->graphics[0]->data.vnc.autoport ? 1 : 0); - if (!def->graphics[0]->data.vnc.autoport) - virBufferAsprintf(&buf, ",vncdisplay=%d", - def->graphics[0]->data.vnc.port - 5900); - if (def->graphics[0]->data.vnc.listenAddr) - virBufferAsprintf(&buf, ",vnclisten=%s", - def->graphics[0]->data.vnc.listenAddr); + virBufferAsprintf(&buf, ",vncunused=%d", autoport ? 1 : 0); + if (!autoport) + virBufferAsprintf(&buf, ",vncdisplay=%d", port - 5900); + if (listenAddr) + virBufferAsprintf(&buf, ",vnclisten=%s", listenAddr); if (def->graphics[0]->data.vnc.auth.passwd) virBufferAsprintf(&buf, ",vncpasswd=%s", def->graphics[0]->data.vnc.auth.passwd); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml new file mode 100644 index 0000000..0d17fde --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='5903' autoport='no'> + <listen type='network' network='Bobsnetwork' port='5903'/> + </graphics> + <video> + <model type='cirrus' vram='9216' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml new file mode 100644 index 0000000..7239699 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <input type='mouse' bus='ps2'/> + <graphics type='vnc'> + <listen type='address' address='1.2.3.4' autoport='yes'/> + <listen type='network' network='Bobsnetwork' port='5903'/> + </graphics> + <video> + <model type='cirrus' vram='9216' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml index 64a6890..cb0a389 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml @@ -22,6 +22,7 @@ <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' port='5903' tlsPort='5904'/> <image compression='auto_glz'/> <jpeg compression='auto'/> <zlib compression='auto'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml index a38550c..e338c20 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml @@ -22,6 +22,7 @@ <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' port='5903' tlsPort='5904'/> <channel name='main' mode='secure'/> <channel name='inputs' mode='insecure'/> </graphics> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml index 6389de5..dbba1c3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml @@ -71,7 +71,9 @@ </console> <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> - <graphics type='spice' port='5900' autoport='no' passwd='sercet' passwdValidTo='2011-05-31T16:11:22' connected='disconnect'/> + <graphics type='spice' port='5900' autoport='no' passwd='sercet' passwdValidTo='2011-05-31T16:11:22' connected='disconnect'> + <listen port='5900'/> + </graphics> <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </sound> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml index 79780c6..ba3103f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml @@ -22,6 +22,7 @@ <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' port='5903' tlsPort='5904'/> <channel name='main' mode='secure'/> <channel name='inputs' mode='insecure'/> <image compression='auto_glz'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml index eb6be7d..0099d02 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml @@ -21,7 +21,9 @@ </disk> <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'/> + <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' port='5903'/> + </graphics> <video> <model type='cirrus' vram='9216' heads='1'/> </video> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml index eb6be7d..0099d02 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml @@ -21,7 +21,9 @@ </disk> <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'/> + <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' port='5903'/> + </graphics> <video> <model type='cirrus' vram='9216' heads='1'/> </video> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml index 6304104..5bd22c1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml @@ -21,7 +21,9 @@ </disk> <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' listen='2001:1:2:3:4:5:1234:1234'/> + <graphics type='vnc' port='5903' autoport='no' listen='2001:1:2:3:4:5:1234:1234'> + <listen type='address' address='2001:1:2:3:4:5:1234:1234' port='5903'/> + </graphics> <video> <model type='cirrus' vram='9216' heads='1'/> </video> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml index 8a5fcd7..534ec05 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml @@ -21,7 +21,9 @@ </disk> <controller type='ide' index='0'/> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'/> + <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' port='5903'/> + </graphics> <video> <model type='xen' vram='4096' heads='1'/> </video> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml new file mode 100644 index 0000000..c9cdd9c --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-listen-network2.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='1.2.3.4'> + <listen type='address' address='1.2.3.4' autoport='yes'/> + <listen type='network' network='Bobsnetwork' port='5903'/> + </graphics> + <video> + <model type='cirrus' vram='9216' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f1900c5..44737d4 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -139,6 +139,7 @@ mymain(void) DO_TEST("disk-drive-cache-v1-wb"); DO_TEST("disk-drive-cache-v1-none"); DO_TEST("disk-scsi-device"); + DO_TEST("graphics-listen-network"); DO_TEST("graphics-vnc"); DO_TEST("graphics-vnc-sasl"); DO_TEST("graphics-vnc-tls"); @@ -194,6 +195,7 @@ mymain(void) DO_TEST_DIFFERENT("disk-scsi-device-auto"); DO_TEST_DIFFERENT("console-virtio"); DO_TEST_DIFFERENT("serial-target-port-auto"); + DO_TEST_DIFFERENT("graphics-listen-network2"); virCapabilitiesFree(driver.caps); diff --git a/tests/sexpr2xmldata/sexpr2xml-curmem.xml b/tests/sexpr2xmldata/sexpr2xml-curmem.xml index 6be4d5d..1fef2c3 100644 --- a/tests/sexpr2xmldata/sexpr2xml-curmem.xml +++ b/tests/sexpr2xmldata/sexpr2xml-curmem.xml @@ -31,6 +31,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen port='-1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml index 3f501e7..1bf0423 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml @@ -43,6 +43,8 @@ </console> <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5925' autoport='yes' keymap='en-us'/> + <graphics type='vnc' port='5925' autoport='yes' keymap='en-us'> + <listen port='5925' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml index d0ead27..d0e17af 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml @@ -39,6 +39,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml index 9e6deae..f367ce1 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml @@ -39,6 +39,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml index da3b3ae..aadfd6e 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml @@ -36,6 +36,8 @@ <target dev='vif3.0'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml b/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml index 4dc218f..48c56ed 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml @@ -37,6 +37,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml index 57b8d27..c814d48 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml @@ -37,6 +37,8 @@ <model type='netfront'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml index e39b638..cd63402 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml @@ -41,6 +41,8 @@ <target port='0'/> </parallel> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml index 5e085f9..2ec86ad 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml @@ -48,6 +48,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml index 5619376..7c60c2c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml @@ -44,6 +44,8 @@ <target type='serial' port='1'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml index db010ad..17a426f 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml @@ -44,6 +44,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml index faeed77..075cfae 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml @@ -42,6 +42,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml index 0967ac7..13528b0 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml @@ -44,6 +44,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml index 3773d3b..b251501 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml @@ -42,6 +42,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml index d46df09..c0d92f3 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml @@ -42,6 +42,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml index 34f25ac..d3c5965 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml @@ -46,6 +46,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml index 6c69214..0fa52e8 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml @@ -46,6 +46,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml index bcc20dd..6dcbc15 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml @@ -46,6 +46,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml index 93f20fa..be4562a 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml @@ -44,6 +44,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5901' autoport='no'/> + <graphics type='vnc' port='5901' autoport='no'> + <listen port='5901'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml index 1635584..4b97715 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml @@ -36,7 +36,9 @@ <target dev='vif3.0'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> <sound model='sb16'/> <sound model='es1370'/> </devices> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml index 1635584..4b97715 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml @@ -36,7 +36,9 @@ <target dev='vif3.0'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> <sound model='sb16'/> <sound model='es1370'/> </devices> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml b/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml index 4870863..b6db288 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml @@ -37,6 +37,8 @@ </interface> <input type='mouse' bus='usb'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml index 74f4bdf..ce28666 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml @@ -37,6 +37,8 @@ </interface> <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml b/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml index 5dbc64d..10f6fed 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml @@ -36,6 +36,8 @@ <target dev='vif3.0'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml b/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml index e3c4e20..a70ff9f 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml @@ -36,6 +36,8 @@ <target dev='vif3.0'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' keymap='ja'/> + <graphics type='vnc' port='-1' autoport='yes' keymap='ja'> + <listen port='-1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv.xml b/tests/sexpr2xmldata/sexpr2xml-fv.xml index 5dbc64d..10f6fed 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv.xml @@ -36,6 +36,8 @@ <target dev='vif3.0'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='ja'/> + <graphics type='vnc' port='5903' autoport='no' keymap='ja'> + <listen port='5903'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml index 5a82775..99d46f4 100644 --- a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml +++ b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml @@ -41,6 +41,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen port='-1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml index 0914bc9..c74b8f6 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new-vncdisplay.xml @@ -24,6 +24,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='5925' autoport='no' listen='0.0.0.0' keymap='ja'/> + <graphics type='vnc' port='5925' autoport='no' listen='0.0.0.0' keymap='ja'> + <listen type='address' address='0.0.0.0' port='5925'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml index 0657fba..bab2c9a 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-new.xml @@ -24,6 +24,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0' keymap='ja'/> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0' keymap='ja'> + <listen type='address' address='0.0.0.0' port='-1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-orig.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-orig.xml index 0657fba..bab2c9a 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-orig.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-orig.xml @@ -24,6 +24,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0' keymap='ja'/> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0' keymap='ja'> + <listen type='address' address='0.0.0.0' port='-1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml index 766c78d..cd5af69 100644 --- a/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml +++ b/tests/sexpr2xmldata/sexpr2xml-pv-vfb-type-crash.xml @@ -29,6 +29,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen port='-1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/vmx2xmldata/vmx2xml-graphics-vnc.xml b/tests/vmx2xmldata/vmx2xml-graphics-vnc.xml index 047e034..08ab324 100644 --- a/tests/vmx2xmldata/vmx2xml-graphics-vnc.xml +++ b/tests/vmx2xmldata/vmx2xml-graphics-vnc.xml @@ -12,7 +12,9 @@ <on_crash>destroy</on_crash> <devices> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='5903' autoport='no' keymap='de' passwd='password'/> + <graphics type='vnc' port='5903' autoport='no' keymap='de' passwd='password'> + <listen port='5903'/> + </graphics> <video> <model type='vmvga' vram='4096'/> </video> diff --git a/tests/xmconfigdata/test-escape-paths.xml b/tests/xmconfigdata/test-escape-paths.xml index 9eaf90c..ebeb9b3 100644 --- a/tests/xmconfigdata/test-escape-paths.xml +++ b/tests/xmconfigdata/test-escape-paths.xml @@ -43,7 +43,9 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> <sound model='sb16'/> <sound model='es1370'/> </devices> diff --git a/tests/xmconfigdata/test-fullvirt-force-hpet.xml b/tests/xmconfigdata/test-fullvirt-force-hpet.xml index a662b97..e2e2e76 100644 --- a/tests/xmconfigdata/test-fullvirt-force-hpet.xml +++ b/tests/xmconfigdata/test-fullvirt-force-hpet.xml @@ -40,6 +40,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-force-nohpet.xml b/tests/xmconfigdata/test-fullvirt-force-nohpet.xml index bcd9af6..f63ab60 100644 --- a/tests/xmconfigdata/test-fullvirt-force-nohpet.xml +++ b/tests/xmconfigdata/test-fullvirt-force-nohpet.xml @@ -40,6 +40,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-localtime.xml b/tests/xmconfigdata/test-fullvirt-localtime.xml index be4f119..b89b8c4 100644 --- a/tests/xmconfigdata/test-fullvirt-localtime.xml +++ b/tests/xmconfigdata/test-fullvirt-localtime.xml @@ -38,6 +38,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-net-ioemu.xml b/tests/xmconfigdata/test-fullvirt-net-ioemu.xml index 4a52a45..4c21896 100644 --- a/tests/xmconfigdata/test-fullvirt-net-ioemu.xml +++ b/tests/xmconfigdata/test-fullvirt-net-ioemu.xml @@ -38,6 +38,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-net-netfront.xml b/tests/xmconfigdata/test-fullvirt-net-netfront.xml index efaf5b4..734c60b 100644 --- a/tests/xmconfigdata/test-fullvirt-net-netfront.xml +++ b/tests/xmconfigdata/test-fullvirt-net-netfront.xml @@ -38,6 +38,8 @@ <model type='netfront'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-new-cdrom.xml b/tests/xmconfigdata/test-fullvirt-new-cdrom.xml index 4a52a45..4c21896 100644 --- a/tests/xmconfigdata/test-fullvirt-new-cdrom.xml +++ b/tests/xmconfigdata/test-fullvirt-new-cdrom.xml @@ -38,6 +38,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-old-cdrom.xml b/tests/xmconfigdata/test-fullvirt-old-cdrom.xml index 97965d5..4a2c371 100644 --- a/tests/xmconfigdata/test-fullvirt-old-cdrom.xml +++ b/tests/xmconfigdata/test-fullvirt-old-cdrom.xml @@ -38,6 +38,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml b/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml index 22c6013..7de2515 100644 --- a/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml +++ b/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml @@ -43,6 +43,8 @@ <target port='0'/> </parallel> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml b/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml index be5f8ee..5b6ffc5 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml @@ -50,6 +50,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml b/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml index 03549f0..8be69e1 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml @@ -46,6 +46,8 @@ <target type='serial' port='1'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-file.xml b/tests/xmconfigdata/test-fullvirt-serial-file.xml index 3e5aaf9..f1bcebc 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-file.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-file.xml @@ -46,6 +46,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-null.xml b/tests/xmconfigdata/test-fullvirt-serial-null.xml index 2fd9bf1..e7f13e1 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-null.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-null.xml @@ -44,6 +44,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-pipe.xml b/tests/xmconfigdata/test-fullvirt-serial-pipe.xml index 9292c25..17565c8 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-pipe.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-pipe.xml @@ -46,6 +46,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-pty.xml b/tests/xmconfigdata/test-fullvirt-serial-pty.xml index c4b189f..a49d6e9 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-pty.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-pty.xml @@ -44,6 +44,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-stdio.xml b/tests/xmconfigdata/test-fullvirt-serial-stdio.xml index 3852977..597ca69 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-stdio.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-stdio.xml @@ -44,6 +44,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml b/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml index 4acdfc8..41e237a 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml @@ -48,6 +48,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-tcp.xml b/tests/xmconfigdata/test-fullvirt-serial-tcp.xml index 48d6cb5..b28701d 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-tcp.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-tcp.xml @@ -48,6 +48,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-udp.xml b/tests/xmconfigdata/test-fullvirt-serial-udp.xml index 8c11b45..23e6cc1 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-udp.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-udp.xml @@ -48,6 +48,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-serial-unix.xml b/tests/xmconfigdata/test-fullvirt-serial-unix.xml index 3071c23..e60711b 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-unix.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-unix.xml @@ -46,6 +46,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-sound.xml b/tests/xmconfigdata/test-fullvirt-sound.xml index f142bfc..7adaaee 100644 --- a/tests/xmconfigdata/test-fullvirt-sound.xml +++ b/tests/xmconfigdata/test-fullvirt-sound.xml @@ -38,7 +38,9 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> <sound model='sb16'/> <sound model='es1370'/> </devices> diff --git a/tests/xmconfigdata/test-fullvirt-usbmouse.xml b/tests/xmconfigdata/test-fullvirt-usbmouse.xml index c96e8f9..ee25495 100644 --- a/tests/xmconfigdata/test-fullvirt-usbmouse.xml +++ b/tests/xmconfigdata/test-fullvirt-usbmouse.xml @@ -39,6 +39,8 @@ </interface> <input type='mouse' bus='usb'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-usbtablet-no-bus.xml b/tests/xmconfigdata/test-fullvirt-usbtablet-no-bus.xml index 9370172..d09cfd2 100644 --- a/tests/xmconfigdata/test-fullvirt-usbtablet-no-bus.xml +++ b/tests/xmconfigdata/test-fullvirt-usbtablet-no-bus.xml @@ -38,6 +38,8 @@ </interface> <input type='tablet'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-usbtablet.xml b/tests/xmconfigdata/test-fullvirt-usbtablet.xml index b7bdfd0..6b35c37 100644 --- a/tests/xmconfigdata/test-fullvirt-usbtablet.xml +++ b/tests/xmconfigdata/test-fullvirt-usbtablet.xml @@ -39,6 +39,8 @@ </interface> <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-fullvirt-utc.xml b/tests/xmconfigdata/test-fullvirt-utc.xml index 4a52a45..4c21896 100644 --- a/tests/xmconfigdata/test-fullvirt-utc.xml +++ b/tests/xmconfigdata/test-fullvirt-utc.xml @@ -38,6 +38,8 @@ <model type='e1000'/> </interface> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-no-source-cdrom.xml b/tests/xmconfigdata/test-no-source-cdrom.xml index a7a27d3..a074759 100644 --- a/tests/xmconfigdata/test-no-source-cdrom.xml +++ b/tests/xmconfigdata/test-no-source-cdrom.xml @@ -43,6 +43,8 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-paravirt-net-e1000.xml b/tests/xmconfigdata/test-paravirt-net-e1000.xml index d709e69..4fd6940 100644 --- a/tests/xmconfigdata/test-paravirt-net-e1000.xml +++ b/tests/xmconfigdata/test-paravirt-net-e1000.xml @@ -28,6 +28,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-paravirt-net-vifname.xml b/tests/xmconfigdata/test-paravirt-net-vifname.xml index b6c4739..9c7cb2c 100644 --- a/tests/xmconfigdata/test-paravirt-net-vifname.xml +++ b/tests/xmconfigdata/test-paravirt-net-vifname.xml @@ -29,6 +29,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-paravirt-new-pvfb-vncdisplay.xml b/tests/xmconfigdata/test-paravirt-new-pvfb-vncdisplay.xml index 39325ea..6c2fe37 100644 --- a/tests/xmconfigdata/test-paravirt-new-pvfb-vncdisplay.xml +++ b/tests/xmconfigdata/test-paravirt-new-pvfb-vncdisplay.xml @@ -27,6 +27,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='5925' autoport='no' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='5925' autoport='no' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' port='5925'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-paravirt-new-pvfb.xml b/tests/xmconfigdata/test-paravirt-new-pvfb.xml index 40c79cb..9c479aa 100644 --- a/tests/xmconfigdata/test-paravirt-new-pvfb.xml +++ b/tests/xmconfigdata/test-paravirt-new-pvfb.xml @@ -27,6 +27,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-paravirt-old-pvfb-vncdisplay.xml b/tests/xmconfigdata/test-paravirt-old-pvfb-vncdisplay.xml index 39325ea..6c2fe37 100644 --- a/tests/xmconfigdata/test-paravirt-old-pvfb-vncdisplay.xml +++ b/tests/xmconfigdata/test-paravirt-old-pvfb-vncdisplay.xml @@ -27,6 +27,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='5925' autoport='no' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='5925' autoport='no' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' port='5925'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-paravirt-old-pvfb.xml b/tests/xmconfigdata/test-paravirt-old-pvfb.xml index 40c79cb..9c479aa 100644 --- a/tests/xmconfigdata/test-paravirt-old-pvfb.xml +++ b/tests/xmconfigdata/test-paravirt-old-pvfb.xml @@ -27,6 +27,8 @@ <target type='xen' port='0'/> </console> <input type='mouse' bus='xen'/> - <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' passwd='123poi'> + <listen type='address' address='127.0.0.1' autoport='yes'/> + </graphics> </devices> </domain> diff --git a/tests/xmconfigdata/test-pci-devs.xml b/tests/xmconfigdata/test-pci-devs.xml index 31df1c7..f823ad7 100644 --- a/tests/xmconfigdata/test-pci-devs.xml +++ b/tests/xmconfigdata/test-pci-devs.xml @@ -43,7 +43,9 @@ <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> - <graphics type='vnc' port='-1' autoport='yes'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen autoport='yes'/> + </graphics> <hostdev mode='subsystem' type='pci' managed='no'> <source> <address domain='0x0001' bus='0x0c' slot='0x1b' function='0x2'/> -- 1.7.3.4

The domain XML now understands the <listen> subelement of its <graphics> element (including when listen type='network'), and the network driver has an internal API that will turn a network name into an IP address, so the final logical step is to put the glue into the qemu driver so that when it is starting up a domain, if it finds <listen type='network' network='xyz'/> in the XML, it will call the network driver to get an IPv4 address associated with network xyz, and tell qemu to listen for vnc (or spice) on that address rather than the default address (localhost). The motivation for this is that a large installation may want the guests' VNC servers listening on physical interfaces rather than localhost, so that users can connect directly from the outside; this requires sending qemu the appropriate IP address to listen on. But this address will of course be different for each host, and if a guest might be migrated around from one host to another, it's important that the guest's config not have any information embedded in it that is specific to one particular host. <listen type='network.../> can solve this problem in the following manner: 1) on each host, define a libvirt network of the same name, associated with the interface on that host that should be used for listening (for example, a simple macvtap network: <forward mode='bridge' dev='eth0'/>, or host bridge network: <forward mode='bridge'/> <bridge name='br0'/> 2) in the <graphics> element of each guest's domain xml, tell vnc to listen on the network name used in step 1: <graphics type='vnc'> <listen type='network'network='example-net' autoport='yes'/> </graphics> (all the above also applies for graphics type='spice'). --- src/qemu/qemu_command.c | 65 +++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.c | 13 +++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4a39078..9ee7658 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4111,7 +4111,6 @@ qemuBuildCommandLine(virConnectPtr conn, if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { int port = virDomainGraphicsListenGetPort(def->graphics[0], 0); - const char *listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); virBuffer opt = VIR_BUFFER_INITIALIZER; if (def->graphics[0]->data.vnc.socket || @@ -4127,7 +4126,37 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->data.vnc.socket); } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; bool escapeAddr; + int ret; + + switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; + } + if (ret < 0) { + qemuReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + goto error; + } + listenAddr = netAddr; + break; + } if (!listenAddr) listenAddr = driver->vncListen; @@ -4139,6 +4168,7 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAdd(&opt, listenAddr, -1); virBufferAsprintf(&opt, ":%d", port - 5900); + VIR_FREE(netAddr); } else { virBufferAsprintf(&opt, "%d", port - 5900); } @@ -4223,7 +4253,10 @@ qemuBuildCommandLine(virConnectPtr conn, virBuffer opt = VIR_BUFFER_INITIALIZER; int port = virDomainGraphicsListenGetPort(def->graphics[0], 0); int tlsPort = virDomainGraphicsListenGetTlsPort(def->graphics[0], 0); - const char *listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + int ret; if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -4236,11 +4269,39 @@ qemuBuildCommandLine(virConnectPtr conn, if (driver->spiceTLS && tlsPort != -1) virBufferAsprintf(&opt, ",tls-port=%u", tlsPort); + switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; + } + if (ret < 0) { + qemuReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + goto error; + } + listenAddr = netAddr; + break; + } + if (!listenAddr) listenAddr = driver->spiceListen; if (listenAddr) virBufferAsprintf(&opt, ",addr=%s", listenAddr); + VIR_FREE(netAddr); + /* In the password case we set it via monitor command, to avoid * making it visible on CLI, so there's no use of password=XXX * in this bit of the code */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 18b5164..6aa3bef 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1055,6 +1055,7 @@ qemuDomainChangeGraphics(struct qemud_driver *driver, int oldPort, newPort, oldTlsPort, newTlsPort; bool oldAutoport, newAutoport; const char *oldListenAddr, *newListenAddr; + const char *oldListenNetwork, *newListenNetwork; int ret = -1; if (!olddev) { @@ -1071,6 +1072,8 @@ qemuDomainChangeGraphics(struct qemud_driver *driver, newTlsPort = virDomainGraphicsListenGetTlsPort(dev, 0); oldListenAddr = virDomainGraphicsListenGetAddress(olddev, 0); newListenAddr = virDomainGraphicsListenGetAddress(dev, 0); + oldListenNetwork = virDomainGraphicsListenGetNetwork(olddev, 0); + newListenNetwork = virDomainGraphicsListenGetNetwork(dev, 0); switch (dev->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: @@ -1086,6 +1089,11 @@ qemuDomainChangeGraphics(struct qemud_driver *driver, _("cannot change listen address setting on vnc graphics")); return -1; } + if (STRNEQ_NULLABLE(oldListenNetwork,newListenNetwork)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot change listen network setting on vnc graphics")); + return -1; + } if (STRNEQ_NULLABLE(olddev->data.vnc.keymap, dev->data.vnc.keymap)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change keymap setting on vnc graphics")); @@ -1132,6 +1140,11 @@ qemuDomainChangeGraphics(struct qemud_driver *driver, _("cannot change listen address setting on spice graphics")); return -1; } + if (STRNEQ_NULLABLE(oldListenNetwork,newListenNetwork)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot change listen network setting on vnc graphics")); + return -1; + } if (STRNEQ_NULLABLE(olddev->data.spice.keymap, dev->data.spice.keymap)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 1.7.3.4

On 07/25/2011 03:00 AM, Laine Stump wrote:
The domain XML now understands the<listen> subelement of its <graphics> element (including when listen type='network'), and the network driver has an internal API that will turn a network name into an IP address, so the final logical step is to put the glue into the qemu driver so that when it is starting up a domain, if it finds <listen type='network' network='xyz'/> in the XML, it will call the network driver to get an IPv4 address associated with network xyz, and tell qemu to listen for vnc (or spice) on that address rather than the default address (localhost).
There may be some fallout to this patch, based on what happens to 1/2, but in general:
@@ -4127,7 +4126,37 @@ qemuBuildCommandLine(virConnectPtr conn, def->graphics[0]->data.vnc.socket);
} else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; bool escapeAddr; + int ret; + + switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork,&netAddr); + if (ret<= -2) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; + } + if (ret< 0) { + qemuReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + goto error; + } + listenAddr = netAddr; + break; + }
this looks good. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Sigh. I'd meant to add more explanation in the [0/2] mail, but forgot to add "--compose" on my git send-mail commandline. Just a couple of points: 1) The old fields in the unions of virDomainGraphicsDef that used to store the attributes from <graphics> (listen, port, tlsPort, autoport) have all been removed. To maintain backward compatibility, if attributes are put into <graphics> (rather than making a separate <listen> subelement), they will be stored in the first virDomainGraphicsListenDef (aka <listen> element) which will be created automatically by the new helper functions if needed. 2) About the helper functions - in the old code, all accesses to the "listen" information was done by referencing the data in virDomainGraphicsDef directly. To continue doing that now that the data structure is more complex would mean that the existing code that uses those attributes would become huge, ugly, and difficult to understand. To avoid this, I made a set of helper functions for getting and setting all the attributes of virDomainGraphicsListenDef. Aw with the data structures themselves, the helper functions are designed to support an array of <listen>s. However, none of our hypervisor drivers know what to do with more than one <listen> right now, so when converting existing code you will see that the index arg is always 0. 3) Patch 1/2 looks large, but much of it is mechanical changes, and it all must be in one patch to maintain bisectability. It can be thought of in three pieces: 1) The parser/formatter additions (including RNG and documentation) 2) updates to hypervisor drivers to access the listen/port data using the new helper functions 3) adding the extra <listen> subelement to every <graphics> element in every test domain XML file (as well as adding a couple of new cases specific to this feature).

On Mon, Jul 25, 2011 at 05:00:51AM -0400, Laine Stump wrote:
Once it's plugged in, the <listen element will be an optional replacement for the "listen", "port", "tlsPort", and "autoport" attributes that graphics elements already have. If the <listen> type='address', it will have an attribute called 'address' which will contain an IP address or dns name that the guest's display server should listen on. If, however, type='network', the <listen> element should have an attribute called 'network' that will be set to the name of a network configuration to get the IP address from.
* docs/schemas/domain.rng: updated to allow the <listen> element
* docs/formatdomain.html.in: document the <listen> element and its attributes.
* src/conf/domain_conf.[hc]:
1) The domain parser, formatter, and data structure are modified to support 0 or more <listen> subelements to each <graphics> element. The old style "legacy" attributes are also still accepted, and will be stored internally just as if they were a separate <listen> element. On output (i.e. format), the first <listen> element will be duplicated in the legacy attributes of the <graphic> element.
2) The following attributes have been removed from the unions in virDomainGRaphicsDef for graphics types vnc, rdp, and spice:
listen port tlsPort autoport
These attributes are all present in the <listen> subelement (aka virDomainGraphicsListenDef)
I think it is a somewhat overkill to have 'autoport' be a setting per-<listen> element. I can't imagine people want a fixed port number on one IP addr, but a dynamic port number on another IP addr. So we could just keep that on the top level element. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 07/26/2011 07:35 AM, Daniel P. Berrange wrote:
Once it's plugged in, the<listen element will be an optional replacement for the "listen", "port", "tlsPort", and "autoport" attributes that graphics elements already have. If the<listen> type='address', it will have an attribute called 'address' which will contain an IP address or dns name that the guest's display server should listen on. If, however, type='network', the<listen> element should have an attribute called 'network' that will be set to the name of a network configuration to get the IP address from.
* docs/schemas/domain.rng: updated to allow the<listen> element
* docs/formatdomain.html.in: document the<listen> element and its attributes.
* src/conf/domain_conf.[hc]:
1) The domain parser, formatter, and data structure are modified to support 0 or more<listen> subelements to each<graphics> element. The old style "legacy" attributes are also still accepted, and will be stored internally just as if they were a separate<listen> element. On output (i.e. format), the first <listen> element will be duplicated in the legacy attributes of the<graphic> element.
2) The following attributes have been removed from the unions in virDomainGRaphicsDef for graphics types vnc, rdp, and spice:
listen port tlsPort autoport
These attributes are all present in the<listen> subelement (aka virDomainGraphicsListenDef) I think it is a somewhat overkill to have 'autoport' be a setting
On Mon, Jul 25, 2011 at 05:00:51AM -0400, Laine Stump wrote: per-<listen> element. I can't imagine people want a fixed port number on one IP addr, but a dynamic port number on another IP addr. So we could just keep that on the top level element.
Although I agree with you for config purposes, it looks to me like the real use of autoport is that in the live XML it allows differentiating between ports that were manually specified and those that were automatically allocated (really, that seems like its main purpose, since simply not giving a port also implies autoport). If we have only a single autoport attribute for all the listens, we'll have to put in extra code that makes sure if they specify port for one listen, they do it for all of them. Otherwise there will be no way to know which ports to strip out when writing the inactive XML (since the same attribute is used for config and status)... So really, nobody should ever need to explicitly say "autoport='yes'" anywhere in their config anyway, it just happens when they don't give any "port='...'".

On 07/26/2011 08:50 AM, Laine Stump wrote:
I think it is a somewhat overkill to have 'autoport' be a setting per-<listen> element. I can't imagine people want a fixed port number on one IP addr, but a dynamic port number on another IP addr. So we could just keep that on the top level element.
Although I agree with you for config purposes, it looks to me like the real use of autoport is that in the live XML it allows differentiating between ports that were manually specified and those that were automatically allocated (really, that seems like its main purpose, since simply not giving a port also implies autoport). If we have only a single autoport attribute for all the listens, we'll have to put in extra code that makes sure if they specify port for one listen, they do it for all of them.
Is it that hard to add that additional validation? Regardless of that answer, and whether this requires a v4, I'll go ahead and review v3 code as if we decide that autoport-per-<listen> is acceptable. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 07/26/2011 07:10 PM, Eric Blake wrote:
On 07/26/2011 08:50 AM, Laine Stump wrote:
I think it is a somewhat overkill to have 'autoport' be a setting per-<listen> element. I can't imagine people want a fixed port number on one IP addr, but a dynamic port number on another IP addr. So we could just keep that on the top level element.
Although I agree with you for config purposes, it looks to me like the real use of autoport is that in the live XML it allows differentiating between ports that were manually specified and those that were automatically allocated (really, that seems like its main purpose, since simply not giving a port also implies autoport). If we have only a single autoport attribute for all the listens, we'll have to put in extra code that makes sure if they specify port for one listen, they do it for all of them.
Is it that hard to add that additional validation?
Anything can be done, given enough time to think, code, and test all the corner cases. I don't see where the gain is in this case, though. I see autoport more as an attribute that's useful when examining live XML than as something useful to set in the config. Having a separate autoport for each port is then not overkill, but just natural. Trying to use a single autoport to indicate whether all of a set of ports were generated automatically or were manually configured is what's overkill - you're going to a lot of trouble to enforce an unnatural restriction for no practical gain. Maybe the best thing is to only allow autoport in <listen> when (flags & INACTIVE) is false (live XML). This would mean that, as far as config was concerned, <listen> would *never* have any autoport in it (which would be fine with me); it would however still be in the data structure, and still output in live XML.
Regardless of that answer, and whether this requires a v4, I'll go ahead and review v3 code as if we decide that autoport-per-<listen> is acceptable.

On 07/27/2011 02:41 AM, Laine Stump wrote:
On 07/26/2011 07:10 PM, Eric Blake wrote:
On 07/26/2011 08:50 AM, Laine Stump wrote:
I think it is a somewhat overkill to have 'autoport' be a setting per-<listen> element. I can't imagine people want a fixed port number on one IP addr, but a dynamic port number on another IP addr. So we could just keep that on the top level element.
Although I agree with you for config purposes, it looks to me like the real use of autoport is that in the live XML it allows differentiating between ports that were manually specified and those that were automatically allocated (really, that seems like its main purpose, since simply not giving a port also implies autoport). If we have only a single autoport attribute for all the listens, we'll have to put in extra code that makes sure if they specify port for one listen, they do it for all of them.
Is it that hard to add that additional validation?
Anything can be done, given enough time to think, code, and test all the corner cases. I don't see where the gain is in this case, though. I see autoport more as an attribute that's useful when examining live XML than as something useful to set in the config. Having a separate autoport for each port is then not overkill, but just natural. Trying to use a single autoport to indicate whether all of a set of ports were generated automatically or were manually configured is what's overkill - you're going to a lot of trouble to enforce an unnatural restriction for no practical gain.
Maybe the best thing is to only allow autoport in <listen> when (flags & INACTIVE) is false (live XML). This would mean that, as far as config was concerned, <listen> would *never* have any autoport in it (which would be fine with me); it would however still be in the data structure, and still output in live XML.
After more thinking, her eare two possible plans: 1) see above - the autoport attribute will only show up (or be recognized) for live XML; when writing the configuration, lack of a port will imply autoport=yes, and presence of port will imply autoport=no. 2) remove autoport from <listen> and store it in <graphics>. During the parsing of <graphics>, after all the listens are parsed, loop through them and verify that either all of them have a port, or none of them do (inactive XML only - for live XML they actually all should have a port). Votes? (After looking through the code again, I'm completely willing to implement (2), just want to make sure that's really what you want.)

On Wed, Jul 27, 2011 at 11:20:42AM -0400, Laine Stump wrote:
On 07/27/2011 02:41 AM, Laine Stump wrote:
On 07/26/2011 07:10 PM, Eric Blake wrote:
On 07/26/2011 08:50 AM, Laine Stump wrote:
I think it is a somewhat overkill to have 'autoport' be a setting per-<listen> element. I can't imagine people want a fixed port number on one IP addr, but a dynamic port number on another IP addr. So we could just keep that on the top level element.
Although I agree with you for config purposes, it looks to me like the real use of autoport is that in the live XML it allows differentiating between ports that were manually specified and those that were automatically allocated (really, that seems like its main purpose, since simply not giving a port also implies autoport). If we have only a single autoport attribute for all the listens, we'll have to put in extra code that makes sure if they specify port for one listen, they do it for all of them.
Is it that hard to add that additional validation?
Anything can be done, given enough time to think, code, and test all the corner cases. I don't see where the gain is in this case, though. I see autoport more as an attribute that's useful when examining live XML than as something useful to set in the config. Having a separate autoport for each port is then not overkill, but just natural. Trying to use a single autoport to indicate whether all of a set of ports were generated automatically or were manually configured is what's overkill - you're going to a lot of trouble to enforce an unnatural restriction for no practical gain.
Maybe the best thing is to only allow autoport in <listen> when (flags & INACTIVE) is false (live XML). This would mean that, as far as config was concerned, <listen> would *never* have any autoport in it (which would be fine with me); it would however still be in the data structure, and still output in live XML.
After more thinking, her eare two possible plans:
1) see above - the autoport attribute will only show up (or be recognized) for live XML; when writing the configuration, lack of a port will imply autoport=yes, and presence of port will imply autoport=no.
This would be a change in behaviour - currently 'autoport' attribute is always present when we output XML, even for inactive guests, so there is a consistent way to determine if autoport is in use.
2) remove autoport from <listen> and store it in <graphics>. During the parsing of <graphics>, after all the listens are parsed, loop through them and verify that either all of them have a port, or none of them do (inactive XML only - for live XML they actually all should have a port).
One further thought, is should we even store 'port' on the listen element. I know technically this lets you configure different port numbers for different interfaces, but this feels somewhat error prone for clients. eg consider a guest A <listen addr='10.0.0.1' port='5902'> <listen addr='192.168.0.1' port='5909'> and guest B <listen addr='10.0.0.1' port='5904'> <listen addr='192.168.0.1' port='5902'> And DNS enter 'somehost.example.com' which has two A records somehost.example.com A 10.0.0.1 somehost.example.com A 192.168.0.1 Connecting to 'somehost.example.com' guest A you need to be very careful not to accidentally get guest B. So it has become much more tedious for a client app to connect to a guest if they have to worry about the fact that a VM may be listening on different ports for each IP address associated with a DNS name. Using different ports may sound unlikely, but it actually could be a fairly common problem if you use 'autoport' feature. I feel it is worthwhile for app developer sanity to *not* allow different port numbers per <listen> element, only IP addresses Should we find we need the extra flexibility in the future (unlikely) then we can still add it to teh XML at that time. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, Jul 27, 2011 at 04:37:13PM +0100, Daniel P. Berrange wrote:
On Wed, Jul 27, 2011 at 11:20:42AM -0400, Laine Stump wrote: One further thought, is should we even store 'port' on the listen element. I know technically this lets you configure different port numbers for different interfaces, but this feels somewhat error prone for clients.
eg consider a guest A
<listen addr='10.0.0.1' port='5902'> <listen addr='192.168.0.1' port='5909'>
and guest B
<listen addr='10.0.0.1' port='5904'> <listen addr='192.168.0.1' port='5902'>
And DNS enter 'somehost.example.com' which has two A records
somehost.example.com A 10.0.0.1 somehost.example.com A 192.168.0.1
Connecting to 'somehost.example.com' guest A you need to be very careful not to accidentally get guest B.
It actually gets even worse if you consider a not uncommon DMZ setup where each host is configured with addresses from a private range like 10.0.0.x, but all users connect to the machine using completely different a public IP address. In such a case there'd be no way to reliably determine which port to use for a guest when conencting to the public IP if multiple different ports per VM allowed. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 07/25/2011 03:00 AM, Laine Stump wrote:
Once it's plugged in, the<listen element will be an optional replacement for the "listen", "port", "tlsPort", and "autoport" attributes that graphics elements already have. If the<listen> type='address', it will have an attribute called 'address' which will contain an IP address or dns name that the guest's display server should listen on. If, however, type='network', the<listen> element should have an attribute called 'network' that will be set to the name of a network configuration to get the IP address from.
91 files changed, 1274 insertions(+), 343 deletions(-)
Big diff, but like you said mostly mechanical.
@@ -2110,6 +2116,70 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+<p> + Rather than putting the information used to setup the listening
setup is a noun, but here you want a verb form: s/setup/set up/
+<dt><code>autoport</code></dt> +<dd>If set to 'yes', a listen port will be determined + automatically at runtime, and reflected in the domain's live + XML. +</dd>
May need tweaks depending on the decision on where autoport should live.
@@ -1500,6 +1503,49 @@ </choice> </element> </define> + +<define name="listenElements"> +<zeroOrMore> +<element name="listen"> +<optional> +<attribute name="type"> +<choice> +<value>address</value> +<value>network</value>
/me curses thunderbird for squashing whitespace in my reply I'm not sure this is right. I think we want to require either type=address (which implies address=nnn must also be present, and network=nnn must not be present) or type=network (which implies that network=nnn must be present, and address=nnn might show up in live XML dumpxml output to show what was resolved, but is ignored in input parsing. Also, autoport=yes and port=nnn should be a forbidden combination. That is, I think this should be as follows (assuming we keep autoport in <listen>, further changes if autoport is global only): <define name="listenElements"> <zeroOrMore> <element name="listen"> <choice> <group> <attribute name="type"> <value>address</value> </attribute> <attribute name="address"> <ref name="addrIPorName"/> </attribute> </group> <group> <attribute name="type"> <value>network</value> </attribute> <attribute name="network"> <text/> </attribute> <optional> <attribute name="address"> <ref name="addrIPorName"/> </attribute> </optional> </group> </choice> <choice> <attribute name="autoport"> <value>yes</value> </attribute> <group> <optional> <attribute name="autoport"> <value>no</value> </attribute> </optional> <optional> <attribute name="port"> <ref name="PortNumber"/> </attribute> </optional> <optional> <attribute name="tlsPort"> <ref name="PortNumber"/> </attribute> </optional> </group> </choice> </element>
+</zeroOrMore> +</define>
Two spaces too much indentation on </define> (not like it shows up any better in my nitpick, though).
@@ -4010,19 +4029,118 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node, return 0; }
+static int +virDomainGraphicsListenParseXML(virDomainGraphicsListenDefPtr def, + xmlNodePtr node, + enum virDomainGraphicsType graphicsType, + unsigned int flags) +{ + int ret = -1; + char *type = virXMLPropString(node, "type"); + char *address = virXMLPropString(node, "address"); + char *network = virXMLPropString(node, "network"); + char *port = virXMLPropString(node, "port"); + char *tlsPort = virXMLPropString(node, "tlsPort"); + char *autoport = virXMLPropString(node, "autoport"); + + if (type&& (def->type = virDomainGraphicsListenTypeFromString(type))< 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("unknown graphics listen type '%s'"), type); + goto error; + }
Shouldn't this fail if type is not found? That is, I think type is a mandatory attribute (must be address or network).
+ + if (address&& address[0]) { + def->address = address; + address = NULL;
Should we reject address if not VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS? I learned with my virDomainSaveImageDefineXML patch that parsing must be symmetric to output format. Since your output function produces address for network mode on live xml but skips it for inactive, you _must_ make the parser leave address as NULL on parsing if (flags & VIR_DOMAIN_XML_INACTIVE) and network mode is detected.
+ } + + if (network&& network[0]) { + if (def->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("network attribute not allowed when listen type is not network")); + goto error; + } + def->network = network; + network = NULL; + }
Are we also missing checks that if type is address, then address was specified; if type is network, then network was specified?
+ + if (port) { + if (virStrToLong_i(port, NULL, 10,&def->port)< 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse listen port %s"), port); + goto error; + } + /* legacy syntax for graphics type=vnc used -1 for + * autoport. We need to maintain it here because the legacy + * attributes in<graphics> must match those in + *<listen>. */
I'm not sure I buy this. We need to output the legacy attributes in <graphics>, but <listen> is not constrained by legacy, so we can require that if you use <listen> you only use the newer syntax.
+ if ((graphicsType == VIR_DOMAIN_GRAPHICS_TYPE_VNC)&& + (def->port == -1)) { + if (flags& VIR_DOMAIN_XML_INACTIVE) + def->port = 0; + def->autoport = true; + } + } else { + def->autoport = true; + } + + if (tlsPort&& + (graphicsType == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + if (virStrToLong_i(tlsPort, NULL, 10,&def->tlsPort)< 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse spice tlsPort %s"), tlsPort); + goto error; + } + }
Should this error out if tlsPort but not GRAPHICS_TYPE_SPICE?
+ + if (autoport&& STREQ(autoport, "yes")) { + if (flags& VIR_DOMAIN_XML_INACTIVE) { + def->port = 0; + def->tlsPort = 0; + } + def->autoport = true;
We should check that autoport=yes and port are not simultaneously specified.
+ if (listenAddr&& STRNEQ_NULLABLE(listenAddr, + virDomainGraphicsListenGetAddress(def, 0))) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("graphics listen attribute %s must match address " + "attribute of first listen element (found %s)"), + listenAddr, virDomainGraphicsListenGetAddress(def, 0)); + goto error; + }
So what if I do: <graphics port="5900"> <listen type="network" network="name" port="5901"/> <listen type="address" address="127.0.0.1" port="5900"/> </graphics> Is that a valid setup, even though the legacy port doesn't match the first <listen> port? That is, are we validating that the legacy must match the first <listen>, or only the first <listen type=address>?
+ +static void +virDomainGraphicsListenDefFormat(virBufferPtr buf, + virDomainGraphicsListenDefPtr def, + enum virDomainGraphicsType graphicsType, + unsigned int flags) +{ + virBufferAddLit(buf, "<listen"); + + if (def->type) { + virBufferAsprintf(buf, " type='%s'", + virDomainGraphicsListenTypeToString(def->type)); + } + + if (def->address&& + ((def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) + || !(flags& VIR_DOMAIN_XML_INACTIVE))) { + /* address may also be set to show current status when type='network', + * but we don't want to print that if INACTIVE data is requested. */ + virBufferAsprintf(buf, " address='%s'", def->address); + }
See the earlier comments - if you honor VIR_DOMAIN_XML_INACTIVE here to control what gets output, you must also honor it on parsing to control what gets skipped.
+ + if (def->network&& + (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) { + virBufferAsprintf(buf, " network='%s'", def->network); + } + + /* For VNC definitions, if the INACTIVE XML was requested and + * autoport is on, we don't print output the port. (Not sure why + * the same thing isn't done for RDP and SPICE, but the existing + * code that this is mimicking doesn't do it.) */
Is that something we should do right, now that we're using new xml? I think it boils down to: In inactive xml, we want to know if the user specified a port (autoport=no and port=nnn) or is okay with a default port (autoport=yes and port can be omitted) - here, the autoport attribute seems redundant. In active xml, we want to know if the user specified a port (autoport=no and port=nnn), or was okay with a default port (autoport=yes) and which port they got (port=nnn, or omitted if they have not got one yet).
+static virDomainGraphicsListenDefPtr +virDomainGraphicsGetListen(virDomainGraphicsDefPtr def, size_t ii, bool force0) +{ + if ((def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + + if (!def->listens&& (ii == 0)&& force0) { + if (VIR_ALLOC(def->listens)< 0) + virReportOOMError();
This fails and leaves a message,
+ else + def->nListens = 1; + } + + if (!def->listens || (def->nListens<= ii)) + return NULL;
but this fails without leaving a message. That can confuse callers (if they got NULL, do they need to report a message, or has one already been reported)? It might be better if you refactor things to always report an error on failure here, except that...
+ +bool +virDomainGraphicsListenGetAutoport(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo) + return false; + return listenInfo->autoport;
this is an example where you don't want an error message (returning false is the right thing to do for out-of-bounds array)
+} + + +int +virDomainGraphicsListenSetAutoport(virDomainGraphicsDefPtr def, size_t ii, bool val) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo) + return -1;
whereas here, you want to know whether the -1 is because of an allocation failure (message was reported, in the current code) or because of a mismatch (<graphics> was not spice or vnc, so it has no listen elements, no error message in the current code). The rest of the patch looks fairly reasonable, so I think any remaining effort is on ensuring a sane domain_conf.c parse and format of the new attribute, how it interacts with autoport, and how the .rng file reflects whatever restrictions are in the code. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 07/26/2011 08:09 PM, Eric Blake wrote:
On 07/25/2011 03:00 AM, Laine Stump wrote:
Once it's plugged in, the<listen element will be an optional replacement for the "listen", "port", "tlsPort", and "autoport" attributes that graphics elements already have. If the<listen> type='address', it will have an attribute called 'address' which will contain an IP address or dns name that the guest's display server should listen on. If, however, type='network', the<listen> element should have an attribute called 'network' that will be set to the name of a network configuration to get the IP address from.
91 files changed, 1274 insertions(+), 343 deletions(-)
Big diff, but like you said mostly mechanical.
@@ -2110,6 +2116,70 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+<p> + Rather than putting the information used to setup the listening
setup is a noun, but here you want a verb form:
s/setup/set up/
+<dt><code>autoport</code></dt> +<dd>If set to 'yes', a listen port will be determined + automatically at runtime, and reflected in the domain's live + XML. +</dd>
May need tweaks depending on the decision on where autoport should live.
@@ -1500,6 +1503,49 @@ </choice> </element> </define> + +<define name="listenElements"> +<zeroOrMore> +<element name="listen"> +<optional> +<attribute name="type"> +<choice> +<value>address</value> +<value>network</value>
/me curses thunderbird for squashing whitespace in my reply
I'm not sure this is right. I think we want to require either type=address (which implies address=nnn must also be present, and network=nnn must not be present) or type=network (which implies that network=nnn must be present, and address=nnn might show up in live XML dumpxml output to show what was resolved, but is ignored in input parsing. Also, autoport=yes and port=nnn should be a forbidden combination.
Actually it's not forbidden, that's really the only practical use I can see for the autoport attribute - in live XML "autoport='yes' port='nnn'" is used to indicate that the port was auto-generated and this time 'nnn' was used. (and not that it's any indication of what is right or wrong, but the current RNG file allows it). (Personally, I would prefer if the currently in-use port was stored in a separate attribute, and "autoport" simply didn't exist. But
That is, I think this should be as follows (assuming we keep autoport in <listen>, further changes if autoport is global only):
I think if we made autoport a single setting global to all instances of listen, and tried to prevent simultaneous (global) autoport and (local to one listen) port, the RNG would be quite unwieldy. (you would have to have a listenElementsPort and a listenElementsAutoport, and the graphics RNG would put the two of them in a <choice> depending on the setting of autoport)
<define name="listenElements"> <zeroOrMore> <element name="listen"> <choice> <group> <attribute name="type"> <value>address</value> </attribute> <attribute name="address"> <ref name="addrIPorName"/> </attribute> </group> <group> <attribute name="type"> <value>network</value> </attribute> <attribute name="network"> <text/> </attribute> <optional> <attribute name="address"> <ref name="addrIPorName"/> </attribute> </optional> </group> </choice> <choice> <attribute name="autoport"> <value>yes</value> </attribute> <group> <optional> <attribute name="autoport"> <value>no</value> </attribute> </optional> <optional> <attribute name="port"> <ref name="PortNumber"/> </attribute> </optional> <optional> <attribute name="tlsPort"> <ref name="PortNumber"/> </attribute> </optional> </group> </choice> </element>
+</zeroOrMore> +</define>
Two spaces too much indentation on </define> (not like it shows up any better in my nitpick, though).
@@ -4010,19 +4029,118 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node, return 0; }
+static int +virDomainGraphicsListenParseXML(virDomainGraphicsListenDefPtr def, + xmlNodePtr node, + enum virDomainGraphicsType graphicsType, + unsigned int flags) +{ + int ret = -1; + char *type = virXMLPropString(node, "type"); + char *address = virXMLPropString(node, "address"); + char *network = virXMLPropString(node, "network"); + char *port = virXMLPropString(node, "port"); + char *tlsPort = virXMLPropString(node, "tlsPort"); + char *autoport = virXMLPropString(node, "autoport"); + + if (type&& (def->type = virDomainGraphicsListenTypeFromString(type))< 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("unknown graphics listen type '%s'"), type); + goto error; + }
Shouldn't this fail if type is not found? That is, I think type is a mandatory attribute (must be address or network).
No. It's also possible that no address/network information is given (in which case type would be "default", which ends up as just no type showing up at all). In that case, the qemu driver has default listen addresses for VNC and for Spice. So it's completely valid to give a port, but no address.
+ + if (address&& address[0]) { + def->address = address; + address = NULL;
Should we reject address if not VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS?
No. Although I haven't found the right place to put it in, it's possible that the listen type may be network, and the resolved address will be filled in so that it can show up as status in the live XML. (I actually originally had exactly that check put in, but removed it for this reason).
I learned with my virDomainSaveImageDefineXML patch that parsing must be symmetric to output format. Since your output function produces address for network mode on live xml but skips it for inactive, you _must_ make the parser leave address as NULL on parsing if (flags & VIR_DOMAIN_XML_INACTIVE) and network mode is detected.
I hadn't thought of that. But should we reject it in that case, or should we *ignore* it? (Will it ever be the case that someone will feed live XML back to the parser as config? And should we complain in that case, or silently ignore the extra, just as we would if it was something completely unknown to us?) (I usually tend to follow the IETF mantra of being lenient in what you accept, and strict in what you give out)
+ } + + if (network&& network[0]) { + if (def->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("network attribute not allowed when listen type is not network")); + goto error; + } + def->network = network; + network = NULL; + }
Are we also missing checks that if type is address, then address was specified; if type is network, then network was specified?
True. I missed that. Definitely that can't be ignored.
+ + if (port) { + if (virStrToLong_i(port, NULL, 10,&def->port)< 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse listen port %s"), port); + goto error; + } + /* legacy syntax for graphics type=vnc used -1 for + * autoport. We need to maintain it here because the legacy + * attributes in<graphics> must match those in + *<listen>. */
I'm not sure I buy this. We need to output the legacy attributes in <graphics>, but <listen> is not constrained by legacy, so we can require that if you use <listen> you only use the newer syntax.
(Keeping in mind that 3 days ago I'd never even thought about any of this, and while writing the code one of my top priorities was that I didn't break/modify any existing behavior of existing attributes, good or bad...) The data in the <listen> and the data in <graphics> are stored in the same place, and are required to match. The data that you're parsing in this listen may have been generated from the following sequence: <graphics type='vnc' listen='1.2.3.4' port='-1'/> parse, format <graphics type='vnc' listen='1.2.3.4' port='-1' autoport='yes'> <listen type='address', address='1.2.3.4' port='-1' autoport='yes'/> </graphics> (now we parse again) What we *don't* want to happen is for valid XML on the first input to turn into invalid XML after a parse/format roundtrip. In the existing code, autoport is always included in the output, and port is always set to -1 if autoport is yes and we're outputting inactive XML. Possibly, I can modify the formatter to (1) never output port=-1 in listen, and 2) only output autoport in listen if we're doing live XML. Then on input, if <graphics has port=-1 that ends up being changed to port=0 + autoport=yes anyway, so the check to verify they match would still work. Okay, yes, I think I was being too pedantic there.
+ if ((graphicsType == VIR_DOMAIN_GRAPHICS_TYPE_VNC)&& + (def->port == -1)) { + if (flags& VIR_DOMAIN_XML_INACTIVE) + def->port = 0; + def->autoport = true; + } + } else { + def->autoport = true; + }
Something I missed here when I combined the multiple places of parsing port (separate for vnc, rdp, and spice) into one - in the case of spice, if no port is specified, it hard codes a default of 5900, rather than setting autoport. Yuck. I guess I have to replicate that, but it certainly doesn't mean I have to like it...
+ + if (tlsPort&& + (graphicsType == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + if (virStrToLong_i(tlsPort, NULL, 10,&def->tlsPort)< 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("cannot parse spice tlsPort %s"), tlsPort); + goto error; + } + }
Should this error out if tlsPort but not GRAPHICS_TYPE_SPICE?
I think this falls in the category of something outside the bounds of what we recognize, so should be ignored, just as it would be ignored if graphics type was VNC and someone put in the attribute "frozzlePort='1234'". These kind of things are a gray area for me though - sometimes it's difficult to decide whether to ignore (it's extra stuff we don't regognize) or give an error (it's attempting to specify something that directly contradicts something else that has also been specified).
+ + if (autoport&& STREQ(autoport, "yes")) { + if (flags& VIR_DOMAIN_XML_INACTIVE) { + def->port = 0; + def->tlsPort = 0; + } + def->autoport = true;
We should check that autoport=yes and port are not simultaneously specified.
Well, existing code doesn't do that. "Do no harm". Certainly in live XML this is acceptable, and for inactive XML there may be cases where someone grabs live XML, modifies it, and sends the results back to the parse as config (aka inactive).
+ if (listenAddr&& STRNEQ_NULLABLE(listenAddr, + virDomainGraphicsListenGetAddress(def, 0))) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("graphics listen attribute %s must match address " + "attribute of first listen element (found %s)"), + listenAddr, virDomainGraphicsListenGetAddress(def, 0)); + goto error; + }
So what if I do:
<graphics port="5900"> <listen type="network" network="name" port="5901"/> <listen type="address" address="127.0.0.1" port="5900"/> </graphics>
Is that a valid setup, even though the legacy port doesn't match the first <listen> port? That is, are we validating that the legacy must match the first <listen>, or only the first <listen type=address>?
Two points here: 1) I did say in earlier conversations that the attributes in <graphics> should match the first <listen> that has type='address'. I didn't have the time to implement it that way, though. Fortunately we currently have no driver that understands more than a single <listen> anyway, so this can be glossed over for the moment (as long as it's not forgotten). 2) Your example brings up an interesting question: the idea of allowing both a <listen> as well as the legacy attributes in <graphics> is really intended to ease compatibility with old applications that only understand the old attributes, and the duplication should *really* only be done by our formatter, not by anyone creating new XML. Our formatter would never copy port='5900' into <graphics> without also putting in listen='127.0.0.1' (since that's in the <listen> too. But the parser only checks that individual attributes match when specified in both places; it doesn't do a "if *any* attribute is specified in both places, they must *all* be specified in both places" check. Do we need to do that, or is it overkill? (this is a place where I can get on the "it's overkill" boat :-)
+ +static void +virDomainGraphicsListenDefFormat(virBufferPtr buf, + virDomainGraphicsListenDefPtr def, + enum virDomainGraphicsType graphicsType, + unsigned int flags) +{ + virBufferAddLit(buf, "<listen"); + + if (def->type) { + virBufferAsprintf(buf, " type='%s'", + virDomainGraphicsListenTypeToString(def->type)); + } + + if (def->address&& + ((def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) + || !(flags& VIR_DOMAIN_XML_INACTIVE))) { + /* address may also be set to show current status when type='network', + * but we don't want to print that if INACTIVE data is requested. */ + virBufferAsprintf(buf, " address='%s'", def->address); + }
See the earlier comments - if you honor VIR_DOMAIN_XML_INACTIVE here to control what gets output, you must also honor it on parsing to control what gets skipped.
Skipped, or forbidden? I've seen you suggest both in different (but similar) contexts.
+ + if (def->network&& + (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) { + virBufferAsprintf(buf, " network='%s'", def->network); + } + + /* For VNC definitions, if the INACTIVE XML was requested and + * autoport is on, we don't print output the port. (Not sure why + * the same thing isn't done for RDP and SPICE, but the existing + * code that this is mimicking doesn't do it.) */
Is that something we should do right, now that we're using new xml? I think it boils down to:
In inactive xml, we want to know if the user specified a port (autoport=no and port=nnn) or is okay with a default port (autoport=yes and port can be omitted) - here, the autoport attribute seems redundant.
Yes, this is an extension of the earlier discussion. I only started thinking about this last Friday afternoon, and at first was nervous about having everything match up exactly all the time. I think I understand the interactions better now, and we can make the <listen> grammar more sane. For starters, I think autoport should never be output in config/inactive XML, and the ugly "port='-1'" should also never show up. In live XML, we should only output autoport if it is "yes" (we already do this). For parsing, I think we should silently accept autoport in live XML (and also in inactive XML as long as the provided value doesn't conflict with the setting of port), but port='-1' should have no special meaning (that will work okay, since the existing code never actually stores a port of -1 as such - it converts it to autoport=yes on input, and back to port=-1 on output.)
In active xml, we want to know if the user specified a port (autoport=no and port=nnn), or was okay with a default port (autoport=yes) and which port they got (port=nnn, or omitted if they have not got one yet).
+static virDomainGraphicsListenDefPtr +virDomainGraphicsGetListen(virDomainGraphicsDefPtr def, size_t ii, bool force0) +{ + if ((def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP) || + (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + + if (!def->listens&& (ii == 0)&& force0) { + if (VIR_ALLOC(def->listens)< 0) + virReportOOMError();
This fails and leaves a message,
+ else + def->nListens = 1; + } + + if (!def->listens || (def->nListens<= ii)) + return NULL;
but this fails without leaving a message. That can confuse callers (if they got NULL, do they need to report a message, or has one already been reported)? It might be better if you refactor things to always report an error on failure here, except that...
+ +bool +virDomainGraphicsListenGetAutoport(virDomainGraphicsDefPtr def, size_t ii) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, false); + + if (!listenInfo) + return false; + return listenInfo->autoport;
this is an example where you don't want an error message (returning false is the right thing to do for out-of-bounds array)
+} + + +int +virDomainGraphicsListenSetAutoport(virDomainGraphicsDefPtr def, size_t ii, bool val) +{ + virDomainGraphicsListenDefPtr listenInfo + = virDomainGraphicsGetListen(def, ii, true); + + if (!listenInfo) + return -1;
whereas here, you want to know whether the -1 is because of an allocation failure (message was reported, in the current code) or because of a mismatch (<graphics> was not spice or vnc, so it has no listen elements, no error message in the current code).
Academically interesting, but currently not significant, since 1) the function is only ever called by code that has already qualified that the type is VNC, SPICE, or RDP (so the "wrong type" error will never happen), 2) the *Set*() functions are always called with ii=0 and force0=true, so for the Sets there will never be a case where we return NULL because the requested listen doesn't exist; it will only return NULL if we ran out of memory, and 3) for the *Get*() functions, again they're only called when type has already been qualified, so the only possible reason for returning NULL would be if the desired listen wasn't there at all, which isn't an error. I see this function as a convenience for a few functions in the same file of the current code, not as an API that has any type of contract that needs to be followed/maintained for future potential callers.
The rest of the patch looks fairly reasonable, so I think any remaining effort is on ensuring a sane domain_conf.c parse and format of the new attribute, how it interacts with autoport, and how the .rng file reflects whatever restrictions are in the code.
I think I was too paranoid in my initial coding about maintaining exact 1:1 correspondence between the new <listen> parse/output and the old <graphics> parse/output. We may disagree about whether certain "extra stuff" should be ignored or rejected, though. I'll go through it again in the morning and see what falls out. Thanks for the review!

On 07/27/2011 01:47 AM, Laine Stump wrote:
I'm not sure this is right. I think we want to require either type=address (which implies address=nnn must also be present, and network=nnn must not be present) or type=network (which implies that network=nnn must be present, and address=nnn might show up in live XML dumpxml output to show what was resolved, but is ignored in input parsing. Also, autoport=yes and port=nnn should be a forbidden combination.
Actually it's not forbidden, that's really the only practical use I can see for the autoport attribute - in live XML "autoport='yes' port='nnn'" is used to indicate that the port was auto-generated and this time 'nnn' was used. (and not that it's any indication of what is right or wrong, but the current RNG file allows it). ...
Well, existing code doesn't do that. "Do no harm".
Certainly in live XML this is acceptable, and for inactive XML there may be cases where someone grabs live XML, modifies it, and sends the results back to the parse as config (aka inactive).
Yeah, on thinking more, it would be nice if the rng file accepted all live xml (it doesn't, because of other aspects like <alias> and <actual>, but in general we tend to ignore rather than reject stuff we don't recognize). So maybe your rng is okay as is - it accepts more than what is strictly necessary, but as long as it doesn't reject anything that the code accepts, we're okay. The question comes down to whether it is easy to make the rng reject stuff that the code rejects, or whether to keep the rng simple and over-permissive.
I think if we made autoport a single setting global to all instances of listen, and tried to prevent simultaneous (global) autoport and (local to one listen) port, the RNG would be quite unwieldy. (you would have to have a listenElementsPort and a listenElementsAutoport, and the graphics RNG would put the two of them in a <choice> depending on the setting of autoport)
Interesting point. Overall, I think it boils down to whether we ever envision having more than one <listen>, where one specified a port and the other let the port be chosen automatically. Conversely, if we start strict, we can always relax later (especially since it won't be till later that we support multiple <listen>), whereas if we start relaxed, it's harder to tighten things up. But I'm starting to swing towards having autoport remain with <listen>.
Shouldn't this fail if type is not found? That is, I think type is a mandatory attribute (must be address or network).
No. It's also possible that no address/network information is given (in which case type would be "default", which ends up as just no type showing up at all). In that case, the qemu driver has default listen addresses for VNC and for Spice. So it's completely valid to give a port, but no address.
Makes sense. So I think this can be expressed in rng as: <optional> <choice> <group> <attribute>...type=address <attribute>...address=iporname </group> <group> <attribute>...type=network <attribute>...network=name <optional> <attribute>...address=ip </optional> </group> </choice> </optional> That is, type is optional, but if present, then it determines whether address or network must also be present. Meanwhile, address can appear with either type, but if address appears, then type must appear.
+ + if (address&& address[0]) { + def->address = address; + address = NULL;
Should we reject address if not VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS?
No. Although I haven't found the right place to put it in, it's possible that the listen type may be network, and the resolved address will be filled in so that it can show up as status in the live XML. (I actually originally had exactly that check put in, but removed it for this reason).
Yep, I agree that we want to be able to parse existing live output, even if we ignore that parse on INACTIVE.
I learned with my virDomainSaveImageDefineXML patch that parsing must be symmetric to output format. Since your output function produces address for network mode on live xml but skips it for inactive, you _must_ make the parser leave address as NULL on parsing if (flags & VIR_DOMAIN_XML_INACTIVE) and network mode is detected.
I hadn't thought of that. But should we reject it in that case, or should we *ignore* it? (Will it ever be the case that someone will feed live XML back to the parser as config? And should we complain in that case, or silently ignore the extra, just as we would if it was something completely unknown to us?) (I usually tend to follow the IETF mantra of being lenient in what you accept, and strict in what you give out)
In general, we ignore (not reject) live-only information when doing an INACTIVE parse. See for example how <alias> is treated - the format routines only output <alias> on live images, and when parsing INACTIVE, any existing <alias> is silently ignored.
Possibly, I can modify the formatter to (1) never output port=-1 in listen, and 2) only output autoport in listen if we're doing live XML. Then on input, if <graphics has port=-1 that ends up being changed to port=0 + autoport=yes anyway, so the check to verify they match would still work.
Seems like a good plan of attack.
Two points here:
1) I did say in earlier conversations that the attributes in <graphics> should match the first <listen> that has type='address'. I didn't have the time to implement it that way, though. Fortunately we currently have no driver that understands more than a single <listen> anyway, so this can be glossed over for the moment (as long as it's not forgotten).
If you don't address it now, be sure to leave some comments to that effect. And if I haven't made it clear before, I'd like to see something like this patch make it into 0.9.4 - we've missed the rc1 freeze, but this only changes xml rather than adding new API, and it can be argued that this patch is necessary to round out a new feature being advertised for 0.9.4 (that is, the feature of being able to isolate host-dependent network information out of domain xml is buggy without this patch).
2) Your example brings up an interesting question: the idea of allowing both a <listen> as well as the legacy attributes in <graphics> is really intended to ease compatibility with old applications that only understand the old attributes, and the duplication should *really* only be done by our formatter, not by anyone creating new XML. Our formatter would never copy port='5900' into <graphics> without also putting in listen='127.0.0.1' (since that's in the <listen> too. But the parser only checks that individual attributes match when specified in both places; it doesn't do a "if *any* attribute is specified in both places, they must *all* be specified in both places" check. Do we need to do that, or is it overkill? (this is a place where I can get on the "it's overkill" boat :-)
I think you've convinced me - pairwise comparison is simple enough to get this patch finished, and while entire set comparison would be stricter, it's probably overkill to worry about it now.
See the earlier comments - if you honor VIR_DOMAIN_XML_INACTIVE here to control what gets output, you must also honor it on parsing to control what gets skipped.
Skipped, or forbidden? I've seen you suggest both in different (but similar) contexts.
You've convinced me - skip (not reject) anything that is valid as live xml but irrelevant as inactive xml (that is, autoport=yes + port=nnn on an INACTIVE parse takes only autoport=yes, and ignores port). We can reserve rejections for something that will 1) never be output as live xml, and 2) would create an actual conflict (like trying to specify tls port and vnc).
We may disagree about whether certain "extra stuff" should be ignored or rejected, though. I'll go through it again in the morning and see what falls out.
I definitely think I found a few things, but my original review mail is probably larger than the actual amount of changes you need to make in a v4.
Thanks for the review!
One idea for splitting this into easier reviews (don't know how hard it would be though): in patch 1, implement the helper functions to get and set values, and convert all callers to use the helper functions, but where the helper functions are one-liner wrappers around direct access. In patch 2, change domain_conf to support <listen> attributes, and update the helper functions, as well as update the test cases to react to the new formatted xml. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Laine Stump