mirror of
https://github.com/Motorhead1991/qemu.git
synced 2025-08-01 14:53:54 -06:00
Revert "qemu-char: Print strerror message on failure" and deps
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 commit6e1db57b2a
. Conflicts: console.c hw/baum.c qemu-char.c This reverts commitaad04cd024
. The parts of commitdb418a0a
"Add stdio char device on windows" that depend on the reverted change fixed up. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
This commit is contained in:
parent
f0cdc966fb
commit
1f51470d04
9 changed files with 84 additions and 122 deletions
|
@ -1510,7 +1510,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
|
|||
chr->init(chr);
|
||||
}
|
||||
|
||||
int text_console_init(QemuOpts *opts, CharDriverState **_chr)
|
||||
CharDriverState *text_console_init(QemuOpts *opts)
|
||||
{
|
||||
CharDriverState *chr;
|
||||
TextConsole *s;
|
||||
|
@ -1542,7 +1542,7 @@ int text_console_init(QemuOpts *opts, CharDriverState **_chr)
|
|||
|
||||
if (!s) {
|
||||
g_free(chr);
|
||||
return -EBUSY;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
s->chr = chr;
|
||||
|
@ -1550,9 +1550,7 @@ int text_console_init(QemuOpts *opts, CharDriverState **_chr)
|
|||
s->g_height = height;
|
||||
chr->opaque = s;
|
||||
chr->chr_set_echo = text_console_set_echo;
|
||||
|
||||
*_chr = chr;
|
||||
return 0;
|
||||
return chr;
|
||||
}
|
||||
|
||||
void text_consoles_set_display(DisplayState *ds)
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue