[Qemu-devel] [PATCH] vnc: fix "info vnc" with "-vnc ..., reverse=on"

[Qemu-devel] [PATCH] vnc: fix "info vnc" with "-vnc ..., reverse=on"

[Paolo Bonzini]

When reverse connection is in use, there is no active VNC server
socket.  Because of this, getsockopt(-1, ...) is attempted and
the following error is emitted:

    $ socat TCP-LISTEN:5900,reuseaddr TCP-LISTEN:5901,reuseaddr &
    $ x86_64-softmmu/qemu-system-x86_64 -vnc localhost:5900,reverse -monitor stdio
    QEMU 1.2.50 monitor - type 'help' for more information
    (qemu) info vnc
    An undefined error has occurred

Because however the host, family, service and auth fields are
optional, we can just exit if there is no active server socket.

    $ x86_64-softmmu/qemu-system-x86_64 -vnc localhost:5900,reverse -monitor stdio
    QEMU 1.2.50 monitor - type 'help' for more information
    (qemu) info vnc
    Server:
    Client:
         address: 127.0.0.1:5900
      x509_dname: none
        username: none

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 ui/vnc.c | 4 ++++
 1 file modificato, 4 inserzioni(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index b8e46ca..44d00a2 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -372,6 +372,10 @@ VncInfo *qmp_query_vnc(Error **errp)
             }
         }
 
+        if (vnc_display->lsock == -1) {
+            return info;
+        }
+
         if (getsockname(vnc_display->lsock, (struct sockaddr *)&sa,
                         &salen) == -1) {
             error_set(errp, QERR_UNDEFINED_ERROR);
-- 
1.7.12.1

Re: [Qemu-devel] [Qemu-trivial] [PATCH] vnc: fix "info vnc" with "-vnc ..., reverse=on"

[Stefan Hajnoczi]

On Wed, Oct 10, 2012 at 02:30:58PM +0200, Paolo Bonzini wrote:
> When reverse connection is in use, there is no active VNC server
> socket.  Because of this, getsockopt(-1, ...) is attempted and
> the following error is emitted:
> 
>     $ socat TCP-LISTEN:5900,reuseaddr TCP-LISTEN:5901,reuseaddr &
>     $ x86_64-softmmu/qemu-system-x86_64 -vnc localhost:5900,reverse -monitor stdio
>     QEMU 1.2.50 monitor - type 'help' for more information
>     (qemu) info vnc
>     An undefined error has occurred
> 
> Because however the host, family, service and auth fields are
> optional, we can just exit if there is no active server socket.
> 
>     $ x86_64-softmmu/qemu-system-x86_64 -vnc localhost:5900,reverse -monitor stdio
>     QEMU 1.2.50 monitor - type 'help' for more information
>     (qemu) info vnc
>     Server:
>     Client:
>          address: 127.0.0.1:5900
>       x509_dname: none
>         username: none
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  ui/vnc.c | 4 ++++
>  1 file modificato, 4 inserzioni(+)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

Re: [Qemu-devel] [PATCH] vnc: fix "info vnc" with "-vnc ..., reverse=on"

[Michael Tokarev]

On 10.10.2012 16:30, Paolo Bonzini wrote:
[]
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -372,6 +372,10 @@ VncInfo *qmp_query_vnc(Error **errp)
>              }
>          }
>  
> +        if (vnc_display->lsock == -1) {

FWIW, can't we use "< 0" condition in all cases like this - for
testing whenever a filedescriptor is not open or if a system call
returned failure?  Why we compare with -1 only?  The "< 0" comparison
is cheaper in the resulting code (very very small difference), but
more to the point, it guarantees that no invalid value is treated
as valid - for example, in case some function returns -errno like
it was with bdrv_something() recently.  I use "< 0" in such places
for over 20 years for this reason, this "technique" saved me
countless number of times, when old API changed, or when I switched
to a new API without reviewing return value checking in all places,
or when I just didn't read the manpage carefully... ;)

Thanks,

/mjt