* [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used
@ 2015-08-14 12:47 Christophe Fergeau
2015-08-14 12:54 ` Daniel P. Berrange
2015-08-14 14:35 ` Daniel P. Berrange
0 siblings, 2 replies; 7+ messages in thread
From: Christophe Fergeau @ 2015-08-14 12:47 UTC (permalink / raw
To: qemu-devel
Before commit b1ea7b79e1, it was possible to start with -spice
disable-ticketing, and then use the "set_password spice" command to
enable ticketing with SPICE. Since commit b1ea7b79e1 this is no longer
possible as qemu_spice_set_ticket() will return an error unless the
'auth' type is "spice". When ticketing is disabled, 'auth' is "none" so
the attempt to set password fails.
This commit allows to call qemu_spice_set_ticket() when 'auth' is "none"
and changes 'auth' to "spice" when this happens.
---
ui/spice-core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 4da3042..3b20c6c 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -882,6 +882,10 @@ static int qemu_spice_set_ticket(bool fail_if_conn, bool disconnect_if_conn)
int qemu_spice_set_passwd(const char *passwd,
bool fail_if_conn, bool disconnect_if_conn)
{
+ if (strcmp(auth, "none") == 0) {
+ /* Allow to set a password when started with 'disable-ticketing' */
+ auth = "spice";
+ }
if (strcmp(auth, "spice") != 0) {
return -1;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used
2015-08-14 12:47 [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used Christophe Fergeau
@ 2015-08-14 12:54 ` Daniel P. Berrange
2015-08-14 13:09 ` Christophe Fergeau
2015-08-14 14:35 ` Daniel P. Berrange
1 sibling, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2015-08-14 12:54 UTC (permalink / raw
To: Christophe Fergeau; +Cc: qemu-devel
On Fri, Aug 14, 2015 at 02:47:15PM +0200, Christophe Fergeau wrote:
> Before commit b1ea7b79e1, it was possible to start with -spice
> disable-ticketing, and then use the "set_password spice" command to
> enable ticketing with SPICE. Since commit b1ea7b79e1 this is no longer
> possible as qemu_spice_set_ticket() will return an error unless the
> 'auth' type is "spice". When ticketing is disabled, 'auth' is "none" so
> the attempt to set password fails.
>
> This commit allows to call qemu_spice_set_ticket() when 'auth' is "none"
> and changes 'auth' to "spice" when this happens.
IMHO we should not be changing the authentication method as a side
effect of trying to set the password.
If app has disabled ticketing, it should remain disabled and the
set password call is right to return an error.
We should have a graphics-set-auth command for changing authentication
parameters on existing graphics backend.
Regards,
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 :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used
2015-08-14 12:54 ` Daniel P. Berrange
@ 2015-08-14 13:09 ` Christophe Fergeau
2015-08-14 14:04 ` Daniel P. Berrange
0 siblings, 1 reply; 7+ messages in thread
From: Christophe Fergeau @ 2015-08-14 13:09 UTC (permalink / raw
To: Daniel P. Berrange; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]
Hey,
On Fri, Aug 14, 2015 at 01:54:59PM +0100, Daniel P. Berrange wrote:
> On Fri, Aug 14, 2015 at 02:47:15PM +0200, Christophe Fergeau wrote:
> > Before commit b1ea7b79e1, it was possible to start with -spice
> > disable-ticketing, and then use the "set_password spice" command to
> > enable ticketing with SPICE. Since commit b1ea7b79e1 this is no longer
> > possible as qemu_spice_set_ticket() will return an error unless the
> > 'auth' type is "spice". When ticketing is disabled, 'auth' is "none" so
> > the attempt to set password fails.
> >
> > This commit allows to call qemu_spice_set_ticket() when 'auth' is "none"
> > and changes 'auth' to "spice" when this happens.
>
> IMHO we should not be changing the authentication method as a side
> effect of trying to set the password.
>
> If app has disabled ticketing, it should remain disabled and the
> set password call is right to return an error.
>
In general I agree with you. However in this case, this used to be
working until ~1 year ago, and this change of behaviour caused a bug in
oVirt (oVirt side is being fixed). This is why I sent this patch.
The intent of commit b1ea7b seems to be to prevent
qemu_spice_set_passwd() from being called when SASL is used, and does
not mention at all whether preventing going from auth being "none" to
"spice" is intentional.
If this change of behaviour was an intentional bug fix, and if we are
fine with asking for oVirt changes for this, then I'm ok with dropping
this patch.
Christophe
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used
2015-08-14 13:09 ` Christophe Fergeau
@ 2015-08-14 14:04 ` Daniel P. Berrange
2015-08-14 14:31 ` Christophe Fergeau
0 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2015-08-14 14:04 UTC (permalink / raw
To: Christophe Fergeau; +Cc: qemu-devel
On Fri, Aug 14, 2015 at 03:09:44PM +0200, Christophe Fergeau wrote:
> Hey,
>
> On Fri, Aug 14, 2015 at 01:54:59PM +0100, Daniel P. Berrange wrote:
> > On Fri, Aug 14, 2015 at 02:47:15PM +0200, Christophe Fergeau wrote:
> > > Before commit b1ea7b79e1, it was possible to start with -spice
> > > disable-ticketing, and then use the "set_password spice" command to
> > > enable ticketing with SPICE. Since commit b1ea7b79e1 this is no longer
> > > possible as qemu_spice_set_ticket() will return an error unless the
> > > 'auth' type is "spice". When ticketing is disabled, 'auth' is "none" so
> > > the attempt to set password fails.
> > >
> > > This commit allows to call qemu_spice_set_ticket() when 'auth' is "none"
> > > and changes 'auth' to "spice" when this happens.
> >
> > IMHO we should not be changing the authentication method as a side
> > effect of trying to set the password.
> >
> > If app has disabled ticketing, it should remain disabled and the
> > set password call is right to return an error.
> >
>
> In general I agree with you. However in this case, this used to be
> working until ~1 year ago, and this change of behaviour caused a bug in
> oVirt (oVirt side is being fixed). This is why I sent this patch.
>
> The intent of commit b1ea7b seems to be to prevent
> qemu_spice_set_passwd() from being called when SASL is used, and does
> not mention at all whether preventing going from auth being "none" to
> "spice" is intentional.
>
> If this change of behaviour was an intentional bug fix, and if we are
> fine with asking for oVirt changes for this, then I'm ok with dropping
> this patch.
Hmm, is oVirt using this via libvirt ? If so, I guess we have to fix
it, as that would be a break in current usage.
Regards,
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 :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used
2015-08-14 14:04 ` Daniel P. Berrange
@ 2015-08-14 14:31 ` Christophe Fergeau
0 siblings, 0 replies; 7+ messages in thread
From: Christophe Fergeau @ 2015-08-14 14:31 UTC (permalink / raw
To: Daniel P. Berrange; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 559 bytes --]
On Fri, Aug 14, 2015 at 03:04:48PM +0100, Daniel P. Berrange wrote:
> Hmm, is oVirt using this via libvirt ? If so, I guess we have to fix
> it, as that would be a break in current usage.
Yes this is done through libvirt.
Before commit qemu-2.1.0-rc2~11^2, you could use virsh update-device
with
<graphics type='spice' autoport='yes' listen='127.0.0.1' passwd='bar'/>
to set the password for a running domain whose graphics node is
<graphics type='spice' autoport='yes' listen='127.0.0.1'/>
After qemu-2.1.0-rc2~11^2, this results in an error.
Christophe
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used
2015-08-14 12:47 [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used Christophe Fergeau
2015-08-14 12:54 ` Daniel P. Berrange
@ 2015-08-14 14:35 ` Daniel P. Berrange
2015-08-14 15:11 ` Christophe Fergeau
1 sibling, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2015-08-14 14:35 UTC (permalink / raw
To: Christophe Fergeau; +Cc: qemu-devel
On Fri, Aug 14, 2015 at 02:47:15PM +0200, Christophe Fergeau wrote:
> Before commit b1ea7b79e1, it was possible to start with -spice
> disable-ticketing, and then use the "set_password spice" command to
> enable ticketing with SPICE. Since commit b1ea7b79e1 this is no longer
> possible as qemu_spice_set_ticket() will return an error unless the
> 'auth' type is "spice". When ticketing is disabled, 'auth' is "none" so
> the attempt to set password fails.
>
> This commit allows to call qemu_spice_set_ticket() when 'auth' is "none"
> and changes 'auth' to "spice" when this happens.
BTW, you need to have a Signed-of-by here
> ---
> ui/spice-core.c | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
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 :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used
2015-08-14 14:35 ` Daniel P. Berrange
@ 2015-08-14 15:11 ` Christophe Fergeau
0 siblings, 0 replies; 7+ messages in thread
From: Christophe Fergeau @ 2015-08-14 15:11 UTC (permalink / raw
To: Daniel P. Berrange; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
On Fri, Aug 14, 2015 at 03:35:21PM +0100, Daniel P. Berrange wrote:
> On Fri, Aug 14, 2015 at 02:47:15PM +0200, Christophe Fergeau wrote:
> > Before commit b1ea7b79e1, it was possible to start with -spice
> > disable-ticketing, and then use the "set_password spice" command to
> > enable ticketing with SPICE. Since commit b1ea7b79e1 this is no longer
> > possible as qemu_spice_set_ticket() will return an error unless the
> > 'auth' type is "spice". When ticketing is disabled, 'auth' is "none" so
> > the attempt to set password fails.
> >
> > This commit allows to call qemu_spice_set_ticket() when 'auth' is "none"
> > and changes 'auth' to "spice" when this happens.
>
> BTW, you need to have a Signed-of-by here
Ah right, thanks, just sent a v2 with this added.
Christophe
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-08-14 15:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-14 12:47 [Qemu-devel] [PATCH] spice: Allow to set password even if disable-ticketing was used Christophe Fergeau
2015-08-14 12:54 ` Daniel P. Berrange
2015-08-14 13:09 ` Christophe Fergeau
2015-08-14 14:04 ` Daniel P. Berrange
2015-08-14 14:31 ` Christophe Fergeau
2015-08-14 14:35 ` Daniel P. Berrange
2015-08-14 15:11 ` Christophe Fergeau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.