[PATCH 11/23] virsh: cmdDomDisplay: Extract loop body fetching display URIs into 'virshGetOneDisplay'

Peter Krempa pkrempa at redhat.com
Wed Mar 2 13:55:10 UTC 2022


Separate the code so that the function is not as massive. Note that this
is a minimal extraction which does not clean up the code meant for
looping.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 tools/virsh-domain.c | 352 ++++++++++++++++++++++---------------------
 1 file changed, 183 insertions(+), 169 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index dc6e3b5020..f82aa49745 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11659,215 +11659,235 @@ static const vshCmdOptDef opts_domdisplay[] = {
     {.name = NULL}
 };

-static bool
-cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
+
+static char *
+virshGetOneDisplay(vshControl *ctl,
+                   const char *scheme,
+                   xmlXPathContext *ctxt)
 {
-    g_autoptr(xmlDoc) xml = NULL;
-    g_autoptr(xmlXPathContext) ctxt = NULL;
-    g_autoptr(virshDomain) dom = NULL;
+    const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/%s)";
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-    bool ret = false;
     char *xpath = NULL;
     char *listen_addr = NULL;
-    int port, tls_port = 0;
+    int port = 0;
+    int 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");
-    const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/%s)";
     virSocketAddr addr;

-    VSH_EXCLUSIVE_OPTIONS("all", "type");
+    /* Create our XPATH lookup for the current display's port */
+    VIR_FREE(xpath);
+    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 */
+    VIR_FREE(listen_addr);
+    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 */
+    VIR_FREE(type_conn);
+    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);
+        }
+    }
+
+    if (!port && !tls_port && !sockpath)
+        goto cleanup;

-        /* Create our XPATH lookup for the current display's address */
-        xpath = g_strdup_printf(xpath_fmt, scheme[iter], "@listen");
+    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 */
+    VIR_FREE(passwd);
+    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);
+
+    /* Free socket to prepare the pointer for the next iteration */
+    VIR_FREE(sockpath);
+
+    /* 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);
-                }
-            }
-        }
+ cleanup:
+    VIR_FREE(xpath);
+    VIR_FREE(type_conn);
+    VIR_FREE(sockpath);
+    VIR_FREE(passwd);
+    VIR_FREE(listen_addr);

-        /* 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 */
+    return virBufferContentAndReset(&buf);
+}

-        /* Create our XPATH lookup for the password */
-        xpath = g_strdup_printf(xpath_fmt, scheme[iter], "@passwd");

-        /* Attempt to get the password */
-        VIR_FREE(passwd);
-        passwd = virXPathString(xpath, ctxt);
-        VIR_FREE(xpath);
+static bool
+cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
+{
+    g_autoptr(xmlDoc) xml = NULL;
+    g_autoptr(xmlXPathContext) ctxt = NULL;
+    g_autoptr(virshDomain) dom = 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");

-        /* 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);
+    VSH_EXCLUSIVE_OPTIONS("all", "type");

-        /* Free socket to prepare the pointer for the next iteration */
-        VIR_FREE(sockpath);
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;

-        /* Add the port */
-        if (port) {
-            if (STREQ(scheme[iter], "vnc")) {
-                /* VNC protocol handlers take their port number as
-                 * 'port' - 5900 */
-                port -= 5900;
-            }
+    if (!virDomainIsActive(dom)) {
+        vshError(ctl, _("Domain is not running"));
+        goto cleanup;
+    }

-            virBufferAsprintf(&buf, ":%d", port);
-        }
+    if (vshCommandOptBool(cmd, "include-password"))
+        flags |= VIR_DOMAIN_XML_SECURE;

-        /* TLS Port */
-        if (tls_port) {
-            virBufferAsprintf(&buf,
-                              "?tls-port=%d",
-                              tls_port);
-            params = true;
-        }
+    if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
+        goto cleanup;

-        if (STREQ(scheme[iter], "spice") && passwd) {
-            virBufferAsprintf(&buf,
-                              "%spassword=%s",
-                              params ? "&" : "?",
-                              passwd);
-            params = true;
-        }
+    if (virshDomainGetXMLFromDom(ctl, dom, flags, &xml, &ctxt) < 0)
+        goto cleanup;

-        /* Print out our full URI */
-        VIR_FREE(output);
-        output = virBufferContentAndReset(&buf);
-        vshPrint(ctl, "%s", output);
+    /* Attempt to grab our display info */
+    for (iter = 0; scheme[iter] != NULL; iter++) {
+        g_autofree char *display = NULL;
+
+        /* Particular scheme requested */
+        if (!all && type && STRNEQ(type, scheme[iter]))
+            continue;
+
+        if (!(display = virshGetOneDisplay(ctl, scheme[iter], ctxt)))
+            continue;
+
+        vshPrint(ctl, "%s", display);

         /* We got what we came for so return successfully */
         ret = true;
@@ -11884,12 +11904,6 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
     }

  cleanup:
-    VIR_FREE(xpath);
-    VIR_FREE(type_conn);
-    VIR_FREE(sockpath);
-    VIR_FREE(passwd);
-    VIR_FREE(listen_addr);
-    VIR_FREE(output);
     return ret;
 }

-- 
2.35.1




More information about the libvir-list mailing list