[Qemu-devel] resetting a stale VNC connection

[Qemu-devel] resetting a stale VNC connection

[David S. Ahern]

Is there a monitor command (or other method) to force a VNC client
connection to close?

It looks like the vnc code does not use keepalive probes. If the
connection is left hanging the VNC server will never drop it. I have hit
this condition a couple of times when a VPN connection drops while the
vncviewer is connected to a VM.

Specific software version at the moment is kvm-81.

david

Re: [Qemu-devel] resetting a stale VNC connection

[Avi Kivity]

David S. Ahern wrote:
> Is there a monitor command (or other method) to force a VNC client
> connection to close?
>
> It looks like the vnc code does not use keepalive probes. If the
> connection is left hanging the VNC server will never drop it. I have hit
> this condition a couple of times when a VPN connection drops while the
> vncviewer is connected to a VM.
>
> Specific software version at the moment is kvm-81.
>   

Why not just enable tcp keepalives?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] resetting a stale VNC connection

[David S. Ahern]

'egrep -r SO_KEEPALIVE qemu' did not turn up any hits for the VNC code.
Can it be enabled within qemu via command line arg?

I am not aware of an OS setting to enable keepalives for all sockets. Is
there one in RHEL5?

david



Avi Kivity wrote:
> David S. Ahern wrote:
>> Is there a monitor command (or other method) to force a VNC client
>> connection to close?
>>
>> It looks like the vnc code does not use keepalive probes. If the
>> connection is left hanging the VNC server will never drop it. I have hit
>> this condition a couple of times when a VPN connection drops while the
>> vncviewer is connected to a VM.
>>
>> Specific software version at the moment is kvm-81.
>>   
> 
> Why not just enable tcp keepalives?
> 

Re: [Qemu-devel] resetting a stale VNC connection

[Avi Kivity]

David S. Ahern wrote:
> 'egrep -r SO_KEEPALIVE qemu' did not turn up any hits for the VNC code.
> Can it be enabled within qemu via command line arg?
>   

It's not in the code AFAIK.  I was suggesting to add it (unconditionally).

> I am not aware of an OS setting to enable keepalives for all sockets. Is
> there one in RHEL5?
>   

I don't think so.  But enabling keepalives for vnc should be a one liner 
(or rather two, since we want to change the default interval).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] resetting a stale VNC connection

[Yaniv Kaul]

On 4/21/2009 7:12 PM, Avi Kivity wrote:
> David S. Ahern wrote:
>> 'egrep -r SO_KEEPALIVE qemu' did not turn up any hits for the VNC code.
>> Can it be enabled within qemu via command line arg?
>
> It's not in the code AFAIK.  I was suggesting to add it 
> (unconditionally).
>
>> I am not aware of an OS setting to enable keepalives for all sockets. Is
>> there one in RHEL5?
>
> I don't think so.  But enabling keepalives for vnc should be a one 
> liner (or rather two, since we want to change the default interval).
>
I think it should (also?) honor the 'shared desktop' flag.

Re: [Qemu-devel] resetting a stale VNC connection

[David S. Ahern]

Yaniv Kaul wrote:
> On 4/21/2009 7:12 PM, Avi Kivity wrote:
>> David S. Ahern wrote:
>>> 'egrep -r SO_KEEPALIVE qemu' did not turn up any hits for the VNC code.
>>> Can it be enabled within qemu via command line arg?
>>
>> It's not in the code AFAIK.  I was suggesting to add it
>> (unconditionally).

Ok, I misunderstood your last response. I'll add it, unconditionally.

>>
>>> I am not aware of an OS setting to enable keepalives for all sockets. Is
>>> there one in RHEL5?
>>
>> I don't think so.  But enabling keepalives for vnc should be a one
>> liner (or rather two, since we want to change the default interval).

What default interval would you prefer over the global values?

>>
> I think it should (also?) honor the 'shared desktop' flag.

What do you mean by the 'shared desktop' flag?

david

Re: [Qemu-devel] resetting a stale VNC connection

[Avi Kivity]

David S. Ahern wrote:
>>> I don't think so.  But enabling keepalives for vnc should be a one
>>> liner (or rather two, since we want to change the default interval).
>>>       
>
> What default interval would you prefer over the global values?
>   

The global one is two hours.

Say sending a probe every two seconds, failing if four probes don't come 
back?



-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] resetting a stale VNC connection

[David Ahern]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]


This patch enables TCP keepalives on VNC connections. After 60-seconds
of idle time, probes are sent every 2 seconds with the connection
resetting after 4 failed probes. This might be a rather aggressive
setting -- 8 seconds until connection is reset.

Also, I believe this is a linux-specific way of doing this; not sure
what the posix/windows method is. Open to suggestions.

Yaniv: I do not believe this interferes with the 'shared desktop'.

Signed-off-by: David Ahern <dsahern@gmail.com>


Avi Kivity wrote:
> David S. Ahern wrote:
>>>> I don't think so.  But enabling keepalives for vnc should be a one
>>>> liner (or rather two, since we want to change the default interval).
>>>>       
>>
>> What default interval would you prefer over the global values?
>>   
> 
> The global one is two hours.
> 
> Say sending a probe every two seconds, failing if four probes don't come
> back?
> 
> 
> 

[-- Attachment #2: vnc-enable-keepalives.patch --]
[-- Type: text/plain, Size: 1724 bytes --]

diff --git a/vnc.c b/vnc.c
index ab1f044..7884a55 100644
--- a/vnc.c
+++ b/vnc.c
@@ -32,6 +32,10 @@
 
 #define VNC_REFRESH_INTERVAL (1000 / 30)
 
+#define VNC_TCP_KEEPIDLE  60
+#define VNC_TCP_KEEPINTVL  2
+#define VNC_TCP_KEEPCNT    4
+
 #include "vnc_keysym.h"
 #include "d3des.h"
 
@@ -2015,12 +2019,40 @@ static void vnc_listen_read(void *opaque)
     VncDisplay *vs = opaque;
     struct sockaddr_in addr;
     socklen_t addrlen = sizeof(addr);
+    int val;
 
     /* Catch-up */
     vga_hw_update();
 
     int csock = accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
     if (csock != -1) {
+
+        /* best effort to enable keep alives */
+        val = 1;
+        if (setsockopt(csock, SOL_SOCKET, SO_KEEPALIVE,
+                       &val, sizeof(val)) < 0) {
+            fprintf(stderr, "VNC: failed to enable keepalives\n");
+        }
+
+        /* after 60-seconds of idle time, send probes every 2 seconds
+         * dropping the connection after 4 failed probes
+         */
+        val = VNC_TCP_KEEPIDLE;
+        if (setsockopt(csock, IPPROTO_TCP, TCP_KEEPIDLE,
+                       &val, sizeof(val)) < 0) {
+            fprintf(stderr, "VNC: failed to set tcp idle interval\n");
+        }
+        val = VNC_TCP_KEEPINTVL;
+        if (setsockopt(csock, IPPROTO_TCP, TCP_KEEPINTVL,
+                       &val, sizeof(val)) < 0) {
+            fprintf(stderr, "VNC: failed to set tcp prove interval\n");
+        }
+        val = VNC_TCP_KEEPCNT;
+        if (setsockopt(csock, IPPROTO_TCP, TCP_KEEPCNT,
+                       &val, sizeof(val)) < 0) {
+            fprintf(stderr, "VNC: failed to set tcp prove interval\n");
+        }
+
         vnc_connect(vs, csock);
     }
 }

Re: [Qemu-devel] resetting a stale VNC connection

[Avi Kivity]

David Ahern wrote:
> This patch enables TCP keepalives on VNC connections. After 60-seconds
> of idle time, probes are sent every 2 seconds with the connection
> resetting after 4 failed probes. This might be a rather aggressive
> setting -- 8 seconds until connection is reset.
>
>   

I think you are right, it's too aggressive.  Maybe reduce it so it takes 
a minute until we give up.

> Also, I believe this is a linux-specific way of doing this; not sure
> what the posix/windows method is. Open to suggestions.
>   

At the very least, you need to wrap this in a #if defined(...) && 
defined (...) ... so it doesn't break compilation on hosts that don't 
support those socket options.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] resetting a stale VNC connection

[Yaniv Kaul]

On 4/21/2009 7:28 PM, David S. Ahern wrote:
> Yaniv Kaul wrote:
>    
>> On 4/21/2009 7:12 PM, Avi Kivity wrote:
>>      
>>> David S. Ahern wrote:
>>>        
>>>> 'egrep -r SO_KEEPALIVE qemu' did not turn up any hits for the VNC code.
>>>> Can it be enabled within qemu via command line arg?
>>>>          
>>> It's not in the code AFAIK.  I was suggesting to add it
>>> (unconditionally).
>>>        
>
> Ok, I misunderstood your last response. I'll add it, unconditionally.
>
>    
>>>> I am not aware of an OS setting to enable keepalives for all sockets. Is
>>>> there one in RHEL5?
>>>>          
>>> I don't think so.  But enabling keepalives for vnc should be a one
>>> liner (or rather two, since we want to change the default interval).
>>>        
>
> What default interval would you prefer over the global values?
>
>    
>> I think it should (also?) honor the 'shared desktop' flag.
>>      
>
> What do you mean by the 'shared desktop' flag?
>    
VNC clients send a flag called 'shared-flag' in their init message. 
>From  http://www.realvnc.com/docs/rfbproto.pdf  , section 6.3.1 ClientInit:
Shared-flag is non-zero (true) if the server should try to share the 
desktop by leaving
other clients connected, zero (false) if it should give exclusive access 
to this client by
disconnecting all other clients.
> david
>    

Re: [Qemu-devel] resetting a stale VNC connection

[Daniel P. Berrange]

On Tue, Apr 21, 2009 at 04:38:55PM +0300, Avi Kivity wrote:
> David S. Ahern wrote:
> >Is there a monitor command (or other method) to force a VNC client
> >connection to close?
> >
> >It looks like the vnc code does not use keepalive probes. If the
> >connection is left hanging the VNC server will never drop it. I have hit
> >this condition a couple of times when a VPN connection drops while the
> >vncviewer is connected to a VM.
> >
> >Specific software version at the moment is kvm-81.
> >  
> 
> Why not just enable tcp keepalives?

Independant of killing off stale VNC clients, it could be desirable to
have a monitor command to kill a client, to allow revoking of end user
access to VNC.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] resetting a stale VNC connection

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Tue, Apr 21, 2009 at 04:38:55PM +0300, Avi Kivity wrote:
>   
>> David S. Ahern wrote:
>>     
>>> Is there a monitor command (or other method) to force a VNC client
>>> connection to close?
>>>
>>> It looks like the vnc code does not use keepalive probes. If the
>>> connection is left hanging the VNC server will never drop it. I have hit
>>> this condition a couple of times when a VPN connection drops while the
>>> vncviewer is connected to a VM.
>>>
>>> Specific software version at the moment is kvm-81.
>>>  
>>>       
>> Why not just enable tcp keepalives?
>>     
>
> Independant of killing off stale VNC clients, it could be desirable to
> have a monitor command to kill a client, to allow revoking of end user
> access to VNC.
>   

I'm surprised change vnc none doesn't kill off all VNC connections.

I don't see a compelling use case to boot off individual clients other 
than we can.  Is there something specific you were thinking someone 
would want to do?

Regards,

Anthony Liguori

> Daniel
>   

Re: [Qemu-devel] resetting a stale VNC connection

[Daniel P. Berrange]

On Tue, Apr 21, 2009 at 11:49:11AM -0500, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >On Tue, Apr 21, 2009 at 04:38:55PM +0300, Avi Kivity wrote:
> >  
> >>David S. Ahern wrote:
> >>    
> >>>Is there a monitor command (or other method) to force a VNC client
> >>>connection to close?
> >>>
> >>>It looks like the vnc code does not use keepalive probes. If the
> >>>connection is left hanging the VNC server will never drop it. I have hit
> >>>this condition a couple of times when a VPN connection drops while the
> >>>vncviewer is connected to a VM.
> >>>
> >>>Specific software version at the moment is kvm-81.
> >>> 
> >>>      
> >>Why not just enable tcp keepalives?
> >>    
> >
> >Independant of killing off stale VNC clients, it could be desirable to
> >have a monitor command to kill a client, to allow revoking of end user
> >access to VNC.
> >  
> 
> I'm surprised change vnc none doesn't kill off all VNC connections.

It may well do that - i've not tried it :-)

> I don't see a compelling use case to boot off individual clients other 
> than we can.  Is there something specific you were thinking someone 
> would want to do?

I've no pressing need for this - i was just thinking of possible use
cases for the command. Now we allow multiple concurrent VNC clients, and
can authenticate them, I just thought that it might be desirable to be
able to drop individual authenticated clients. eg if you've revoked
their credentials, but they still have a session open. Turning off+on
the whole VNC server would work as a big stick too.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] resetting a stale VNC connection

[David S. Ahern]

Daniel P. Berrange wrote:
> On Tue, Apr 21, 2009 at 11:49:11AM -0500, Anthony Liguori wrote:
>> Daniel P. Berrange wrote:
>>> On Tue, Apr 21, 2009 at 04:38:55PM +0300, Avi Kivity wrote:
>>>  
>>>> David S. Ahern wrote:
>>>>    
>>>>> Is there a monitor command (or other method) to force a VNC client
>>>>> connection to close?
>>>>>
>>>>> It looks like the vnc code does not use keepalive probes. If the
>>>>> connection is left hanging the VNC server will never drop it. I have hit
>>>>> this condition a couple of times when a VPN connection drops while the
>>>>> vncviewer is connected to a VM.
>>>>>
>>>>> Specific software version at the moment is kvm-81.
>>>>>
>>>>>      
>>>> Why not just enable tcp keepalives?
>>>>    
>>> Independant of killing off stale VNC clients, it could be desirable to
>>> have a monitor command to kill a client, to allow revoking of end user
>>> access to VNC.
>>>  
>> I'm surprised change vnc none doesn't kill off all VNC connections.
> 
> It may well do that - i've not tried it :-)

It did not kill off established connections for me; it closed the server
socket, but not client connections.

david


> 
>> I don't see a compelling use case to boot off individual clients other 
>> than we can.  Is there something specific you were thinking someone 
>> would want to do?
> 
> I've no pressing need for this - i was just thinking of possible use
> cases for the command. Now we allow multiple concurrent VNC clients, and
> can authenticate them, I just thought that it might be desirable to be
> able to drop individual authenticated clients. eg if you've revoked
> their credentials, but they still have a session open. Turning off+on
> the whole VNC server would work as a big stick too.
> 
> Daniel

Re: [Qemu-devel] resetting a stale VNC connection

[Anthony Liguori]

David S. Ahern wrote:
> Daniel P. Berrange wrote:
>   
>> It may well do that - i've not tried it :-)
>>     
>
> It did not kill off established connections for me; it closed the server
> socket, but not client connections.
>   

I'm really not sure if that's a bug or not.  I think I could understand 
arguments for either behavior.

Regards,

Anthony Liguori

> david

Re: [Qemu-devel] resetting a stale VNC connection

[Gerd Hoffmann]

On 04/21/09 19:17, Daniel P. Berrange wrote:
> On Tue, Apr 21, 2009 at 11:49:11AM -0500, Anthony Liguori wrote:
> I've no pressing need for this - i was just thinking of possible use
> cases for the command. Now we allow multiple concurrent VNC clients, and
> can authenticate them, I just thought that it might be desirable to be
> able to drop individual authenticated clients. eg if you've revoked
> their credentials, but they still have a session open. Turning off+on
> the whole VNC server would work as a big stick too.

I would link that to acl commands.  You have to send that anyway to make 
qemu not accept new connections for $user.  And qemu can check for open 
sessions owned by $user and zap them ...

cheers,
   Gerd