All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
@ 2013-07-22 21:03 Seiji Aguchi
  2013-07-22 21:23 ` Andreas Färber
  0 siblings, 1 reply; 9+ messages in thread
From: Seiji Aguchi @ 2013-07-22 21:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: lersek, tomoki.sekiyama, lcapitulino

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-devel&m=137331442628866&w=2

Signed-off-by: Seiji Aguchi <seiji.aguchi@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(-)

diff --git a/arch_init.c b/arch_init.c
index e9dd96f..da83560 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1085,8 +1085,8 @@ void do_acpitable_option(const QemuOpts *opts)
 
     acpi_table_add(opts, &err);
     if (err) {
-        fprintf(stderr, "Wrong acpi table provided: %s\n",
-                error_get_pretty(err));
+        error_report("Wrong acpi table provided: %s\n",
+                     error_get_pretty(err));
         error_free(err);
         exit(1);
     }
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 6025592..de54a42 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -27,6 +27,7 @@
 #include "sysemu/char.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
+#include "qemu/error-report.h"
 
 //#define DEBUG_SERIAL
 
@@ -696,7 +697,7 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
     s->chr = chr;
     serial_realize_core(s, &err);
     if (err != NULL) {
-        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_report("%s\n", error_get_pretty(err));
         error_free(err);
         exit(1);
     }
@@ -760,7 +761,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
 
     serial_realize_core(s, &err);
     if (err != NULL) {
-        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_report("%s\n", error_get_pretty(err));
         error_free(err);
         exit(1);
     }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2a87563..375fe7a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -980,7 +980,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
         cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
                          icc_bridge, &error);
         if (error) {
-            fprintf(stderr, "%s\n", error_get_pretty(error));
+            error_report("%s\n", error_get_pretty(error));
             error_free(error);
             exit(1);
         }
@@ -1087,8 +1087,8 @@ void pc_acpi_init(const char *default_dsdt)
 
         acpi_table_add(opts, &err);
         if (err) {
-            fprintf(stderr, "WARNING: failed to load %s: %s\n", filename,
-                    error_get_pretty(err));
+            error_report("WARNING: failed to load %s: %s\n", filename,
+                         error_get_pretty(err));
             error_free(err);
         }
         g_free(arg);
diff --git a/qemu-char.c b/qemu-char.c
index c86ce4b..b1f4e95 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3299,7 +3299,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
 
     chr = qemu_chr_new_from_opts(opts, init, &err);
     if (error_is_set(&err)) {
-        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_report("%s\n", error_get_pretty(err));
         error_free(err);
     }
     if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e3f75a8..5991c9d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1839,7 +1839,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 
 out:
     if (error) {
-        fprintf(stderr, "%s\n", error_get_pretty(error));
+        error_report("%s\n", error_get_pretty(error));
         error_free(error);
         if (cpu != NULL) {
             object_unref(OBJECT(cpu));
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 79bfcd8..865b6b1 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -27,6 +27,7 @@
 #include "cpu-models.h"
 #include "mmu-hash32.h"
 #include "mmu-hash64.h"
+#include "qemu/error-report.h"
 
 //#define PPC_DUMP_CPU
 //#define PPC_DEBUG_SPR
@@ -8174,7 +8175,7 @@ PowerPCCPU *cpu_ppc_init(const char *cpu_model)
 
     object_property_set_bool(OBJECT(cpu), true, "realized", &err);
     if (err != NULL) {
-        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_report("%s\n", error_get_pretty(err));
         error_free(err);
         object_unref(OBJECT(cpu));
         return NULL;
diff --git a/vl.c b/vl.c
index 25b8f2f..c2d0a1c 100644
--- a/vl.c
+++ b/vl.c
@@ -2393,7 +2393,7 @@ static int chardev_init_func(QemuOpts *opts, void *opaque)
 
     qemu_chr_new_from_opts(opts, NULL, &local_err);
     if (error_is_set(&local_err)) {
-        fprintf(stderr, "%s\n", error_get_pretty(local_err));
+        error_report("%s\n", error_get_pretty(local_err));
         error_free(local_err);
         return -1;
     }
@@ -4375,8 +4375,8 @@ int main(int argc, char **argv, char **envp)
         vnc_display_init(ds);
         vnc_display_open(ds, vnc_display, &local_err);
         if (local_err != NULL) {
-            fprintf(stderr, "Failed to start VNC server on `%s': %s\n",
-                    vnc_display, error_get_pretty(local_err));
+            error_report("Failed to start VNC server on `%s': %s\n",
+                         vnc_display, error_get_pretty(local_err));
             error_free(local_err);
             exit(1);
         }
@@ -4419,7 +4419,8 @@ int main(int argc, char **argv, char **envp)
         Error *local_err = NULL;
         qemu_start_incoming_migration(incoming, &local_err);
         if (local_err) {
-            fprintf(stderr, "-incoming %s: %s\n", incoming, error_get_pretty(local_err));
+            error_report("-incoming %s: %s\n", incoming,
+                         error_get_pretty(local_err));
             error_free(local_err);
             exit(1);
         }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-22 21:03 [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp Seiji Aguchi
@ 2013-07-22 21:23 ` Andreas Färber
  2013-07-23  6:47   ` Markus Armbruster
  2013-07-29 21:20   ` Luiz Capitulino
  0 siblings, 2 replies; 9+ messages in thread
From: Andreas Färber @ 2013-07-22 21:23 UTC (permalink / raw)
  To: Seiji Aguchi; +Cc: lersek, qemu-devel, tomoki.sekiyama, lcapitulino

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-devel&m=137331442628866&w=2
> 
> Signed-off-by: Seiji Aguchi <seiji.aguchi@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-22 21:23 ` Andreas Färber
@ 2013-07-23  6:47   ` Markus Armbruster
  2013-07-29 21:20   ` Luiz Capitulino
  1 sibling, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2013-07-23  6:47 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Seiji Aguchi, lersek, qemu-devel, tomoki.sekiyama, lcapitulino

Andreas Färber <afaerber@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-devel&m=137331442628866&w=2
>> 
>> Signed-off-by: Seiji Aguchi <seiji.aguchi@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

[...]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-22 21:23 ` Andreas Färber
  2013-07-23  6:47   ` Markus Armbruster
@ 2013-07-29 21:20   ` Luiz Capitulino
  2013-07-29 21:23     ` Andreas Färber
  1 sibling, 1 reply; 9+ messages in thread
From: Luiz Capitulino @ 2013-07-29 21:20 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Seiji Aguchi, lersek, qemu-devel, tomoki.sekiyama

On Mon, 22 Jul 2013 23:23:29 +0200
Andreas Färber <afaerber@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-devel&m=137331442628866&w=2
> > 
> > Signed-off-by: Seiji Aguchi <seiji.aguchi@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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-29 21:20   ` Luiz Capitulino
@ 2013-07-29 21:23     ` Andreas Färber
  2013-07-29 21:45       ` Luiz Capitulino
  0 siblings, 1 reply; 9+ messages in thread
From: Andreas Färber @ 2013-07-29 21:23 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: Seiji Aguchi, lersek, qemu-devel, tomoki.sekiyama

Am 29.07.2013 23:20, schrieb Luiz Capitulino:
> On Mon, 22 Jul 2013 23:23:29 +0200
> Andreas Färber <afaerber@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-29 21:23     ` Andreas Färber
@ 2013-07-29 21:45       ` Luiz Capitulino
  2013-07-30  0:00         ` Markus Armbruster
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Capitulino @ 2013-07-29 21:45 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Seiji Aguchi, lersek, qemu-devel, tomoki.sekiyama

On Mon, 29 Jul 2013 23:23:32 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 29.07.2013 23:20, schrieb Luiz Capitulino:
> > On Mon, 22 Jul 2013 23:23:29 +0200
> > Andreas Färber <afaerber@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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-29 21:45       ` Luiz Capitulino
@ 2013-07-30  0:00         ` Markus Armbruster
  2013-07-30  0:34           ` Luiz Capitulino
  0 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2013-07-30  0:00 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Seiji Aguchi, lersek, Andreas Färber, tomoki.sekiyama, qemu-devel

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Mon, 29 Jul 2013 23:23:32 +0200
> Andreas Färber <afaerber@suse.de> wrote:
>
>> Am 29.07.2013 23:20, schrieb Luiz Capitulino:
>> > On Mon, 22 Jul 2013 23:23:29 +0200
>> > Andreas Färber <afaerber@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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-30  0:00         ` Markus Armbruster
@ 2013-07-30  0:34           ` Luiz Capitulino
  2013-07-30  1:22             ` Seiji Aguchi
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Capitulino @ 2013-07-30  0:34 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Seiji Aguchi, lersek, Andreas Färber, tomoki.sekiyama, qemu-devel

On Tue, 30 Jul 2013 02:00:40 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Mon, 29 Jul 2013 23:23:32 +0200
> > Andreas Färber <afaerber@suse.de> wrote:
> >
> >> Am 29.07.2013 23:20, schrieb Luiz Capitulino:
> >> > On Mon, 22 Jul 2013 23:23:29 +0200
> >> > Andreas Färber <afaerber@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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp
  2013-07-30  0:34           ` Luiz Capitulino
@ 2013-07-30  1:22             ` Seiji Aguchi
  0 siblings, 0 replies; 9+ messages in thread
From: Seiji Aguchi @ 2013-07-30  1:22 UTC (permalink / raw)
  To: Luiz Capitulino, Markus Armbruster
  Cc: lersek, Andreas Färber, Tomoki Sekiyama, qemu-devel

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:lcapitulino@redhat.com]
> Sent: Monday, July 29, 2013 8:35 PM
> To: Markus Armbruster
> Cc: Andreas Färber; Seiji Aguchi; lersek@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 <armbru@redhat.com> wrote:
> 
> > Luiz Capitulino <lcapitulino@redhat.com> writes:
> >
> > > On Mon, 29 Jul 2013 23:23:32 +0200
> > > Andreas Färber <afaerber@suse.de> wrote:
> > >
> > >> Am 29.07.2013 23:20, schrieb Luiz Capitulino:
> > >> > On Mon, 22 Jul 2013 23:23:29 +0200
> > >> > Andreas Färber <afaerber@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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-07-30  1:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-22 21:03 [Qemu-devel] [PATCH] Convert stderr message calling error_get_pretty() to error_report() to prepend timestamp Seiji Aguchi
2013-07-22 21:23 ` Andreas Färber
2013-07-23  6:47   ` Markus Armbruster
2013-07-29 21:20   ` Luiz Capitulino
2013-07-29 21:23     ` Andreas Färber
2013-07-29 21:45       ` Luiz Capitulino
2013-07-30  0:00         ` Markus Armbruster
2013-07-30  0:34           ` Luiz Capitulino
2013-07-30  1:22             ` Seiji Aguchi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.