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

[Dimitris Aragiorgis <dimara@arrikto.com>]

[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]

Hi,

* Paolo Bonzini <pbonzini@redhat.com> [2016-02-08 10:29:55 +0100]:

> 
> 
> On 16/12/2015 17:56, Alex Pyrgiotis wrote:
> > +
> > +        log = qemu_get_log_filename();
> > +        if (log != NULL) {
> > +            TFR(fd = qemu_open(log, O_RDWR | O_APPEND | O_CREAT, 0640));
> 
> Here you are opening the same file twice, but the FILE* that is opened
> in do_qemu_set_log does not necessarily have O_APPEND.
> 

This is partially true :) I am opening this file twice only if the -d option
is passed along with -D. Still, point taken.

> I like the idea of moving stderr to the logfile, but I'm not sure how to
> do it.  For now, can you prepare a simple patch that only does the "dup"
> if "isatty" returns true for the file descriptor?  This lets you use
> redirection at the shell level to save the stdout and stderr.
> 

This could be a workaround. Still, I think it is a bit unorthodox to
change the core behavior of logging depending on the type of output (tty
or not). I understand that checking the type of output is sometimes used
to enable/disable colors automatically, for example.

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. A simple solution would be to
redirect the stderr to the logfile when -D is used, but this may confuse
people who expect the current behavior for their logfiles.

Therefore, our suggestion is to introduce another option, "-log-stderr".
If this is given then we can 'dup2' stderr to the already opened logfile
inside do_qemu_set_log(). And we should not close it even if -d is not
given.

Afterwards, os_setup_post() in case of daemonize, would always dup2 0,
1 to /dev/null and only if qemu_logfile is not NULL would dup2 2 to
/dev/null as well.

To sum up if one wants to log stderr to the logfile, one should pass -D
along with -log-stderr. Eventually qemu_log(), qemu_log_mask(), and
fprintf(stderr, ...) will end up writing to our logfile. The -daemonize
option should respect the other options.

What do you think?

dimara

> Paolo
> 
> > +        } else {
> > +            TFR(fd = qemu_open("/dev/null", O_RDWR));
> > +        }
> >          if (fd == -1) {
> > +            fprintf(stderr, "Cannot open \"%s\" for logging\n", log);
> >              exit(1);
> >          }
> > +        g_free(log);
> >      }
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]