Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On Thu, Nov 22, 2012 at 11:32:38AM +0100, Ján Tomko wrote: On 11/22/12 11:03, Martin Kletzander wrote: I'll rework it to make it as URI as possible then. Just one question, though (for anyone, I guess); why do we append port as a parameter for spice scheme? Martin I think it's because of what spicy supported initially (since v0.1.0): http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=f0693b9f949ba spice://host:port is supported since v0.8 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=b4c72ece9ca3b and spice://host:port/ (with the trailing slash) since v0.12 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=50add15ef69cd Another thing to keep in mind is that you can have both a port and a secure port (over which data will transit through SSL). Christophe pgpWUa4VXBGk6.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/23/2012 09:20 AM, Christophe Fergeau wrote: On Thu, Nov 22, 2012 at 11:32:38AM +0100, Ján Tomko wrote: On 11/22/12 11:03, Martin Kletzander wrote: I'll rework it to make it as URI as possible then. Just one question, though (for anyone, I guess); why do we append port as a parameter for spice scheme? Martin I think it's because of what spicy supported initially (since v0.1.0): http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=f0693b9f949ba spice://host:port is supported since v0.8 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=b4c72ece9ca3b and spice://host:port/ (with the trailing slash) since v0.12 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=50add15ef69cd Another thing to keep in mind is that you can have both a port and a secure port (over which data will transit through SSL). Christophe I'm keeping that in the parameter as there will be both port and tlsPort available in this case. I also modified it so we have a vnc version and spice version of the output, which can be seen in v2 (rewrite cmdDomDisplay). Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/2012 11:50 PM, Doug Goldstein wrote: On Wed, Nov 21, 2012 at 10:13 AM, Martin Kletzander mklet...@redhat.com wrote: On 11/21/2012 05:06 PM, Ján Tomko wrote: On 11/21/12 16:37, Martin Kletzander wrote: The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. This is just a simple patch for that to work properly. --- tools/virsh-domain.c | 17 + 1 file changed, 17 insertions(+) Wouldn't it be better to put the password before the host (vnc://:passwd@host:port) so that at least some clients (krdc from the few that I've tried) can understand the whole URI? Jan TBH, I don't get this URI-styled remote display definitions because I don't know what program can use those (universally, not just from libvirt, I mean, and I haven't heard about krdc until now) and if there is a recommended scheme for that, however this is how the parameter is appended for spice connections and unless there is a scheme for that and we can say The previous version was a bug, this is how it should be, I don't think we want rick breaking something. OTOH this is just a virsh call, not an API. Martin I originally did that because we only had vncdisplay with no way to query SPICE info via virsh. So to generically support all graphics protocols I added domdisplay which provides hostname:port like vncdisplay so I needed a way to tell you of the protocol. I figured generic URI RFC style for VNC would be ok since we follow SPICE's URI spec and RDP's URI spec so why special case VNC to not make it a URI. I'll rework it to make it as URI as possible then. Just one question, though (for anyone, I guess); why do we append port as a parameter for spice scheme? Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/22/12 11:03, Martin Kletzander wrote: I'll rework it to make it as URI as possible then. Just one question, though (for anyone, I guess); why do we append port as a parameter for spice scheme? Martin I think it's because of what spicy supported initially (since v0.1.0): http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=f0693b9f949ba spice://host:port is supported since v0.8 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=b4c72ece9ca3b and spice://host:port/ (with the trailing slash) since v0.12 http://cgit.freedesktop.org/spice/spice-gtk/commit/?id=50add15ef69cd Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On Wed, Nov 21, 2012 at 04:37:35PM +0100, Martin Kletzander wrote:
The 'virsh domdisplay' command is able to display the password
configured for spice, but it was missing for vnc type graphics.
This is just a simple patch for that to work properly.
---
tools/virsh-domain.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cc47383..18aa869 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7073,6 +7073,23 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
if (STREQ(scheme[iter], vnc)) {
/* VNC protocol handlers take their port number as 'port' - 5900
*/
port -= 5900;
+
+if (vshCommandOptBool(cmd, include-password)) {
+/* Create our XPATH lookup for the SPICE password */
s/spice/vnc/
+virAsprintf(xpath,
+string(/domain/devices/graphics
+[@type='%s']/@passwd),
+scheme[iter]);
+if (!xpath) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* Attempt to get the SPICE password */
s/spice/vnc/
+passwd = virXPathString(xpath, ctxt);
+VIR_FREE(xpath);
+}
+
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/2012 04:39 PM, Daniel P. Berrange wrote:
On Wed, Nov 21, 2012 at 04:37:35PM +0100, Martin Kletzander wrote:
The 'virsh domdisplay' command is able to display the password
configured for spice, but it was missing for vnc type graphics.
This is just a simple patch for that to work properly.
---
tools/virsh-domain.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cc47383..18aa869 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7073,6 +7073,23 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
if (STREQ(scheme[iter], vnc)) {
/* VNC protocol handlers take their port number as 'port' -
5900 */
port -= 5900;
+
+if (vshCommandOptBool(cmd, include-password)) {
+/* Create our XPATH lookup for the SPICE password */
s/spice/vnc/
+virAsprintf(xpath,
+string(/domain/devices/graphics
+[@type='%s']/@passwd),
+scheme[iter]);
+if (!xpath) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* Attempt to get the SPICE password */
s/spice/vnc/
+passwd = virXPathString(xpath, ctxt);
+VIR_FREE(xpath);
+}
+
Daniel
Oh, right, dumb me. I tested the functionality, but left the copy-paste
errors in the comments. Does that mean I can push with those fixes?
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/12 16:37, Martin Kletzander wrote: The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. This is just a simple patch for that to work properly. --- tools/virsh-domain.c | 17 + 1 file changed, 17 insertions(+) Wouldn't it be better to put the password before the host (vnc://:passwd@host:port) so that at least some clients (krdc from the few that I've tried) can understand the whole URI? Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/2012 05:06 PM, Ján Tomko wrote: On 11/21/12 16:37, Martin Kletzander wrote: The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. This is just a simple patch for that to work properly. --- tools/virsh-domain.c | 17 + 1 file changed, 17 insertions(+) Wouldn't it be better to put the password before the host (vnc://:passwd@host:port) so that at least some clients (krdc from the few that I've tried) can understand the whole URI? Jan TBH, I don't get this URI-styled remote display definitions because I don't know what program can use those (universally, not just from libvirt, I mean, and I haven't heard about krdc until now) and if there is a recommended scheme for that, however this is how the parameter is appended for spice connections and unless there is a scheme for that and we can say The previous version was a bug, this is how it should be, I don't think we want rick breaking something. OTOH this is just a virsh call, not an API. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On 11/21/12 17:13, Martin Kletzander wrote: On 11/21/2012 05:06 PM, Ján Tomko wrote: Wouldn't it be better to put the password before the host (vnc://:passwd@host:port) so that at least some clients (krdc from the few that I've tried) can understand the whole URI? Jan TBH, I don't get this URI-styled remote display definitions because I don't know what program can use those (universally, not just from libvirt, I mean, and I haven't heard about krdc until now) and if there is a recommended scheme for that, however this is how the parameter is appended for spice connections and unless there is a scheme for that and we can say The previous version was a bug, this is how it should be, I don't think we want rick breaking something. OTOH this is just a virsh call, not an API. Martin From a quick google search it seems like it's used by RealVNC for iOS, other than that there isn't any recommended URI style. (Maybe just the generic URI RFC) It does seem more usable than using the SPICE scheme for VNC to me, but I'm OK with both. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Correct include-password option for domdisplay
On Wed, Nov 21, 2012 at 10:13 AM, Martin Kletzander mklet...@redhat.com wrote: On 11/21/2012 05:06 PM, Ján Tomko wrote: On 11/21/12 16:37, Martin Kletzander wrote: The 'virsh domdisplay' command is able to display the password configured for spice, but it was missing for vnc type graphics. This is just a simple patch for that to work properly. --- tools/virsh-domain.c | 17 + 1 file changed, 17 insertions(+) Wouldn't it be better to put the password before the host (vnc://:passwd@host:port) so that at least some clients (krdc from the few that I've tried) can understand the whole URI? Jan TBH, I don't get this URI-styled remote display definitions because I don't know what program can use those (universally, not just from libvirt, I mean, and I haven't heard about krdc until now) and if there is a recommended scheme for that, however this is how the parameter is appended for spice connections and unless there is a scheme for that and we can say The previous version was a bug, this is how it should be, I don't think we want rick breaking something. OTOH this is just a virsh call, not an API. Martin I originally did that because we only had vncdisplay with no way to query SPICE info via virsh. So to generically support all graphics protocols I added domdisplay which provides hostname:port like vncdisplay so I needed a way to tell you of the protocol. I figured generic URI RFC style for VNC would be ok since we follow SPICE's URI spec and RDP's URI spec so why special case VNC to not make it a URI. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
