From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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