mirror of
https://github.com/qemu/qemu.git
synced 2025-10-27 22:03:47 +00:00
The commit's purpose is laudable:
The only way for chardev drivers to communicate an error was to
return a NULL pointer, which resulted in an error message that
said _that_ something went wrong, but not _why_.
It attempts to achieve it by changing the interface to return 0/-errno
and update qemu_chr_open_opts() to use strerror() to display a more
helpful error message. Unfortunately, it has serious flaws:
1. Backends "socket" and "udp" return bogus error codes, because
qemu_chr_open_socket() and qemu_chr_open_udp() assume that
unix_listen_opts(), unix_connect_opts(), inet_listen_opts(),
inet_connect_opts() and inet_dgram_opts() fail with errno set
appropriately. That assumption is wrong, and the commit turns
unspecific error messages into misleading error messages. For
instance:
$ qemu-system-x86_64 -nodefaults -vnc :0 -chardev socket,id=bar,host=xxx
inet_connect: host and/or port not specified
chardev: opening backend "socket" failed: No such file or directory
ENOENT is what happens to be in my errno when the backend returns
-errno. Let's put ERANGE there just for giggles:
$ qemu-system-x86_64 -nodefaults -vnc :0 -chardev socket,id=bar,host=xxx -drive if=none,iops=99999999999999999999
inet_connect: host and/or port not specified
chardev: opening backend "socket" failed: Numerical result out of range
Worse: when errno happens to be zero, return -errno erroneously
signals success, and qemu_chr_new_from_opts() dies dereferencing
uninitialized chr. I observe this with "-serial unix:".
2. All qemu_chr_open_opts() knows about the error is an errno error
code. That's simply not enough for a decent message. For instance,
when inet_dgram() can't resolve the parameter host, which errno code
should it use? What if it can't resolve parameter localaddr?
Clue: many backends already report errors in their open methods.
Let's revert the flawed commit along with its dependencies, and fix up
the silent error paths instead.
This reverts commit
|
||
|---|---|---|
| .. | ||
| cocoa.m | ||
| curses_keys.h | ||
| curses.c | ||
| d3des.c | ||
| d3des.h | ||
| keymaps.c | ||
| keymaps.h | ||
| qemu-spice.h | ||
| sdl_keysym.h | ||
| sdl_zoom_template.h | ||
| sdl_zoom.c | ||
| sdl_zoom.h | ||
| sdl.c | ||
| spice-core.c | ||
| spice-display.c | ||
| spice-display.h | ||
| spice-input.c | ||
| vnc_keysym.h | ||
| vnc-auth-sasl.c | ||
| vnc-auth-sasl.h | ||
| vnc-auth-vencrypt.c | ||
| vnc-auth-vencrypt.h | ||
| vnc-enc-hextile-template.h | ||
| vnc-enc-hextile.c | ||
| vnc-enc-tight.c | ||
| vnc-enc-tight.h | ||
| vnc-enc-zlib.c | ||
| vnc-enc-zrle-template.c | ||
| vnc-enc-zrle.c | ||
| vnc-enc-zrle.h | ||
| vnc-enc-zywrle-template.c | ||
| vnc-enc-zywrle.h | ||
| vnc-jobs-async.c | ||
| vnc-jobs-sync.c | ||
| vnc-jobs.h | ||
| vnc-palette.c | ||
| vnc-palette.h | ||
| vnc-tls.c | ||
| vnc-tls.h | ||
| vnc.c | ||
| vnc.h | ||
| x_keymap.c | ||
| x_keymap.h | ||