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

22 Jun 2012

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

    

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

Michal Privoznik