| Message ID | 1349872258-21952-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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);
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(+)