7:10 a.m.
On 11/22/12 11:34, Martin Kletzander wrote:
The 'virsh domdisplay' command is able to display the
password
configured for spice, but it was missing for vnc type graphics.
Also, there were some inconsistencies that are cleaned now.
---
tools/virsh-domain.c | 74 +++++++++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 32 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cc47383..cc5c830 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7069,14 +7074,36 @@ cmdDomDisplay(vshControl *ctl, const vshCmd
*cmd)
listen_addr = virXPathString(xpath, ctxt);
VIR_FREE(xpath);
+ /* We can query this info for all the graphics types since we'll
+ * get nothing for the unsupported ones (just rdp for now) */
+ if (vshCommandOptBool(cmd, "include-password")) {
+ /* Create our XPATH lookup for the password */
+ virAsprintf(&xpath,
+ "string(/domain/devices/graphics"
+ "[@type='%s']/@passwd)",
+ scheme[iter]);
The usual way is to check the return value of virAsprintf here instead
of checking the allocated memory.
+
+ if (!xpath) {
+ virReportOOMError();
+ goto cleanup;
Hm, a no_memory label would decrease the line count somewhat, but that's
not necessary ...
+ }
+
+ /* Attempt to get the password */
+ passwd = 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 */
+ /* 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]);
+ virAsprintf(&xpath,
+ "string(/domain/devices/graphics[@type='%s']"
+ "/@tlsPort)",
+ scheme[iter]);
Same as the first comment (although it was pre-existing).
if (!xpath) {
virReportOOMError();
goto cleanup;
@@ -7087,25 +7114,17 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
VIR_FREE(xpath);
if (tmp)
tls_port = 0;
-
- if (vshCommandOptBool(cmd, "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]);
+ /* There is no user, so just append password if there's any */
+ if (passwd) {
+ virBufferAsprintf(&buf, ":%s@", passwd);
Doesn't this break something that was working previously? I'm not using
this but the original way was to append "?password=adsgf".
+ VIR_FREE(passwd);
Move this free to the cleanup section.
+ }
+
/* Then host name or IP */
if (!listen_addr || STREQ((const char *)listen_addr, "0.0.0.0"))
virBufferAddLit(&buf, "localhost");
@@ -7115,20 +7134,11 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
VIR_FREE(listen_addr);
/* Add the port */
- if (STREQ(scheme[iter], "spice"))
- virBufferAsprintf(&buf, "?port=%d", port);
- else
- virBufferAsprintf(&buf, ":%d", port);
+ 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);
- }
+ virBufferAsprintf(&buf, "?tls-port=%d", tls_port);
/* Ensure we can print our URI */
if (virBufferError(&buf)) {
I'm not sure about the change of the password parameter. Could you back
that up somehow?
Peter