Re: [Qemu-devel] [PATCH] os-posix: Log to logfile in case of daemonize

[Dimitris Aragiorgis]

Hi,

* Paolo Bonzini <address@hidden> [2016-02-11 13:31:17 +0100]:

> 
> 
> On 11/02/2016 13:12, Dimitris Aragiorgis wrote:
> > Besides that, when one executes a daemon, shell redirection is
> > hardly, if ever, used. More so if the daemon already has a logfile
> > option.
> > 
> > So, we decided to give it a go and find the least painful way to
> > log the stderr of a QEMU process to a logfile.
> > 
> > To our understanding, the logfile (-D option) is used only for
> > messages generated by qemu_log()/qemu_log_mask(). The current
> > situation however is that fprintf(stderr, ...) is used in various
> > places throughout the codebase for logging/debug purposes.
> 
> Right, mostly through error_report.
> 
> Actually I like your approach (log to -D if daemonize is used).  I
> just was not sure of the best way to implement it.
> 

Well my apprach was based on the extra option so that the current
behavior does not change and logfiles become too noisy out of the blue.

I have a patch already that:

  - modifies do_qemu_set_log() so that if -log-stderr is given to:
    + open the logfile
    + dup stderr to it
    + skip qemu_log_close()
  - modifies os_setup_post() to dup stderr to /dev/null only if
    qemu_logfile is NULL.

If we do not want this extra option, I guess we are talking about
logging stderr to -D *only* if daemonize is used, right?

> Perhaps when the logfile is opened you can replace the straight fopen with
> 
>     qemu_logfile = fopen(...);
>     if (daemonized) {
>         dup2(fileno(qemu_logfile), STDERR_FILENO);
>         fclose(qemu_logfile);
>         qemu_logfile = stderr;
>     }
> 
> Then the logfile will never be closed by qemu_log_close, and stderr
> will always be sent to it.  Does this look sane?
> 

I guess the above snippet will go in do_qemu_set_log(), right?

If true, we have to open the logfile even if the -d option is not given.
Besides that, log.c should become aware of daemonize.

Depending on your answers I'll prepare the corresponding patch.

Thanks,
dimara

> Paolo

signature.asc
Description: Digital signature