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