From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54615) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RvW2Q-0000ZY-8c for qemu-devel@nongnu.org; Thu, 09 Feb 2012 10:36:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RvW2K-0001kw-HJ for qemu-devel@nongnu.org; Thu, 09 Feb 2012 10:35:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33845) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RvW2K-0001km-7m for qemu-devel@nongnu.org; Thu, 09 Feb 2012 10:35:52 -0500 Message-ID: <4F33E8A5.1010203@redhat.com> Date: Thu, 09 Feb 2012 16:39:17 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1328623766-12287-1-git-send-email-armbru@redhat.com> <1328623766-12287-11-git-send-email-armbru@redhat.com> <4F3148BC.7000400@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/19] qemu-char: Chardev open error reporting, !_WIN32 part List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, Luiz Capitulino Am 09.02.2012 16:16, schrieb Markus Armbruster: > Kevin Wolf 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 >>> --- >>> 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. > 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, 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. (But the information is missing from QMP - should it be added?) Kevin