[libvirt PATCH v3 07/16] virsh: refactor/split cmdDomDisplay()

marcandre.lureau at redhat.com marcandre.lureau at redhat.com
Wed Dec 22 19:43:36 UTC 2021


From: Marc-André Lureau <marcandre.lureau at redhat.com>

The function is large and quite complex. Move the inner loop for each
scheme in a separate function cmdDomDisplayScheme().

Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
---
 tools/virsh-domain.c | 341 +++++++++++++++++++++----------------------
 1 file changed, 169 insertions(+), 172 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f086c2dd4b58..002cfc4be6af 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11507,209 +11507,213 @@ static const vshCmdOptDef opts_domdisplay[] = {
 };
 
 static bool
-cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
+cmdDomDisplayScheme(vshControl *ctl, const char *scheme,
+                    xmlXPathContext *ctxt, virBuffer *buf)
 {
-    g_autoptr(xmlDoc) xml = NULL;
-    g_autoptr(xmlXPathContext) ctxt = NULL;
-    g_autoptr(virshDomain) dom = NULL;
-    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-    bool ret = false;
-    char *xpath = NULL;
-    char *listen_addr = NULL;
-    int port, tls_port = 0;
-    char *type_conn = NULL;
-    char *sockpath = NULL;
-    char *passwd = NULL;
-    char *output = NULL;
-    const char *scheme[] = { "vnc", "spice", "rdp", NULL };
-    const char *type = NULL;
-    int iter = 0;
-    int tmp;
-    int flags = 0;
-    bool params = false;
-    bool all = vshCommandOptBool(cmd, "all");
+    g_autofree char *xpath = NULL;
+    g_autofree char *listen_addr = NULL;
+    g_autofree char *type_conn = NULL;
+    g_autofree char *sockpath = NULL;
+    g_autofree char *passwd = NULL;
     const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/%s)";
     virSocketAddr addr;
+    int port, tls_port = 0;
+    bool params = false;
+    int tmp;
 
-    VSH_EXCLUSIVE_OPTIONS("all", "type");
+    /* Create our XPATH lookup for the current display's port */
+    xpath = g_strdup_printf(xpath_fmt, scheme, "@port");
 
-    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-        return false;
+    /* Attempt to get the port number for the current graphics scheme */
+    tmp = virXPathInt(xpath, ctxt, &port);
+    VIR_FREE(xpath);
 
-    if (!virDomainIsActive(dom)) {
-        vshError(ctl, _("Domain is not running"));
-        goto cleanup;
-    }
+    /* If there is no port number for this type, then jump to the next
+     * scheme */
+    if (tmp)
+        port = 0;
 
-    if (vshCommandOptBool(cmd, "include-password"))
-        flags |= VIR_DOMAIN_XML_SECURE;
+    /* Create our XPATH lookup for TLS Port (automatically skipped
+     * for unsupported schemes */
+    xpath = g_strdup_printf(xpath_fmt, scheme, "@tlsPort");
 
-    if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
-        goto cleanup;
+    /* Attempt to get the TLS port number */
+    tmp = virXPathInt(xpath, ctxt, &tls_port);
+    VIR_FREE(xpath);
+    if (tmp)
+        tls_port = 0;
 
-    if (virshDomainGetXMLFromDom(ctl, dom, flags, &xml, &ctxt) < 0)
-        goto cleanup;
+    /* Create our XPATH lookup for the current display's address */
+    xpath = g_strdup_printf(xpath_fmt, scheme, "@listen");
 
-    /* Attempt to grab our display info */
-    for (iter = 0; scheme[iter] != NULL; iter++) {
-        /* Particular scheme requested */
-        if (!all && type && STRNEQ(type, scheme[iter]))
-            continue;
+    /* Attempt to get the listening addr if set for the current
+     * graphics scheme */
+    listen_addr = virXPathString(xpath, ctxt);
+    VIR_FREE(xpath);
 
-        /* Create our XPATH lookup for the current display's port */
-        VIR_FREE(xpath);
-        xpath = g_strdup_printf(xpath_fmt, scheme[iter], "@port");
+    /* Create our XPATH lookup for the current spice type. */
+    xpath = g_strdup_printf(xpath_fmt, scheme, "listen/@type");
 
-        /* Attempt to get the port number for the current graphics scheme */
-        tmp = virXPathInt(xpath, ctxt, &port);
-        VIR_FREE(xpath);
+    /* Attempt to get the type of spice connection */
+    type_conn = virXPathString(xpath, ctxt);
+    VIR_FREE(xpath);
 
-        /* If there is no port number for this type, then jump to the next
-         * scheme */
-        if (tmp)
-            port = 0;
+    if (STREQ_NULLABLE(type_conn, "socket")) {
+        if (!sockpath) {
+            xpath = g_strdup_printf(xpath_fmt, scheme, "listen/@socket");
 
-        /* Create our XPATH lookup for TLS Port (automatically skipped
-         * for unsupported schemes */
-        xpath = g_strdup_printf(xpath_fmt, scheme[iter], "@tlsPort");
+            sockpath = virXPathString(xpath, ctxt);
 
-        /* Attempt to get the TLS port number */
-        tmp = virXPathInt(xpath, ctxt, &tls_port);
-        VIR_FREE(xpath);
-        if (tmp)
-            tls_port = 0;
+            VIR_FREE(xpath);
+        }
+    }
 
-        /* Create our XPATH lookup for the current display's address */
-        xpath = g_strdup_printf(xpath_fmt, scheme[iter], "@listen");
+    if (!port && !tls_port && !sockpath)
+        return false;
+
+    if (!listen_addr) {
+        /* The subelement address - <listen address='xyz'/> -
+         * *should* have been automatically backfilled into its
+         * parent <graphics listen='xyz'> (which we just tried to
+         * retrieve into listen_addr above) but in some cases it
+         * isn't, so we also do an explicit check for the
+         * subelement (which, by the way, doesn't exist on libvirt
+         * < 0.9.4, so we really do need to check both places)
+         */
+        xpath = g_strdup_printf(xpath_fmt, scheme, "listen/@address");
 
-        /* Attempt to get the listening addr if set for the current
-         * graphics scheme */
-        VIR_FREE(listen_addr);
         listen_addr = virXPathString(xpath, ctxt);
         VIR_FREE(xpath);
+    } else {
+        /* If listen_addr is 0.0.0.0 or [::] we should try to parse URI and set
+         * listen_addr based on current URI. */
+        if (virSocketAddrParse(&addr, listen_addr, AF_UNSPEC) > 0 &&
+            virSocketAddrIsWildcard(&addr)) {
+
+            virConnectPtr conn = ((virshControl *)(ctl->privData))->conn;
+            char *uriStr = virConnectGetURI(conn);
+            virURI *uri = NULL;
+
+            if (uriStr) {
+                uri = virURIParse(uriStr);
+                VIR_FREE(uriStr);
+            }
 
-        /* Create our XPATH lookup for the current spice type. */
-        xpath = g_strdup_printf(xpath_fmt, scheme[iter], "listen/@type");
+            /* It's safe to free the listen_addr even if parsing of URI
+             * fails, if there is no listen_addr we will print "localhost". */
+            VIR_FREE(listen_addr);
 
-        /* Attempt to get the type of spice connection */
-        VIR_FREE(type_conn);
-        type_conn = virXPathString(xpath, ctxt);
-        VIR_FREE(xpath);
+            if (uri) {
+                listen_addr = g_strdup(uri->server);
+                virURIFree(uri);
+            }
+        }
+    }
 
-        if (STREQ_NULLABLE(type_conn, "socket")) {
-            if (!sockpath) {
-                xpath = g_strdup_printf(xpath_fmt, scheme[iter], "listen/@socket");
+    /* We can query this info for all the graphics types since we'll
+     * get nothing for the unsupported ones (just rdp for now).
+     * Also the parameter '--include-password' was already taken
+     * care of when getting the XML */
 
-                sockpath = virXPathString(xpath, ctxt);
+    /* Create our XPATH lookup for the password */
+    xpath = g_strdup_printf(xpath_fmt, scheme, "@passwd");
 
-                VIR_FREE(xpath);
-            }
+    /* Attempt to get the password */
+    passwd = virXPathString(xpath, ctxt);
+    VIR_FREE(xpath);
+
+    /* Build up the full URI, starting with the scheme */
+    if (sockpath)
+        virBufferAsprintf(buf, "%s+unix://", scheme);
+    else
+        virBufferAsprintf(buf, "%s://", scheme);
+
+    /* There is no user, so just append password if there's any */
+    if (STREQ(scheme, "vnc") && passwd)
+        virBufferAsprintf(buf, ":%s@", passwd);
+
+    /* Then host name or IP */
+    if (!listen_addr && !sockpath)
+        virBufferAddLit(buf, "localhost");
+    else if (!sockpath && strchr(listen_addr, ':'))
+        virBufferAsprintf(buf, "[%s]", listen_addr);
+    else if (sockpath)
+        virBufferAsprintf(buf, "%s", sockpath);
+    else
+        virBufferAsprintf(buf, "%s", listen_addr);
+
+    /* Add the port */
+    if (port) {
+        if (STREQ(scheme, "vnc")) {
+            /* VNC protocol handlers take their port number as
+             * 'port' - 5900 */
+            port -= 5900;
         }
 
-        if (!port && !tls_port && !sockpath)
-            continue;
+        virBufferAsprintf(buf, ":%d", port);
+    }
 
-        if (!listen_addr) {
-            /* The subelement address - <listen address='xyz'/> -
-             * *should* have been automatically backfilled into its
-             * parent <graphics listen='xyz'> (which we just tried to
-             * retrieve into listen_addr above) but in some cases it
-             * isn't, so we also do an explicit check for the
-             * subelement (which, by the way, doesn't exist on libvirt
-             * < 0.9.4, so we really do need to check both places)
-             */
-            xpath = g_strdup_printf(xpath_fmt, scheme[iter], "listen/@address");
-
-            listen_addr = virXPathString(xpath, ctxt);
-            VIR_FREE(xpath);
-        } else {
-            /* If listen_addr is 0.0.0.0 or [::] we should try to parse URI and set
-             * listen_addr based on current URI. */
-            if (virSocketAddrParse(&addr, listen_addr, AF_UNSPEC) > 0 &&
-                virSocketAddrIsWildcard(&addr)) {
-
-                virConnectPtr conn = ((virshControl *)(ctl->privData))->conn;
-                char *uriStr = virConnectGetURI(conn);
-                virURI *uri = NULL;
-
-                if (uriStr) {
-                    uri = virURIParse(uriStr);
-                    VIR_FREE(uriStr);
-                }
+    /* TLS Port */
+    if (tls_port) {
+        virBufferAsprintf(buf,
+                          "?tls-port=%d",
+                          tls_port);
+        params = true;
+    }
 
-                /* It's safe to free the listen_addr even if parsing of URI
-                 * fails, if there is no listen_addr we will print "localhost". */
-                VIR_FREE(listen_addr);
+    if (STREQ(scheme, "spice") && passwd) {
+        virBufferAsprintf(buf,
+                          "%spassword=%s",
+                          params ? "&" : "?",
+                          passwd);
+        params = true;
+    }
 
-                if (uri) {
-                    listen_addr = g_strdup(uri->server);
-                    virURIFree(uri);
-                }
-            }
-        }
+    return true;
+}
 
-        /* We can query this info for all the graphics types since we'll
-         * get nothing for the unsupported ones (just rdp for now).
-         * Also the parameter '--include-password' was already taken
-         * care of when getting the XML */
+static bool
+cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
+{
+    g_autoptr(xmlDoc) xml = NULL;
+    g_autoptr(xmlXPathContext) ctxt = NULL;
+    g_autoptr(virshDomain) dom = NULL;
+    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+    g_autofree char *output = NULL;
+    bool ret = false;
+    const char *scheme[] = { "vnc", "spice", "rdp", NULL };
+    const char *type = NULL;
+    int iter = 0;
+    int flags = 0;
+    bool all = vshCommandOptBool(cmd, "all");
 
-        /* Create our XPATH lookup for the password */
-        xpath = g_strdup_printf(xpath_fmt, scheme[iter], "@passwd");
+    VSH_EXCLUSIVE_OPTIONS("all", "type");
 
-        /* Attempt to get the password */
-        VIR_FREE(passwd);
-        passwd = virXPathString(xpath, ctxt);
-        VIR_FREE(xpath);
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
 
-        /* Build up the full URI, starting with the scheme */
-        if (sockpath)
-            virBufferAsprintf(&buf, "%s+unix://", scheme[iter]);
-        else
-            virBufferAsprintf(&buf, "%s://", scheme[iter]);
-
-        /* There is no user, so just append password if there's any */
-        if (STREQ(scheme[iter], "vnc") && passwd)
-            virBufferAsprintf(&buf, ":%s@", passwd);
-
-        /* Then host name or IP */
-        if (!listen_addr && !sockpath)
-            virBufferAddLit(&buf, "localhost");
-        else if (!sockpath && strchr(listen_addr, ':'))
-            virBufferAsprintf(&buf, "[%s]", listen_addr);
-        else if (sockpath)
-            virBufferAsprintf(&buf, "%s", sockpath);
-        else
-            virBufferAsprintf(&buf, "%s", listen_addr);
+    if (!virDomainIsActive(dom)) {
+        vshError(ctl, _("Domain is not running"));
+        return false;
+    }
 
-        /* Free socket to prepare the pointer for the next iteration */
-        VIR_FREE(sockpath);
+    if (vshCommandOptBool(cmd, "include-password"))
+        flags |= VIR_DOMAIN_XML_SECURE;
 
-        /* Add the port */
-        if (port) {
-            if (STREQ(scheme[iter], "vnc")) {
-                /* VNC protocol handlers take their port number as
-                 * 'port' - 5900 */
-                port -= 5900;
-            }
+    if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
+        return false;
 
-            virBufferAsprintf(&buf, ":%d", port);
-        }
+    if (virshDomainGetXMLFromDom(ctl, dom, flags, &xml, &ctxt) < 0)
+        return false;
 
-        /* TLS Port */
-        if (tls_port) {
-            virBufferAsprintf(&buf,
-                              "?tls-port=%d",
-                              tls_port);
-            params = true;
-        }
+    /* Attempt to grab our display info */
+    for (iter = 0; scheme[iter] != NULL; iter++) {
+        /* Particular scheme requested */
+        if (!all && type && STRNEQ(type, scheme[iter]))
+            continue;
 
-        if (STREQ(scheme[iter], "spice") && passwd) {
-            virBufferAsprintf(&buf,
-                              "%spassword=%s",
-                              params ? "&" : "?",
-                              passwd);
-            params = true;
-        }
+        if (!cmdDomDisplayScheme(ctl, scheme[iter], ctxt, &buf))
+            continue;
 
         /* Print out our full URI */
         VIR_FREE(output);
@@ -11730,13 +11734,6 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
             vshError(ctl, _("No graphical display found"));
     }
 
- cleanup:
-    VIR_FREE(xpath);
-    VIR_FREE(type_conn);
-    VIR_FREE(sockpath);
-    VIR_FREE(passwd);
-    VIR_FREE(listen_addr);
-    VIR_FREE(output);
     return ret;
 }
 
-- 
2.34.1.8.g35151cf07204




More information about the libvir-list mailing list