[libvirt] [PATCH] virsh: Add domdisplay command for VNC and SPICE

Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks. Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 128 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 4d34d49..88d4681 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13624,6 +13624,133 @@ cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* + * "display" command + */ +static const vshCmdInfo info_domdisplay[] = { + {"help", N_("domain display connection URI")}, + {"desc", N_("Output the IP address and port number for the graphical display.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domdisplay[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) +{ + xmlDocPtr xml = NULL; + xmlXPathObjectPtr obj = NULL; + xmlXPathContextPtr ctxt = NULL; + virDomainPtr dom; + bool ret = false; + int port = 0; + char *doc; + char *xpath; + const char *scheme[] = { "vnc", "spice", NULL }; + int iter = 0; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + doc = virDomainGetXMLDesc(dom, 0); + if (!doc) + goto cleanup; + + xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); + VIR_FREE(doc); + if (!xml) + goto cleanup; + + /* Attempt to grab our display info */ + for (iter = 0; scheme[iter] != NULL; iter++) { + /* Create our XPATH lookup for the current display */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@port)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + obj = xmlXPathEval(BAD_CAST xpath, ctxt); + VIR_FREE(xpath); + if (obj == NULL || obj->type != XPATH_STRING || + obj->stringval == NULL) { + if (obj) { + /* Clean up memory */ + xmlXPathFreeObject(obj); + obj = NULL; + } + continue; + } + + /* If there was a port number, then the guest uses the current scheme */ + if (obj->stringval[0] != 0) { + + /* Make sure this is a valid port number */ + if (virStrToLong_i((const char *)obj->stringval, NULL, 10, &port)) { + vshError(ctl, _("Unable to interpret '%s' as a port number."), + obj->stringval); + goto cleanup; + } + + if (port < 0) { + vshError(ctl, _("Invalid port '%d', guest likely not started."), + port); + goto cleanup; + } + + /* Clean up memory */ + xmlXPathFreeObject(obj); + obj = NULL; + + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@listen)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + obj = xmlXPathEval(BAD_CAST xpath, ctxt); + VIR_FREE(xpath); + + /* VNC protocol handlers take their port number as X - 5900 */ + if (scheme[iter] == "vnc") + port -= 5900; + + if (obj == NULL || obj->type != XPATH_STRING || + obj->stringval == NULL || obj->stringval[0] == 0 || + STREQ((const char *)obj->stringval, "0.0.0.0")) { + vshPrint(ctl, "%s://localhost:%d\n", scheme[iter], port); + } else { + vshPrint(ctl, "%s://%s:%d\n", scheme[iter], + (const char *)obj->stringval, port); + } + + /* Clean up memory */ + xmlXPathFreeObject(obj); + obj = NULL; + + /* We got what we came for so return successfully */ + ret = true; + break; + } + + } + +cleanup: + xmlXPathFreeObject(obj); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + virDomainFree(dom); + return ret; +} + +/* * "vncdisplay" command */ static const vshCmdInfo info_vncdisplay[] = { @@ -17702,6 +17829,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-disk", cmdDetachDisk, opts_detach_disk, info_detach_disk, 0}, {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, + {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, -- 1.7.3.4

On Fri, Jun 22, 2012 at 12:30 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
I also toyed with the idea of adding this instead as an API call to libvirt called something like virDomainDisplay() and allowing other backends to advertise their URIs to the domain's display. I believe VMWare has VMWare View which I would assume would have a method by which to call it and provide the machine to connect to as command line arguments. Then virsh would simply hook the 'domdisplay' command to virDomainDisplay() and spit out that output. It would probably be able to be used by virt-manager and vdsm as well. Just a thought. Let me know. -- Doug Goldstein

On 22.06.2012 07:38, Doug Goldstein wrote:
On Fri, Jun 22, 2012 at 12:30 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
I also toyed with the idea of adding this instead as an API call to libvirt called something like virDomainDisplay() and allowing other backends to advertise their URIs to the domain's display. I believe VMWare has VMWare View which I would assume would have a method by which to call it and provide the machine to connect to as command line arguments. Then virsh would simply hook the 'domdisplay' command to virDomainDisplay() and spit out that output. It would probably be able to be used by virt-manager and vdsm as well.
Just a thought. Let me know.
Yeah, that's what I was thinking of when reading your patch. I think we can add a new API that would return all info needed to connect to a display. I think it could look like this: char * virDomainGetGraphicsInfo(virDomainPtr dom, unsigned int flags); It would return a XML doc containing all interesting knobs: <graphics type='spice' port='5904'> <listen type='address' address='1.2.3.4'/> <channel name='main' mode='secure'/> <channel name='record' mode='insecure'/> <image compression='auto_glz'/> <streaming mode='filter'/> <clipboard copypaste='no'/> <mouse mode='client'/> </graphics> yeah it's basically copy&paste-d from domain's XML, but there is huge difference. Currently vncdisplay command (and from quick look at your patch domdisplay too) don't show correct listen address if none set in domain's XML. Because in that case we take the one from qemu.conf. However, this cannot be stored into domain's XML as it would discard all future changes to vnc_listen|spice_listen in qemu.conf. For instance, user doesn't set any listenAddress in domain's XML but set vnc_listen=1.2.3.4; Then we want vncdisplay (and subsequently domdisplay) to return 1.2.3.4:<port> Moreover, it's the daemon only who know what has been set in config files. Summing up, I think we need a new API which would take into account all written above. And to make our lives easier it can return an XML with RNG taken from domain XML (docs/schemas/domaincommon.rng); And since there may be security info we can use VIR_GRAPHICS_XML_SECURE flag to show it or not. Michal

On Fri, Jun 22, 2012 at 10:19:06AM +0200, Michal Privoznik wrote:
On 22.06.2012 07:38, Doug Goldstein wrote:
On Fri, Jun 22, 2012 at 12:30 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
I also toyed with the idea of adding this instead as an API call to libvirt called something like virDomainDisplay() and allowing other backends to advertise their URIs to the domain's display. I believe VMWare has VMWare View which I would assume would have a method by which to call it and provide the machine to connect to as command line arguments. Then virsh would simply hook the 'domdisplay' command to virDomainDisplay() and spit out that output. It would probably be able to be used by virt-manager and vdsm as well.
Just a thought. Let me know.
Yeah, that's what I was thinking of when reading your patch. I think we can add a new API that would return all info needed to connect to a display. I think it could look like this:
char * virDomainGetGraphicsInfo(virDomainPtr dom, unsigned int flags);
It would return a XML doc containing all interesting knobs:
<graphics type='spice' port='5904'> <listen type='address' address='1.2.3.4'/> <channel name='main' mode='secure'/> <channel name='record' mode='insecure'/> <image compression='auto_glz'/> <streaming mode='filter'/> <clipboard copypaste='no'/> <mouse mode='client'/> </graphics>
yeah it's basically copy&paste-d from domain's XML, but there is huge difference. Currently vncdisplay command (and from quick look at your
I don't think we should be inventing a new API that just replicates the data from the main XML.
patch domdisplay too) don't show correct listen address if none set in domain's XML. Because in that case we take the one from qemu.conf. However, this cannot be stored into domain's XML as it would discard all future changes to vnc_listen|spice_listen in qemu.conf. For instance, user doesn't set any listenAddress in domain's XML but set vnc_listen=1.2.3.4; Then we want vncdisplay (and subsequently domdisplay) to return 1.2.3.4:<port>
It is possible to distinguish an original 'listen' addr from an auto-added already. The auto-added one will only appear in the live XML, so apps can see the original value via the inactive XML. 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 22.06.2012 12:07, Daniel P. Berrange wrote:
On Fri, Jun 22, 2012 at 10:19:06AM +0200, Michal Privoznik wrote:
On 22.06.2012 07:38, Doug Goldstein wrote:
On Fri, Jun 22, 2012 at 12:30 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
I also toyed with the idea of adding this instead as an API call to libvirt called something like virDomainDisplay() and allowing other backends to advertise their URIs to the domain's display. I believe VMWare has VMWare View which I would assume would have a method by which to call it and provide the machine to connect to as command line arguments. Then virsh would simply hook the 'domdisplay' command to virDomainDisplay() and spit out that output. It would probably be able to be used by virt-manager and vdsm as well.
Just a thought. Let me know.
Yeah, that's what I was thinking of when reading your patch. I think we can add a new API that would return all info needed to connect to a display. I think it could look like this:
char * virDomainGetGraphicsInfo(virDomainPtr dom, unsigned int flags);
It would return a XML doc containing all interesting knobs:
<graphics type='spice' port='5904'> <listen type='address' address='1.2.3.4'/> <channel name='main' mode='secure'/> <channel name='record' mode='insecure'/> <image compression='auto_glz'/> <streaming mode='filter'/> <clipboard copypaste='no'/> <mouse mode='client'/> </graphics>
yeah it's basically copy&paste-d from domain's XML, but there is huge difference. Currently vncdisplay command (and from quick look at your
I don't think we should be inventing a new API that just replicates the data from the main XML.
patch domdisplay too) don't show correct listen address if none set in domain's XML. Because in that case we take the one from qemu.conf. However, this cannot be stored into domain's XML as it would discard all future changes to vnc_listen|spice_listen in qemu.conf. For instance, user doesn't set any listenAddress in domain's XML but set vnc_listen=1.2.3.4; Then we want vncdisplay (and subsequently domdisplay) to return 1.2.3.4:<port>
It is possible to distinguish an original 'listen' addr from an auto-added already. The auto-added one will only appear in the live XML, so apps can see the original value via the inactive XML.
No it's not. When building qemu cmd line: listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); ... if (!listenAddr) listenAddr = driver->vncListen; escapeAddr = strchr(listenAddr, ':') != NULL; if (escapeAddr) virBufferAsprintf(&opt, "[%s]", listenAddr); else virBufferAdd(&opt, listenAddr, -1); And when formatting (even live) domain XML: listenAddr = virDomainGraphicsListenGetAddress(def, i); ... if (listenAddr) virBufferAsprintf(buf, " listen='%s'", listenAddr); What might cause confusion are migration cookies where the listen address is presented every time. That's because the same command sequence as for building cmd line is used. But okay, if we don't want to introduce a new API we must find a way of inserting an attribute just into live XML not inactive (=make info temporary). I guess I believe how to achieve this. Michal

On Fri, Jun 22, 2012 at 01:18:49PM +0200, Michal Privoznik wrote:
On 22.06.2012 12:07, Daniel P. Berrange wrote:
On Fri, Jun 22, 2012 at 10:19:06AM +0200, Michal Privoznik wrote:
On 22.06.2012 07:38, Doug Goldstein wrote:
On Fri, Jun 22, 2012 at 12:30 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
I also toyed with the idea of adding this instead as an API call to libvirt called something like virDomainDisplay() and allowing other backends to advertise their URIs to the domain's display. I believe VMWare has VMWare View which I would assume would have a method by which to call it and provide the machine to connect to as command line arguments. Then virsh would simply hook the 'domdisplay' command to virDomainDisplay() and spit out that output. It would probably be able to be used by virt-manager and vdsm as well.
Just a thought. Let me know.
Yeah, that's what I was thinking of when reading your patch. I think we can add a new API that would return all info needed to connect to a display. I think it could look like this:
char * virDomainGetGraphicsInfo(virDomainPtr dom, unsigned int flags);
It would return a XML doc containing all interesting knobs:
<graphics type='spice' port='5904'> <listen type='address' address='1.2.3.4'/> <channel name='main' mode='secure'/> <channel name='record' mode='insecure'/> <image compression='auto_glz'/> <streaming mode='filter'/> <clipboard copypaste='no'/> <mouse mode='client'/> </graphics>
yeah it's basically copy&paste-d from domain's XML, but there is huge difference. Currently vncdisplay command (and from quick look at your
I don't think we should be inventing a new API that just replicates the data from the main XML.
patch domdisplay too) don't show correct listen address if none set in domain's XML. Because in that case we take the one from qemu.conf. However, this cannot be stored into domain's XML as it would discard all future changes to vnc_listen|spice_listen in qemu.conf. For instance, user doesn't set any listenAddress in domain's XML but set vnc_listen=1.2.3.4; Then we want vncdisplay (and subsequently domdisplay) to return 1.2.3.4:<port>
It is possible to distinguish an original 'listen' addr from an auto-added already. The auto-added one will only appear in the live XML, so apps can see the original value via the inactive XML.
No it's not. When building qemu cmd line: listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); ... if (!listenAddr) listenAddr = driver->vncListen;
escapeAddr = strchr(listenAddr, ':') != NULL; if (escapeAddr) virBufferAsprintf(&opt, "[%s]", listenAddr); else virBufferAdd(&opt, listenAddr, -1);
And when formatting (even live) domain XML: listenAddr = virDomainGraphicsListenGetAddress(def, i); ... if (listenAddr) virBufferAsprintf(buf, " listen='%s'", listenAddr);
What might cause confusion are migration cookies where the listen address is presented every time. That's because the same command sequence as for building cmd line is used.
But okay, if we don't want to introduce a new API we must find a way of inserting an attribute just into live XML not inactive (=make info temporary). I guess I believe how to achieve this.
We already do this for the port number, so I don't see why the listen addr should be any harder 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 :|

Hey, On Fri, Jun 22, 2012 at 12:30:37AM -0500, Doug Goldstein wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 128 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 4d34d49..88d4681 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13624,6 +13624,133 @@ cmdSysinfo (vshControl *ctl, const vshCmd + + if (obj == NULL || obj->type != XPATH_STRING || + obj->stringval == NULL || obj->stringval[0] == 0 || + STREQ((const char *)obj->stringval, "0.0.0.0")) { + vshPrint(ctl, "%s://localhost:%d\n", scheme[iter], port); + } else { + vshPrint(ctl, "%s://%s:%d\n", scheme[iter], + (const char *)obj->stringval, port);
For SPICE, a URI you can pass to remote-viewer is spice://hostname?port=xx&tls-port=yy&password=zzzzzzz For simple cases, spice://hostname:port will be enough, but if you are using secure channels you need the former. Christophe

On Fri, Jun 22, 2012 at 3:56 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
On Fri, Jun 22, 2012 at 12:30:37AM -0500, Doug Goldstein wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 128 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 4d34d49..88d4681 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13624,6 +13624,133 @@ cmdSysinfo (vshControl *ctl, const vshCmd + + if (obj == NULL || obj->type != XPATH_STRING || + obj->stringval == NULL || obj->stringval[0] == 0 || + STREQ((const char *)obj->stringval, "0.0.0.0")) { + vshPrint(ctl, "%s://localhost:%d\n", scheme[iter], port); + } else { + vshPrint(ctl, "%s://%s:%d\n", scheme[iter], + (const char *)obj->stringval, port);
For SPICE, a URI you can pass to remote-viewer is spice://hostname?port=xx&tls-port=yy&password=zzzzzzz For simple cases, spice://hostname:port will be enough, but if you are using secure channels you need the former.
Christophe
Thanks for that. Do I need to worry about the authority field for anything? -- Doug Goldstein

On Sun, Jun 24, 2012 at 06:54:10PM -0500, Doug Goldstein wrote:
On Fri, Jun 22, 2012 at 3:56 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
For SPICE, a URI you can pass to remote-viewer is spice://hostname?port=xx&tls-port=yy&password=zzzzzzz For simple cases, spice://hostname:port will be enough, but if you are using secure channels you need the former.
Christophe
Thanks for that. Do I need to worry about the authority field for anything?
I don't know what the authority field is :-/ Christophe

On 22.06.2012 07:30, Doug Goldstein wrote:
Add a new 'domdisplay' command that provides a URI for both VNC and SPICE connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 128 insertions(+), 0 deletions(-)
Overall, this patch needed a bit tweaking just to be able to apply it. If you can make your e-mail client to not wrap long line it would be perfect. Or just use git send-email.
diff --git a/tools/virsh.c b/tools/virsh.c index 4d34d49..88d4681 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13624,6 +13624,133 @@ cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }
/* + * "display" command + */ +static const vshCmdInfo info_domdisplay[] = { + {"help", N_("domain display connection URI")}, + {"desc", N_("Output the IP address and port number for the graphical display.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domdisplay[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) +{ + xmlDocPtr xml = NULL; + xmlXPathObjectPtr obj = NULL; + xmlXPathContextPtr ctxt = NULL; + virDomainPtr dom; + bool ret = false; + int port = 0; + char *doc; + char *xpath; + const char *scheme[] = { "vnc", "spice", NULL }; + int iter = 0t + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; +
if (!virDomainIsActive(dom)) { vshError(ctl, _("Domain is not running")); goto cleanup; } rather than [1]
+ doc = virDomainGetXMLDesc(dom, 0); + if (!doc) + goto cleanup; + + xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); + VIR_FREE(doc); + if (!xml) + goto cleanup; + + /* Attempt to grab our display info */ + for (iter = 0; scheme[iter] != NULL; iter++) { + /* Create our XPATH lookup for the current display */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@port)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + obj = xmlXPathEval(BAD_CAST xpath, ctxt);
we prefer virXPathLong which does all the checks and returns just long.
+ VIR_FREE(xpath); + if (obj == NULL || obj->type != XPATH_STRING || + obj->stringval == NULL) { + if (obj) { + /* Clean up memory */ + xmlXPathFreeObject(obj); + obj = NULL; + } + continue; + } + + /* If there was a port number, then the guest uses the current scheme */ + if (obj->stringval[0] != 0) { + + /* Make sure this is a valid port number */ + if (virStrToLong_i((const char *)obj->stringval, NULL, 10, &port)) { + vshError(ctl, _("Unable to interpret '%s' as a port number."), + obj->stringval); + goto cleanup; + } + + if (port < 0)
...[1] this:
+ vshError(ctl, _("Invalid port '%d', guest likely not started."), + port); + goto cleanup; + } + + /* Clean up memory */ + xmlXPathFreeObject(obj); + obj = NULL; + + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@listen)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + obj = xmlXPathEval(BAD_CAST xpath, ctxt); + VIR_FREE(xpath); + + /* VNC protocol handlers take their port number as X - 5900 */ + if (scheme[iter] == "vnc")
if (STREQ(scheme[iter], "vnc"))
+ port -= 5900; + + if (obj == NULL || obj->type != XPATH_STRING || + obj->stringval == NULL || obj->stringval[0] == 0 || + STREQ((const char *)obj->stringval, "0.0.0.0")) { + vshPrint(ctl, "%s://localhost:%d\n", scheme[iter], port); + } else { + vshPrint(ctl, "%s://%s:%d\n", scheme[iter], + (const char *)obj->stringval, port); + } + + /* Clean up memory */ + xmlXPathFreeObject(obj); + obj = NULL; + + /* We got what we came for so return successfully */ + ret = true; + break; + } + + } + +cleanup: + xmlXPathFreeObject(obj); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + virDomainFree(dom); + return ret; +} + +/* * "vncdisplay" command */ static const vshCmdInfo info_vncdisplay[] = { @@ -17702,6 +17829,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-disk", cmdDetachDisk, opts_detach_disk, info_detach_disk, 0}, {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, + {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0},
And you need to update tools/virsh.pod as well. I think using libvirt wrappers for XML/XPath will make this patch much smaller. I like the idea though. Michal

v2: - Refactored to use virBuffer - Refactored to use virXPath wrappers - Added support for tls-port and password for SPICE - Added optional flag to disable SPICE password to the URI - Added support for RDP - Fixed code reviews Add a new 'domdisplay' command that provides a URI for VNC, SPICE and RDP connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE and RDP. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks. For SPICE connections it supports secure and unsecure channels and optionally providing the password for the SPICE channel. Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 6 ++ 2 files changed, 179 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 0354822..da80477 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13827,6 +13827,178 @@ cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* + * "display" command + */ +static const vshCmdInfo info_domdisplay[] = { + {"help", N_("domain display connection URI")}, + {"desc", N_("Output the IP address and port number for the graphical display.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domdisplay[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"include-password", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("includes the password into the connection URI if available")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) +{ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + virDomainPtr dom; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool ret = false; + char *doc; + char *xpath; + char *listen_addr; + int port, tls_port = 0; + char *passwd = NULL; + const char *scheme[] = { "vnc", "spice", "rdp", NULL }; + int iter = 0; + int tmp; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (!virDomainIsActive(dom)) { + vshError(ctl, _("Domain is not running")); + goto cleanup; + } + + doc = virDomainGetXMLDesc(dom, 0); + if (!doc) + goto cleanup; + + xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); + VIR_FREE(doc); + if (!xml) + goto cleanup; + + /* Attempt to grab our display info */ + for (iter = 0; scheme[iter] != NULL; iter++) { + /* Create our XPATH lookup for the current display's port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@port)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the port number for the current graphics scheme */ + tmp = virXPathInt(xpath, ctxt, &port); + VIR_FREE(xpath); + + /* If there is no port number for this type, then jump to the next + * scheme + */ + if (tmp) + continue; + + /* Create our XPATH lookup for the current display's address */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@listen)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the listening addr if set for the current + * graphics scheme + */ + listen_addr = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + + /* Per scheme data mangling */ + if (STREQ(scheme[iter], "vnc")) { + /* VNC protocol handlers take their port number as 'port' - 5900 */ + port -= 5900; + } else if (STREQ(scheme[iter], "spice")) { + /* Create our XPATH lookup for the SPICE TLS Port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@tlsPort)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the TLS port number for SPICE */ + tmp = virXPathInt(xpath, ctxt, &tls_port); + VIR_FREE(xpath); + if (tmp) + tls_port = 0; + + if (vshCommandOptBool(cmd, "daemon")) { + /* Create our XPATH lookup for the SPICE password */ + virAsprintf(&xpath, "string(/domain/devices/graphics" + "[@type='%s']/@passwd)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the SPICE password */ + passwd = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + } + } + + /* Build up the full URI, starting with the scheme */ + virBufferAsprintf(&buf, "%s://", scheme[iter]); + + /* Then host name or IP */ + if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0")) + virBufferAddLit(&buf, "localhost"); + else + virBufferAsprintf(&buf, "%s", listen_addr); + + /* Clean up our memory */ + if (listen_addr) + VIR_FREE(listen_addr); + + /* Add the port */ + if (STREQ(scheme[iter], "spice")) + virBufferAsprintf(&buf, "?port=%d", port); + else + virBufferAsprintf(&buf, ":%d", port); + + /* TLS Port */ + if (tls_port) + virBufferAsprintf(&buf, "&tls-port=%d", tls_port); + + /* Password */ + if (passwd) { + virBufferAsprintf(&buf, "&password=%s", passwd); + VIR_FREE(passwd); + } + + /* Ensure we can print our URI */ + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to create display URI")); + goto cleanup; + } + + /* Print out our full URI */ + output = virBufferContentAndReset(&buf); + vshPrint(ctl, "%s", output); + VIR_FREE(output); + + /* We got what we came for so return successfully */ + ret = true; + break; + } + +cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + virDomainFree(dom); + return ret; +} + +/* * "vncdisplay" command */ static const vshCmdInfo info_vncdisplay[] = { @@ -17974,6 +18146,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-disk", cmdDetachDisk, opts_detach_disk, info_detach_disk, 0}, {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, + {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index f83a29d..558105f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -820,6 +820,12 @@ I<size> is a scaled integer (see B<NOTES> above) which defaults to KiB "B" to get bytes (note that for historical reasons, this differs from B<vol-resize> which defaults to bytes without a suffix). +=item B<domdisplay> I<domain-id> [I<--include-password>] + +Output a URI which can be used to connect to the graphical display of the +domain via VNC, SPICE or RDP. If I<--include-password> is specified, the +SPICE channel password will be included in the URI. + =item B<dominfo> I<domain-id> Returns basic information about the domain. -- 1.7.3.4

On Sun, Jun 24, 2012 at 4:14 PM, Doug Goldstein <cardoe@cardoe.com> wrote:
v2: - Refactored to use virBuffer - Refactored to use virXPath wrappers - Added support for tls-port and password for SPICE - Added optional flag to disable SPICE password to the URI - Added support for RDP - Fixed code reviews
Add a new 'domdisplay' command that provides a URI for VNC, SPICE and RDP connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE and RDP. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks. For SPICE connections it supports secure and unsecure channels and optionally providing the password for the SPICE channel.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 6 ++ 2 files changed, 179 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 0354822..da80477 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13827,6 +13827,178 @@ cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }
/* + * "display" command + */ +static const vshCmdInfo info_domdisplay[] = { + {"help", N_("domain display connection URI")}, + {"desc", N_("Output the IP address and port number for the graphical display.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domdisplay[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"include-password", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("includes the password into the connection URI if available")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) +{ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + virDomainPtr dom; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool ret = false; + char *doc; + char *xpath; + char *listen_addr; + int port, tls_port = 0; + char *passwd = NULL; + const char *scheme[] = { "vnc", "spice", "rdp", NULL }; + int iter = 0; + int tmp; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (!virDomainIsActive(dom)) { + vshError(ctl, _("Domain is not running")); + goto cleanup; + } + + doc = virDomainGetXMLDesc(dom, 0); + if (!doc) + goto cleanup; + + xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); + VIR_FREE(doc); + if (!xml) + goto cleanup; + + /* Attempt to grab our display info */ + for (iter = 0; scheme[iter] != NULL; iter++) { + /* Create our XPATH lookup for the current display's port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@port)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the port number for the current graphics scheme */ + tmp = virXPathInt(xpath, ctxt, &port); + VIR_FREE(xpath); + + /* If there is no port number for this type, then jump to the next + * scheme + */ + if (tmp) + continue; + + /* Create our XPATH lookup for the current display's address */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@listen)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the listening addr if set for the current + * graphics scheme + */ + listen_addr = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + + /* Per scheme data mangling */ + if (STREQ(scheme[iter], "vnc")) { + /* VNC protocol handlers take their port number as 'port' - 5900 */ + port -= 5900; + } else if (STREQ(scheme[iter], "spice")) { + /* Create our XPATH lookup for the SPICE TLS Port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@tlsPort)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the TLS port number for SPICE */ + tmp = virXPathInt(xpath, ctxt, &tls_port); + VIR_FREE(xpath); + if (tmp) + tls_port = 0; + + if (vshCommandOptBool(cmd, "daemon")) { + /* Create our XPATH lookup for the SPICE password */ + virAsprintf(&xpath, "string(/domain/devices/graphics" + "[@type='%s']/@passwd)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the SPICE password */ + passwd = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + } + } + + /* Build up the full URI, starting with the scheme */ + virBufferAsprintf(&buf, "%s://", scheme[iter]); + + /* Then host name or IP */ + if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0")) + virBufferAddLit(&buf, "localhost"); + else + virBufferAsprintf(&buf, "%s", listen_addr); + + /* Clean up our memory */ + if (listen_addr) + VIR_FREE(listen_addr); + + /* Add the port */ + if (STREQ(scheme[iter], "spice")) + virBufferAsprintf(&buf, "?port=%d", port); + else + virBufferAsprintf(&buf, ":%d", port); + + /* TLS Port */ + if (tls_port) + virBufferAsprintf(&buf, "&tls-port=%d", tls_port); + + /* Password */ + if (passwd) { + virBufferAsprintf(&buf, "&password=%s", passwd); + VIR_FREE(passwd); + } + + /* Ensure we can print our URI */ + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to create display URI")); + goto cleanup; + } + + /* Print out our full URI */ + output = virBufferContentAndReset(&buf); + vshPrint(ctl, "%s", output); + VIR_FREE(output); + + /* We got what we came for so return successfully */ + ret = true; + break; + } + +cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + virDomainFree(dom); + return ret; +} + +/* * "vncdisplay" command */ static const vshCmdInfo info_vncdisplay[] = { @@ -17974,6 +18146,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-disk", cmdDetachDisk, opts_detach_disk, info_detach_disk, 0}, {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, + {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index f83a29d..558105f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -820,6 +820,12 @@ I<size> is a scaled integer (see B<NOTES> above) which defaults to KiB "B" to get bytes (note that for historical reasons, this differs from B<vol-resize> which defaults to bytes without a suffix).
+=item B<domdisplay> I<domain-id> [I<--include-password>] + +Output a URI which can be used to connect to the graphical display of the +domain via VNC, SPICE or RDP. If I<--include-password> is specified, the +SPICE channel password will be included in the URI. + =item B<dominfo> I<domain-id>
Returns basic information about the domain. -- 1.7.3.4
Nudge for review & potential inclusion into 0.9.13. -- Doug Goldstein

On 24.06.2012 23:14, Doug Goldstein wrote:
v2: - Refactored to use virBuffer - Refactored to use virXPath wrappers - Added support for tls-port and password for SPICE - Added optional flag to disable SPICE password to the URI - Added support for RDP - Fixed code reviews
Add a new 'domdisplay' command that provides a URI for VNC, SPICE and RDP connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE and RDP. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks. For SPICE connections it supports secure and unsecure channels and optionally providing the password for the SPICE channel.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 6 ++ 2 files changed, 179 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 0354822..da80477 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13827,6 +13827,178 @@ cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }
/* + * "display" command + */
In fact it's domdisplay command.
+static const vshCmdInfo info_domdisplay[] = { + {"help", N_("domain display connection URI")}, + {"desc", N_("Output the IP address and port number for the graphical display.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domdisplay[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"include-password", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("includes the password into the connection URI if available")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) +{ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + virDomainPtr dom; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool ret = false; + char *doc; + char *xpath; + char *listen_addr; + int port, tls_port = 0; + char *passwd = NULL; + const char *scheme[] = { "vnc", "spice", "rdp", NULL }; + int iter = 0; + int tmp; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (!virDomainIsActive(dom)) { + vshError(ctl, _("Domain is not running")); + goto cleanup; + } + + doc = virDomainGetXMLDesc(dom, 0); + if (!doc) + goto cleanup; + + xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); + VIR_FREE(doc); + if (!xml) + goto cleanup; + + /* Attempt to grab our display info */ + for (iter = 0; scheme[iter] != NULL; iter++) { + /* Create our XPATH lookup for the current display's port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@port)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the port number for the current graphics scheme */ + tmp = virXPathInt(xpath, ctxt, &port); + VIR_FREE(xpath); + + /* If there is no port number for this type, then jump to the next + * scheme + */ + if (tmp) + continue; + + /* Create our XPATH lookup for the current display's address */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@listen)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the listening addr if set for the current + * graphics scheme + */ + listen_addr = virXPathString(xpath, ctxt); + VIR_FREE(xpath); +
Adding a blank line ^^
+ /* Per scheme data mangling */ + if (STREQ(scheme[iter], "vnc")) { + /* VNC protocol handlers take their port number as 'port' - 5900 */ + port -= 5900; + } else if (STREQ(scheme[iter], "spice")) { + /* Create our XPATH lookup for the SPICE TLS Port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@tlsPort)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the TLS port number for SPICE */ + tmp = virXPathInt(xpath, ctxt, &tls_port); + VIR_FREE(xpath); + if (tmp) + tls_port = 0; + + if (vshCommandOptBool(cmd, "daemon")) {
s/daemon/include-password/
+ /* Create our XPATH lookup for the SPICE password */ + virAsprintf(&xpath, "string(/domain/devices/graphics" + "[@type='%s']/@passwd)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the SPICE password */ + passwd = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + } + } + + /* Build up the full URI, starting with the scheme */ + virBufferAsprintf(&buf, "%s://", scheme[iter]); + + /* Then host name or IP */ + if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0")) + virBufferAddLit(&buf, "localhost"); + else + virBufferAsprintf(&buf, "%s", listen_addr); + + /* Clean up our memory */ + if (listen_addr) + VIR_FREE(listen_addr);
This check is redundant. free(NULL) is safe.
+ + /* Add the port */ + if (STREQ(scheme[iter], "spice")) + virBufferAsprintf(&buf, "?port=%d", port); + else + virBufferAsprintf(&buf, ":%d", port); + + /* TLS Port */ + if (tls_port) + virBufferAsprintf(&buf, "&tls-port=%d", tls_port); + + /* Password */ + if (passwd) { + virBufferAsprintf(&buf, "&password=%s", passwd); + VIR_FREE(passwd); + } + + /* Ensure we can print our URI */ + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to create display URI")); + goto cleanup; + } + + /* Print out our full URI */ + output = virBufferContentAndReset(&buf); + vshPrint(ctl, "%s", output); + VIR_FREE(output); + + /* We got what we came for so return successfully */ + ret = true; + break; + } + +cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + virDomainFree(dom); + return ret; +} + +/* * "vncdisplay" command */ static const vshCmdInfo info_vncdisplay[] = { @@ -17974,6 +18146,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-disk", cmdDetachDisk, opts_detach_disk, info_detach_disk, 0}, {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, + {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index f83a29d..558105f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -820,6 +820,12 @@ I<size> is a scaled integer (see B<NOTES> above) which defaults to KiB "B" to get bytes (note that for historical reasons, this differs from B<vol-resize> which defaults to bytes without a suffix).
+=item B<domdisplay> I<domain-id> [I<--include-password>] + +Output a URI which can be used to connect to the graphical display of the +domain via VNC, SPICE or RDP. If I<--include-password> is specified, the +SPICE channel password will be included in the URI. + =item B<dominfo> I<domain-id>
Returns basic information about the domain.
Otherwise looking good. ACK. I would pushed this but we are in freeze phase. On the other hand - v1 was sent before freeze. Combined with fact that number of patches sent into the list has fallen down I guess we can push this in. But I'd like to hear a second opinion. Michal

On Wed, Jun 27, 2012 at 03:21:48PM +0200, Michal Privoznik wrote:
On 24.06.2012 23:14, Doug Goldstein wrote:
v2: - Refactored to use virBuffer - Refactored to use virXPath wrappers - Added support for tls-port and password for SPICE - Added optional flag to disable SPICE password to the URI - Added support for RDP - Fixed code reviews
Add a new 'domdisplay' command that provides a URI for VNC, SPICE and RDP connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE and RDP. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks. For SPICE connections it supports secure and unsecure channels and optionally providing the password for the SPICE channel.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 6 ++ 2 files changed, 179 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 0354822..da80477 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13827,6 +13827,178 @@ cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }
/* + * "display" command + */
In fact it's domdisplay command.
+static const vshCmdInfo info_domdisplay[] = { + {"help", N_("domain display connection URI")}, + {"desc", N_("Output the IP address and port number for the graphical display.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domdisplay[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"include-password", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("includes the password into the connection URI if available")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) +{ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + virDomainPtr dom; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool ret = false; + char *doc; + char *xpath; + char *listen_addr; + int port, tls_port = 0; + char *passwd = NULL; + const char *scheme[] = { "vnc", "spice", "rdp", NULL }; + int iter = 0; + int tmp; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (!virDomainIsActive(dom)) { + vshError(ctl, _("Domain is not running")); + goto cleanup; + } + + doc = virDomainGetXMLDesc(dom, 0); + if (!doc) + goto cleanup; + + xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); + VIR_FREE(doc); + if (!xml) + goto cleanup; + + /* Attempt to grab our display info */ + for (iter = 0; scheme[iter] != NULL; iter++) { + /* Create our XPATH lookup for the current display's port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@port)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the port number for the current graphics scheme */ + tmp = virXPathInt(xpath, ctxt, &port); + VIR_FREE(xpath); + + /* If there is no port number for this type, then jump to the next + * scheme + */ + if (tmp) + continue; + + /* Create our XPATH lookup for the current display's address */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@listen)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the listening addr if set for the current + * graphics scheme + */ + listen_addr = virXPathString(xpath, ctxt); + VIR_FREE(xpath); +
Adding a blank line ^^
+ /* Per scheme data mangling */ + if (STREQ(scheme[iter], "vnc")) { + /* VNC protocol handlers take their port number as 'port' - 5900 */ + port -= 5900; + } else if (STREQ(scheme[iter], "spice")) { + /* Create our XPATH lookup for the SPICE TLS Port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@tlsPort)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the TLS port number for SPICE */ + tmp = virXPathInt(xpath, ctxt, &tls_port); + VIR_FREE(xpath); + if (tmp) + tls_port = 0; + + if (vshCommandOptBool(cmd, "daemon")) {
s/daemon/include-password/
+ /* Create our XPATH lookup for the SPICE password */ + virAsprintf(&xpath, "string(/domain/devices/graphics" + "[@type='%s']/@passwd)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the SPICE password */ + passwd = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + } + } + + /* Build up the full URI, starting with the scheme */ + virBufferAsprintf(&buf, "%s://", scheme[iter]); + + /* Then host name or IP */ + if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0")) + virBufferAddLit(&buf, "localhost"); + else + virBufferAsprintf(&buf, "%s", listen_addr); + + /* Clean up our memory */ + if (listen_addr) + VIR_FREE(listen_addr);
This check is redundant. free(NULL) is safe.
+ + /* Add the port */ + if (STREQ(scheme[iter], "spice")) + virBufferAsprintf(&buf, "?port=%d", port); + else + virBufferAsprintf(&buf, ":%d", port); + + /* TLS Port */ + if (tls_port) + virBufferAsprintf(&buf, "&tls-port=%d", tls_port); + + /* Password */ + if (passwd) { + virBufferAsprintf(&buf, "&password=%s", passwd); + VIR_FREE(passwd); + } + + /* Ensure we can print our URI */ + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to create display URI")); + goto cleanup; + } + + /* Print out our full URI */ + output = virBufferContentAndReset(&buf); + vshPrint(ctl, "%s", output); + VIR_FREE(output); + + /* We got what we came for so return successfully */ + ret = true; + break; + } + +cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + virDomainFree(dom); + return ret; +} + +/* * "vncdisplay" command */ static const vshCmdInfo info_vncdisplay[] = { @@ -17974,6 +18146,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-disk", cmdDetachDisk, opts_detach_disk, info_detach_disk, 0}, {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, + {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index f83a29d..558105f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -820,6 +820,12 @@ I<size> is a scaled integer (see B<NOTES> above) which defaults to KiB "B" to get bytes (note that for historical reasons, this differs from B<vol-resize> which defaults to bytes without a suffix).
+=item B<domdisplay> I<domain-id> [I<--include-password>] + +Output a URI which can be used to connect to the graphical display of the +domain via VNC, SPICE or RDP. If I<--include-password> is specified, the +SPICE channel password will be included in the URI. + =item B<dominfo> I<domain-id>
Returns basic information about the domain.
Otherwise looking good. ACK. I would pushed this but we are in freeze phase. On the other hand - v1 was sent before freeze. Combined with fact that number of patches sent into the list has fallen down I guess we can push this in. But I'd like to hear a second opinion.
it only touches virsh, so the risks are limited. And overall a new command is really low risk too, so ACK, let's push it now. I will probably make the rc2 in 12hours if you could push before fine :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 27.06.2012 15:28, Daniel Veillard wrote:
On Wed, Jun 27, 2012 at 03:21:48PM +0200, Michal Privoznik wrote:
On 24.06.2012 23:14, Doug Goldstein wrote:
v2: - Refactored to use virBuffer - Refactored to use virXPath wrappers - Added support for tls-port and password for SPICE - Added optional flag to disable SPICE password to the URI - Added support for RDP - Fixed code reviews
Add a new 'domdisplay' command that provides a URI for VNC, SPICE and RDP connections. Presently the 'vncdisplay' command provides you with the port info that QEMU is listening on but there is no counterpart for SPICE and RDP. Additionally this provides you with the bind address as specified in the XML, which the existing 'vncdisplay' lacks. For SPICE connections it supports secure and unsecure channels and optionally providing the password for the SPICE channel.
Signed-off-by: Doug Goldstein <cardoe@cardoe.com> --- tools/virsh.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 6 ++ 2 files changed, 179 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 0354822..da80477 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13827,6 +13827,178 @@ cmdSysinfo (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) }
/* + * "display" command + */
In fact it's domdisplay command.
+static const vshCmdInfo info_domdisplay[] = { + {"help", N_("domain display connection URI")}, + {"desc", N_("Output the IP address and port number for the graphical display.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domdisplay[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"include-password", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("includes the password into the connection URI if available")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) +{ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + virDomainPtr dom; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool ret = false; + char *doc; + char *xpath; + char *listen_addr; + int port, tls_port = 0; + char *passwd = NULL; + const char *scheme[] = { "vnc", "spice", "rdp", NULL }; + int iter = 0; + int tmp; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (!virDomainIsActive(dom)) { + vshError(ctl, _("Domain is not running")); + goto cleanup; + } + + doc = virDomainGetXMLDesc(dom, 0); + if (!doc) + goto cleanup; + + xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt); + VIR_FREE(doc); + if (!xml) + goto cleanup; + + /* Attempt to grab our display info */ + for (iter = 0; scheme[iter] != NULL; iter++) { + /* Create our XPATH lookup for the current display's port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@port)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the port number for the current graphics scheme */ + tmp = virXPathInt(xpath, ctxt, &port); + VIR_FREE(xpath); + + /* If there is no port number for this type, then jump to the next + * scheme + */ + if (tmp) + continue; + + /* Create our XPATH lookup for the current display's address */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@listen)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the listening addr if set for the current + * graphics scheme + */ + listen_addr = virXPathString(xpath, ctxt); + VIR_FREE(xpath); +
Adding a blank line ^^
+ /* Per scheme data mangling */ + if (STREQ(scheme[iter], "vnc")) { + /* VNC protocol handlers take their port number as 'port' - 5900 */ + port -= 5900; + } else if (STREQ(scheme[iter], "spice")) { + /* Create our XPATH lookup for the SPICE TLS Port */ + virAsprintf(&xpath, "string(/domain/devices/graphics[@type='%s']" + "/@tlsPort)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the TLS port number for SPICE */ + tmp = virXPathInt(xpath, ctxt, &tls_port); + VIR_FREE(xpath); + if (tmp) + tls_port = 0; + + if (vshCommandOptBool(cmd, "daemon")) {
s/daemon/include-password/
+ /* Create our XPATH lookup for the SPICE password */ + virAsprintf(&xpath, "string(/domain/devices/graphics" + "[@type='%s']/@passwd)", scheme[iter]); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + + /* Attempt to get the SPICE password */ + passwd = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + } + } + + /* Build up the full URI, starting with the scheme */ + virBufferAsprintf(&buf, "%s://", scheme[iter]); + + /* Then host name or IP */ + if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0")) + virBufferAddLit(&buf, "localhost"); + else + virBufferAsprintf(&buf, "%s", listen_addr); + + /* Clean up our memory */ + if (listen_addr) + VIR_FREE(listen_addr);
This check is redundant. free(NULL) is safe.
+ + /* Add the port */ + if (STREQ(scheme[iter], "spice")) + virBufferAsprintf(&buf, "?port=%d", port); + else + virBufferAsprintf(&buf, ":%d", port); + + /* TLS Port */ + if (tls_port) + virBufferAsprintf(&buf, "&tls-port=%d", tls_port); + + /* Password */ + if (passwd) { + virBufferAsprintf(&buf, "&password=%s", passwd); + VIR_FREE(passwd); + } + + /* Ensure we can print our URI */ + if (virBufferError(&buf)) { + vshPrint(ctl, "%s", _("Failed to create display URI")); + goto cleanup; + } + + /* Print out our full URI */ + output = virBufferContentAndReset(&buf); + vshPrint(ctl, "%s", output); + VIR_FREE(output); + + /* We got what we came for so return successfully */ + ret = true; + break; + } + +cleanup: + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + virDomainFree(dom); + return ret; +} + +/* * "vncdisplay" command */ static const vshCmdInfo info_vncdisplay[] = { @@ -17974,6 +18146,7 @@ static const vshCmdDef domManagementCmds[] = { {"detach-disk", cmdDetachDisk, opts_detach_disk, info_detach_disk, 0}, {"detach-interface", cmdDetachInterface, opts_detach_interface, info_detach_interface, 0}, + {"domdisplay", cmdDomDisplay, opts_domdisplay, info_domdisplay, 0}, {"domid", cmdDomid, opts_domid, info_domid, 0}, {"domif-setlink", cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 0}, {"domiftune", cmdDomIftune, opts_domiftune, info_domiftune, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index f83a29d..558105f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -820,6 +820,12 @@ I<size> is a scaled integer (see B<NOTES> above) which defaults to KiB "B" to get bytes (note that for historical reasons, this differs from B<vol-resize> which defaults to bytes without a suffix).
+=item B<domdisplay> I<domain-id> [I<--include-password>] + +Output a URI which can be used to connect to the graphical display of the +domain via VNC, SPICE or RDP. If I<--include-password> is specified, the +SPICE channel password will be included in the URI. + =item B<dominfo> I<domain-id>
Returns basic information about the domain.
Otherwise looking good. ACK. I would pushed this but we are in freeze phase. On the other hand - v1 was sent before freeze. Combined with fact that number of patches sent into the list has fallen down I guess we can push this in. But I'd like to hear a second opinion.
it only touches virsh, so the risks are limited. And overall a new command is really low risk too, so ACK, let's push it now.
I will probably make the rc2 in 12hours if you could push before fine :-)
Daniel
Okay, pushed now. Thanks. Michal
participants (5)
-
Christophe Fergeau
-
Daniel P. Berrange
-
Daniel Veillard
-
Doug Goldstein
-
Michal Privoznik