Re: [Qemu-devel] [PATCH] vl.c: Replace fprintf(stderr) with error_report()

[Eduardo Habkost]

On Mon, Oct 26, 2015 at 06:43:43PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
> 
> > This replaces most fprintf(stderr) calls on vl.c with error_report().
> >
> > The trailing newlines, "qemu:" and "error:" message prefixes were
> > removed.
> 
> Good!
> 
> Some of the messages end in a period, which could also be dropped.
> Example:
> 
>     Usage: -virtfs fsdriver,mount_tag=tag.

I will do that as a follow-up, or in case I have to send a new version.

> 
> > The only remaining fprintf(stderr) calls are the ones at
> > qemu_kill_report(), because the error mesage is split in multiple
> > fprintf() calls.
> 
> I think the straightforward solution would be two error_report() calls:
> 
> @@ -1624,14 +1624,14 @@ static int qemu_shutdown_requested(void)
>  static void qemu_kill_report(void)
>  {
>      if (!qtest_driver() && shutdown_signal != -1) {
> -        fprintf(stderr, "qemu: terminating on signal %d", shutdown_signal);
>          if (shutdown_pid == 0) {
> +            error_report("terminating on signal %d", shutdown_signal);
>              /* This happens for eg ^C at the terminal, so it's worth
>               * avoiding printing an odd message in that case.
>               */
> -            fputc('\n', stderr);
>          } else {
> -            fprintf(stderr, " from pid " FMT_pid "\n", shutdown_pid);
> +            error_report("terminating on signal %d from pid" FMT_pid,

Missing whitespace here. I will fix it and submit a patch.

> +                         shutdown_signal, shutdown_pid);
>          }
>          shutdown_signal = -1;
>      }
> 
> > Signed-off-by: Eduardo Habkost <address@hidden>
> > ---
> > Not sure if this is appropriate post soft-freeze, but if we are going to 
> > apply
> > the max-cpus patch from Drew before 2.5.0, we could simply change all the
> > fprintf() calls in a single step.
> 
> I think it is just fine post soft freeze.  It's soft, not hard freeze.
> 
> Neither of my suggestions is important enough to deny my
> Reviewed-by: Markus Armbruster <address@hidden>

Thanks!

-- 
Eduardo