[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting,
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part |
Date: |
Thu, 09 Feb 2012 17:19:00 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Am 09.02.2012 16:16, schrieb Markus Armbruster:
>> Kevin Wolf <address@hidden> writes:
>>
>>> Am 07.02.2012 15:09, schrieb Markus Armbruster:
>>>> This part takes care of backends "file", "pipe", "pty" and "stdio".
>>>> Unlike many other backends, these leave open error reporting to their
>>>> caller. Because the caller doesn't know what went wrong, this results
>>>> in a pretty useless error message.
>>>>
>>>> Change them to report their errors. Improves comically user-hostile
>>>> messages like this one for "-chardev file,id=foo,path=/x"
>>>>
>>>> chardev: opening backend "file" failed
>>>>
>>>> to
>>>>
>>>> qemu-system-x86_64: -chardev file,id=foo,path=/x: Can't create file
>>>> '/x': Permission denied
>>>> chardev: opening backend "file" failed
>>>>
>>>> The useless "opening backend failed" message will be cleaned up
>>>> shortly.
>>>>
>>>> Signed-off-by: Markus Armbruster <address@hidden>
>>>> ---
>>>> qemu-char.c | 19 +++++++++++++++----
>>>> 1 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>>> @@ -1002,7 +1013,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts
>>>> *opts)
>>>> chr->filename = g_malloc(len);
>>>> snprintf(chr->filename, len, "pty:%s", q_ptsname(master_fd));
>>>> qemu_opt_set(opts, "path", q_ptsname(master_fd));
>>>> - fprintf(stderr, "char device redirected to %s\n",
>>>> q_ptsname(master_fd));
>>>> + error_printf("char device redirected to %s\n", q_ptsname(master_fd));
>>>>
>>>> s = g_malloc0(sizeof(PtyCharDriver));
>>>> chr->opaque = s;
>>>
>>> Not really an error message. Does it make any sense at all to have this
>>> message?
>>
>> error_printf() prints to the error channel (monitor or stderr), but not
>> necessarily an error message. See for instance drive_init()'s use of it
>> to print format help.
>
> Ah, right. I confused it with error_report() which includes an error
> location. That would be rather unexpected.
Indeed.
>> Not sure whether it makes sense to have this message. I guess it exists
>> because the pty is chosen automatically, but the user might still want
>> to know which one was chosen.
>
> Does "the user" include management tools?
>
> If we had a chardev_add monitor command, its output would be moved from
> stderr to the monitor. We don't have that,
Yet! One of the reasons for doing this series was preparing the ground
for chardev_add.
> but there might be commands
> that create chardevs internally: gdbserver is one, not sure if I missed
> others.
>
> We probably don't care much about breaking tools that use 'gdbserver
> pty' and read the device from stderr.
And do so using a monitor chardev other than stdio. That would be
weird, wouldn't it?
> (But the information is missing
> from QMP - should it be added?)
Right when somebody asks for it.
For what it's worth, some chardev backend open() methods return such
information via their opts argument. E.g. inet_listen_opts() receives a
port range in opts (options "port" and "to"), and overwrites option
"port" with the port actually chosen. I hate that, should use separate
options.
- Re: [Qemu-devel] [PATCH 04/19] qemu-char: qemu_chr_open_fd() can't fail, don't check, (continued)
- [Qemu-devel] [PATCH 07/19] sockets: Drop sockets_debug debug code, Markus Armbruster, 2012/02/07
- [Qemu-devel] [PATCH 06/19] gdbstub: Error locations for -gdb, Markus Armbruster, 2012/02/07
- [Qemu-devel] [PATCH 14/19] console: Eliminate text_consoles[], Markus Armbruster, 2012/02/07
- [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part, Markus Armbruster, 2012/02/07
- Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part, Kevin Wolf, 2012/02/07
- Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part, Markus Armbruster, 2012/02/09
- Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part, Kevin Wolf, 2012/02/09
- Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part, Luiz Capitulino, 2012/02/09
- Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part, Markus Armbruster, 2012/02/09
[Qemu-devel] [PATCH 03/19] qemu-char: Re-apply style fixes from just reverted aad04cd0, Markus Armbruster, 2012/02/07
[Qemu-devel] [PATCH 17/19] baum: Chardev open error reporting, braille part, Markus Armbruster, 2012/02/07
[Qemu-devel] [PATCH 11/19] qemu-char: Chardev open error reporting, _WIN32 part, Markus Armbruster, 2012/02/07
[Qemu-devel] [PATCH 15/19] console: Chardev open error reporting, console part, Markus Armbruster, 2012/02/07
[Qemu-devel] [PATCH 19/19] qemu-char: Fix legacy chardev syntax error reporting, Markus Armbruster, 2012/02/07
[Qemu-devel] [PATCH 18/19] qemu-char: Chardev open error reporting, generic part, Markus Armbruster, 2012/02/07
Re: [Qemu-devel] [PATCH 00/19] Fix and improve chardev open error messages, Kevin Wolf, 2012/02/07