[libvirt] [PATCH] tools: virsh: Adding unix socket support to 'domdisplay' command.

This commit adds the unix socket URL support to 'domdisplay' command. Before, even if an user was using unix socket to define a spice graphics, the command 'domdisplay' showed that the settings were not supported. Now, the command shows the proper URL: spice+unix://foo/bar.sock. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1336720 Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-domain.c | 49 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0684979..090714b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10948,6 +10948,8 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) char *xpath = NULL; char *listen_addr = NULL; int port, tls_port = 0; + char *type_conn = NULL; + char *socket = NULL; char *passwd = NULL; char *output = NULL; const char *scheme[] = { "vnc", "spice", "rdp", NULL }; @@ -11008,9 +11010,6 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) if (tmp) tls_port = 0; - if (!port && !tls_port) - continue; - /* Create our XPATH lookup for the current display's address */ if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "@listen") < 0) goto cleanup; @@ -11021,6 +11020,29 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) listen_addr = virXPathString(xpath, ctxt); VIR_FREE(xpath); + /* Create our XPATH lookup for the current spice type. */ + if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "listen/@type") < 0) + goto cleanup; + + /* Attempt to get the type of spice connection */ + VIR_FREE(type_conn); + type_conn = virXPathString(xpath, ctxt); + VIR_FREE(xpath); + + if (STREQ_NULLABLE(type_conn, "socket")) { + if (!socket) { + if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "listen/@socket") < 0) + goto cleanup; + + socket = virXPathString(xpath, ctxt); + + VIR_FREE(xpath); + } + } + + if (!port && !tls_port && !socket) + continue; + if (!listen_addr) { /* The subelement address - <listen address='xyz'/> - * *should* have been automatically backfilled into its @@ -11035,11 +11057,9 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) listen_addr = virXPathString(xpath, ctxt); VIR_FREE(xpath); - } - - /* If listen_addr is 0.0.0.0 or [::] we should try to parse URI and set - * listen_addr based on current URI. */ - if (listen_addr) { + } 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)) { @@ -11078,17 +11098,22 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) VIR_FREE(xpath); /* Build up the full URI, starting with the scheme */ - virBufferAsprintf(&buf, "%s://", scheme[iter]); + if (socket) + 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) + if (!listen_addr && !socket) virBufferAddLit(&buf, "localhost"); - else if (strchr(listen_addr, ':')) + else if (!socket && strchr(listen_addr, ':')) virBufferAsprintf(&buf, "[%s]", listen_addr); + else if (STREQ(scheme[iter], "spice")) + virBufferAsprintf(&buf, "%s", socket); else virBufferAsprintf(&buf, "%s", listen_addr); @@ -11148,6 +11173,8 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) cleanup: VIR_FREE(xpath); + VIR_FREE(socket); + VIR_FREE(type_conn); VIR_FREE(passwd); VIR_FREE(listen_addr); VIR_FREE(output); -- 2.7.4

On 07/20/2017 11:11 PM, Julio Faracco wrote:
This commit adds the unix socket URL support to 'domdisplay' command. Before, even if an user was using unix socket to define a spice graphics, the command 'domdisplay' showed that the settings were not supported. Now, the command shows the proper URL: spice+unix://foo/bar.sock.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1336720
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-domain.c | 49 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 11 deletions(-)
Hey, this looks roughly okay. But, the changes you made cause a small problem: ./tools/virsh domdisplay --all fedora vnc://localhost:0 spice+unix:///tmp/spice.sock rdp+unix://(null) And I don't have any RDP configured for my domain. I only have vnc and spice. Can you please look into that and send v2? Thanks. Michal

Thanks for the update, Michal. I did some other fixes related to spice too. Libvirt is freezed right now. So, I will wait the next wave to send the V2. 2017-07-24 10:42 GMT-03:00 Michal Privoznik <mprivozn@redhat.com>:
On 07/20/2017 11:11 PM, Julio Faracco wrote:
This commit adds the unix socket URL support to 'domdisplay' command. Before, even if an user was using unix socket to define a spice graphics, the command 'domdisplay' showed that the settings were not supported. Now, the command shows the proper URL: spice+unix://foo/bar.sock.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1336720
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-domain.c | 49 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 11 deletions(-)
Hey, this looks roughly okay. But, the changes you made cause a small problem:
./tools/virsh domdisplay --all fedora vnc://localhost:0 spice+unix:///tmp/spice.sock rdp+unix://(null)
And I don't have any RDP configured for my domain. I only have vnc and spice. Can you please look into that and send v2? Thanks.
Michal

On 07/27/2017 04:51 PM, Julio Faracco wrote:
Thanks for the update, Michal.
I did some other fixes related to spice too. Libvirt is freezed right now. So, I will wait the next wave to send the V2.
That's okay. Libvirt freezes take about a week. Moreover, patches can be reviewed meanwhile, it's only merging of patches that is delayed (unless the patch is a bug fix). IOW, feel free to post patch anytime. Michal
participants (2)
-
Julio Faracco
-
Michal Privoznik