Hi, * Paolo Bonzini <pbonz...@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);
> > }
>
signature.asc
Description: Digital signature
