Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
On Mon, 22 Jul 2013 23:23:29 +0200 Andreas Färber afaer...@suse.de wrote: Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). Timestamp is prepended by -msg timstamp option with it. This is suggested by Luiz Capitulino. http://marc.info/?l=qemu-develm=137331442628866w=2 Signed-off-by: Seiji Aguchi seiji.agu...@hds.com --- arch_init.c |4 ++-- hw/char/serial.c|5 +++-- hw/i386/pc.c|6 +++--- qemu-char.c |2 +- target-i386/cpu.c |2 +- target-ppc/translate_init.c |3 ++- vl.c|9 + 7 files changed, 17 insertions(+), 14 deletions(-) How is this related to error_get_pretty()? Yeah, we're converting fprintf(stderr,) calls to error_report() so that error messages get a timestamp. And does error_report() expect a trailing \n or not? I would assume no, but spotted some in this patch. Yes, already confirmed by Markus.
Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
Am 29.07.2013 23:20, schrieb Luiz Capitulino: On Mon, 22 Jul 2013 23:23:29 +0200 Andreas Färber afaer...@suse.de wrote: Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). How is this related to error_get_pretty()? Yeah, we're converting fprintf(stderr,) calls to error_report() so that error messages get a timestamp. Want to add that to my http://wiki.qemu.org/DeveloperNews so that people reading it stop adding new ones? :) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
On Mon, 29 Jul 2013 23:23:32 +0200 Andreas Färber afaer...@suse.de wrote: Am 29.07.2013 23:20, schrieb Luiz Capitulino: On Mon, 22 Jul 2013 23:23:29 +0200 Andreas Färber afaer...@suse.de wrote: Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). How is this related to error_get_pretty()? Yeah, we're converting fprintf(stderr,) calls to error_report() so that error messages get a timestamp. Want to add that to my http://wiki.qemu.org/DeveloperNews so that people reading it stop adding new ones? :) Big IMHO here, but honestly speaking I'm not a huge fan of error_report() because I think that random code shouldn't be allowed to print to the monitor (only HMP code should). But it's widespread and it's where the timestamp lives, so calling fprintf() instead of error_report() will probably start hurting soon.
Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
Luiz Capitulino lcapitul...@redhat.com writes: On Mon, 29 Jul 2013 23:23:32 +0200 Andreas Färber afaer...@suse.de wrote: Am 29.07.2013 23:20, schrieb Luiz Capitulino: On Mon, 22 Jul 2013 23:23:29 +0200 Andreas Färber afaer...@suse.de wrote: Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). How is this related to error_get_pretty()? Yeah, we're converting fprintf(stderr,) calls to error_report() so that error messages get a timestamp. Want to add that to my http://wiki.qemu.org/DeveloperNews so that people reading it stop adding new ones? :) Big IMHO here, but honestly speaking I'm not a huge fan of error_report() because I think that random code shouldn't be allowed to print to the monitor (only HMP code should). A conversion from fprintf() to error_report() is always an improvement, because 1. It makes the nature of the message explicit in the code. 2. It prints the error message in the proper form, with error location when available. 3. When called in monitor context, it prints to the monitor rather than stderr. Printing directly to the monitor may not be clean, but it sure beats printing to stderr, where the monitor user can't see it unless the monitor happens to be on stdio. Besides, there's plenty of code that doesn't ever run in monitor context, thus needn't worry about QMP vs. HMP. But it's widespread and it's where the timestamp lives, so calling fprintf() instead of error_report() will probably start hurting soon.
Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
On Tue, 30 Jul 2013 02:00:40 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Mon, 29 Jul 2013 23:23:32 +0200 Andreas Färber afaer...@suse.de wrote: Am 29.07.2013 23:20, schrieb Luiz Capitulino: On Mon, 22 Jul 2013 23:23:29 +0200 Andreas Färber afaer...@suse.de wrote: Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). How is this related to error_get_pretty()? Yeah, we're converting fprintf(stderr,) calls to error_report() so that error messages get a timestamp. Want to add that to my http://wiki.qemu.org/DeveloperNews so that people reading it stop adding new ones? :) Big IMHO here, but honestly speaking I'm not a huge fan of error_report() because I think that random code shouldn't be allowed to print to the monitor (only HMP code should). A conversion from fprintf() to error_report() is always an improvement, because No disagreement here.
Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
Markus, Luiz A conversion from fprintf() to error_report() is always an improvement, because No disagreement here. Thank you for discussing this. I will update my patch by converting from fprintf(stderr, ..) to error_report(). Seiji -Original Message- From: Luiz Capitulino [mailto:lcapitul...@redhat.com] Sent: Monday, July 29, 2013 8:35 PM To: Markus Armbruster Cc: Andreas Färber; Seiji Aguchi; ler...@redhat.com; qemu-devel@nongnu.org; Tomoki Sekiyama Subject: Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp On Tue, 30 Jul 2013 02:00:40 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Mon, 29 Jul 2013 23:23:32 +0200 Andreas Färber afaer...@suse.de wrote: Am 29.07.2013 23:20, schrieb Luiz Capitulino: On Mon, 22 Jul 2013 23:23:29 +0200 Andreas Färber afaer...@suse.de wrote: Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). How is this related to error_get_pretty()? Yeah, we're converting fprintf(stderr,) calls to error_report() so that error messages get a timestamp. Want to add that to my http://wiki.qemu.org/DeveloperNews so that people reading it stop adding new ones? :) Big IMHO here, but honestly speaking I'm not a huge fan of error_report() because I think that random code shouldn't be allowed to print to the monitor (only HMP code should). A conversion from fprintf() to error_report() is always an improvement, because No disagreement here.
Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
Andreas Färber afaer...@suse.de writes: Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). Timestamp is prepended by -msg timstamp option with it. This is suggested by Luiz Capitulino. http://marc.info/?l=qemu-develm=137331442628866w=2 Signed-off-by: Seiji Aguchi seiji.agu...@hds.com --- arch_init.c |4 ++-- hw/char/serial.c|5 +++-- hw/i386/pc.c|6 +++--- qemu-char.c |2 +- target-i386/cpu.c |2 +- target-ppc/translate_init.c |3 ++- vl.c|9 + 7 files changed, 17 insertions(+), 14 deletions(-) How is this related to error_get_pretty()? And does error_report() expect a trailing \n or not? I would assume no, but spotted some in this patch. You're correct, and the patch needs fixing. See commits 312fd5f error: Strip trailing '\n' from error string arguments (again) be62a2e Strip trailing '\n' from error_report()'s first argument (again) 6daf194 Strip trailing '\n' from error_report()'s first argument [...]
Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
Am 22.07.2013 23:03, schrieb Seiji Aguchi: Convert stderr messages calling error_get_pretty() to error_report(). Timestamp is prepended by -msg timstamp option with it. This is suggested by Luiz Capitulino. http://marc.info/?l=qemu-develm=137331442628866w=2 Signed-off-by: Seiji Aguchi seiji.agu...@hds.com --- arch_init.c |4 ++-- hw/char/serial.c|5 +++-- hw/i386/pc.c|6 +++--- qemu-char.c |2 +- target-i386/cpu.c |2 +- target-ppc/translate_init.c |3 ++- vl.c|9 + 7 files changed, 17 insertions(+), 14 deletions(-) How is this related to error_get_pretty()? And does error_report() expect a trailing \n or not? I would assume no, but spotted some in this patch. If you feel strongly about having error_report() consistently, please let me know if there are any in my qom-next queue that need fixing. https://github.com/afaerber/qemu-cpu/commits/qom-next Thanks, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
