Re: [Qemu-devel] [PULL 14/19] log: Redirect stderr to logfile if deamonized

[Ján Tomko]

On Mon, Feb 29, 2016 at 11:17:25AM +0100, Paolo Bonzini wrote:
> 
> 
> On 29/02/2016 11:04, Ján Tomko wrote:
> > On Mon, Feb 29, 2016 at 10:07:47AM +0100, Paolo Bonzini wrote:
> >>
> >>
> >> On 29/02/2016 09:57, Ján Tomko wrote:
> >>> qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -M none
> >>>  -qmp unix:/var/lib/libvirt/qemu/capabilities.monitor.sock,server,nowait
> >>>  -pidfile /var/lib/libvirt/qemu/capabilities.pidfile -daemonize
> >>
> >> Can you test this patch?
> >>
> >> diff --git a/util/log.c b/util/log.c
> >> index 4dff00b..9402880 100644
> >> --- a/util/log.c
> >> +++ b/util/log.c
> >> @@ -56,7 +56,8 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
> >>  #ifdef CONFIG_TRACE_LOG
> >>      qemu_loglevel |= LOG_TRACE;
> >>  #endif
> >> -    if ((qemu_loglevel || is_daemonized()) && !qemu_logfile) {
> >> +    if (!qemu_logfile &&
> >> +        (qemu_loglevel || (is_daemonized() && logfilename)) {
> >>          if (logfilename) {
> >>              qemu_logfile = fopen(logfilename, log_append ? "a" : "w");
> >>              if (!qemu_logfile) {
> >>
> >> Thanks,
> > 
> > Still hangs,
> > 
> > (gdb) list
> > 54      {
> > 55          qemu_loglevel = log_flags;
> > 56      #ifdef CONFIG_TRACE_LOG
> > 57          qemu_loglevel |= LOG_TRACE;
> > 58      #endif
> > 59          sleep(10);
> > 60          if (!qemu_logfile &&
> > 61              (qemu_loglevel || (is_daemonized() && logfilename))) {
> > 62              if (logfilename) {
> > 63                  qemu_logfile = fopen(logfilename, log_append ? "a" : 
> > "w");
> > (gdb) p qemu_logfile
> > $1 = (FILE *) 0x0
> > (gdb) p qemu_loglevel
> > $2 = 32768
> > (gdb) p logfilename
> > $3 = 0x0
> > (gdb) call is_daemonized()
> > $4 = true
> 
> Second attempt:
> 
> diff --git a/util/log.c b/util/log.c
> index 4dff00b..1530a60 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -56,7 +56,8 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
>  #ifdef CONFIG_TRACE_LOG
>      qemu_loglevel |= LOG_TRACE;
>  #endif
> -    if ((qemu_loglevel || is_daemonized()) && !qemu_logfile) {
> +    if (!qemu_logfile &&
> +        (is_daemonized() ? logfilename : qemu_loglevel)) {
>          if (logfilename) {
>              qemu_logfile = fopen(logfilename, log_append ? "a" : "w");
>              if (!qemu_logfile) {
> @@ -72,6 +73,7 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
>              }
>          } else {
>              /* Default to stderr if no log file specified */
> +            assert(!is_daemonized());
>              qemu_logfile = stderr;
>          }
>          /* must avoid mmap() usage of glibc by setting a buffer "by hand" */
> 
> 

This works for me, but produces a warning:
  CC    util/log.o
util/log.c: In function ‘do_qemu_set_log’:
util/log.c:60:40: error: pointer/integer type mismatch in conditional 
expression [-Werror]
         (is_daemonized() ? logfilename : qemu_loglevel)) {

Jan

> Paolo