From patchwork Wed Oct 19 15:16:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kurz X-Patchwork-Id: 13011992 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 92653C43219 for ; Wed, 19 Oct 2022 15:21:37 +0000 (UTC) Received: from localhost ([::1]:52136 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1olAsy-00068p-Dx for qemu-devel@archiver.kernel.org; Wed, 19 Oct 2022 11:21:36 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:33416) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1olAob-00043d-1i for qemu-devel@nongnu.org; Wed, 19 Oct 2022 11:17:05 -0400 Received: from us-smtp-delivery-44.mimecast.com ([207.211.30.44]:35654) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1olAoY-00022C-Sf for qemu-devel@nongnu.org; Wed, 19 Oct 2022 11:17:04 -0400 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-41-IxPVhthtOTucQyILKEnXCQ-1; Wed, 19 Oct 2022 11:16:55 -0400 X-MC-Unique: IxPVhthtOTucQyILKEnXCQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 246F63C0219A; Wed, 19 Oct 2022 15:16:55 +0000 (UTC) Received: from bahia.redhat.com (unknown [10.39.192.144]) by smtp.corp.redhat.com (Postfix) with ESMTP id 215349459C; Wed, 19 Oct 2022 15:16:54 +0000 (UTC) From: Greg Kurz To: qemu-devel@nongnu.org Cc: =?utf-8?q?Alex_Benn=C3=A9e?= , =?utf-8?q?Daniel_?= =?utf-8?q?P_=2E_Berrang=C3=A9?= , Richard Henderson , Paolo Bonzini , Greg Kurz Subject: [PATCH v2 1/2] util/log: Derive thread id from getpid() on hosts w/o gettid() syscall Date: Wed, 19 Oct 2022 17:16:50 +0200 Message-Id: <20221019151651.334334-2-groug@kaod.org> In-Reply-To: <20221019151651.334334-1-groug@kaod.org> References: <20221019151651.334334-1-groug@kaod.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 Received-SPF: softfail client-ip=207.211.30.44; envelope-from=groug@kaod.org; helo=us-smtp-delivery-44.mimecast.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" A subsequent patch needs to be able to differentiate the main QEMU thread from other threads. An obvious way to do so is to compare log_thread_id() and getpid(), based on the fact that they are equal for the main thread on systems that have the gettid() syscall (e.g. linux). Adapt the fallback code for systems without gettid() to provide the same assumption. Suggested-by: Paolo Bonzini Signed-off-by: Greg Kurz --- util/log.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/util/log.c b/util/log.c index d6eb0378c3a3..e1c2535cfcd2 100644 --- a/util/log.c +++ b/util/log.c @@ -72,8 +72,13 @@ static int log_thread_id(void) #elif defined(SYS_gettid) return syscall(SYS_gettid); #else + static __thread int my_id = -1; static int counter; - return qatomic_fetch_inc(&counter); + + if (my_id == -1) { + my_id = getpid() + qatomic_fetch_inc(&counter); + } + return my_id; #endif } From patchwork Wed Oct 19 15:16:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kurz X-Patchwork-Id: 13011993 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CED9CC4332F for ; Wed, 19 Oct 2022 15:22:03 +0000 (UTC) Received: from localhost ([::1]:52142 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1olAtN-0006NB-A5 for qemu-devel@archiver.kernel.org; Wed, 19 Oct 2022 11:22:01 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45246) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1olAoh-000457-1k for qemu-devel@nongnu.org; Wed, 19 Oct 2022 11:17:11 -0400 Received: from us-smtp-delivery-44.mimecast.com ([205.139.111.44]:48773) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1olAof-00022N-8m for qemu-devel@nongnu.org; Wed, 19 Oct 2022 11:17:10 -0400 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-618-OosLUMQFPBu7SCnqojLufQ-1; Wed, 19 Oct 2022 11:16:57 -0400 X-MC-Unique: OosLUMQFPBu7SCnqojLufQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 664B43802B82; Wed, 19 Oct 2022 15:16:56 +0000 (UTC) Received: from bahia.redhat.com (unknown [10.39.192.144]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6263A9459C; Wed, 19 Oct 2022 15:16:55 +0000 (UTC) From: Greg Kurz To: qemu-devel@nongnu.org Cc: =?utf-8?q?Alex_Benn=C3=A9e?= , =?utf-8?q?Daniel_?= =?utf-8?q?P_=2E_Berrang=C3=A9?= , Richard Henderson , Paolo Bonzini , Greg Kurz Subject: [PATCH v2 2/2] util/log: Always send errors to logfile when daemonized Date: Wed, 19 Oct 2022 17:16:51 +0200 Message-Id: <20221019151651.334334-3-groug@kaod.org> In-Reply-To: <20221019151651.334334-1-groug@kaod.org> References: <20221019151651.334334-1-groug@kaod.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 Received-SPF: softfail client-ip=205.139.111.44; envelope-from=groug@kaod.org; helo=us-smtp-delivery-44.mimecast.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" When QEMU is started with `-daemonize`, all stdio descriptors get redirected to `/dev/null`. This basically means that anything printed with error_report() and friends is lost. One could hope that passing `-D ${logfile}` would cause the messages to go to `${logfile}`, as the documentation tends to suggest: -D logfile Output log in logfile instead of to stderr Unfortunately, `-D` belongs to the logging framework and it only does this redirection if some log item is also enabled with `-d` or if QEMU was configured with `--enable-trace-backend=log`. A typical production setup doesn't do tracing or fine-grain debugging but it certainly needs to collect errors. Ignore the check on enabled log items when QEMU is daemonized. Previous behaviour is retained for the non-daemonized case. The logic is unrolled as an `if` for better readability. Since qemu_set_log_internal() caches the final log level and the per-thread property in global variables, it seems more correct to check these instead of intermediary local variables. Special care is needed for the `-D ${logfile} -d tid` case : `${logfile}` is expected to be a template that contains exactly one `%d` that should be expanded to a PID or TID. The logic in qemu_log_trylock() already takes care of that for per-thread logs. Do it as well for the QEMU main thread when opening the file. Note that qemu_log_trylock() now must ensure that the main QEMU thread only uses the global log file ; qemu_log_unlock() must be adapted as well by checking thread_file which is always equal to NULL for the main thread. Signed-off-by: Greg Kurz --- util/log.c | 49 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/util/log.c b/util/log.c index e1c2535cfcd2..0fa23729c78c 100644 --- a/util/log.c +++ b/util/log.c @@ -82,6 +82,11 @@ static int log_thread_id(void) #endif } +static bool is_main_log_thread(void) +{ + return log_thread_id() == getpid(); +} + /* Lock/unlock output. */ FILE *qemu_log_trylock(void) @@ -90,7 +95,8 @@ FILE *qemu_log_trylock(void) logfile = thread_file; if (!logfile) { - if (log_per_thread) { + /* Main thread to use the global file only */ + if (log_per_thread && !is_main_log_thread()) { g_autofree char *filename = g_strdup_printf(global_filename, log_thread_id()); logfile = fopen(filename, "w"); @@ -124,7 +130,7 @@ void qemu_log_unlock(FILE *logfile) if (logfile) { fflush(logfile); qemu_funlockfile(logfile); - if (!log_per_thread) { + if (!thread_file) { rcu_read_unlock(); } } @@ -253,16 +259,21 @@ static bool qemu_set_log_internal(const char *filename, bool changed_name, #endif qemu_loglevel = log_flags; - /* - * In all cases we only log if qemu_loglevel is set. - * Also: - * If per-thread, open the file for each thread in qemu_log_lock. - * If not daemonized we will always log either to stderr - * or to a file (if there is a filename). - * If we are daemonized, we will only log if there is a filename. - */ daemonized = is_daemonized(); - need_to_open_file = log_flags && !per_thread && (!daemonized || filename); + need_to_open_file = false; + if (!daemonized) { + /* + * If not daemonized we only log if qemu_loglevel is set, either to + * stderr or to a file (if there is a filename). + * If per-thread, open the file for each thread in qemu_log_trylock(). + */ + need_to_open_file = qemu_loglevel && !log_per_thread; + } else { + /* + * If we are daemonized, we will only log if there is a filename. + */ + need_to_open_file = filename != NULL; + } if (logfile && (!need_to_open_file || changed_name)) { qatomic_rcu_set(&global_file, NULL); @@ -276,10 +287,22 @@ static bool qemu_set_log_internal(const char *filename, bool changed_name, if (!logfile && need_to_open_file) { if (filename) { - logfile = fopen(filename, log_append ? "a" : "w"); + g_autofree char *fname = NULL; + + /* + * If per-thread, filename contains a single %d that should be + * converted. + */ + if (per_thread) { + fname = g_strdup_printf(filename, getpid()); + } else { + fname = g_strdup(filename); + } + + logfile = fopen(fname, log_append ? "a" : "w"); if (!logfile) { error_setg_errno(errp, errno, "Error opening logfile %s", - filename); + fname); return false; } /* In case we are a daemon redirect stderr to logfile */