From: Paolo Bonzini <pbonzini@redhat.com>
To: Dimitris Aragiorgis <dimara@arrikto.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] log: Redirect stderr to logfile if deamonized
Date: Fri, 19 Feb 2016 18:25:35 +0100 [thread overview]
Message-ID: <56C7500F.2070107@redhat.com> (raw)
In-Reply-To: <20160218171250.GA21453@arr>
On 18/02/2016 18:12, Dimitris Aragiorgis wrote:
> * Paolo Bonzini <pbonzini@redhat.com> [2016-02-18 16:22:13 +0100]:
>
>>
>>
>> On 18/02/2016 12:38, Dimitris Aragiorgis wrote:
>>> In case of daemonize, use the logfile passed with the -D option in
>>> order to redirect stderr to it instead of /dev/null.
>>>
>>> Also remove some unused code in log.h.
>>>
>>> Signed-off-by: Dimitris Aragiorgis <dimara@arrikto.com>
>>> ---
>>> include/qemu/log.h | 6 ------
>>> os-posix.c | 6 +++++-
>>> util/log.c | 11 +++++++++--
>>> 3 files changed, 14 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/include/qemu/log.h b/include/qemu/log.h
>>> index 30817f7..dda65fd 100644
>>> --- a/include/qemu/log.h
>>> +++ b/include/qemu/log.h
>>> @@ -94,12 +94,6 @@ static inline void qemu_log_close(void)
>>> }
>>> }
>>>
>>> -/* Set up a new log file */
>>> -static inline void qemu_log_set_file(FILE *f)
>>> -{
>>> - qemu_logfile = f;
>>> -}
>>> -
>>> /* define log items */
>>> typedef struct QEMULogItem {
>>> int mask;
>>> diff --git a/os-posix.c b/os-posix.c
>>> index cce62ed..92fa3ba 100644
>>> --- a/os-posix.c
>>> +++ b/os-posix.c
>>> @@ -37,6 +37,7 @@
>>> #include "qemu-options.h"
>>> #include "qemu/rcu.h"
>>> #include "qemu/error-report.h"
>>> +#include "qemu/log.h"
>>>
>>> #ifdef CONFIG_LINUX
>>> #include <sys/prctl.h>
>>> @@ -275,7 +276,10 @@ void os_setup_post(void)
>>>
>>> dup2(fd, 0);
>>> dup2(fd, 1);
>>> - dup2(fd, 2);
>>> + /* In case -D is given do not redirect stderr to /dev/null */
>>> + if (!qemu_logfile) {
>>> + dup2(fd, 2);
>>> + }
>>>
>>> close(fd);
>>>
>>> diff --git a/util/log.c b/util/log.c
>>> index 2709e98..a7ddc7e 100644
>>> --- a/util/log.c
>>> +++ b/util/log.c
>>> @@ -56,13 +56,20 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
>>> #ifdef CONFIG_TRACE_LOG
>>> qemu_loglevel |= LOG_TRACE;
>>> #endif
>>> - if (qemu_loglevel && !qemu_logfile) {
>>> + if ((qemu_loglevel || is_daemonized()) && !qemu_logfile) {
>>> if (logfilename) {
>>> qemu_logfile = fopen(logfilename, log_append ? "a" : "w");
>>> if (!qemu_logfile) {
>>> perror(logfilename);
>>> _exit(1);
>>> }
>>> + /* In case we are a daemon redirect stderr to logfile */
>>> + if (is_daemonized()) {
>>> + dup2(fileno(qemu_logfile), STDERR_FILENO);
>>> + fclose(qemu_logfile);
>>> + /* This will skip closing logfile in qemu_log_close() */
>>> + qemu_logfile = stderr;
>>> + }
>>> } else {
>>> /* Default to stderr if no log file specified */
>>> qemu_logfile = stderr;
>>> @@ -82,7 +89,7 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
>>> log_append = 1;
>>> }
>>> }
>>> - if (!qemu_loglevel && qemu_logfile) {
>>> + if (!qemu_loglevel && !is_daemonized() && qemu_logfile) {
>>> qemu_log_close();
>>> }
>>
>> Why is this necessary? Perhaps qemu_log_close should dup(1,2) if QEMU
>> is daemonized. The rest looks great.
>>
>
> Without !is_daemonized(), if we use -daemon with -D without -d,
> qemu_log_close() will eventually set qemu_logfile to NULL. This
> will make os_setup_post() redirect stderr to /dev/null, which
> is unwanted.
Sorry, I missed the context of this hunk. The patch is okay, thanks for
putting up with me!
Paolo
next prev parent reply other threads:[~2016-02-19 17:25 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-16 16:56 [Qemu-devel] [PATCH] os-posix: Log to logfile in case of daemonize Alex Pyrgiotis
2016-02-08 9:29 ` Paolo Bonzini
2016-02-11 12:12 ` Dimitris Aragiorgis
2016-02-11 12:31 ` Paolo Bonzini
2016-02-11 16:49 ` Dimitris Aragiorgis
2016-02-11 17:56 ` Paolo Bonzini
2016-02-18 11:38 ` [Qemu-devel] [PATCH] log: Redirect stderr to logfile if deamonized Dimitris Aragiorgis
2016-02-18 15:22 ` Paolo Bonzini
2016-02-18 17:12 ` Dimitris Aragiorgis
2016-02-19 17:25 ` Paolo Bonzini [this message]
2016-03-01 11:15 ` Gerd Hoffmann
2016-03-01 11:47 ` Daniel P. Berrange
2016-03-01 11:51 ` Paolo Bonzini
2016-03-01 11:58 ` Daniel P. Berrange
2016-03-01 12:03 ` Paolo Bonzini
2016-03-01 13:54 ` Daniel P. Berrange
2016-03-01 11:50 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56C7500F.2070107@redhat.com \
--to=pbonzini@redhat.com \
--cc=dimara@arrikto.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).